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

Add a tool to run x.py from any subdirectory #78658

Merged
merged 1 commit into from
Nov 8, 2020
Merged

Conversation

casey
Copy link
Contributor

@casey casey commented Nov 2, 2020

This adds a binary called x in src/tools/x. All it does is check the current directory and its ancestors for a file called x.py, and if it finds one, runs it.

By installing x, you can easily run x.py from any subdirectory, and only need to type x.

It can be installed with cargo install --path src/tools/x

This is a copy of a binary I've been using myself when working on rust, currently published to crates.io as bootstrap.

It could be changed to avoid indirecting through x.py, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2020
@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 Nov 2, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

Assuming this is accepted, can you also make a PR to the rustc-dev-guide documenting how to use it? It would go in https://rustc-dev-guide.rust-lang.org/building/how-to-build-and-run.html.

@casey
Copy link
Contributor Author

casey commented Nov 2, 2020

Assuming this is accepted, can you also make a PR to the rustc-dev-guide documenting how to use it? It would go in https://rustc-dev-guide.rust-lang.org/building/how-to-build-and-run.html.

Yup, definitely, I'd be happy to.

src/tools/x/src/main.rs Outdated Show resolved Hide resolved
src/tools/x/Cargo.lock Outdated Show resolved Hide resolved
src/tools/x/src/main.rs Outdated Show resolved Hide resolved
@casey
Copy link
Contributor Author

casey commented Nov 2, 2020

Thanks for the review! I think I addressed everything.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

I am a bit worried this might break the process group creation on Windows where we attempt to associate rustbuild with the Python process (IIRC) -- can you check if ctrl+c on Windows when running through this tool kills Cargo and any running rustc processes?

src/tools/x/src/main.rs Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Member

I am generally okay with adding this but if it's a cargo project you won't be able to run it (in general) without running x.py first unless you're manually managing submodules (otherwise Cargo.toml will not resolve). I am a bit hesitant as a result to add it. But not really opposed, it seems low maintenance cost, modulo my comments on Windows process scheduling.

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

if it's a cargo project you won't be able to run it (in general) without running x.py first unless you're manually managing submodules (otherwise Cargo.toml will not resolve)

Could we not make it part of the workspace, so it doesn't need to resolve any project other than its own directory?

@Mark-Simulacrum
Copy link
Member

That would work, I think, yes. I suppose the downside is that we can't make it the default-run in the workspace, but since that's not a good idea anyway due to the submodule problem, seems fine.

Please also squash commits.

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

We could always add it to the workspace if we figure out the submodule problem later.

@camelid
Copy link
Member

camelid commented Nov 2, 2020

I think the name x, is not a common binary name. There is X, I think usually a shortcut that launches X11, but on a case-sensitive system, there isn't any conflict.

I want to note that macOS is unfortunately case-insensitive, so this doesn’t hold for macOS.

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

@camelid X11 isn't installed on MacOS, though, right?

@camelid
Copy link
Member

camelid commented Nov 2, 2020

I don't think it's installed by default, but people may install it themselves (see also XQuartz). I just wanted to note that macOS is case-insensitive, so this will collide with anything named x or X.

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

I think that's a reasonable tradeoff - you'd have to install both of the binaries yourself, at which point you'd know that they overlap.

@jyn514
Copy link
Member

jyn514 commented Nov 3, 2020

@casey I think this is waiting on you to remove src/tools/x from the workspace and test that it can be run without first running x.py.

@camelid camelid 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 Nov 3, 2020
This adds a binary called `x` in `src/tools/x`. All it does is check the
current directory and its ancestors for a file called `x.py`, and if it
finds one, runs it.

By installing x, you can easily `x.py` from any subdirectory.

It can be installed globally with `cargo install --path src/tools/x`
@casey
Copy link
Contributor Author

casey commented Nov 4, 2020

I checked on windows, and ctrl-c seems to work when running the wrapper.

I removed it from the workspace, and added it to the exclude list. It now works when run in a fresh checkout, before submodules are initialized.

I also squashed the commits.

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2020
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

Thanks!

@bors
Copy link
Contributor

bors commented Nov 6, 2020

📌 Commit 5fc22f1 has been approved by Mark-Simulacrum

@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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Add a tool to run `x.py` from any subdirectory

This adds a binary called `x` in `src/tools/x`. All it does is check the current directory and its ancestors for a file called `x.py`, and if it finds one, runs it.

By installing x, you can easily run `x.py` from any subdirectory, and only need to type `x`.

It can be installed with `cargo install --path src/tools/x`

This is a copy of a [binary I've been using myself when working on rust](https://github.com/casey/bootstrap), currently published to crates.io as `bootstrap`.

It could be changed to avoid indirecting through `x.py`, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Add a tool to run `x.py` from any subdirectory

This adds a binary called `x` in `src/tools/x`. All it does is check the current directory and its ancestors for a file called `x.py`, and if it finds one, runs it.

By installing x, you can easily run `x.py` from any subdirectory, and only need to type `x`.

It can be installed with `cargo install --path src/tools/x`

This is a copy of a [binary I've been using myself when working on rust](https://github.com/casey/bootstrap), currently published to crates.io as `bootstrap`.

It could be changed to avoid indirecting through `x.py`, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Add a tool to run `x.py` from any subdirectory

This adds a binary called `x` in `src/tools/x`. All it does is check the current directory and its ancestors for a file called `x.py`, and if it finds one, runs it.

By installing x, you can easily run `x.py` from any subdirectory, and only need to type `x`.

It can be installed with `cargo install --path src/tools/x`

This is a copy of a [binary I've been using myself when working on rust](https://github.com/casey/bootstrap), currently published to crates.io as `bootstrap`.

It could be changed to avoid indirecting through `x.py`, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 7, 2020
Add a tool to run `x.py` from any subdirectory

This adds a binary called `x` in `src/tools/x`. All it does is check the current directory and its ancestors for a file called `x.py`, and if it finds one, runs it.

By installing x, you can easily run `x.py` from any subdirectory, and only need to type `x`.

It can be installed with `cargo install --path src/tools/x`

This is a copy of a [binary I've been using myself when working on rust](https://github.com/casey/bootstrap), currently published to crates.io as `bootstrap`.

It could be changed to avoid indirecting through `x.py`, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 8, 2020
Add a tool to run `x.py` from any subdirectory

This adds a binary called `x` in `src/tools/x`. All it does is check the current directory and its ancestors for a file called `x.py`, and if it finds one, runs it.

By installing x, you can easily run `x.py` from any subdirectory, and only need to type `x`.

It can be installed with `cargo install --path src/tools/x`

This is a copy of a [binary I've been using myself when working on rust](https://github.com/casey/bootstrap), currently published to crates.io as `bootstrap`.

It could be changed to avoid indirecting through `x.py`, and instead call the bootstrap module directly. However, this seemed like the simplest thing possible, and won't break if the details of how the bootstrap module is invoked change.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2020
Rollup of 19 pull requests

Successful merges:

 - rust-lang#76097 (Stabilize hint::spin_loop)
 - rust-lang#76227 (Stabilize `Poll::is_ready` and `is_pending` as const)
 - rust-lang#78065 (make concurrency helper more pleasant to read)
 - rust-lang#78570 (Remove FIXME comment in print_type_sizes ui test suite)
 - rust-lang#78572 (Use SOCK_CLOEXEC and accept4() on more platforms.)
 - rust-lang#78658 (Add a tool to run `x.py` from any subdirectory)
 - rust-lang#78706 (Fix run-make tests running when LLVM is disabled)
 - rust-lang#78728 (Constantify `UnsafeCell::into_inner` and related)
 - rust-lang#78775 (Bump Rustfmt and RLS)
 - rust-lang#78788 (Correct unsigned equivalent of isize to be usize)
 - rust-lang#78811 (Make some std::io functions `const`)
 - rust-lang#78828 (use single char patterns for split() (clippy::single_char_pattern))
 - rust-lang#78841 (Small cleanup in `TypeFoldable` derive macro)
 - rust-lang#78842 (Honor the rustfmt setting in config.toml)
 - rust-lang#78843 (Less verbose debug logging from inlining integrator)
 - rust-lang#78852 (Convert a bunch of intra-doc links)
 - rust-lang#78860 (rustc_resolve: Use `#![feature(format_args_capture)]`)
 - rust-lang#78861 (typo and formatting)
 - rust-lang#78865 (Don't fire `CONST_ITEM_MUTATION` lint when borrowing a deref)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7d9ad6d into rust-lang:master Nov 8, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 8, 2020
@casey casey deleted the x branch November 8, 2020 20:49
@casey
Copy link
Contributor Author

casey commented Nov 8, 2020

Nice! Thank you all for reviewing ^_^

@jyn514
Copy link
Member

jyn514 commented Nov 8, 2020

@casey do you mind making that PR to the dev-guide?

@casey
Copy link
Contributor Author

casey commented Nov 8, 2020

@jyn514 Yup, definitely, it's on my todo list.

@casey
Copy link
Contributor Author

casey commented Nov 8, 2020

I just opened rust-lang/rustc-dev-guide#945 to document this.

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

Successfully merging this pull request may close these issues.

8 participants