Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure: Support --disable-option-checking #31169

Merged
merged 2 commits into from Apr 22, 2016
Merged

configure: Support --disable-option-checking #31169

merged 2 commits into from Apr 22, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jan 24, 2016

I'm trying to package Rust for Fedora (this is nothing official (yet)).

The standard RPM packaging process involves running ./configure with a whole lot of options that are commonly recognized by autotools configure scripts, but not by Rust's one. Since it does not make much sense to support all of this options, I think it would be great to support at least --disable-option-checking, so Rust's configure script would not fail.

The old attempt to package Rust used a sed script (at line 72), but this is not the recommended way to do that.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned nikomatsakis Jan 25, 2016
@brson
Copy link
Contributor

brson commented Jan 29, 2016

@gmbonnet Do you know which options Fedora is passing that we don't support, and also why Fedora can't be told not to pass them?

@@ -580,6 +580,7 @@ fi
BOOL_OPTIONS=""
VAL_OPTIONS=""

opt option-checking 1 "complain about unrecognized options"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this at the end of the list so it's displayed last in the UI?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. Should I move it at the end of all options or just after most --enable/--disable-style options (before --localstatedir) ?

@ghost
Copy link
Author

ghost commented Jan 29, 2016

In RPM, configure scripts are called via the %configure macro. On my system, this macro expands to:

  CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -I/usr/lib64/gfortran/modules}" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -I/usr/lib64/gfortran/modules}" ; export FCFLAGS ; 
  LDFLAGS="${LDFLAGS:--Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld}"; export LDFLAGS; 
  [ "1" = 1 ] && for i in $(find $(dirname ./configure) -name config.guess -o -name config.sub) ; do 
      [ -f /usr/lib/rpm/redhat/$(basename $i) ] && /usr/bin/rm -f $i && /usr/bin/cp -fv /usr/lib/rpm/redhat/$(basename $i) $i ; 
  done ; 
  [ "1" = 1 ] && [ x != "x-specs=/usr/lib/rpm/redhat/redhat-hardened-ld" ] && 
      for i in $(find . -name ltmain.sh) ; do 
        /usr/bin/sed -i.backup -e 's~compiler_flags=$~compiler_flags="-specs=/usr/lib/rpm/redhat/redhat-hardened-ld"~' $i 
      done ; 
  ./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu \
    --program-prefix= \
    --disable-dependency-tracking \
    --prefix=/usr \
    --exec-prefix=/usr \
    --bindir=/usr/bin \
    --sbindir=/usr/sbin \
    --sysconfdir=/etc \
    --datadir=/usr/share \
    --includedir=/usr/include \
    --libdir=/usr/lib64 \
    --libexecdir=/usr/libexec \
    --localstatedir=/var \
    --sharedstatedir=/var/lib \
    --mandir=/usr/share/man \
    --infodir=/usr/share/info

Values of options may depend on the target platform. I also believe that this macro's value may differ on other RPM based distributions.

It would be possible to do not use the macro at all and to run the configure script with only needed options and environment variables, but this would be error-prone since options added in newer versions of Rust would not be set by default.

@ghost
Copy link
Author

ghost commented Feb 3, 2016

I moved the flag to the location that seems the more appropriate (--enable/--disable-style options seem to be grouped in the source file), but feel free to say me if it should really be the very last option in the list.

@bors
Copy link
Contributor

bors commented Feb 12, 2016

☔ The latest upstream changes (presumably #31123) made this pull request unmergeable. Please resolve the merge conflicts.

@ghost
Copy link
Author

ghost commented Feb 13, 2016

I rebased this pull request.

@alexcrichton
Copy link
Member

ping r? @brson

(just doing a routine cleanup of the queue)

@bors
Copy link
Contributor

bors commented Mar 18, 2016

☔ The latest upstream changes (presumably #32080) made this pull request unmergeable. Please resolve the merge conflicts.

@ghost
Copy link
Author

ghost commented Mar 19, 2016

It's rebased again.

@bors
Copy link
Contributor

bors commented Apr 3, 2016

☔ The latest upstream changes (presumably #32168) made this pull request unmergeable. Please resolve the merge conflicts.

@brson
Copy link
Contributor

brson commented Apr 20, 2016

@bors r+

Sorry for the long delay.

@bors
Copy link
Contributor

bors commented Apr 20, 2016

📌 Commit 1a732c5 has been approved by brson

@bors
Copy link
Contributor

bors commented Apr 22, 2016

🔒 Merge conflict

@ghost
Copy link
Author

ghost commented Apr 22, 2016

Sorry, I forgot to rebase my commits. Now it's done.

@alexcrichton
Copy link
Member

@bors: r=brson 46b75eb

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 22, 2016
…=brson

configure: Support --disable-option-checking

I'm trying to package Rust for Fedora (this is nothing official (yet)).

The standard RPM packaging process involves running `./configure` with a whole lot of options that are commonly recognized by autotools configure scripts, but not by Rust's one. Since it does not make much sense to support all of this options, I think it would be great to support at least `--disable-option-checking`, so Rust's configure script would not fail.

[The old attempt](https://github.com/fabiand/rust-spec/blob/master/rustc.spec) to package Rust used a sed script (at line 72), but this is not the recommended way to do that.
@bors
Copy link
Contributor

bors commented Apr 22, 2016

⌛ Testing commit 46b75eb with merge af000a7...

bors added a commit that referenced this pull request Apr 22, 2016
configure: Support --disable-option-checking

I'm trying to package Rust for Fedora (this is nothing official (yet)).

The standard RPM packaging process involves running `./configure` with a whole lot of options that are commonly recognized by autotools configure scripts, but not by Rust's one. Since it does not make much sense to support all of this options, I think it would be great to support at least `--disable-option-checking`, so Rust's configure script would not fail.

[The old attempt](https://github.com/fabiand/rust-spec/blob/master/rustc.spec) to package Rust used a sed script (at line 72), but this is not the recommended way to do that.
@alexcrichton
Copy link
Member

All tests passed but I confused bors, so merging manually.

@alexcrichton alexcrichton merged commit 46b75eb into rust-lang:master Apr 22, 2016
cuviper added a commit to cuviper/cargo that referenced this pull request Jul 24, 2016
This is mirroring rust-lang/rust#31169.  The RPM %configure macro sets a
lot of useful paths for typical configure scripts, but some of these
values are not recognized here in Cargo.  It's nice to have an option to
ignore those, rather than failing on `validate_opt`.
bors added a commit to rust-lang/cargo that referenced this pull request Jul 25, 2016
configure: Support --disable-option-checking

This is mirroring rust-lang/rust#31169.  The RPM %configure macro sets a
lot of useful paths for typical configure scripts, but some of these
values are not recognized here in Cargo.  It's nice to have an option to
ignore those, rather than failing on `validate_opt`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants