For bug 1100624, we will be pushing build artifacts to s3 from mozharness, so we need a way to communicate to mozharness what files should be uploaded. Right now I'm leaning towards writing the contents of UPLOAD_FILES into mach_build_properties.json, though we could also move all build artifacts into a specific directory.

The latter definitely sounds simpler. Do we still have things that need to preserve relative paths? (I can't remember.) It might also be nice at some point to move the definition of UPLOAD_FILES out of a Makefile maybe into moz.build, but I don't know how much work that is.

My only real concern with a specific upload directory is needing to have extra I/O copying things around (though I guess we can get away with symlinks on some platforms), or re-writing build rules to output in a specific directory. Probably not a big deal though - I'll give it a try. The mach_build_properties.json route has the downside of needing to know UPLOAD_FILES, which is currently defined in packager.mk. Including that in moz-automation.mk results in conflicting build targets :/. I think it's possible to move all the variables out into package-name.mk and just keep the rules in packager.mk, though. The UPLOAD_FILES logic is most of packager.mk - I'd prefer not to block this bug on a moz.build conversion if we can avoid it, since there's already a lot of moving pieces here.

Write out files containing a list of absolute paths to a well-defined location? Solving this with |echo filename >> file| seems like a pretty easy path forward.

(In reply to Gregory Szorc [:gps] from comment #3) > Write out files containing a list of absolute paths to a well-defined > location? Solving this with |echo filename >> file| seems like a pretty easy > path forward. This is pretty much what we'd achieve by putting it in mach_build_properties.json - that's how we communicate other bits of data from the build system to mozharness, so it would mostly be getting access to UPLOAD_FILES and adding a new argument to gen_mach_buildprops.py.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1) > Do we still have things that need to > preserve relative paths? (I can't remember.) I forgot to ask - what things are you concerned about here?

(In reply to Michael Shal [:mshal] from comment #5) > (In reply to Ted Mielczarek [:ted.mielczarek] from comment #1) > > Do we still have things that need to > > preserve relative paths? (I can't remember.) > > I forgot to ask - what things are you concerned about here? I'm thinking specifically of PKG_PATH I believe http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/package-name.mk#100 which is used in a few paths: http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#875 and we preserve relative paths in upload.py: http://dxr.mozilla.org/mozilla-central/source/build/upload.py#101

For the put-files-in-mach-build-properties route, we need access to UPLOAD_FILES (and later SOURCE_UPLOAD_FILES). This pulls out the data into upload-files.mk. We can't re-use package-name.mk for this since some other Makefiles include it, and it messes with DIST_FILES.

Add uploadFiles to the json file. Then mozharness can pull out the file list and upload them to s3.

I should mention, I've no idea of the impact on #c6 on these patches yet. As far as I can tell, it looks like all files get uploaded into a flat directory hierarchy. I'll need to dig a bit more, but if anyone has more knowledge about that please let me know!

Comment on attachment 8533968 [details] [diff] [review] 0001-Bug-1109136-Move-variable-definitions-from-packager..patch Review of attachment 8533968 [details] [diff] [review]: ----------------------------------------------------------------- Could you reemit this patch with upload-files.mk starting as a copy of packager.mk? That would make the review so much easier.

This time with hg copy ... The few extra differences in upload-files.mk are just some trailing spaces that I removed.

Turns out we need to unset DIST_FILES in moz-automation.mk, otherwise android builds break. r+ carried forward

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2bdb8a0cb8b0 inbound: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/9cc41324a5e1 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c207cbc339fd

https://hg.mozilla.org/mozilla-central/rev/9cc41324a5e1 https://hg.mozilla.org/mozilla-central/rev/c207cbc339fd

https://hg.mozilla.org/releases/mozilla-esr31/rev/2cc4b7b3d98a https://hg.mozilla.org/releases/mozilla-esr31/rev/d9711f1b2116

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/2d5a4861e5ac https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dd83f7689676

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c4455359445c https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/9d4c021fd0df