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

Add renameat2 and change their flags' type to c_uint #2116

Merged
merged 1 commit into from
Mar 17, 2021
Merged

Add renameat2 and change their flags' type to c_uint #2116

merged 1 commit into from
Mar 17, 2021

Conversation

JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Mar 17, 2021 •
edited
Loading

Taking over #1508. This changes the type of some flags but it should've been c_uint.

Closes #1508

@JohnTitor @Mic92 @phi-gamma
Add renameat2 and change their flags' type to c_uint
b90fda7 
Co-Authored-By: Jörg Thalheim <github@thalheim.io>
Co-Authored-By: Philipp Gesang <phg@phi-gamma.net>
@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@JohnTitor JohnTitor mentioned this pull request Mar 17, 2021
Merged
@Amanieu
Copy link
Member

Amanieu commented Mar 17, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 17, 2021

📌 Commit b90fda7 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Mar 17, 2021

⌛ Testing commit b90fda7 with merge e3dd608...

@bors
Copy link
Contributor

bors commented Mar 17, 2021

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing e3dd608 to master...

@bors bors merged commit e3dd608 into rust-lang:master Mar 17, 2021
@JohnTitor JohnTitor deleted the renameat2 branch March 17, 2021 23:44
@bgilbert bgilbert mentioned this pull request Mar 19, 2021
Closed
@bgilbert
Copy link

The type change causes a build failure for openat (tailhook/openat#35). Should this have gone through a deprecation period per the breaking change policy?

@lucab lucab mentioned this pull request Mar 19, 2021
Closed
@phi-gamma
Copy link
Contributor

Thanks for merging this!

cgwalters added a commit to cgwalters/openat that referenced this pull request Mar 19, 2021
@cgwalters
dir: Cast RENAME_EXCHANGE back to libc::c_int
42f87fa 
Workaround tailhook#35
AKA rust-lang/libc#2116

Unfortunately since we made this libc::c_int in our
public API, we can't easily change it right now.

Closes: tailhook#35
@cgwalters cgwalters mentioned this pull request Mar 19, 2021
Merged
@mosyp mosyp mentioned this pull request Mar 21, 2021
Merged
@JohnTitor
Copy link
Member Author

The type change causes a build failure for openat (tailhook/openat#35). Should this have gone through a deprecation period per the breaking change policy?

@bgilbert I'd consider it as bugfix since renameat2 needs c_uint as their flag and the initial definition (i.e. c_int) should be wrong in that context. But hmm is there any use of the flags as c_int?

@bgilbert
Copy link

Well, it can be a bugfix and still be a breaking change. 🙂 But meanwhile, openat has been updated with a workaround in tailhook/openat#36.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
S-waiting-on-bors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JohnTitor @rust-highfive @Amanieu @bors @bgilbert @phi-gamma