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

Register src/tools/unicode-table-generator as a runnable tool #131647

Merged
merged 4 commits into from
Oct 20, 2024

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Oct 13, 2024

It seems like src/tools/unicode-table-generator is not currently managed by bootstrap. This PR wires it up with bootstrap as a runnable tool.

This tool seems to take two possible args:

  1. (Mandatory) path to library/core/src/unicode/unicode_data.rs, and
  2. (Optional) path to generate a test file.

I only passed the mandatory path to unicode_data.rs in bootstrap and didn't do anything about (2). I'm not sure about how this tool is supposed to be run.

Cargo.lock is modified because I renamed unicode-table-generator's bin name to match the tool name, as bootstrap's tool running logic expects the bin name to be derived from the tool name.

I also added a triagebot message to remind to not manually edit the library source file and edit the tool then regenerate instead, but this should probably be a tidy check (if that's desirable then that can be in a follow-up PR, though may be overkill).

Helps with #131640 but does not close it because still no docs.

r? @Mark-Simulacrum (since I think you authored this tool?)

Bootstrap assumes that the binary name is the same as tool name, just
makes everyone's lives easier.
@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 Oct 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@jieyouxu

This comment was marked as resolved.

These comments were updated on master but not through this tool, so the
comments in the tool became outdated. Sync the comments to stay
consistent.
…` that it needs to be generated by tool

And should not be directly edited.
@rustbot rustbot added the A-meta Area: Issues & PRs about the rust-lang/rust repository itself label Oct 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2024

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

@Mark-Simulacrum
Copy link
Member

580a634 added the test file. I don't really recall that, but poking around it looks likely that the intent was to produce it and then manually run e.g. rustc --test path/to/test/file.rs -O && ./file or something like that. I suppose we could automate that in bootstrap?

re:tidy checking, I think it's likely to be expensive to re-run the tool - and probably incompatible with offline builds - but we could maybe commit a hash of the file to source code and use that as a signal of manual modification?

FWIW historically this was just run from the directory (e.g., cargo run --manifest-path ...) from what I recall. Wiring to bootstrap seems fine though (probably good).

@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2024

📌 Commit be89da5 has been approved by Mark-Simulacrum

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#121560 (Allow `#[deny]` inside `#[forbid]` as a no-op)
 - rust-lang#131365 (Fix missing rustfmt in msi installer rust-lang#101993)
 - rust-lang#131647 (Register `src/tools/unicode-table-generator` as a runnable tool)
 - rust-lang#131843 (compiler: Error on layout of enums with invalid reprs)
 - rust-lang#131926 (Align boolean option descriptions in `configure.py`)
 - rust-lang#131961 (compiletest: tidy up how `tidy` and `tidy` (html version) are disambiguated)
 - rust-lang#131962 (Make `llvm::set_section` take a `&CStr`)
 - rust-lang#131964 (add latest crash tests)
 - rust-lang#131965 (remove outdated comment)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fb42a45 into rust-lang:master Oct 20, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2024
Rollup merge of rust-lang#131647 - jieyouxu:unicode-table-generator, r=Mark-Simulacrum

Register `src/tools/unicode-table-generator` as a runnable tool

It seems like `src/tools/unicode-table-generator` is not currently managed by bootstrap. This PR wires it up with bootstrap as a runnable tool.

This tool seems to take two possible args:

1. (Mandatory) path to `library/core/src/unicode/unicode_data.rs`, and
2. (Optional) path to generate a test file.

I only passed the mandatory path to `unicode_data.rs` in bootstrap and didn't do anything about (2). I'm not sure about how this tool is supposed to be run.

`Cargo.lock` is modified because I renamed `unicode-table-generator`'s bin name to match the tool name, as bootstrap's tool running logic expects the bin name to be derived from the tool name.

I also added a triagebot message to remind to not manually edit the library source file and edit the tool then regenerate instead, but this should probably be a tidy check (if that's desirable then that can be in a follow-up PR, though may be overkill).

Helps with rust-lang#131640 but does not close it because still no docs.

r? `@Mark-Simulacrum` (since I think you authored this tool?)
@jieyouxu jieyouxu deleted the unicode-table-generator branch October 20, 2024 23:59
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 2, 2024
unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 3, 2024
unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 3, 2024
unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 3, 2024
unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 3, 2024
unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2024
Rollup merge of rust-lang#132499 - RalfJung:unicode_data.rs, r=tgross35

unicode_data.rs: show command for generating file

rust-lang#131647 made this an easily runnable tool, now we just have to mention that in the comment. :)

Fixes rust-lang#131640.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository 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.

4 participants