1265537 - Remove the version number from artifact names of Windows Firefox builds in taskcluster

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Selena Deckelmann :selenamarie :selena]

The TaskCluster Windows build doesn't need to include version numbers in the name. We'll have an index path that has all the version information in it. 

How do we remove this? Is it mozconfig or environment variables or something else?

Comment 1

[Pete Moore [:pmoore][:pete]]

This is described in bug 1203573.

Comment 2

[Selena Deckelmann :selenamarie :selena]

Rob, Pete - is this change required for the build?

Comment 3

[Pete Moore [:pmoore][:pete]]

Yes, we should add it.

It essentially means setting the env var MOZ_SIMPLE_PACKAGE_NAME in the build task to either `target` or to `{{build_product}}.{{build_product_locale}}.{{build_name}}`. I prefer the second option, as it is static per job, but varied across platforms/locales. This makes it safer against bugs where code thinks it is looking at a Polish thunderbird linux build, but is actually looking at a Windows 32bit en-US firefox build.

Whichever option we go for, we could probably then lose the references to `build_product_version` in the taskcluster configs. If we go with `target` we might be able to lose even more properties.

This part is not done yet, so leaving this bug open.

Comment 4

[Selena Deckelmann :selenamarie :selena]

Just waiting for the code to land in-tree, but bug is complete.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

in the windows build we had a successful run to the end (except for artifact uploads).  pmoore quickly rolled out a new build to support uploading all artifacts and included a fix for MOZ_SIMPLE_PACKAGE_NAME as well.  Unfortunately the build failed in the l10n-check step (unlike the earlier builds):
7:43:09     INFO -  mozmake.EXE[3]: Leaving directory 'c:/Users/Task_1462453392/build/src/obj-firefox/browser/locales'
17:43:09     INFO -  mozmake.EXE[4]: *** No rule to make target 'c:/Users/Task_1462453392/build/src/obj-firefox/dist/install/sea/firefox-49.0a1.en-US.win32.installer.exe', needed by 'repackage-win32-installer'.  Stop.
17:43:09     INFO -  Makefile:139: recipe for target 'repackage-win32-installer-x-test' failed
17:43:09     INFO -  mozmake.EXE[3]: *** [repackage-win32-installer-x-test] Error 2
17:43:09     INFO -  Makefile:207: recipe for target 'l10n-check' failed
17:43:09     INFO -  mozmake.EXE[2]: *** [l10n-check] Error 2
17:43:09     INFO -  c:/Users/Task_1462453392/build/src/browser/build.mk:42: recipe for target 'l10n-check' failed
17:43:09     INFO -  mozmake.EXE[1]: *** [l10n-check] Error 2
17:43:09     INFO -  c:/Users/Task_1462453392/build/src/build/moz-automation.mk:120: recipe for target 'automation/l10n-check' failed
17:43:09     INFO -  mozmake.EXE: *** [automation/l10n-check] Error 2
17:43:09     INFO -  mozmake.EXE: Leaving directory 'c:/Users/Task_1462453392/build/src/obj-firefox'
17:43:09     INFO -  242 compiler warnings present.


looking at https://dxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#205, we see that MOZ_SIMPLE_PACKAGE_NAME is set to nothing (unless I am not good at reading makefiles).

Maybe there is more work to do prior to rolling this out.

Comment 6

[Pete Moore [:pmoore][:pete]]

Hi Greg,

See https://public-artifacts.taskcluster.net/XrZsmnSaQ6ur_G5wUZHQuA/3/public/logs/all_commands.log

Basically, when I set MOZ_SIMPLE_PACKAGE_NAME it breaks l10n checks ("INFO -  mozmake.EXE[4]: *** No rule to make target 'c:/Users/Task_1462453392/build/src/obj-firefox/dist/install/sea/firefox-49.0a1.en-US.win32.installer.exe', needed by 'repackage-win32-installer'.  Stop.")

I'm guessing this might be that the l10 check still expects the original file name (since the error message seems to be looking for firefox-49.0a1.en-US.win32.installer.exe), rather than the "simple" name. This l10n problem did not occur when I did not set MOZ_SIMPLE_PACKAGE_NAME. Do you have an idea how I might be able to fix it?

Many thanks in advance!
Pete

Comment 7

[Gregory Szorc [:gps]]

The l10n jobs are working with the installer exe, whose name contains the version string (see PKG_BASENAME and PKG_INST_BASENAME in toolkit/mozapps/installer/package-name.mk).

You'll need to add logic to that file to use the MOZ_SIMPLE_PACKAGE_NAME variable where appropriate.

Comment 8

[Pete Moore [:pmoore][:pete]]

Thanks, I'll take a look.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

are there next steps on this bug?  it is marked as blocking bug 1265531, although we have builds on try.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Pete, can you comment on what the status of this bug is?  Do we still want to do this?  are there action items remaining or blocking us?

Comment 11

[Greg Arndt [:garndt]]

This sounds like it's more about a build and task configuration change rather than a worker change.  I think :grenade has been working on the builds/tests.

Comment 12

[Pete Moore [:pmoore][:pete]]

Yeah, Rob probably has the latest on this...

Comment 13

[Rob Thijssen [:grenade (EET/UTC+0300)]]

I don't feel confident implementing change to something affecting l10n. I don't have any experience or knowledge of the kinds of things likely to break if this change is implemented but I think there is a strong chance of breaking things by changing the package name. I'm not saying it shouldn't be done, just that I don't think I'm in the best position to do the implementation.

Comment 14

[Greg Arndt [:garndt]]

So is this bug now dependent on 1300901 ?  If this is blocking windows 2012 builds on try...what are our next steps here?

Comment 15

[Pete Moore [:pmoore][:pete]]

(In reply to Greg Arndt [:garndt] from comment #14)
> So is this bug now dependent on 1300901 ?  If this is blocking windows 2012
> builds on try...what are our next steps here?

Nice spot, the bug dependencies need a bit of tweaking here. I agree this bug depends 1300901. Also, this is a soft block on tests, rather than blocking tier 2 windows taskcluster builds of firefox desktop.

It will be *extra work* for taskcluster windows tests to know the name of the build artifact to download until bug 1300901 is resolved. However, the tests can still determine the artifact name by querying the artifacts of the known build task, and matching the discovered artifact names with the name of the build using e.g. regex. So there is a way around the problem before 1300901 is solved, it just isn't very nice. So let's make it a blocker.

Comment 16

[Pete Moore [:pmoore][:pete]]

Note, I removed the blocker on having Windows builds in taskcluster, and instead made it a block on running tests. I only found the win7 tests in taskcluster bug, I guess later there will be more bugs for tests for the other windows desktop versions too.

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

I think we're using simple names (without a version) now, just like on linux -- right?

Comment 18

[Rob Thijssen [:grenade (EET/UTC+0300)]]

no, we have complex names on windows build packages. see: https://dxr.mozilla.org/mozilla-central/rev/f65ad27efe839ce9df0283840a1a40b4bbc9ead0/taskcluster/taskgraph/transforms/tests/make_task_description.py#329

Comment 19

[Justin Wood (:Callek)]

Attachment: Bug 1265537 - Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME.

Review commit: https://reviewboard.mozilla.org/r/135892/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135892/

Comment 20

[Justin Wood (:Callek)]

Comment on attachment 8864257 [details]
Bug 1265537 - Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135892/diff/1-2/

Comment 21

[Justin Wood (:Callek)]

Attachment: Bug 1265537 - Adjust Windows TC Builds to generate simple package names, adjust tests to use them.

Review commit: https://reviewboard.mozilla.org/r/135910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135910/

Comment 22

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Comment on attachment 8864284 [details]
Bug 1265537 - Adjust Windows TC Builds to generate simple package names, adjust tests to use them.

https://reviewboard.mozilla.org/r/135910/#review138988

Comment 23

[(not currently active) Ted Mielczarek]

Comment on attachment 8864257 [details]
Bug 1265537 - Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME.

https://reviewboard.mozilla.org/r/135892/#review139296

Comment 24

[Justin Wood (:Callek)]

 https://hg.mozilla.org/projects/date/rev/4b1ee31522f48497bf1eb6f5fa51106bde42ef05
 https://hg.mozilla.org/projects/date/rev/3bb90c64e2a86f92334bc0b9045ee1ff20e243e9

Comment 25

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 4b1ee31522f4 -d a646b729971e: rebasing 393826:4b1ee31522f4 "Bug 1265537 - Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME. r=ted"
rebasing 393827:3bb90c64e2a8 "Bug 1265537 - Adjust Windows TC Builds to generate simple package names, adjust tests to use them. r=grenade" (tip)
merging taskcluster/taskgraph/transforms/job/mozharness.py
merging taskcluster/taskgraph/transforms/job/mozharness_test.py
merging taskcluster/taskgraph/transforms/tests.py
merging testing/mozharness/mozharness/mozilla/building/buildbase.py
warning: conflicts while merging taskcluster/taskgraph/transforms/job/mozharness_test.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 26

[Justin Wood (:Callek)]

Comment on attachment 8864257 [details]
Bug 1265537 - Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135892/diff/2-3/

Comment 27

[Justin Wood (:Callek)]

Comment on attachment 8864284 [details]
Bug 1265537 - Adjust Windows TC Builds to generate simple package names, adjust tests to use them.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135910/diff/1-2/

Comment 28

[Justin Wood (:Callek)]

Comment on attachment 8864284 [details]
Bug 1265537 - Adjust Windows TC Builds to generate simple package names, adjust tests to use them.

https://reviewboard.mozilla.org/r/135910/#review139644

::: taskcluster/taskgraph/transforms/tests.py:762
(Diff revisions 1 - 2)
>      return path.replace('/', '\\')
> +
> +
> +def get_firefox_version():
> +    with open('browser/config/version.txt', 'r') as f:
> +        return f.readline().strip()

In the interdiff after the failed autoland, I had to add this back in. It ended up being used for OSX BBB stuff. So had to add this back in.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a9d3c56aec9
Pass WIN32 Installer Filename to l10n-check explicitly, while simultaneously disabling MOZ_SIMPLE_PACKAGE_NAME. r=ted
https://hg.mozilla.org/integration/autoland/rev/cbf2fbeead1f
Adjust Windows TC Builds to generate simple package names, adjust tests to use them. r=grenade

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5a9d3c56aec9
https://hg.mozilla.org/mozilla-central/rev/cbf2fbeead1f