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

Improve handing of env vars during bootstrap process #101783

Merged
merged 1 commit into from
Sep 17, 2022
Merged

Improve handing of env vars during bootstrap process #101783

merged 1 commit into from
Sep 17, 2022

Conversation

chriswailes
Copy link
Contributor

This CL modifies the handing of env vars during the bootstrap process in two ways:

  1. Replaces '-' characters with '_' characters in target names to increase compatibility with different shells
  2. Passes Stage0 snapshot compiler related env vars to early invocations of Cargo

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 13, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jyn514 (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2022
jyn514
jyn514 suggested changes Sep 14, 2022
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The python changes make sense to me but need changes to the implementation, I left comments inline. It would be helpful to know when you need to set these variables, I'm surprised no one has complained before.

The rust changes I'm unsure about , I left a comment inline.

src/bootstrap/bootstrap.py Outdated Show resolved Hide resolved
src/bootstrap/bootstrap.py Outdated Show resolved Hide resolved
src/bootstrap/bootstrap.py Outdated Show resolved Hide resolved
src/bootstrap/builder.rs Show resolved Hide resolved
@jyn514 jyn514 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 Sep 14, 2022
@chriswailes
Copy link
Contributor Author

It would be helpful to know when you need to set these variables, I'm surprised no one has complained before.

We ran into this issue when the lzma-sys crate's build.rs file found the default system cc on an Ubuntu 14.04 image, which was too old to successfully compile the crate's C (or assembly, I can't remember) files.

@chriswailes
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot 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 Sep 14, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 14, 2022

@chriswailes you force pushed but I don't see any changes; maybe you forgot to commit them?

@chriswailes
Copy link
Contributor Author

@jyn514 Yup, and then I had to pull, rebase, re-compile... A full yak shave. Sorry about that.

jyn514
jyn514 approved these changes Sep 14, 2022
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

r=me with the last nit fixed. Thanks!

src/bootstrap/bootstrap.py Outdated Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Sep 14, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 14, 2022

📌 Commit 8df181d has been approved by jyn514

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 Sep 14, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 15, 2022
@Dylan-DPC
Rollup merge of rust-lang#101783 - chriswailes:env-vars, r=jyn514
3e7f24d 
Improve handing of env vars during bootstrap process

This CL modifies the handing of env vars during the bootstrap process in two ways:
1. Replaces '-' characters with '_' characters in target names to increase compatibility with different shells
2. Passes Stage0 snapshot compiler related env vars to early invocations of Cargo
@Dylan-DPC Dylan-DPC mentioned this pull request Sep 15, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 15, 2022
@Dylan-DPC
Rollup merge of rust-lang#101783 - chriswailes:env-vars, r=jyn514
ed156fb 
Improve handing of env vars during bootstrap process

This CL modifies the handing of env vars during the bootstrap process in two ways:
1. Replaces '-' characters with '_' characters in target names to increase compatibility with different shells
2. Passes Stage0 snapshot compiler related env vars to early invocations of Cargo
@Dylan-DPC Dylan-DPC mentioned this pull request Sep 15, 2022
Closed
@aDotInTheVoid aDotInTheVoid mentioned this pull request Sep 15, 2022
Merged
@Dylan-DPC
Copy link
Member

@bors r-

failed in rollup

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 15, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 15, 2022

Oh right, we support python 2 still. Should be a simple fix, just use .format instead.

jyn514
jyn514 suggested changes Sep 16, 2022
View reviewed changes
src/bootstrap/bootstrap.py Outdated Show resolved Hide resolved
@chriswailes
Improve handing of env vars during bootstrap process
d1291dc 
This CL modifies the handing of env vars during the bootstrap process in
two ways:
1. Replaces '-' characters with '_' characters in target names to
   increase compatibility with different shells
2. Passes Stage0 snapshot compiler related env vars to early invocations
   of Cargo
@jyn514
Copy link
Member

jyn514 commented Sep 16, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2022

📌 Commit d1291dc has been approved by jyn514

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 Sep 16, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 17, 2022
@matthiaskrgr
Rollup merge of rust-lang#101783 - chriswailes:env-vars, r=jyn514
32ddc71 
Improve handing of env vars during bootstrap process

This CL modifies the handing of env vars during the bootstrap process in two ways:
1. Replaces '-' characters with '_' characters in target names to increase compatibility with different shells
2. Passes Stage0 snapshot compiler related env vars to early invocations of Cargo
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 17, 2022
Closed
This was referenced Sep 17, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 17, 2022
@bors
Auto merge of rust-lang#101946 - matthiaskrgr:rollup-jqkhsku, r=matth…
98ad6a5 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#101672 (array docs - advertise how to get array from slice)
 - rust-lang#101781 (Extend list of targets that support dyanmic linking for llvm tools)
 - rust-lang#101783 (Improve handing of env vars during bootstrap process)
 - rust-lang#101801 (add note for `layout_of` when query depth overflows)
 - rust-lang#101824 (rustdoc: add test cases for turning ``[Vec<T>]`` into ``[`Vec<T>`]``)
 - rust-lang#101861 (Update stdarch)
 - rust-lang#101873 (Allow building `rust-analyzer-proc-macro-srv` as a standalone tool)
 - rust-lang#101918 (rustdoc: clean up CSS for All Items and All Crates lists)
 - rust-lang#101934 (Continue migration of CSS themes)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9c32773 into rust-lang:master Sep 17, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 17, 2022
@chriswailes chriswailes deleted the env-vars branch September 19, 2022 17:07
@cuviper cuviper modified the milestones: 1.65.0, 1.66.0 Oct 4, 2022
chriswailes added a commit to chriswailes/rust that referenced this pull request Feb 3, 2023
@chriswailes
Add a linker argument back to boostrap.py
134a5ae 
In rust-lang#101783 I accidentally removed
a load-bearing linker argument.  This PR adds it back in.
@chriswailes chriswailes mentioned this pull request Feb 3, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2023
@bors
Auto merge of rust-lang#107618 - chriswailes:linker-arg, r=albertlars…
4aa6afa 
…an68

Add a linker argument back to boostrap.py

In rust-lang#101783 I accidentally removed a load-bearing linker argument.  This PR adds it back in.

r? jyn514
bors added a commit to rust-lang/miri that referenced this pull request Feb 7, 2023
@bors
Auto merge of #107618 - chriswailes:linker-arg, r=albertlarsan68
d0fd429 
Add a linker argument back to boostrap.py

In rust-lang/rust#101783 I accidentally removed a load-bearing linker argument.  This PR adds it back in.

r? jyn514
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
@bors
Auto merge of #107618 - chriswailes:linker-arg, r=albertlarsan68
07b5b21 
Add a linker argument back to boostrap.py

In rust-lang/rust#101783 I accidentally removed a load-bearing linker argument.  This PR adds it back in.

r? jyn514
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #107618 - chriswailes:linker-arg, r=albertlarsan68
b9ba69c 
Add a linker argument back to boostrap.py

In rust-lang/rust#101783 I accidentally removed a load-bearing linker argument.  This PR adds it back in.

r? jyn514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 requested changes

Assignees

@jyn514 jyn514

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

Successfully merging this pull request may close these issues.
7 participants
@chriswailes @rust-highfive @jyn514 @bors @Dylan-DPC @cuviper @rustbot