Closed Bug 1357562 Opened 8 years ago Closed 8 years ago

Do not replace any variant builds with artifact builds

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

See dependent bugs. There may be other places this is happening, we need to make this do the right thing by default, probably by opting-in to artifact build replacement rather than out as Ted suggests in bug 1349180.
Hm, this is kind of tricky given how mozharness configs are structured as I understand it: the base configs contain what's necessary for a vanilla opt build, and that's added to or overridden by a sub config for any variany build (mozharness considers debug to be a variant build). So to specify this in the base config would be to specify it for the remaining variants, and we're back to opting out in variant configs.
Assignee: nobody → cmanchester
Comment on attachment 8861172 [details] Bug 1357562 - Specify artifact build replacement via mozharness for non-variant builds. https://reviewboard.mozilla.org/r/133124/#review135994 ::: testing/mozharness/scripts/fx_desktop_build.py:142 (Diff revision 1) > # > # This is temporary, until we find a way to introduce an "artifact > # build dimension" like "opt"/"debug" into the CI configurations. > self.info('Artifact build requested in try syntax.') > > - default = 'artifact' > + original_variant = c.get('build_variant') This is really ``` DEFAULT_VARIANT = 'debug-artifact' if c.get('build_variant') in DEBUG_VARIANTS else 'artifact' c.get('artifact_flag_build_variant_in_try', DEFAULT_VARIANT) ``` right? I feel like this could be simplified, but I leave it to your judgement. (I think I worked through the cases to verify your refactor, but it's late and I'm tired.)
Attachment #8861172 - Flags: review?(nalexander) → review+
Comment on attachment 8861173 [details] Bug 1357562 - Remove 'artifact_build_variant_in_try': None entries where no longer necessary. https://reviewboard.mozilla.org/r/133126/#review135996 ::: commit-message-0c9e6:3 (Diff revision 1) > +Bug 1357562 - Remove 'artifact_build_variant_in_try': None entries where no longer necessary. r=nalexander > + > +A previous commit changed our default behavior to make these entries no longer nit: _the_ previous commit, or just fold this into the previous commit.
Attachment #8861173 - Flags: review?(nalexander) → review+
Comment on attachment 8861174 [details] Bug 1357562 - Prevent windows clang builds from being replaced by an artifact build. https://reviewboard.mozilla.org/r/133128/#review135998 ::: commit-message-98500:3 (Diff revision 1) > +Bug 1357562 - Prevent windows clang builds from being replaced by an artifact build. r=nalexander > + > +These builds do not have a distinct variant but they do not have an artifact nit: s/but/and/, or expand: "These builds do not have a distinct variant, so they default to 'artifact' when --artifact is passed to `mach try`, but they don't have an artifact build equivalent so ...
Attachment #8861174 - Flags: review?(nalexander) → review+
Comment on attachment 8861172 [details] Bug 1357562 - Specify artifact build replacement via mozharness for non-variant builds. https://reviewboard.mozilla.org/r/133124/#review135994 > This is really > ``` > DEFAULT_VARIANT = 'debug-artifact' if c.get('build_variant') in DEBUG_VARIANTS else 'artifact' > c.get('artifact_flag_build_variant_in_try', DEFAULT_VARIANT) > ``` > right? I feel like this could be simplified, but I leave it to your judgement. (I think I worked through the cases to verify your refactor, but it's late and I'm tired.) Not exactly... the essence of this change is that if we have a variant and it's not in DEBUG_VARIANTS we don't provide a replacement by default. I'll see if this can be simplified to be more obvious.
Attachment #8861172 - Flags: review+ → review?(nalexander)
Comment on attachment 8861172 [details] Bug 1357562 - Specify artifact build replacement via mozharness for non-variant builds. https://reviewboard.mozilla.org/r/133124/#review137010 If you're happy, I am too!
Attachment #8861172 - Flags: review?(nalexander) → review+
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/50295481fb98 Specify artifact build replacement via mozharness for non-variant builds. r=nalexander https://hg.mozilla.org/integration/autoland/rev/4b9adc99502a Remove 'artifact_build_variant_in_try': None entries where no longer necessary. r=nalexander https://hg.mozilla.org/integration/autoland/rev/2d6216823293 Prevent windows clang builds from being replaced by an artifact build. r=nalexander
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: