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 should not silently overwrite an existing config.toml #110109

Closed
jyn514 opened this issue Apr 9, 2023 · 4 comments · Fixed by #110123
Closed

./configure should not silently overwrite an existing config.toml #110109

jyn514 opened this issue Apr 9, 2023 · 4 comments · Fixed by #110123
Assignees
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 9, 2023

I tried this code:

; cat config.toml 
# Includes one of the default files in src/bootstrap/defaults
profile = "compiler"
changelog-seen = 2
; ./configure

I expected to see this happen: ./configure notices that I have an existing config.toml and gives a hard error unless I remove it (or automatically backs up the file somewhere).

Instead, this happened: ./configure overwrites the file without warning.

; ./configure
configure: processing command line
configure: 
configure: build.configure-args := []
configure: 
configure: writing `config.toml` in current directory
configure: 
configure: run `python /home/jyn/src/rust2/x.py --help`
; cat config.toml
# Keeps track of the last version of `x.py` used.
# If `changelog-seen` does not match the version that is currently running,
# `x.py` will prompt you to update it and to read the changelog.
# See `src/bootstrap/CHANGELOG.md` for more information.
changelog-seen = 2

[llvm]

[build]

# Arguments passed to the `./configure` script, used during distcheck. You
# probably won't fill this in but rather it's filled in by the `./configure`
# script. Useful for debugging.
configure-args = []

[install]

[rust]

[target.x86_64-unknown-linux-gnu]

[dist]

Meta

HEAD is branched from eb3e9c1.

@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Apr 9, 2023
@madsravn
Copy link
Contributor

madsravn commented Apr 9, 2023

@rustbot claim

@jyn514 Would an acceptable solution for you be something like: WARNING: Existing 'config.toml' detected. Do you wish to override: [y/N]?

@jyn514
Copy link
Member Author

jyn514 commented Apr 9, 2023

Sure, that seems fine :) in that case we should probably also add a -y or --force flag, since ./configure is usually meant to be used in an automated context.

@madsravn
Copy link
Contributor

madsravn commented Apr 9, 2023

Sure, that seems fine :) in that case we should probably also add a -y or --force flag, since ./configure is usually meant to be used in an automated context.

Would you rather have it give a hard error? I could be leaning that way - user input could be an annoying thing to keep doing. I am not a big fan of the rename-solution. Because if you then run it twice, you would probably have overridden your backup.

And roger on the flags.

@jyn514
Copy link
Member Author

jyn514 commented Apr 9, 2023

Yeah, let's do the hard error after all, ./configure is very quick to run so having it be interactive isn't saving you much time. If we do a hard error there's no need for new flags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants