Move parts of configure.py into sandboxed moz.configure

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 3 bugs)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(4 attachments)

Configure.py currently handles reading the mozconfig to find autoconf, run autoconf, and execute the old configure.
Most of this should move into sandboxed moz.configure.
Blocks: 1250296
Blocks: 1251497
Blocks: 1250301
Depends on: 1247836
Depends on: 1253464
Depends on: 1253466
Blocks: 1253502
The upcoming move of the configure.py initialization to sandboxed
moz.configure changes the path separators for topsrcdir and topobjdir
from native to always use forward-slashes, which confuses the hell out
of the test_base.py test. Settle the issue by declaring that
MozbuildObject will always use forward-slashed paths for topsrcdir
and topobjdir.

Review commit: https://reviewboard.mozilla.org/r/38107/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38107/
Attachment #8726621 - Flags: review?(gps)
Generally speaking, the configuration needs forward-slashes in paths.
We might as well make it hard(er) to set configuration items with
backslash separators on Windows by exposing mozpath.* functions in place
of os.path functions. The downside is that functions explicitly
importing os will still get the real os.path functions.

Review commit: https://reviewboard.mozilla.org/r/38109/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38109/
Attachment #8726622 - Flags: review?(nalexander)
Attachment #8726622 - Flags: review?(cmanchester)
comm-central doesn't set it since bug 1040009.

Review commit: https://reviewboard.mozilla.org/r/38111/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38111/
Attachment #8726623 - Flags: review?(nalexander)
Attachment #8726623 - Flags: review?(cmanchester)
This moves all the reading mozconfig, finding autoconf, refreshing the
old configure, and running the old configure into sandboxed
moz.configure. This effectively bootstraps the sandboxed python configure.

Review commit: https://reviewboard.mozilla.org/r/38113/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38113/
Attachment #8726624 - Flags: review?(nalexander)
Attachment #8726624 - Flags: review?(cmanchester)
Blocks: 1253553
Comment on attachment 8726622 [details]
MozReview Request: Bug 1253203 - Use mozpath functions for sandboxed os.path in configure.py

https://reviewboard.mozilla.org/r/38109/#review34867
Attachment #8726622 - Flags: review?(nalexander) → review+
Comment on attachment 8726623 [details]
MozReview Request: Bug 1253203 - Remove everything COMM_BUILD

https://reviewboard.mozilla.org/r/38111/#review34869
Attachment #8726623 - Flags: review?(nalexander) → review+
https://reviewboard.mozilla.org/r/38107/#review34875

::: python/mozbuild/mozbuild/test/test_base.py:63
(Diff revision 1)
> -            self.assertTrue(base.topobjdir.startswith(topsrcdir))
> +            print(base.topobjdir, base.topsrcdir)

Left over print?
Comment on attachment 8726622 [details]
MozReview Request: Bug 1253203 - Use mozpath functions for sandboxed os.path in configure.py

https://reviewboard.mozilla.org/r/38109/#review34877
Attachment #8726622 - Flags: review?(cmanchester) → review+
Comment on attachment 8726623 [details]
MozReview Request: Bug 1253203 - Remove everything COMM_BUILD

https://reviewboard.mozilla.org/r/38111/#review34879
Attachment #8726623 - Flags: review?(cmanchester) → review+
Comment on attachment 8726624 [details]
MozReview Request: Bug 1253203 - Move parts of configure.py into sandboxed moz.configure

https://reviewboard.mozilla.org/r/38113/#review34873

This is a mind-bending patch.

::: build/moz.configure/init.configure:71
(Diff revision 1)
> +def mozconfig(current_project, mozconfig,  build_env, help):

Why do you depend on, and then not use, `help`?

::: configure.py:32
(Diff revision 1)
> -        for k, v in raw_config['substs']
> +        if k not in ('DEFINES', 'non_global_defines', 'TOPSRCDIR', 'TOPOBJDIR')

nit: indentation should be improved here.
Attachment #8726624 - Flags: review?(nalexander) → review+
Attachment #8726624 - Flags: review?(cmanchester) → review+
Comment on attachment 8726624 [details]
MozReview Request: Bug 1253203 - Move parts of configure.py into sandboxed moz.configure

https://reviewboard.mozilla.org/r/38113/#review35111

::: build/moz.configure/init.configure:89
(Diff revision 1)
> +@advanced
> +def command_line_helper():

Why does this need @advanced?

::: build/moz.configure/init.configure:100
(Diff revision 1)
> +        helper = command_line_helper()

Can we inline this?
Attachment #8726621 - Flags: review?(gps) → review+
Comment on attachment 8726621 [details]
MozReview Request: Bug 1253203 - Normalize path separators in MozbuildObject

https://reviewboard.mozilla.org/r/38107/#review35179

Looks good aside from the print() chmanchester also found.
https://reviewboard.mozilla.org/r/38113/#review35111

> Why does this need @advanced?

without @advanced, this fails with: RuntimeError: restricted attribute

> Can we inline this?

Unfortunately no, because @depends functions don't expose depends, or any other function that would allow the same kind of trick to access the CommandLineHelper.
Blocks: 1254410
Depends on: 1254795
This seems to have broken builds on Mac OS X with homebrew's autoconf213.
I specify:
  mk_add_options AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213
in my .mozconfig, and get the error:
  js/src> Could not find autoconf 2.13

There's also a typo on https://hg.mozilla.org/mozilla-central/rev/e654e71b61b5#l4.53
  if find:
should be
  if fink:
Maybe we can integrate something like https://bug1250294.bmoattachments.org/attachment.cgi?id=8724127 so that all the homebrew users won't need to specify AUTOCONF in our .mozconfigs, and old.configure will just find it automatically?  Please?
Please file a followup?
Depends on: 1256528
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.