Skip to content

bootstrap: open llvm-config as r+w #127850

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
Jul 17, 2024
Merged

bootstrap: open llvm-config as r+w #127850

merged 1 commit into from
Jul 17, 2024

Conversation

jieyouxu
Copy link
Member

This previously failed on Windows and prevented building on Windows for compiler stuff because the llvm-config file was open as read-only.

Tested locally on a Windows machine.

Fixes #127849.

@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2024

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
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 Jul 17, 2024
ChrisDenton
ChrisDenton reviewed Jul 17, 2024
View reviewed changes
@jieyouxu
bootstrap: open llvm_config as r+w
3bee507 
This previously failed on Windows because the `llvm_config` file was
open as read-only.
@jieyouxu jieyouxu force-pushed the bootstrap-llvm-config branch from 643b764 to 3bee507 Compare July 17, 2024 09:00
onur-ozkan
onur-ozkan approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@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, feel free to r=me

@jieyouxu
Copy link
Member Author

Made suggested changes, and tested it to make sure it builds past llvm step locally on Windows.
@bors r=@onur-ozkan rollup

@bors
Copy link
Collaborator

bors commented Jul 17, 2024

📌 Commit 3bee507 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 Jul 17, 2024
@tgross35 tgross35 mentioned this pull request Jul 17, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127851 - tgross35:rollup-i39um74, r=tgross35
39f3504 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#127763 (Make more Windows functions `#![deny(unsafe_op_in_unsafe_fn)]`)
 - rust-lang#127813 (Prevent double reference in generic futex)
 - rust-lang#127847 (Reviewer on vacation)
 - rust-lang#127850 (bootstrap: open `llvm-config` as r+w)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127851 - tgross35:rollup-i39um74, r=tgross35
08cdc2f 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#127763 (Make more Windows functions `#![deny(unsafe_op_in_unsafe_fn)]`)
 - rust-lang#127813 (Prevent double reference in generic futex)
 - rust-lang#127847 (Reviewer on vacation)
 - rust-lang#127850 (bootstrap: open `llvm-config` as r+w)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f99f61b into rust-lang:master Jul 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@rust-timer
Unrolled build for rust-lang#127850
5de9771 
Rollup merge of rust-lang#127850 - jieyouxu:bootstrap-llvm-config, r=onur-ozkan

bootstrap: open `llvm-config` as r+w

This previously failed on Windows and prevented building on Windows for compiler stuff because the `llvm-config` file was open as read-only.

Tested locally on a Windows machine.

Fixes rust-lang#127849.
@rustbot rustbot added this to the 1.81.0 milestone Jul 17, 2024
@jieyouxu jieyouxu deleted the bootstrap-llvm-config branch July 17, 2024 16:30
@jieyouxu jieyouxu mentioned this pull request Aug 11, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 12, 2024
@matthiaskrgr
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
9ef4afb 
Only try to modify file times of a writable file

- First commit fixes a failure that I was running into locally trying to build a stage 1 `./x build library --stage 1` on windows due to trying to modify file times of a read-only file.
- Second commit introduces a `set_file_times` helper which opens a given path as a file in r+w mode and then sets file times. This should hopefully make setting file times less error prone, since trying to set file times on read-only file also happened in rust-lang#127850. (And apparently it only fails locally on Windows or something weird like that.)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 12, 2024
@matthiaskrgr
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
dd3fe6a 
Only try to modify file times of a writable file

- First commit fixes a failure that I was running into locally trying to build a stage 1 `./x build library --stage 1` on windows due to trying to modify file times of a read-only file.
- Second commit introduces a `set_file_times` helper which opens a given path as a file in r+w mode and then sets file times. This should hopefully make setting file times less error prone, since trying to set file times on read-only file also happened in rust-lang#127850. (And apparently it only fails locally on Windows or something weird like that.)
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2024
@bors
Auto merge of rust-lang#128977 - jieyouxu:writable-file, r=<try>
edbc734 
Only try to modify file times of a writable file

- First commit fixes a failure that I was running into locally trying to build a stage 1 `./x build library --stage 1` on windows due to trying to modify file times of a read-only file.
- Second commit introduces a `set_file_times` helper which opens a given path as a file in r+w mode and then sets file times. This should hopefully make setting file times less error prone, since trying to set file times on read-only file also happened in rust-lang#127850. (And apparently it only fails locally on Windows or something weird like that.)

try-job: dist-loongarch64-musl
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2024
@bors
Auto merge of rust-lang#128977 - jieyouxu:writable-file, r=<try>
f2d4ef6 
Only try to modify file times of a writable file

- First commit fixes a failure that I was running into locally trying to build a stage 1 `./x build library --stage 1` on windows due to trying to modify file times of a read-only file.
- Second commit introduces a `set_file_times` helper which opens a given path as a file in r+w mode and then sets file times. This should hopefully make setting file times less error prone, since trying to set file times on read-only file also happened in rust-lang#127850. (And apparently it only fails locally on Windows or something weird like that.)

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 15, 2024
@matthiaskrgr
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
7f98870 
Only try to modify file times of a writable file on Windows

Introduces a `set_file_times` helper which opens a given path as a file in r+w mode on Windows and then sets file times. Previously the file was open as read-only for Windows which caused permission errors locally.

This should hopefully make setting file times less error prone, since trying to set file times on read-only file on Windows also happened in rust-lang#127850.

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
@workingjubilee
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
4ed9ed3 
Only try to modify file times of a writable file on Windows

Introduces a `set_file_times` helper which opens a given path as a file in r+w mode on Windows and then sets file times. Previously the file was open as read-only for Windows which caused permission errors locally.

This should hopefully make setting file times less error prone, since trying to set file times on read-only file on Windows also happened in rust-lang#127850.

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
@workingjubilee
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
72f657a 
Only try to modify file times of a writable file on Windows

Introduces a `set_file_times` helper which opens a given path as a file in r+w mode on Windows and then sets file times. Previously the file was open as read-only for Windows which caused permission errors locally.

This should hopefully make setting file times less error prone, since trying to set file times on read-only file on Windows also happened in rust-lang#127850.

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
@workingjubilee
Rollup merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
370a01b 
Only try to modify file times of a writable file on Windows

Introduces a `set_file_times` helper which opens a given path as a file in r+w mode on Windows and then sets file times. Previously the file was open as read-only for Windows which caused permission errors locally.

This should hopefully make setting file times less error prone, since trying to set file times on read-only file on Windows also happened in rust-lang#127850.

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
@bors
Auto merge of rust-lang#128977 - jieyouxu:writable-file, r=Kobzol
8fbdc04 
Only try to modify file times of a writable file on Windows

Introduces a `set_file_times` helper which opens a given path as a file in r+w mode on Windows and then sets file times. Previously the file was open as read-only for Windows which caused permission errors locally.

This should hopefully make setting file times less error prone, since trying to set file times on read-only file on Windows also happened in rust-lang#127850.

try-job: dist-loongarch64-musl
try-job: x86_64-msvc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisDenton ChrisDenton ChrisDenton left review comments

+1 more reviewer

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

Reviewers whose approvals may not affect merge requirements
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.81.0
Development

Successfully merging this pull request may close these issues.

Bootstrap using set_times on a read-only file
5 participants
@jieyouxu @rustbot @bors @ChrisDenton @onur-ozkan