Closed Bug 1265537 Opened 4 years ago Closed 3 years ago

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

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla55

People

(Reporter: selenamarie, Assigned: Callek)

References

Details

Attachments

(2 files)

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?
This is described in bug 1203573.
See Also: → 1203573
Summary: How to remove the version number from artifact names → Remove the version number from artifact names of Windows Firefox builds in taskcluster
Rob, Pete - is this change required for the build?
Flags: needinfo?(rthijssen)
Flags: needinfo?(pmoore)
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.
Flags: needinfo?(rthijssen)
Flags: needinfo?(pmoore)
Just waiting for the code to land in-tree, but bug is complete.
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.
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
Flags: needinfo?(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.
Flags: needinfo?(gps)
Thanks, I'll take a look.
are there next steps on this bug?  it is marked as blocking bug 1265531, although we have builds on try.
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?
Flags: needinfo?(pmoore)
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.
Yeah, Rob probably has the latest on this...
Flags: needinfo?(pmoore) → needinfo?(rthijssen)
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.
Flags: needinfo?(rthijssen)
See Also: → 1300901
So is this bug now dependent on 1300901 ?  If this is blocking windows 2012 builds on try...what are our next steps here?
(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.
Blocks: 1280474
No longer blocks: 1265531
Depends on: 1300901
See Also: 1300901
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.
No longer blocks: 1280474
I think we're using simple names (without a version) now, just like on linux -- right?
Flags: needinfo?(pmoore)
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
Attachment #8864284 - Flags: review?(rthijssen) → review+
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
Attachment #8864257 - Flags: review?(ted) → review+
Assignee: nobody → bugspam.Callek
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 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.
Pushed by Callek@gmail.com:
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
https://hg.mozilla.org/mozilla-central/rev/5a9d3c56aec9
https://hg.mozilla.org/mozilla-central/rev/cbf2fbeead1f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1277591
Blocks: 1362511
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.