add renameat2 and flags #1508

phi-gamma · 2019-09-11T10:50:02Z

This used to be PR #554.

Adapted to match the current state of libc. Also, adapt for the
fact that in Linux, the flags argument is of type unsigned,
not int.

rust-highfive · 2019-09-11T10:50:06Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

phi-gamma · 2019-09-11T10:54:04Z

Note that I changed the definition of the flags to match the
signature of the syscall argument. IMO this is the right thing
to do since renameat2() is the sole syscall the macros
are used with.

This means the type now diverges from those in
src/fuchsia/mod.rs. Not sure whether this is an issue
for you guys.

gnzlbg · 2019-09-11T11:23:10Z

This means the type now diverges from those in
src/fuchsia/mod.rs. Not sure whether this is an issue
for you guys.

Not an issue, but @josephlr might want to take a look at those.

Looking at the CI errors, these are apparently broken on musl. You might want to move these to the gnu module. Other than that, this looks good to merge to me.

josephlr · 2019-09-11T20:21:55Z

Not an issue, but @josephlr might want to take a look at those.

Seems fine. This divergence is expected and shouldn't break anything.

See rust-lang/libc#1508 The type of the RENAME_* constants has been adjusted to match the type (unsigned int) of the syscall parameter. Signed-off-by: Philipp Gesang <phg@phi-gamma.net>

The renameat2 syscall is available as a glibc wrapper since the libc crate now supports glibc 2.28+. See rust-lang/libc#1508

gnzlbg · 2019-09-13T20:02:58Z

@bors: r+

bors · 2019-09-13T20:02:59Z

📌 Commit 8463a87 has been approved by gnzlbg

bors · 2019-09-13T22:42:45Z

⌛ Testing commit 8463a87 with merge ae9350e...

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2019-09-13T23:37:45Z

💔 Test failed - status-azure

phi-gamma · 2019-09-16T07:02:12Z

Regarding the CI issues: I have rustfmt 1.3 (stable) here which doesn’t catch
that formatting issue, so I applied the changes as the version in CI suggests.

Can’t comment on that linker error on sparc or what seems to be an update
problem on arm.

gnzlbg · 2019-09-16T07:03:53Z

cargo +nightly fmt --all should use the nightly rustfmt if you have a nightly toolchain installed, even if it is not your default.

phi-gamma · 2019-09-16T08:55:29Z

Sorry, there is no nightly toolchain on stock Fedora 30 and rustup
is allergic against the system toolchain. I’m going to have to do that
at home later.

gnzlbg · 2019-09-16T09:00:37Z

@cuviper might be able to give some advice.

I don't think this problem has a great solution. Every stable rustfmt point release produces different formatting as well, due to bugs being fixed, etc. So picking one single rustfmt version isn't really a great solution to this problem either :/ I'm open to suggestions. We definitely have some freedom about which rustfmt version we pick.

We could also just use rustfmt in an advisory position, and just reformat the library before each release. But sadly rustfmt isn't always idempotent (e.g. cargo fmt --all might produce output that fails a subsequent cargo fmt --all -- --check), so I worry that would create a higher maintanence burden. I obviously find it easier if we only merge PRs that are properly formatted.

mati865 · 2019-09-16T11:12:43Z

Rustup can be made to live alongside system Rust toolchain: https://github.com/rust-lang/rustup.rs#working-with-distribution-rust-packages

cuviper · 2019-09-16T11:45:05Z

Yeah, I also use rustup that way myself.

Picking a rustfmt version for CI is hard, but I don't think tracking nightly is the best. If nothing else, it's annoying for contributors if they have to fix unrelated formatting in their PR, just because it changed in nightly. Implicitly tracking stable is a little better because that changes less frequently. But I think it's best to choose an explicit stable version, even if as a maintainer you always bump it to latest stable -- that act of bumping can catch up on any formatting changes at the same time.

gnzlbg · 2019-09-16T12:14:33Z

FWIW the main reason libc uses nightly rustfmt is that you need nightly rust to run libc tests, so you already need to do cargo +nightly test --all to prepare a PR if nightly isn't your default toolchain.

cuviper · 2019-09-16T14:07:09Z

Sure, that's reasonable.

phi-gamma · 2019-09-17T06:24:18Z

@mati865: Those instructions proved incomplete for me; rustup outright
refuses to work unless I pass RUSTUP_INIT_SKIP_PATH_CHECK=yes.

gnzlbg · 2019-09-21T09:35:05Z

@bors: r+

bors · 2019-09-21T09:35:06Z

📌 Commit fea39c1 has been approved by gnzlbg

bors · 2019-09-21T11:23:12Z

⌛ Testing commit fea39c1 with merge 8e81c08...

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2019-09-21T11:43:56Z

💔 Test failed - checks-cirrus-freebsd-12

gnzlbg · 2019-10-15T23:11:21Z

@bors: r+

bors · 2019-10-15T23:11:22Z

💡 This pull request was already approved, no need to approve it again.

This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
There's another pull request that is currently being tested, blocking this pull request: Support vxWorks in libc-test #1543

bors · 2019-10-15T23:11:23Z

📌 Commit fea39c1 has been approved by gnzlbg

bors · 2019-10-16T01:14:50Z

⌛ Testing commit fea39c1 with merge be44a92...

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2019-10-16T02:50:44Z

💔 Test failed - status-azure

bors · 2019-11-18T09:22:15Z

⌛ Testing commit fea39c1 with merge 13664cc...

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

gnzlbg · 2019-11-18T09:26:47Z

@bors r- retry

gnzlbg · 2019-11-18T16:26:55Z

@bors: retry

gnzlbg · 2019-11-28T16:39:38Z

@bors: r+

bors · 2019-11-28T16:39:39Z

📌 Commit fea39c1 has been approved by gnzlbg

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2019-11-28T17:44:01Z

⌛ Testing commit fea39c1 with merge 9d1230e...

bors · 2019-11-28T18:55:39Z

💔 Test failed - status-azure

gnzlbg · 2019-11-29T08:39:52Z

@bors: retry

There is a failure in sparc64, and it appears legit, but lets try one more time.

bors · 2019-11-29T08:40:00Z

⌛ Testing commit fea39c1 with merge d22b2de...

add renameat2 and flags This used to be PR #554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2019-11-29T09:54:18Z

💔 Test failed - status-azure

bors · 2020-02-20T23:16:53Z

☔ The latest upstream changes (presumably #1639) made this pull request unmergeable. Please resolve the merge conflicts.

This used to be PR rust-lang#554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

bors · 2020-02-28T02:58:49Z

☔ The latest upstream changes (presumably #1638) made this pull request unmergeable. Please resolve the merge conflicts.

fbernier · 2020-12-23T20:56:33Z

Hey, seems like there was an issue merging this in but the build is now gone. Can we try again and I'll check out the failure if there's one?

rust-highfive assigned gnzlbg Sep 11, 2019

phi-gamma force-pushed the renameat2 branch from 09464cf to 8463a87 Compare September 11, 2019 12:12

phi-gamma added a commit to phi-gamma/openat that referenced this pull request Sep 12, 2019

Use libc::renameat2 instead of syscall(2)

eefe239

The renameat2 syscall is available as a glibc wrapper since the libc crate now supports glibc 2.28+. See rust-lang/libc#1508

phi-gamma force-pushed the renameat2 branch from 8463a87 to ad29be4 Compare September 16, 2019 06:55

phi-gamma force-pushed the renameat2 branch from ad29be4 to fea39c1 Compare September 16, 2019 08:40

add renameat2 and flags

087c2f3

This used to be PR rust-lang#554. Adapted to match the current state of *libc*. Also, adapt for the fact that in Linux, the *flags* argument is of type *unsigned*, not *int*.

phi-gamma force-pushed the renameat2 branch from fea39c1 to 087c2f3 Compare February 22, 2020 23:18

JohnTitor added the S-waiting-on-review label Oct 15, 2020

wpbrown mentioned this pull request Dec 12, 2020

Need atomic file replacement for state and configuration updates blockcaptain/blockcaptain#23

Open

JohnTitor mentioned this pull request Mar 17, 2021

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

Merged

bors closed this in e3dd608 Mar 17, 2021

add renameat2 and flags #1508

add renameat2 and flags #1508

Conversation

phi-gamma commented Sep 11, 2019

rust-highfive commented Sep 11, 2019

phi-gamma commented Sep 11, 2019

gnzlbg commented Sep 11, 2019

josephlr commented Sep 11, 2019

gnzlbg commented Sep 13, 2019

bors commented Sep 13, 2019

bors commented Sep 13, 2019

bors commented Sep 13, 2019

phi-gamma commented Sep 16, 2019

gnzlbg commented Sep 16, 2019

phi-gamma commented Sep 16, 2019

gnzlbg commented Sep 16, 2019

mati865 commented Sep 16, 2019

cuviper commented Sep 16, 2019

gnzlbg commented Sep 16, 2019 • edited Loading

cuviper commented Sep 16, 2019

phi-gamma commented Sep 17, 2019

gnzlbg commented Sep 21, 2019

bors commented Sep 21, 2019

bors commented Sep 21, 2019

bors commented Sep 21, 2019

gnzlbg commented Oct 15, 2019

bors commented Oct 15, 2019

bors commented Oct 15, 2019

bors commented Oct 16, 2019

bors commented Oct 16, 2019

bors commented Nov 18, 2019

gnzlbg commented Nov 18, 2019

gnzlbg commented Nov 18, 2019

gnzlbg commented Nov 28, 2019

bors commented Nov 28, 2019

bors commented Nov 28, 2019

bors commented Nov 28, 2019

gnzlbg commented Nov 29, 2019

bors commented Nov 29, 2019

bors commented Nov 29, 2019

bors commented Feb 20, 2020

bors commented Feb 28, 2020

fbernier commented Dec 23, 2020

gnzlbg commented Sep 16, 2019 •

edited

Loading