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

fix: assure possibly blocking non-files (like FIFOs) won't be picked up for publishing. #14977

Merged
merged 2 commits into from
Dec 24, 2024

Conversation

Byron
Copy link
Member

@Byron Byron commented Dec 23, 2024

Follow-up of #14975, related to GitoxideLabs/gitoxide#1629.

This would otherwise cause the publish to hang if it tries to read from a fifo.

@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2024

r? @ehuss

rustbot has assigned @ehuss.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 23, 2024
@Byron Byron marked this pull request as ready for review December 23, 2024 07:13
…up for publishing.

This would otherwise cause the publish to hang.
@Byron
Copy link
Member Author

Byron commented Dec 23, 2024

The test-failure seems unrelated:

---- expected: tests/testsuite/test.rs:5014:13
++++ actual:   stderr
   1    1 | [RUNNING] `[..]--crate-name a [..]-C panic=abort[..]`
   2    2 | [RUNNING] `[..]--crate-name foo [..]-C panic=abort[..]`
   3      - [RUNNING] `[..]--crate-name foo [..]-C panic=abort[..]--test[..]`
   4    3 | ...∅

@Rustin170506
Copy link
Member

The test-failure seems unrelated:

Yes. It is unrelated. I tried it locally, and it works well. I will help you rerun it.

Copy link
Member

@weihanglo weihanglo 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 fix!

tests/testsuite/package.rs Outdated Show resolved Hide resolved
tests/testsuite/git.rs Outdated Show resolved Hide resolved
Personally I liked that the test was only dependent on what really matters, the lack of presence of a particular filename. Now the test would fail if Cargo one day adds more (generated) files to the package.

Co-authored-by: Weihang Lo <[email protected]>
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks!

@weihanglo weihanglo added this pull request to the merge queue Dec 24, 2024
Merged via the queue into rust-lang:master with commit 8721d52 Dec 24, 2024
20 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 25, 2024
Update cargo

6 commits in 652623b779c88fe44afede28bf7f1c9c07812511..c86f4b3a1b153218e6e50861214b0b4b4e695f23
2024-12-20 15:44:42 +0000 to 2024-12-24 17:49:48 +0000
- fix(package): check dirtiness of path fields in manifest (rust-lang/cargo#14966)
- test: make path arguments more generic and flexible (rust-lang/cargo#14979)
- Moved manifest metadata tracking from fingerprint to dep info (rust-lang/cargo#14973)
- fix: assure possibly blocking non-files (like FIFOs) won't be picked up for publishing. (rust-lang/cargo#14977)
- simplify SourceID Hash (rust-lang/cargo#14800)
- upgrade `gix` to the latest release 0.69. (rust-lang/cargo#14975)
@rustbot rustbot added this to the 1.85.0 milestone Dec 25, 2024
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.

5 participants