sccache setup with UPLOAD_EXTRA_FILES makes some files uploaded multiple times

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla43
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Assignee: nobody → mh+mozilla
Attachment #8640347 - Flags: review?(mshal)
Comment on attachment 8640347 [details] [diff] [review]
Unexport UPLOAD_EXTRA_FILES from upload-files.mk

Mmmm this worked locally, but doesn't seem to work on try.
Attachment #8640347 - Flags: review?(mshal)
The way UPLOAD_EXTRA_FILES is currently exported makes the files added
to the list added in each directory that is being traversed recursively
because of the += in .mozconfig.mk, combined with "export".

The easy way out is to remove the export altogether, but being unsure of
the side effects of putting all non exported mk_add_options variables
in .mozconfig.mk, limit this to UPLOAD_EXTRA_FILES.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=21829aaf72e1
Attachment #8640347 - Attachment is obsolete: true
Sorry, previous patch still had debugging $(warning)s.
Attachment #8644177 - Attachment is obsolete: true
Attachment #8644179 - Flags: review?(mshal)
Can we go back to something similar to the pre-bug-1042432 way of passing the filenames to make and just use a new variable to avoid the export issue? The complicated interactions with export+make and the fact that we'd have to special-case UPLOAD_EXTRA_FILES in client.mk makes me think we're going about this in a weird way.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=545e57b20c93
Attachment #8644667 - Flags: review?(mh+mozilla)
Comment on attachment 8644667 [details] [diff] [review]
0001-Bug-1188766-Use-UPLOAD_MOZCONFIG_FILES-for-files-add.patch

Review of attachment 8644667 [details] [diff] [review]:
-----------------------------------------------------------------

The problem is that this approach had its own problem. Note that UPLOAD_EXTRA_FILES is special cased in my patch because I don't want to figure out now what the consequences are of putting all non export variables in .mozconfig.mk, but the end goal would be for that to happen, so my patch actually removes the weird special-casing there was for UPLOAD_EXTRA_FILES, although it uses a temporary different special-case to do so.
Attachment #8644667 - Flags: review?(mh+mozilla) → review-
Can you clarify what problem that is with this approach? I realize this was problematic when we appended to "UPLOAD_EXTRA_FILES" both in the mozconfig and in the Makefile, but this uses a separate variable that is only set by the mozconfig and read by the Makefile.
Flags: needinfo?(mh+mozilla)
To be honest, I don't remember what was going wrong with the state pre-bug-1042432, but the fact is that it only happened on release/beta, not on try, and that just by looking at the state pre-bug-1042432, I have no idea why this even failed in the first place, so I want to be extra cautious not to reintroduce the conditions that made it fail in the first place. We do know that the .mozconfig.mk trick works, and the only caveat it has is the fact that there is an "export" in there, which my patch removes.
Flags: needinfo?(mh+mozilla)
And as I said, I want for all mk_add_options to end up in mozconfig.mk eventually, I just don't want to deal with unexpected side effects from that just to fix this particular issue.
Comment on attachment 8644179 [details] [diff] [review]
Avoid exporting UPLOAD_EXTRA_FILES from mozconfig

Fair enough.
Attachment #8644179 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/0288a5a47478
https://hg.mozilla.org/mozilla-central/rev/cd0fd190e964
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
url:        https://hg.mozilla.org/comm-central/rev/b1c282fe435af169b2a46df96aed132eadb95b87
changeset:  b1c282fe435af169b2a46df96aed132eadb95b87
user:       aleth <aleth@instantbird.org>
date:       Wed Aug 12 01:14:53 2015 +0200
description:
Keep build/ in sync (port bug 1188766). rs=bustage-fix
Blocks: 1195246
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.