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.

Sign up for GitHub

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

Jump to bottom

Make configure script respect (and save) values for CC, CXX, CFLAGS etc. #13823

Merged
merged 1 commit into from
May 21, 2014
Merged

Make configure script respect (and save) values for CC, CXX, CFLAGS etc. #13823

merged 1 commit into from
May 21, 2014

Conversation

pnkfelix
Copy link
Member

Make configure script respect (and save) values for CC, CXX, CFLAGS etc.

I mostly tried to remain backwards compatible with old invocations of
the configure script; if you do not want to use CC et al., you
should not have to; you can keep using --enable-clang and/or
--enable-ccache.

The overall intention is to capture the following precedences for
guessing the C compiler:

  1. Value of CC at make invocation time.
  2. Value of CC at configure invocation time.
  3. Compiler inferred at configure invocation time (gcc or clang).

The strategy is to check (at configure time) if each of the
environment variables is set, and if so, save its value in a
corresponding CFG_ variable (e.g. CFG_CC).

The configure script also does not attempt to infer the compiler if
CC is set; but if --enable-clang was passed, then it does still
attempt to validate that the clang version is compatible.

Then, in the makefiles, if CC is not set but CFG_CC is, then we
use the CFG_CC setting as CC.

Fix #13805.

@pnkfelix
Copy link
Member Author

@sstewartgallus can you try applying this commit locally and seeing if it resolves your problems?

@pnkfelix pnkfelix changed the title Make configure script respect (and save) values for CC, CXX, `CFLAGS... Make configure script respect (and save) values for CC, CXX, CFLAGS etc. Apr 28, 2014
@alexcrichton
Copy link
Member

It seems that travis has some problems, which may be a result of CC=gcc and ./configure --enable-clang.

@mstewartgallus
Copy link
Contributor

@pnkfelix Thank you very much for trying to fix this but after trying the PATH workaround I found that not only did Rust's LLVM need a newer GCC but that it seem to need a newer C++ runtime as well.

@pnkfelix
Copy link
Member Author

@alexcrichton ha ha, of course, on my newish OS X machine, gcc --version actually runs clang, so I cannot replicate that problem on the outermost OS.

I'll try it again in a VM (or in a Linux boot) tomorrow and fix that up, thanks for pointing it out.

@pnkfelix
Copy link
Member Author

So I can see a few ways to try to address the problem noted by @alexcrichton on travis.

The problem: travis for some reason does CC=gcc ./configure --enable-clang. I was assuming that if someone sets CC, then they really want to use that as their compiler, and the --enable-clang thing should not change the choice of CC. However, I also figured we should preserve the current checks that validate that we are using a sufficiently new version of clang, and the way I decided to drive that check by the presence of the --enable-clang option.

In short: I was assuming that no one would both set CC=gcc and pass --enable-clang, but Travis does.

So, here are the main solutions I can see that still preserve the properties mentioned above (namely: that setting CC takes precedence, and we still want to try to check the clang version when reasonable):

Options:

  1. Only do the clang version check during the C compiler inference code (i.e. when CC was unset) and when --enable-clang was passed, or
  2. Have the clang version check be driven by the value of CC (be it inferred or explicitly provided) rather than by whether --enable-clang was passed. E.g. perhaps if CC ends with "clang" (so that "ccache clang" would match) then do the version check. If you pass some name for CC that does not match but is an alias for clang (e.g. a symlink or local build with a different name), then you don't get the configure-time check: its up to you to know what you're doing in that case.

Option 1 is very conservative. Option 2 seems better to me. I think I will adopt Option 2.

@pnkfelix
Copy link
Member Author

pnkfelix commented May 1, 2014

Okay I implemented option 2, and cleaned up the code some. I've tested this on my Mac and on a Linux VM. (On my mac the LLVM sub-configure step fails if I feed it CC=gcc with a message about the std lib not being a sufficiently new version, but it is fine with CC=clang or not providing a CC at all, which I am satisfied with.)

r? anyone.

@eliovir eliovir mentioned this pull request May 2, 2014
Closed
@pnkfelix
Copy link
Member Author

pnkfelix commented May 2, 2014

hmm, from rust-rosetta/rust-rosetta#44, apparently this ticket is higher priority than I had realized. :)

@pnkfelix pnkfelix mentioned this pull request May 2, 2014
Closed
@pnkfelix
Copy link
Member Author

pnkfelix commented May 2, 2014

travis approves! r? anyone? :)

@eliovir
Copy link
Contributor

eliovir commented May 6, 2014

Yes, it is important: there is not any new package for rust-nightly for Ubuntu precise since April, 18th!

So

  • all Travis-CI instances are failing due to new features/language or stdlib changes.
  • on such platform, we can't try new Rust.

Thanks for fixing.

@pnkfelix
Copy link
Member Author

pnkfelix commented May 7, 2014

part of #8058

@pnkfelix
Copy link
Member Author

pnkfelix commented May 7, 2014

(I will rebase and attempt to land this again after #14000 lands.)

@hansjorg hansjorg mentioned this pull request May 9, 2014
Closed
@pnkfelix
Copy link
Member Author

okay, #14000 has landed, time to rebase this.

@pnkfelix
Make configure respect (and save) values for CC, CXX, CFLAGS, etc.
ae67b74 
I mostly tried to remain backwards compatible with old invocations of
the `configure` script; if you do not want to use `CC` et al., you
should not have to; you can keep using `--enable-clang` and/or
`--enable-ccache`.

The overall intention is to capture the following precedences for
guessing the C compiler:

 1. Value of `CC` at make invocation time.
 2. Value of `CC` at configure invocation time.
 3. Compiler inferred at configure invocation time (`gcc` or `clang`).

The strategy is to check (at `configure` time) if each of the
environment variables is set, and if so, save its value in a
corresponding `CFG_` variable (e.g. `CFG_CC`).

Then, in the makefiles, if `CC` is not set but `CFG_CC` is, then we
use the `CFG_CC` setting as `CC`.

Also, I fold the potential user-provided `CFLAGS` and `CXXFLAGS`
values into all of the per-platform `CFLAGS` and `CXXFLAGS` settings.
(This was opposed to adding `$(CFLAGS)` in an ad-hoc manner to various
parts of the mk files.)

Fix rust-lang#13805.

----

Note that if you try to set the compiler to clang via the `CC` and
`CXX` environment variables, you will probably need to also set
`CXXFLAGS` to `--enable-libcpp` so that LLVM will be configured
properly.

----

Introduce CFG_USING_CLANG, which is distinguished from
CFG_ENABLE_CLANG because the former represents "we think we're using
clang, choose appropriate warning-control options" while the latter
represents "we asked configure (or the host required) that we attempt
to use clang, so check that we have an appropriate version of clang."

The main reason I added this is that I wanted to allow the user to
choose clang via setting the `CC` environment variable, but I did not
want that method of selection to get confused with the user passing
the `--enable-clang` option.

----

A digression: The `configure` script does not infer the compiler
setting if `CC` is set; but if `--enable-clang` was passed, then it
*does* still attempt to validate that the clang version is compatible.

Supporting this required revising `CLANG_VERSION` check to be robust
in face of user-provided `CC` value.

In particular, on Travis, the `CC` is set to `gcc` and so the natural
thing to do is to attempt to use `gcc` as the compiler, but Travis is
also passing `--enable-clang` to configure.  So, what is the right
answer in the face of these contradictory requests?

One approach would be to have `--enable-clang` supersede the setting
for `CC` (and instead just call whatever we inferred for `CFG_CLANG`).
That sounds maximally inflexible to me (pnkfelix): a developer
requesting a `CC` value probably wants it respected, and should be
able to set it to something else; it is harder for that developer to
hack our configure script to change its inferred path to clang.

A second approach would be to blindly use the `CC` value but keep
going through the clang version check when `--enable-clang` is turned
on.  But on Travis (a Linux host), the `gcc` invocation won't print a
clang version, so we would not get past the CLANG_VERSION check in
that context.

A third approach would be to never run the CLANG_VERSION check if `CC`
is explicitly set.  That is not a terrible idea; but if the user uses
`CC` to pass in a path to some other version of clang that they want
to test, probably should still send that through the `CLANG_VERSION`
check.

So in the end I (pnkfelix) took a fourth approach: do the
CLANG_VERSION check if `CC` is unset *or* if `CC` is set to a string
ending with `clang`.  This way setting `CC` to things like
`path/to/clang` or `ccache clang` will still go through the
CLANG_VERSION check, while setting `CC` to `gcc` or some unknown
compiler will skip the CLANG_VERSION check (regardless of whether the
user passed --enable-clang to `configure`).

----

Drive-by fixes:

* The call that sets `CFG_CLANG_VERSION` was quoting `"$CFG_CC"` in
  its invocation, but that does not play nicely with someone who sets
  `$CFG_CC` to e.g. `ccache clang`, since you do not want to intepret
  that whole string as a command.

  (On the other hand, a path with spaces might need the quoted
  invocation.  Not sure which one of these corner use-cases is more
  important to support.)

* Fix chk_cc error message to point user at `gcc` not `cc`.
@pnkfelix
Copy link
Member Author

r? @alexcrichton

fyi this did (eventually) pass the try server, once I wised up about my handling of CFLAGS: you can see the epic tale here: http://buildbot.rust-lang.org/console?branch=try&refresh=15

bors added a commit that referenced this pull request May 21, 2014
@bors
auto merge of #13823 : pnkfelix/rust/fsk-fix-13805, r=alexcrichton
f30382d 
Make configure script respect (and save) values for `CC`, `CXX`, `CFLAGS` etc.

I mostly tried to remain backwards compatible with old invocations of
the `configure` script; if you do not want to use `CC` et al., you
should not have to; you can keep using `--enable-clang` and/or
`--enable-ccache`.

The overall intention is to capture the following precedences for
guessing the C compiler:

 1. Value of `CC` at `make` invocation time.
 2. Value of `CC` at `configure` invocation time.
 3. Compiler inferred at configure invocation time (`gcc` or `clang`).

The strategy is to check (at `configure` time) if each of the
environment variables is set, and if so, save its value in a
corresponding `CFG_` variable (e.g. `CFG_CC`).

The `configure` script also does not attempt to infer the compiler if
`CC` is set; but if `--enable-clang` was passed, then it *does* still
attempt to validate that the clang version is compatible.

Then, in the makefiles, if `CC` is not set but `CFG_CC` is, then we
use the `CFG_CC` setting as `CC`.

Fix #13805.
@bors bors merged commit ae67b74 into rust-lang:master May 21, 2014
@alexcrichton
Copy link
Member

I just noticed today, but this appears to have caused the freebsd build to break. (see http://buildbot.rust-lang.org/builders/auto-bsd-64-opt for the list).

It looks like it's attempting to use g++ to compile our LLVM glue, when g++ doesn't exist. The weird part is that the configure script picked up clang++ and exported it!

@alexcrichton
Copy link
Member

For now I've configured the bsd bots to have CXX=clang++, but I can turn it off so you can test a fix if necessary.

@pnkfelix
Copy link
Member Author

hmm interesting I think I just thought that freebsd was already broken. But i guess that was not the case.

I'll file a ticket for this.

@pnkfelix
Copy link
Member Author

filed as #14381

arcnmx pushed a commit to arcnmx/rust that referenced this pull request Jan 9, 2023
@bors
Auto merge of rust-lang#13853 - veber-alex:diag_fix, r=Veykril
50801b7 
Use diagnostic code as link to full message

fixes rust-lang#13823 by adding a vscode setting that will keeping the existing diagnostic code and use it as a link to the full compiler error message.
While I was there I also fixed `index` to fallback to `rendered.length` to make the previewRustcOutput feature work.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 15, 2024
@xFrednet
chore: use multipart_suggestion in significant_drop_tightening lint (r…
5ac1805 
…ust-lang#13823)

This addresses rust-lang/rust-clippy#13099 for
the significant_drop_tightening lint.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No way to specify custom C and C++ compilers in configuration
5 participants
@pnkfelix @alexcrichton @mstewartgallus @eliovir @bors