move --enable{,-install}-strip to moz.configure

NEW
Assigned to

Status

()

Core
Build Config
11 months ago
11 months ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

This is a straightforward conversion; the only trickery is the bit
around --enable-install-strip.
Created attachment 8819352 [details] [diff] [review]
move --enable{,-install}-strip to moz.configure

I think I got this right; checking the config.status variables with various
combinations of options before and after seems to work.  --enable-install-strip
kind of makes my head hurt, though.
Attachment #8819352 - Flags: review?(cmanchester)
Comment on attachment 8819352 [details] [diff] [review]
move --enable{,-install}-strip to moz.configure

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

::: moz.configure
@@ +150,5 @@
>  
> +
> +option('--enable-strip',
> +       help='Enable stripping of libraries and executables.')
> +set_config('ENABLE_STRIP', depends('--enable-strip')(lambda x: bool(x)))

At first glance it looks like these both want to be `js_option`, because they're mentioned in js/src/old-configure.in, but then neither of the variables are mentioned under js/src, so this seems like the right thing. If that's intended let's add something to the commit message about it.

@@ +155,5 @@
> +
> +
> +option('--enable-install-strip', default=True,
> +       help='Enable stripping of libraries and executables when packaging')
> +set_config('PKG_SKIP_STRIP', depends('--enable-install-strip')(lambda x: not x))

It looks like we have a different default on Windows here due to http://searchfox.org/mozilla-central/rev/f680e72cc6579f90b992b63ca14d923d2afea612/old-configure.in#974
Attachment #8819352 - Flags: review?(cmanchester)

Comment 3

11 months ago
Comment on attachment 8819352 [details] [diff] [review]
move --enable{,-install}-strip to moz.configure

You want this in toolchain.configure, not top-level moz.configure.
(In reply to Mike Hommey [:glandium] from comment #3)
> Comment on attachment 8819352 [details] [diff] [review]
> move --enable{,-install}-strip to moz.configure
> 
> You want this in toolchain.configure, not top-level moz.configure.

Why does this distinction matter?  stripping binaries doesn't really feel like a toolchain-y thing.
Flags: needinfo?(mh+mozilla)

Comment 5

11 months ago
Stripping binaries doesn't happen if you don't have a toolchain. And the option makes no sense when you build with --disable-compile-environment.
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.