Closed Bug 1109136 Opened 9 years ago Closed 9 years ago

Give mozharness a list of artifacts to upload

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox-esr31 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed, b2g-v2.1S fixed)

RESOLVED FIXED
mozilla37
Tracking Status
firefox-esr31 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(2 files, 2 obsolete files)

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
Assignee: nobody → mshal
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.
Attachment #8533968 - Flags: review?(mh+mozilla)
Add uploadFiles to the json file. Then mozharness can pull out the file list and upload them to s3.
Attachment #8533969 - Flags: review?(mh+mozilla)
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.
Attachment #8533968 - Flags: review?(mh+mozilla)
Attachment #8533969 - Flags: review?(mh+mozilla) → review+
This time with hg copy ...

The few extra differences in upload-files.mk are just some trailing spaces that I removed.
Attachment #8533968 - Attachment is obsolete: true
Attachment #8534614 - Flags: review?(mh+mozilla)
Attachment #8534614 - Flags: review?(mh+mozilla) → review+
Blocks: 1112252
Turns out we need to unset DIST_FILES in moz-automation.mk, otherwise android builds break. r+ carried forward
Attachment #8533969 - Attachment is obsolete: true
Attachment #8537453 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/9cc41324a5e1
https://hg.mozilla.org/mozilla-central/rev/c207cbc339fd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.