Skip to content

make x look for x.py if shell script does not exist #108021

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
Merged
merged 1 commit into from
Feb 16, 2023
Merged

make x look for x.py if shell script does not exist #108021

merged 1 commit into from
Feb 16, 2023

Conversation

zephaniahong
Copy link
Contributor

@zephaniahong zephaniahong commented Feb 14, 2023 •
edited
Loading

Fixes #107907

Manually tested by doing the following after changes were made:

  1. cargo install --path src/tools/x
  2. checked out old version: commit hash 775c3c0 from Add x.sh and x.ps1 shell scripts #99992
  3. Ran x --help and it works. Previously, it was giving the error x.py not found

@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2023

r? @albertlarsan68

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 14, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2023

Could not assign reviewer from: albertlarsan68.
User(s) albertlarsan68 are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

@jyn514
Copy link
Member

jyn514 commented Feb 14, 2023

Thanks! Can you also bump the version of x in Cargo.toml so people get a warning if they're using an old version?

@jyn514
Copy link
Member

jyn514 commented Feb 14, 2023

(I need to set up a rustbot ping comment that posts that automatically...)

@zephaniahong
Copy link
Contributor Author

zephaniahong commented Feb 14, 2023 •
edited
Loading

No problem! Do i bump it to 0.1.1? or 0.2.0?

@zephaniahong
Copy link
Contributor Author

zephaniahong commented Feb 14, 2023 •
edited
Loading

(I need to set up a rustbot ping comment that posts that automatically...)

If you could give me some details about your requirements, I could give it a shot! 👍

@jyn514
Copy link
Member

jyn514 commented Feb 14, 2023

No problem! Do i bump it to 0.1.1? or 0.2.0?

0.1.1 seems fine, in practice tidy is just checking if the version increased or not.

@jyn514
Copy link
Member

jyn514 commented Feb 14, 2023

(I need to set up a rustbot ping comment that posts that automatically...)

If you could give me some details about your requirements, I could give it a shot! 👍

That would be great, thanks! You'd just add a line around

rust/triagebot.toml

Line 463 in 5348a89

[mentions."Cargo.lock"]
that posts a message if src/tools/x is modified.

@zephaniahong zephaniahong mentioned this pull request Feb 14, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 14, 2023
@matthiaskrgr
Rollup merge of rust-lang#108024 - zephaniahong:master, r=jyn514
43b42c5 
add message to update Cargo.toml when x is changed

`@jyn514` Is this correct?

As mentioned in rust-lang#108021
albertlarsan68
albertlarsan68 requested changes Feb 14, 2023
View reviewed changes
Copy link
Member

@albertlarsan68 albertlarsan68 left a comment

Choose a reason for hiding this comment

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

And you can squash your commits after the changes requested.
Thanks!

@albertlarsan68 albertlarsan68 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2023

src/tools/x was changed. Bump version of Cargo.toml in src/tools/x so tidy will suggest installing the new version.

@albertlarsan68
Copy link
Member

Seems good.
Thanks for the PR!
@bors r+

@bors
Copy link
Collaborator

bors commented Feb 15, 2023

📌 Commit 54cfc10 has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 15, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 15, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 16, 2023
@bors
Auto merge of rust-lang#108096 - matthiaskrgr:rollup-ncexzf6, r=matth…
dc7a676 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#107034 (Migrating rustc_infer to session diagnostics (part 4))
 - rust-lang#107972 (Fix unintentional UB in ui tests)
 - rust-lang#108010 (Make `InferCtxt::can_eq` and `InferCtxt::can_sub` return booleans)
 - rust-lang#108021 (make x look for x.py if shell script does not exist)
 - rust-lang#108047 (Use `target` instead of `machine` for mir interpreter integer handling.)
 - rust-lang#108049 (Don't suggest `#[doc(hidden)]` trait methods with matching return type)
 - rust-lang#108066 (Better names for illegal impl trait positions)
 - rust-lang#108076 (rustdoc: Use more let chain)
 - rust-lang#108088 (clarify correctness of `black_box`)
 - rust-lang#108094 (Demonstrate I/O in File examples)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 32891e1 into rust-lang:master Feb 16, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albertlarsan68 albertlarsan68 albertlarsan68 requested changes

Assignees

@albertlarsan68 albertlarsan68

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.

src/tools/x doesn't work on old checkouts of the compiler
5 participants
@zephaniahong @rustbot @jyn514 @albertlarsan68 @bors