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.

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

add TestFloatParse to tools.rs for bootstrap #131731

Merged
merged 1 commit into from
Oct 22, 2024
Merged

add TestFloatParse to tools.rs for bootstrap #131731

merged 1 commit into from
Oct 22, 2024

Conversation

lucarlig
Copy link
Contributor

@lucarlig lucarlig commented Oct 15, 2024 •
edited by onur-ozkan
Loading

add TestFloatParse to tools for bootstrap, I am not sure this is what the issue #128012 discussion wants.

try-job: aarch64-apple

@rustbot
Copy link
Collaborator

rustbot commented Oct 15, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 15, 2024
@jieyouxu
Copy link
Member

@tgross35 could you elaborate what you want for test-float-parse again?

@jieyouxu
Copy link
Member

Marking this as blocked waiting for @tgross35 to help me understand what is the desired changes.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2024
@tgross35
Copy link
Contributor

For test-float-parse specifically I think I wanted a build step so ./x build test-float-parse works. It seems like this is what is here. The more general thing is that to me it seems like prepare_tool_cargo should just call builder.ensure to compile std, since it seemed like anything that calls prepare_tool_cargo needs to call builder.ensure anyway.

I honestly don't remember the exact change that I was planning to do and forgot. @onur-ozkan do you know what change we should do based on #128012?

@onur-ozkan
Copy link
Member

I will review this once I get some free time.

r? onur-ozkan

@rustbot rustbot assigned onur-ozkan and unassigned jieyouxu Oct 16, 2024
@jieyouxu jieyouxu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Oct 16, 2024
@bors
Copy link
Contributor

bors commented Oct 16, 2024

☔ The latest upstream changes (presumably #131767) made this pull request unmergeable. Please resolve the merge conflicts.

@lucarlig
Copy link
Contributor Author

For test-float-parse specifically I think I wanted a build step so ./x build test-float-parse works. It seems like this is what is here. The more general thing is that to me it seems like prepare_tool_cargo should just call builder.ensure to compile std, since it seemed like anything that calls prepare_tool_cargo needs to call builder.ensure anyway.

I honestly don't remember the exact change that I was planning to do and forgot. @onur-ozkan do you know what change we should do based on #128012?

I think that would require passing the Step directly to prepare_too_cargo also I noticed that too many args are present on it, the solution could be to pass the CargoCommand directly instead of all the parameters, what else is required from CargoCommand can be derived inside the prepare_too_cargo.

@rustbot
Copy link
Collaborator

rustbot commented Oct 17, 2024

Some changes occurred in src/tools/cargo

cc @ehuss

@lucarlig lucarlig force-pushed the master branch 2 times, most recently from 1ce25bb to fd87f50 Compare October 17, 2024 10:09
@lucarlig
Copy link
Contributor Author

Some changes occurred in src/tools/cargo

cc @ehuss

removed those

@lucarlig lucarlig mentioned this pull request Oct 18, 2024
Closed
onur-ozkan
onur-ozkan requested changes Oct 19, 2024
View reviewed changes
src/bootstrap/src/core/build_steps/test.rs Outdated Show resolved Hide resolved
src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
@rustbot rustbot 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 Oct 19, 2024
@onur-ozkan
Copy link
Member

@rustbot author

@lucarlig lucarlig force-pushed the master branch 2 times, most recently from 607db5d to 8f3378a Compare October 19, 2024 14:39
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2024
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 19, 2024
@lucarlig
Copy link
Contributor Author

@rustbot ready

onur-ozkan
onur-ozkan reviewed Oct 19, 2024
View reviewed changes
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Last notes:

src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved
@onur-ozkan
Copy link
Member

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2024
@bors
Auto merge of rust-lang#131731 - lucarlig:master, r=<try>
727ce5b 
add `TestFloatParse` to `tools.rs` for bootstrap

add TestFloatParse to tools for bootstrap, I am not sure this is what the issue rust-lang#128012 discussion wants.

try-job: aarch64-apple
@bors
Copy link
Contributor

bors commented Oct 19, 2024

⌛ Trying commit 5be83d2 with merge 727ce5b...

@bors
Copy link
Contributor

bors commented Oct 19, 2024

☀️ Try build successful - checks-actions
Build commit: 727ce5b (727ce5bd6982a133701fa5dc646615745a846aba)

onur-ozkan
onur-ozkan approved these changes Oct 19, 2024
View reviewed changes
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM - Could you please squash your commits?

@lucarlig
Copy link
Contributor Author

LGTM - Could you please squash your commits?

done @onur-ozkan

@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 22, 2024

📌 Commit bc9fc71 has been approved by onur-ozkan

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 22, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 22, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2024
@bors
Auto merge of rust-lang#132035 - matthiaskrgr:rollup-ty1e4q0, r=matth…
86d69c7 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125205 (Fixup Windows verbatim paths when used with the `include!` macro)
 - rust-lang#131049 (Validate args are correct for `UnevaluatedConst`, `ExistentialTraitRef`/`ExistentialProjection`)
 - rust-lang#131549 (Add a note for `?` on a `impl Future<Output = Result<..>>` in sync function)
 - rust-lang#131731 (add `TestFloatParse` to `tools.rs` for bootstrap)
 - rust-lang#131732 (Add doc(plugins), doc(passes), etc. to INVALID_DOC_ATTRIBUTES)
 - rust-lang#132006 (don't stage-off to previous compiler when CI rustc is available)
 - rust-lang#132022 (Move `cmp_in_dominator_order` out of graph dominator computation)
 - rust-lang#132033 (compiletest: Make `line_directive` return a `DirectiveLine`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cbb85f1 into rust-lang:master Oct 22, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2024
@rust-timer
Unrolled build for rust-lang#131731
ee6a338 
Rollup merge of rust-lang#131731 - lucarlig:master, r=onur-ozkan

add `TestFloatParse` to `tools.rs` for bootstrap

add TestFloatParse to tools for bootstrap, I am not sure this is what the issue rust-lang#128012 discussion wants.

try-job: aarch64-apple
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onur-ozkan onur-ozkan onur-ozkan approved these changes

Assignees

@onur-ozkan onur-ozkan

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

Successfully merging this pull request may close these issues.
6 participants
@lucarlig @rustbot @jieyouxu @tgross35 @onur-ozkan @bors