Closed Bug 1313787 Opened 3 years ago Closed 3 years ago

Ensure packaged debug windows artifact builds are usable

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

There were found in bug 1305534 to fail in tests jobs. From testing them locally, I think we're missing some dlls from the package.
certutil.exe at least appears in the common.tests.zip file, so I think I was probably wrong about it being missing but it does seem to be erroring on use, so something isn't quite right.
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review91350

::: old-configure.in:183
(Diff revision 1)
>  dnl ========================================================
>  dnl Checks for compilers.
>  dnl ========================================================
>  
> +MSVC_C_RUNTIME_DLL_NAME=vcruntime140.dll
> +MSVC_CXX_RUNTIME_DLL_NAME=msvcp140.dll

This is going to break in the future if we update to a newer MSVC. It would be nice if we could figure out a better strategy for artifact builds longer-term without forcing them to run all of configure.
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review91350

> This is going to break in the future if we update to a newer MSVC. It would be nice if we could figure out a better strategy for artifact builds longer-term without forcing them to run all of configure.

How so? We'll have to update these names as soon as we update, this patch makes these variables the only place they get set in the main old-configure.
Because there's always a period where we support >1 version of MSVC, which is why they were in the case block. Maybe we could just move that stuff into moz.configure and find a better way to use it from artifact builds? (In a separate bug, you don't have to fix that now.)
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review91438

I think this needs to be dealt with in some other way, because:
- there are other files involved that are not in the list you have here (specifically all the ucrt files, other d3d compilers (iirc we ship several))
- On top of ted's comment, in a close future where the next version of MSVC is released, the version used on automation will not necessarily match the one used locally.

I think a better approach would be to make use of what the artifacts build actually contains, not a hardcoded list of what we expect it contains.
Attachment #8808762 - Flags: review?(mh+mozilla)
It seems like the core problem here is that with a lot of things with artifact builds we're guessing or hardcoding a bunch of values because we aren't running configure, and we don't have the configure results from the actual build we're using available. What if we uploaded config.status or something like that, and used it to fill in the gaps of the configure tests we didn't run?
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> It seems like the core problem here is that with a lot of things with
> artifact builds we're guessing or hardcoding a bunch of values because we
> aren't running configure

we are

> and we don't have the configure results from the
> actual build we're using available.

that, however is true.

> What if we uploaded config.status or
> something like that, and used it to fill in the gaps of the configure tests
> we didn't run?

we already have most if not all the relevant information in the build itself, in the form of the list of files it contains, as well as, for some specific things, buildconfig.html, platform.ini, etc.
config.status contains way too many things that are irrelevant.
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review91834

::: browser/installer/package-manifest.in:117
(Diff revision 2)
> +; We don't have a complete view of which dlls to expect when doing an artifact
> +; build because we haven't run all of configure, so we trust what's in
> +; dist/bin, because everything there was extracted from our original build's
> +; package.
> +#ifdef MOZ_ARTIFACT_BUILDS
> +@BINPATH@/*.dll

Mmmm if this doesn't fail on non-Windows builds, there's a bug to file to make it so.
Attachment #8808762 - Flags: review?(mh+mozilla)
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review91834

> Mmmm if this doesn't fail on non-Windows builds, there's a bug to file to make it so.

On a linux build we get:

Warning: /home/worker/workspace/build/src/browser/installer/package-manifest.in:117: Missing file(s): bin/*.dll

That seems about right, because we're unsetting `MOZ_PKG_FATAL_WARNINGS` (this was part of bug 1240134, which means we're no longer generating .xpt files during an artifact builds).
Comment on attachment 8808762 [details]
Bug 1313787 - Find relevant dlls on Windows when packaging an artifact build.

https://reviewboard.mozilla.org/r/91488/#review92974
Attachment #8808762 - Flags: review?(mh+mozilla) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d6d40b8751d
Find relevant dlls on Windows when packaging an artifact build. r=glandium
https://hg.mozilla.org/mozilla-central/rev/7d6d40b8751d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.