-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Replace ./configure with config.toml in README.md and CONTRIBUTING.md #40056
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
If you're running inside of an msys shell, however, you can run: | ||
|
||
```sh | ||
$ ./configure --build=x86_64-pc-windows-msvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifying x86_64-pc-windows-msvc
is necessary, otherwise x86_64-pc-windows-gnu
will be built.
README.md
Outdated
@@ -18,7 +18,6 @@ Read ["Installing Rust"] from [The Book]. | |||
|
|||
* `g++` 4.7 or later or `clang++` 3.x | |||
* `python` 2.7 (but not 3.x) | |||
* GNU `make` 3.81 or later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make
is required for running test suite (run-make
tests)
CONTRIBUTING.md
Outdated
./configure | ||
``` | ||
To change configuration, you must copy the file `src/bootstrap/config.toml.example` | ||
to `config.toml` in the project root, and change the settings provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.toml
should be in the build directory, not in the project root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean ./build/config.toml when you say this? Or "directory from which you will be running the build", as the config.toml.example file states?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directory from which you will be running the build
CONTRIBUTING.md
Outdated
|
||
To see a full list of options, run `./configure --help`. | ||
#### `[rust]`: | ||
- `debuginfo = true` - Build a debug version of the compiler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could say "Build a compiler with debuginfo" b/c this doesn't disable optimizations or enable debug assertions
CONTRIBUTING.md
Outdated
@@ -185,6 +189,9 @@ To learn about all possible rules you can execute, run: | |||
python x.py build --help --verbose | |||
``` | |||
|
|||
Note: Previously `./configure` and `make` were used to build this project, | |||
but these are now deprecated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Deprecated" is a little strong here I think because we don't plan on ever removing them. Moreso we just recommend "x.py" for a better build experience nowadays.
README.md
Outdated
@@ -35,15 +35,16 @@ Read ["Installing Rust"] from [The Book]. | |||
3. Build and install: | |||
|
|||
```sh | |||
$ ./configure | |||
$ ./x.py build && sudo ./x.py dist --install | |||
$ make && sudo make install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be deleted I believe?
README.md
Outdated
@@ -117,8 +116,7 @@ shell with: | |||
If you're running inside of an msys shell, however, you can run: | |||
|
|||
```sh | |||
$ ./configure --build=x86_64-pc-windows-msvc | |||
$ make && make install | |||
$ ./x.py build --build=x86_64-pc-windows-msvc && ./x.py dist --install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may not work because the second command doesn't have --build
. Perhaps this could just recommend changing the rust.build
value in config.toml
?
Looks great, thanks! Could we perhaps still mention in the README that for those who prefer the ./configure + make system still exists? |
Nice, this will fix #39739 |
README.md
Outdated
$ ./configure --build=x86_64-pc-windows-msvc | ||
$ make && make install | ||
$ ./x.py build --build=x86_64-pc-windows-msvc | ||
$ ./x.py dist --build=x86_64-pc-windows-msvc --install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong. Maybe we should just get rid of this msys section? It is no longer relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it no longer relevant? MSYS2 is still the best environment to work with both GNU and MSVC flavors.
Because PowerShell is goddamn horrible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a configuration that is recommended for new users on Windows? If not, it's probably fine removing it. (I don't do any dev on Windows, so I don't know the way to go here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really matter whether Msys2 is a good choice or not. The fact is that with rustbuild the commands are identical no matter what shell you're in (aside from python x.py
vs ./x.py
), so there's really no point in covering msys2 anymore. There was a point when this section mentioned you could use configure and make in msys2, but since it uses direct rustbuild now it provides no useful information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it just mention --build=x86_64-pc-windopws-msvc
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commands are identical no matter what shell you're in
Default build triples are different.
That's why you need to explicitly specify x86_64-pc-windopws-msvc
in msys/mingw and x86_64-pc-windopws-gnu
in powershell/cmd.
This is probably the only notable difference in commandlines, but it needs to be mentioned.
--build=x86_64-pc-windopws-msvc
looks like a correct invocation, but I'm not sure (I use config.toml
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keeperofdakeys Kinda funny that the only section on msys2 simply tells you to target msvc, despite rustbuild defaulting to gnu if it detects an msys environment. Maybe we could replace this section with a thing that describes this default target behavior based on whether you're in an msys environment (really whether you have uname
in your path), and how you can use config.toml
to override the default.
19ec338
to
bf878c8
Compare
README.md
Outdated
the GNU ABI in powershell) by using an explicit build triple. The available | ||
Windows build triples are: | ||
- `x86_64-pc-windows-gnu` - The GNU ABI (using GCC) | ||
- `x86_64-pc-windows-msvc` - The MSVC ABI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about i686? I mean granted nobody should be using i686 but there are people that might want it.
@bors: r+ Thanks @keeperofdakeys! |
📌 Commit 2c695d7 has been approved by |
…crichton Replace ./configure with config.toml in README.md and CONTRIBUTING.md Replace ./configure with config.toml in README.md and CONTRIBUTING.md, so that new users aren't confused about which build system to use, and how to configure the build process.
…crichton Replace ./configure with config.toml in README.md and CONTRIBUTING.md Replace ./configure with config.toml in README.md and CONTRIBUTING.md, so that new users aren't confused about which build system to use, and how to configure the build process.
Small note: this should be rollup. |
@bors rollup |
2c695d7
to
fb2d763
Compare
Just squashing the commits. |
@keeperofdakeys In the future, please do so before pushing (unless the commits are quite large). |
@bors r=alexcrichton |
There's no need of squashing, really. Just keep what you do as a concrete history. |
📌 Commit fb2d763 has been approved by |
…crichton Replace ./configure with config.toml in README.md and CONTRIBUTING.md Replace ./configure with config.toml in README.md and CONTRIBUTING.md, so that new users aren't confused about which build system to use, and how to configure the build process.
Whoops, sorry about that. I didn't realise a rollup had happened. I'll try not to do that in the future. |
…crichton Replace ./configure with config.toml in README.md and CONTRIBUTING.md Replace ./configure with config.toml in README.md and CONTRIBUTING.md, so that new users aren't confused about which build system to use, and how to configure the build process.
Replace ./configure with config.toml in README.md and CONTRIBUTING.md, so that new users aren't confused about which build system to use, and how to configure the build process.