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

Don't use fd-lock on Solaris in bootstrap #108607

Merged
merged 1 commit into from
Mar 14, 2023
Merged

Don't use fd-lock on Solaris in bootstrap #108607

merged 1 commit into from
Mar 14, 2023

Conversation

psumbera
Copy link
Contributor

@psumbera psumbera commented Mar 1, 2023

...as Solaris is missing flock()

fixes #103630

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2023

r? @albertlarsan68

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

@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 Mar 1, 2023
@psumbera psumbera mentioned this pull request Mar 7, 2023
Closed
@albertlarsan68
Copy link
Member

Can you add a line in the rustc dev guide to warn that on some platforms using rust-analyzer and x at the same time may corrupt things?
r=me once done.
@bors rollup

@albertlarsan68 albertlarsan68 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 Mar 10, 2023
@psumbera
Copy link
Contributor Author

Can you add a line in the rustc dev guide to warn that on some platforms using rust-analyzer and x at the same time may corrupt things? r=me once done. @bors rollup

I would like to. But not sure where exactly it should belong. Also not sure about all consequences of not locking. Can you help me with it?

@albertlarsan68
Copy link
Member

I would put it in https://rustc-dev-guide.rust-lang.org/building/suggested.html#visual-studio-code, warning that on Solaris and platforms other than Unix and Windows, having multiple x.py running at the same time in the same repo will likely corrupt files and/or do bad things.
You will want to rephrase, but it is the main part I want to add a warning about.

@psumbera
Copy link
Contributor Author

I would put it in https://rustc-dev-guide.rust-lang.org/building/suggested.html#visual-studio-code, warning that on Solaris and platforms other than Unix and Windows, having multiple x.py running at the same time in the same repo will likely corrupt files and/or do bad things. You will want to rephrase, but it is the main part I want to add a warning about.

I don't think it's good idea. Visual Studio is not available for Solaris. And all Visual Studio supported platforms do have working fd-lock.

@albertlarsan68
Copy link
Member

Having it in the release notes for bootstrap is definitely a good thing.
Ignore what I said about the dev guide, since this is not likely to happen, and there is already a warning.

@psumbera
Copy link
Contributor Author

Having it in the release notes for bootstrap is definitely a good thing. Ignore what I said about the dev guide, since this is not likely to happen, and there is already a warning.

Shall I do any change then? Where exactly?

@albertlarsan68
Copy link
Member

You should add a line in https://github.com/rust-lang/rust/blob/master/src/bootstrap/CHANGELOG.md.

@psumbera
Don't use fd-lock on Solaris in bootstrap
04dfedb 
...as Solaris is missing flock()

fixes rust-lang#103630
@albertlarsan68
Copy link
Member

Thanks for the PR!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 13, 2023

📌 Commit 04dfedb has been approved by albertlarsan68

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 Mar 13, 2023
@albertlarsan68
Copy link
Member

@bors ping

@bors
Copy link
Contributor

bors commented Mar 13, 2023

😪 I'm awake I'm awake

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2023
@matthiaskrgr
Rollup merge of rust-lang#108607 - psumbera:solaris-no-flock-bootstra…
bb5417d 
…p, r=albertlarsan68

Don't use fd-lock on Solaris in bootstrap

...as Solaris is missing flock()

fixes rust-lang#103630
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 13, 2023
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 13, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2023
@bors
Auto merge of rust-lang#109097 - matthiaskrgr:rollup-6ydc4ri, r=matth…
c54af45 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#108419 (Stabilize `atomic_as_ptr`)
 - rust-lang#108507 (use `as_ptr` to determine the address of atomics)
 - rust-lang#108607 (Don't use fd-lock on Solaris in bootstrap)
 - rust-lang#108830 (Treat projections with infer as placeholder during fast reject in new solver)
 - rust-lang#109055 (create `config::tests::detect_src_and_out` test for bootstrap)
 - rust-lang#109058 (Document BinOp::is_checkable)
 - rust-lang#109081 (simd-wide-sum test: adapt for LLVM 17 codegen change)
 - rust-lang#109083 (Update books)
 - rust-lang#109088 (Gracefully handle `#[target_feature]` on statics)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b4c7fc4 into rust-lang:master Mar 14, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 14, 2023
@gco
Copy link
Contributor

gco commented Mar 14, 2023

Just a note to say I sent @psumbera a compatibility stub of flock implemented using lockf for traditional Unix systems a while back.

@psumbera
Copy link
Contributor Author

Just a note to say I sent @psumbera a compatibility stub of flock implemented using lockf for traditional Unix systems a while back.

Originally I was proposing this: #103630 . But it was (reasonably) rejected because of different semantics.

Now there is also basic fcntl-style locking support in Rustix: bytecodealliance/rustix#555

Maybe this can be later leveraged somehow (either directly or via fd-lock?!).

This was referenced Apr 18, 2023
Closed
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2023
@GuillaumeGomez
Rollup merge of rust-lang#110485 - albertlarsan68:fix-bootstrap-lock,…
aa1247a 
… r=ozkanonur

Fix bootstrap locking

Fix the regression introduced in rust-lang#108607

Fixes rust-lang#109967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@albertlarsan68 albertlarsan68

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

Successfully merging this pull request may close these issues.

rust 1.64 now uses fd-lock->rustix->flock() for bootstraping where flock() is not available on Solaris
5 participants
@psumbera @rustbot @albertlarsan68 @bors @gco