configure.py doesn't parse global autoconf options like --libdir

RESOLVED FIXED in Firefox 49

Status

Firefox Build System
General
RESOLVED FIXED
2 years ago
3 months ago

People

(Reporter: Ian Stakenvicius, Unassigned)

Tracking

48 Branch
mozilla51

Firefox Tracking Flags

(firefox49 fixed, firefox50 fixed, firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8765989 [details] [diff] [review]
add global autotools options to @old_configure_options

Starting with firefox-48.0_beta , configure.py now seems to parse all of the .mozconfig options before passing them through to ./old-configure; part of this parsing includes all of the options that ./old-configure normally accepts, as is specified in "@old_configure_options" from build/moz.configure/old.configure.  However, not in this list are the standard options that every autotools package presents, such as those that define basic system install paths:  --bindir, --libdir, --datadir, etc. etc. etc..

Although not all of these standard options are necessarily implemented, some of them (especially --libdir) need to be passed through on linux.  I took a guess at which ones might be useful based on Makefile.in's throughout the codebase, and made up the attached patch to try and help with this issue.
(Reporter)

Updated

2 years ago
Attachment #8765989 - Flags: review?(mh+mozilla)
Attachment #8765989 - Attachment is patch: true
Attachment #8765989 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 8765989 [details] [diff] [review]
add global autotools options to @old_configure_options

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

Please provide a commit message.

::: build/moz.configure/old.configure
@@ +154,4 @@
>  
>  
>  @old_configure_options(
> +    '--bindir',

Let's not add the ones that aren't used: bindir, libexecdir, mandir.
Attachment #8765989 - Flags: review?(mh+mozilla)
(Reporter)

Comment 2

2 years ago
Created attachment 8767718 [details] [diff] [review]
Add path specifier options to @old_configure_options

Try2 -- comment added, extra options removed
Attachment #8765989 - Attachment is obsolete: true
Attachment #8767718 - Flags: review?(mh+mozilla)
Attachment #8767718 - Flags: review?(mh+mozilla) → review+
Why is this path missing in tree? I guess distros (at least Fedora) needs that to build the package. Firefox is always installed to /usr/lib without this patch.
Flags: needinfo?(axs)
(Reporter)

Comment 4

2 years ago
(In reply to Martin Stránský from comment #3)
> Why is this path missing in tree? I guess distros (at least Fedora) needs
> that to build the package. Firefox is always installed to /usr/lib without
> this patch.

I'm not exactly understanding your question as phrased -- I will take it to mean you don't understand the use-case where firefox should be built with a 'libdir' path that isn't the default /usr/lib ?

/usr/lib is not the de-facto native-abi installation path on all linux systems, many often install to /usr/lib64 for instance if they support multilib (that is, concurrent 64bit and 32bit) installations (while /usr/lib32 is the 32bit path).

Also, it is quite possible that someone may want to build firefox with --libdir=/opt or some other path.  It often does not suffice to just use --prefix for these things, the other path overrides that autotools allows should also be exposed so that the codebase can be built appropriately.
Flags: needinfo?(axs)
Duplicate of this bug: 1295092
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/f5bfd9e3ebcf
Add ability to specify system paths to @old_configure_options. r=glandium
Keywords: checkin-needed
Comment on attachment 8767718 [details] [diff] [review]
Add path specifier options to @old_configure_options

Approval Request Comment
[Feature/regressing bug #]: regression from python configure
[User impact if declined]: Distros where /usr/lib64 is the library directory (e.g. Fedora, SuSE, etc.) can't build without going through hoops to support that directory. Most notably, that breaks finding system plugins.
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: Technically NPOTB: the patch only adds 3 supported flags to configure which aren't used for mozilla builds (if they were, the build would simply fail, because the flags aren't supported currently), and sends those flags to the still-existing autoconf-based configure.
[String/UUID change made/needed]: n/a
Attachment #8767718 - Flags: approval-mozilla-beta?
Attachment #8767718 - Flags: approval-mozilla-aurora?

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f5bfd9e3ebcf
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

Comment 9

2 years ago
Comment on attachment 8767718 [details] [diff] [review]
Add path specifier options to @old_configure_options

NPOTB, Aurora50+, Beta49+
Attachment #8767718 - Flags: approval-mozilla-beta?
Attachment #8767718 - Flags: approval-mozilla-beta+
Attachment #8767718 - Flags: approval-mozilla-aurora?
Attachment #8767718 - Flags: approval-mozilla-aurora+

Updated

2 years ago
status-firefox49: --- → affected
status-firefox50: --- → affected

Comment 10

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/be48ab980c56
status-firefox50: affected → fixed

Comment 11

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/c44faf03e1b4
status-firefox49: affected → fixed

Updated

3 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.