Closed Bug 1203573 Opened 4 years ago Closed 4 years ago

Support creation of simple filenames for upload

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox43 fixed)

RESOLVED FIXED
Tracking Status
firefox43 --- fixed

People

(Reporter: dustin, Assigned: ted)

References

Details

Attachments

(1 file)

The "TaskCluster way" is for build jobs to generate artifacts with simple, predictable names -- no version numbers or revisions, at least.  These can then be written into the task definitions in advance (at task-graph creation time).

Ted has suggested adding MOZ_SIMPLE_PACKAGE_NAMES which, if set, would cause the build process to create such artifacts.
What is your timeline to work on this?

I'm currently trying to solve this differently by letting Mozharness ask the parent task where are the installer and test_packages.json files.

I'm a bit concerned that changing the current naming will break somebody's assumption somewhere downstream.

I don't know if this a valid thought or not; I don't know how I feel about adding an assumption or a promise of where the files will be uploaded rather than enquirying and being explicit.
It would nevertheless remove the need to talk to taskcluster to determine where the files are.
This shouldn't break any assumptions since there's nothing to be assumed, yet :)

In the world where builds and tests are both running in TaskCluster, the task-graph creation code will take care of getting the filenames from the build task definition (which is why they need to be simple -- we don't have a Firefox version number at that time) and inserting them into the test task definition.  So depending on where you need the information, it may already be available in your own task definition (and thus in an environment variable or something like that) or easily queried from either the build task (if you want to find the build's output) or the test task (if you want to find the task's input).
For reference, Taskcluster builds already rename the existing packages:
https://dxr.mozilla.org/mozilla-central/rev/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/testing/taskcluster/scripts/builder/build-linux.sh#150

What we want to do here is simply move that into the build system and remove that bit of shell, since the build system is a better place for it.
bug 1203573 - add a MOZ_SIMPLE_PACKAGE_NAME variable to simplify package naming for taskcluster's benefit. r?gps
Attachment #8659445 - Flags: review?(gps)
I tested locally with:
```
export MOZ_SIMPLE_PACKAGE_NAME=target.foo
```

and got:
```
$ ls ../debug-mozilla-central/dist/target*
../debug-mozilla-central/dist/target.foo.json          ../debug-mozilla-central/dist/target.foo.tar.bz2
../debug-mozilla-central/dist/target.foo.jsshell.zip   ../debug-mozilla-central/dist/target.foo.txt
../debug-mozilla-central/dist/target.foo.mozinfo.json
```
Comment on attachment 8659445 [details]
MozReview Request: bug 1203573 - add a MOZ_SIMPLE_PACKAGE_NAME variable to simplify package naming for taskcluster's benefit. r?gps

https://reviewboard.mozilla.org/r/18833/#review16851

::: toolkit/mozapps/installer/package-name.mk:57
(Diff revision 1)
> +ifndef MOZ_SIMPLE_PACKAGE_NAME
>  PKG_BASENAME = $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).$(MOZ_PKG_PLATFORM)
> +else
> +PKG_BASENAME := $(MOZ_SIMPLE_PACKAGE_NAME)
> +endif

Please invert the logic to an ifdef.

::: toolkit/mozapps/installer/package-name.mk:171
(Diff revision 1)
> +ifndef MOZ_SIMPLE_PACKAGE_NAME
>  JSSHELL_NAME = jsshell-$(MOZ_PKG_PLATFORM).zip
> +else
> +JSSHELL_NAME := $(MOZ_SIMPLE_PACKAGE_NAME).jsshell.zip
> +endif

ditto
Attachment #8659445 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/0c9269ad0d26
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Depends on: 1228289
It would seem reasonable to still include product, locale, platform, architecture in filename, since this is static per task, and thus "predictable". The version number varies over time, so it seems fair to strip it.

In other words:

  export MOZ_SIMPLE_PACKAGE_NAME=firefox.en-US.linux-x86_64

seems preferable to:

  export MOZ_SIMPLE_PACKAGE_NAME=target


I'm not sure in which bugs the implementation is being done to use this new build tool feature - hence why I added here.

Certainly this seems like it would help with a class of problems such as seen in bug 1228289.
(In reply to Pete Moore [:pmoore][:pete] from comment #9)
> It would seem reasonable to still include product, locale, platform,
> architecture in filename, since this is static per task, and thus
> "predictable". The version number varies over time, so it seems fair to
> strip it.
> 
> In other words:
> 
>   export MOZ_SIMPLE_PACKAGE_NAME=firefox.en-US.linux-x86_64
> 
> seems preferable to:
> 
>   export MOZ_SIMPLE_PACKAGE_NAME=target

Except that's still hardcoded, so l10n-check will still be using the same name and overwrite the produced package.

But really, the non-MOZ_SIMPLE_PACKAGE_NAME is also predictable. It is $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).$(MOZ_PKG_PLATFORM) where:
- MOZ_PKG_APPNAME is set from MOZ_APP_NAME, which is either set in confvars.sh or derived as the lowercase form of MOZ_APP_BASENAME, which is set in confvars.sh.
- MOZ_PKG_VERSION is set from MOZ_APP_VERSION, which is derived from browser/config/version.txt
- AB_CD is the locale for the build
- MOZ_PKG_PLATFORM is fixed for each platform we build for.

It may not be convenient, but it is predictable.
bug 1198179 switched the TaskCluster builds to use this feature. You can put whatever you want in MOZ_SIMPLE_PACKAGE_NAME, the key is just that it's defined in the task, not by the build system, so it's predictable.
(In reply to Mike Hommey [:glandium] from comment #10)
> It may not be convenient, but it is predictable.

It's not *static* though, which is a PITA.
(In reply to Mike Hommey [:glandium] from comment #13)
> Testing a simple patch: https://hg.mozilla.org/try/rev/8eaaaba4798f

Which won't work because of the typo. Plus, this is the wrong bug for discussion.
See Also: → 1265537
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.