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

[DO NOT MERGE] make before_exec an unsafe fn #56129

Closed
wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

This is an experiment for #39575.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2018
@RalfJung
Copy link
Member Author

@bors try

@bors

This comment has been minimized.

@RalfJung
Copy link
Member Author

@bors try

@bors

This comment has been minimized.

@RalfJung
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 21, 2018

⌛ Trying commit 39c3fd7 with merge aaadaed...

bors added a commit that referenced this pull request Nov 21, 2018
[DO NOT MERGE] make before_exec an unsafe fn

This is an experiment for #39575.
@rust-highfive

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Nov 21, 2018

☀️ Test successful - status-travis
State: approved= try=True

@RalfJung
Copy link
Member Author

@rust-lang/infra could we get a check-only crater run?

@alexcrichton
Copy link
Member

@craterbot run start=master#289ad6e9922683807d455ca0020dc2a8f7bd1a7b end=try#aaadaed28c4f579449b007d5228f7e512be9af5f mode=check-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-56129 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2018
@craterbot
Copy link
Collaborator

🚧 Experiment pr-56129 is now running on agent aws-2.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-56129 is completed!
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 23, 2018
@pietroalbini
Copy link
Member

102 regressions, including Cargo (and thus a ton of Cargo plugins).

@pietroalbini
Copy link
Member

Ran crater-tree on the results. As usual, the graph is far from perfect and it misses some things, but it can be useful anyway.

Results
cargo-disassemble
cargo-geiger
cargo-debstatus
cargo-tally
cargo-scriptify
mdbook-epub
stund
lateral
cargo-tree
cargo
| cargo-apk
| cargo-faircode
| cargo-bom
| cargo-sphinx
| cargo-outdated
| cargo-ebuild
| cargo-dinghy
| dinghy-lib
| cargo-clone
| cargo-authors
| dinghy
| cuo
| cargo-bitbake
| cargo-vendor
| cargo-travis
| cargo-raze
| cargo-pack
| | cargo-pack-docker
| oysterpack_built
| | oysterpack
| | oysterpack_log
| nannou-new
riffol
cargo-watch
watchexec
exclave
varlink-cli
sccache
rtq
initrs
tracetree
sinit
jobserver
tokio-pty-process
axon
cntr
process_viewer
tmux-hints
rtss
varlink_stdinterfaces
varlink
commandspec
tty
eloggr
runny
spawn-ptrace
pseudoterm

@kennytm
Copy link
Member

kennytm commented Nov 23, 2018

The cargo failure was caused by jobserver, which the tree above doesn't show 🤔

Nov 23 04:18:59.734 INFO [stderr] error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
Nov 23 04:18:59.734 INFO [stderr]    --> /cargo-home/registry/src/github.com-1ecc6299db9ec823/jobserver-0.1.11/src/lib.rs:566:13
Nov 23 04:18:59.734 INFO [stderr]     |
Nov 23 04:18:59.734 INFO [stderr] 566 | /             cmd.before_exec(move || {
Nov 23 04:18:59.734 INFO [stderr] 567 | |                 set_cloexec(read, false)?;
Nov 23 04:18:59.734 INFO [stderr] 568 | |                 set_cloexec(write, false)?;
Nov 23 04:18:59.734 INFO [stderr] 569 | |                 Ok(())
Nov 23 04:18:59.734 INFO [stderr] 570 | |             });
Nov 23 04:18:59.734 INFO [stderr]     | |______________^ call to unsafe function
Nov 23 04:18:59.734 INFO [stderr]     |
Nov 23 04:18:59.734 INFO [stderr]     = note: consult the function's documentation for information on how to avoid undefined behavior

@pietroalbini
Copy link
Member

Ok, did a simpler grep, and there are 32 crates actually failing (without counting dependencies):

List
axon
bind_spawn
cntr
commandspec
containers
fisher
initrs
jobserver
kickoff
lateral
mathsbot
minit
playlist_mgr
process_viewer
pseudoterm
ps-util
pty
repo-manager
riffol
rtq
rtss
runny
rush
sinit
spawn-ptrace
symare
tmux-hints
tokio-pty-process
tty
tydra
varlink
watchexec

@RalfJung
Copy link
Member Author

Thanks for the analysis @pietroalbini! Let's discuss at #39575.

@RalfJung RalfJung closed this Nov 24, 2018
@RalfJung RalfJung deleted the before-exec branch February 17, 2019 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants