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

rustbuild: Rewrite the configure script in Python #44107

Merged
merged 1 commit into from
Aug 28, 2017

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Aug 26, 2017

This commit rewrites our ancient ./configure script from shell into Python.
The impetus for this change is to remove config.mk which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through config.toml.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a config.toml which is based off config.toml.example.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@Mark-Simulacrum
Copy link
Member

I believe this will also close #43295, #42255, and #38058. Could you verify and update the description?

I've looked through the code and everything looks good to me. If you'd like a rubber stamp approval, feel free to r=me -- I'm not familiar with Python though so can't really review code quality there.

configure Outdated

p("processing command line")
i = 1
while i < len(sys.argv):
Copy link
Member

Choose a reason for hiding this comment

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

Could we use argparse (or optparse if we care about supporting the dead hand of Python 2.6) rather than rolling our own?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's possible, yes, but I'm pretty unfamiliar with both (I barely know python) and would prefer to focus on compatibility for now

@alexcrichton
Copy link
Member Author

Thanks for reviewing @Mark-Simulacrum!

I think I'll hold off on merging this until after beta branches, no need to land a rewrite that has a high likelihood for regressions just before!

@alexcrichton alexcrichton force-pushed the no-shell-configure branch 3 times, most recently from ecc3451 to 1db05ac Compare August 27, 2017 01:45
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

📌 Commit 1db05ac has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

⌛ Testing commit 1db05acbfa2bbe241323b3c7e98ed2407a921b60 with merge a10b0ddf528dd8e29853347fa20ca8c016d573d0...

@bors
Copy link
Contributor

bors commented Aug 27, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

📌 Commit e206216 has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

⌛ Testing commit e2062165d58211ff81c10bc9332e14eb68c6970b with merge f52c915024bac5dbab77ecb715c445cfe7a40dca...

@bors
Copy link
Contributor

bors commented Aug 27, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

Probably a matter of copying the config.toml.example into dist.

00:10:05] configure: processing command line
[00:10:05] configure: 
[00:10:05] configure: rust.debug-assertions := True
[00:10:05] configure: build.vendor         := True
[00:10:05] configure: build.submodules     := False
[00:10:05] configure: llvm.assertions      := True
[00:10:05] configure: build.locked-deps    := True
[00:10:05] configure: llvm.ccache          := sccache
[00:10:05] configure: build.openssl-static := True
[00:10:05] configure: build.build          := x86_64-unknown-linux-gnu
[00:10:05] configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--enable ...
[00:10:05]     for line in open(rust_dir + '/config.toml.example').read().split("\n"):
[00:10:05] IOError: [Errno 2] No such file or directory: '/checkout/obj/build/tmp/distcheck/config.toml.example'
[00:10:05] 
[00:10:05] 
[00:10:05] command did not execute successfully: "./configure" "--build=x86_64-unknown-linux-gnu" "--enable-sccache" "--disable-manage-submodules" "--enable-locked-deps" "--enable-cargo-openssl-static" "--enable-debug-assertions" "--enable-llvm-assertions" "--enable-vendor"
[00:10:05] expected success, got: exit code: 1

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes rust-lang#40730
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 28, 2017

📌 Commit a9b0a7b has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 28, 2017

⌛ Testing commit a9b0a7b with merge 469d147...

bors added a commit that referenced this pull request Aug 28, 2017
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
@bors
Copy link
Contributor

bors commented Aug 28, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

@bors
Copy link
Contributor

bors commented Aug 28, 2017

⌛ Testing commit a9b0a7b with merge a0c3bd2...

bors added a commit that referenced this pull request Aug 28, 2017
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
@bors
Copy link
Contributor

bors commented Aug 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing a0c3bd2 to master...

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