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

src/tools/x doesn't work on old checkouts of the compiler #107907

Closed
jyn514 opened this issue Feb 10, 2023 · 12 comments · Fixed by #108021
Closed

src/tools/x doesn't work on old checkouts of the compiler #107907

jyn514 opened this issue Feb 10, 2023 · 12 comments · Fixed by #108021
Assignees
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Feb 10, 2023

Since #105844, src/tools/x unconditionally looks for the shell scripts when running. That's good in most cases because it avoids having to run python. However, on old checkouts the shell scripts don't exist and it needs to fall back to using python.

cc @albertlarsan68, do you have time to fix this?

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Feb 10, 2023
@albertlarsan68
Copy link
Member

@rustbot claim
If it can wait for the weekend to pass, then yes.
Anyone, feel free to claim of you can do it in the weekend

@zephaniahong
Copy link
Contributor

@albertlarsan68 i could help! Could you give me some pointers on what needs to be done?

@albertlarsan68
Copy link
Member

What you should do is re-add the code that was removed in #105844, using it when the shell scripts are not found, but x.py is.
You could even add an env var that forces the use of python, but that's a later step.

@zephaniahong
Copy link
Contributor

zephaniahong commented Feb 11, 2023

I see! Okay I'll give this a shot! But I may need more time than just the weekends cause I probably need some time to figure things out. But if it's urgent then feel free to unassign me😅
@rustbot claim

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

You could even add an env var that forces the use of python, but that's a later step.

I don't think this is necessary, there shouldn't be any functionality that's available in python but not shell.

@albertlarsan68
Copy link
Member

It is future-proofing for when python does get used only when building bootstrap, and the shell scripts won't be able to call python

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2023

That can be done with a function argument so it's not exposed to users.

@albertlarsan68
Copy link
Member

Yes, it can work like this.

@zephaniahong
Copy link
Contributor

zephaniahong commented Feb 12, 2023

Hi @jyn514 ! Currently working on this. Could you list out the steps needed to replicate this issue so I could get a bit more context of the problem?

Thank you!

@jyn514
Copy link
Member Author

jyn514 commented Feb 12, 2023

@zephaniahong install it from a current version of the repository, then check out any commit from before c16b969 and run x --help. It will says "x.py not found" even though it exists in the repository.

@jyn514
Copy link
Member Author

jyn514 commented Feb 12, 2023

Err sorry, that was the commit that changes src/tools/x - you want the commit that initially added the ./x and x.ps1 scripts.

@albertlarsan68
Copy link
Member

One reason may be because they work on a pr that started before this commit, and that (miraculously) didn't conflict.

Another reason would have been because the work was done on stable/beta, but I think the scripts are now on stable (correct me if I'm wrong)

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 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.

3 participants