1418122 - Investigate necessity of UPLOAD_EXTRA_FILES handling in .mozconfig.mk

Status: NEW

(Firefox Build System :: General, enhancement)

Description

[Gregory Szorc [:gps]]

We have special code for handling UPLOAD_EXTRA_FILES in the auto-generated .mozconfig.mk file. I cargo culted it in bug 1417264. I suspect it is either not needed or can be refactored to something reasonable.

Comment 1

[(not currently active) Ted Mielczarek]

OK, so I tracked this back through some vcs history. glandium made a change in bug 1005495 to add `AC_SUBST(MOZ_UPLOAD_FILES)`:
https://hg.mozilla.org/mozilla-central/rev/41708cef5ec1

This broke some things: bug 1042432 removed that subst and added a special-case for it in client.mk:
https://hg.mozilla.org/mozilla-central/rev/fab1f203cab0

This caused an additional issue with the sccache log setup: bug 1188766 changed that and added the special code that you just touched:
https://hg.mozilla.org/mozilla-central/rev/0288a5a47478

Comment 2

[(not currently active) Ted Mielczarek]

I'm struggling to find what was actually broken in bug 1042432--I think maybe the UPLOAD_EXTRA_FILES bit there was from release automation? I'm pretty sure that has all changed drastically in the meantime, so maybe we don't have that problem anymore?

A search of current m-c doesn't show much usage of it:
https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=UPLOAD_EXTRA_FILES

Comment 3

[Gregory Szorc [:gps]]

This all feels like it is related to `make upload`, which mozharness and/or the build system used to call separately to facilitate file uploading mid-job. This was to facilitate kicking off tests before `make check` and friends started. In other words, a buildbot hack.

Now that we're in TaskCluster, our "upload a file" logic is literally "put a file in a directory." Sometimes you need to annotate that file's existence in Taskgraph land. But for most artifacts, we write it somewhere and we're done.

The only place I see us defining UPLOAD_EXTRA_FILES in mozconfig land is for sccache.log (build/mozconfig.cache). I'm not even sure why this is using UPLOAD_EXTRA_FILES. We could probably write sccache.log directly to the artifacts directory and be done with it.

Comment 4

[Mike Hommey [:glandium]]

Note it's sometimes useful to have a declarative way, in mozconfigs, to say "I want this file as an artifact". I've used that in the past. OTOH, this only really works when the build doesn't fail, and I've had more cases of "I want this file as an artifact when this fails" than "I want this file as an artifact when this is green"...