Closed Bug 1593948 Opened 1 year ago Closed 11 months ago

clobber needed on 71 beta source tarballs ?

Categories

(Firefox Build System :: General, defect)

71 Branch
All
OpenBSD
defect
Not set
normal

Tracking

(firefox-esr68 unaffected, firefox70 unaffected, firefox71+ fixed, firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 + fixed
firefox72 --- fixed

People

(Reporter: gaston, Assigned: mshal)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Our OpenBSD packaging system runs builds using plain 'make' and that used to be fine up to 70. in 71, make hits https://hg.mozilla.org/releases/mozilla-beta/file/tip/Makefile.in#l65 which fails the build from the beginning.

The CLOBBER file is the one present in the source tarball, and we're building out-of-src-tree as usual - i have no idea what changed in the build system for 71, but it would be nice to still be able to use traditional ./configure && make sequence instead of the mach madness.

I tried touching CLOBBER to make it old, removing it, updating it, no matter what, the target is still hit.

Flags: needinfo?(mh+mozilla)

This is a regression from bug 1496746. The CLOBBER rule only worked by chance before, because when objdir/CLOBBER doesn't exist, we picked srcdir/CLOBBER, and that made the dependencies happy.. Mike, can you take a look?

Flags: needinfo?(mh+mozilla) → needinfo?(mshal)
Regressed by: 1496746

I tried this out, and hit 2 separate problems. First, running configure directly with no options results in an empty MOZ_CONFIGURE_OPTIONS parameter, and although bug 1580533 is supposed to force MOZ_CONFIGURE_OPTIONS to be unicode, it doesn't work if there are no options. I think just encoding the result makes sense here. Note that './mach configure' sets MOZ_CONFIGURE_OPTIONS to be "MAKE=/usr/bin/make" even with an empty mozconfig, so we don't normally run into this unless configure is executed directly.

Second, I do hit the issue with there being no CLOBBER file. I'm not really sure of the right approach here, but creating it in configure.py if it doesn't exist does fix the issue. The part I'm unsure about is what the desired role for ensure_objdir_state() was in clobber.py, and whether or not skipping the objdir/CLOBBER file creation there has any side effects. I removed that function and updated the test cases to account for the fact that objdir/CLOBBER isn't created after a clobber, but now is only created during configure. I'd appreciate some extra review here to see if there's any use cases I'm missing.

Flags: needinfo?(mshal)
Assignee: nobody → mshal

Although this was mostly fixed by bug 1580533, if the result is []
because there are no options, then quote(*result) returns a string
rather than unicode since there are no unicode values to force the type.

If configure is invoked manually, clobber.py is bypassed and the CLOBBER
file doesn't get touched. The clobber check in Makefile.in gets
triggered causing the build to stop.

Moving the objdir/CLOBBER file creation into configure.py should cause
it to be created regardless of how configure is invoked.

Depends on D53290

Attachment #9109185 - Attachment is obsolete: true

[Tracking Requested - why for this release]: downstreams need to patch Makefile.in

Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/51204686aab6
Touch clobber in configure.py instead of clobber.py; r=firefox-build-system-reviewers,chmanchester
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Michael, do you want to make an uplift request for beta so as that it gets in 71?

Flags: needinfo?(mshal)

yes please, nice to have in 71.0b12 :)

Comment on attachment 9109186 [details]
Bug 1593948 - Touch clobber in configure.py instead of clobber.py; r?#firefox-build-system-reviewers

Beta/Release Uplift Approval Request

  • User impact if declined: If declined, it will be more difficult to package Firefox for OpenBSD.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch affects the user interface of the build system, not the browser itself. Since we are able to still build in automation and locally with this patch, it should be a low impact.
  • String changes made/needed:
Flags: needinfo?(mshal)
Attachment #9109186 - Flags: approval-mozilla-beta?

Comment on attachment 9109186 [details]
Bug 1593948 - Touch clobber in configure.py instead of clobber.py; r?#firefox-build-system-reviewers

Downstream patch, has tests and was on Nightly for a week, uplift approved for the beta branch before RC.

Attachment #9109186 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.