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

Add wrappers for futimens(2) and utimesat(2) #944

Merged
merged 1 commit into from Sep 30, 2018
Merged

Conversation

@jmmv
Copy link
Contributor

@jmmv jmmv commented Sep 24, 2018

Hello again!

Let's see how this looks like. I don't think there is any other way of doing this via the standard library, though I see there is a filetime crate in the Rust nursery to add this missing functionality. Not sure what your policy for adding features into nix is.

@jmmv jmmv force-pushed the jmmv:utimes branch from 63b6851 to 539f9f1 Sep 25, 2018
@asomers
Copy link
Member

@asomers asomers commented Sep 25, 2018

utimes(2) is obsolete. futimens and utimensat are the newer replacements. So you should be using one of those instead. Or are you working on an OS that doesn't have them?

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 25, 2018

Why is utimes obsolete? I don't see it documented as such in the manual page (of macOS).

But I can change this to implement the other two calls if you prefer; I'm not bound to using utimes at the moment. Is there anything else major to change?

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 25, 2018

Grr, except that libc lacks both utimensat and futimens support for macOS. I guess we should fix that first...

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 25, 2018

Sent PR for libc in rust-lang/libc#1084.

@jmmv jmmv force-pushed the jmmv:utimes branch from 539f9f1 to 1089c08 Sep 25, 2018
@jmmv jmmv changed the title Add a wrapper for utimes(2) Add wrappers for futimens(2) and utimesat(2) Sep 25, 2018
@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 25, 2018

Alright, dependent PR addressed, so updated this one with new wrappers as you proposed.

@asomers
Copy link
Member

@asomers asomers commented Sep 25, 2018

Sigh. It looks like libc needs a PR for FreeBSD, too.

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 25, 2018

Sent rust-lang/libc#1085 for FreeBSD and DragonFly.

bors added a commit to rust-lang/libc that referenced this pull request Sep 25, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 26, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 26, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 26, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 26, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 27, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
bors added a commit to rust-lang/libc that referenced this pull request Sep 28, 2018
Expose futimens and utimensat on FreeBSD and DragonFly

Should have sent this out with #1084 but I didn't notice then that these were missing (see nix-rust/nix#944).

Haven't been able to test these locally as I do not have access to these platforms. Will rely on CI to verify these for me.
@jmmv jmmv force-pushed the jmmv:utimes branch from 1089c08 to 313f857 Sep 28, 2018
@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 28, 2018

Dependent libc change submitted. Forced a push to the PR to trigger another CI run here.

Copy link
Member

@asomers asomers left a comment

Apart from that one malformed comment, it LGTM.

src/sys/stat.rs Outdated Show resolved Hide resolved
@jmmv jmmv force-pushed the jmmv:utimes branch from 313f857 to 0f2a5bc Sep 29, 2018
@asomers
Copy link
Member

@asomers asomers commented Sep 29, 2018

Thanks. But one more thing: please add an entry in CHANGELOG.

@jmmv jmmv force-pushed the jmmv:utimes branch 2 times, most recently from 717debc to 2be6bec Sep 29, 2018
@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Sep 29, 2018

CHANGELOG entry added and branch rebased on master to avoid conflicts.

CHANGELOG.md Show resolved Hide resolved
@jmmv jmmv force-pushed the jmmv:utimes branch from 2be6bec to 886c76c Sep 30, 2018
@asomers
Copy link
Member

@asomers asomers commented Sep 30, 2018

Thanks for your contribution.
bors r+

bors bot added a commit that referenced this pull request Sep 30, 2018
Merge #944
944: Add wrappers for futimens(2) and utimesat(2) r=asomers a=jmmv

Hello again!

Let's see how this looks like. I don't think there is any other way of doing this via the standard library, though I see there is a `filetime` crate in the Rust nursery to add this missing functionality. Not sure what your policy for adding features into `nix` is.

Co-authored-by: Julio Merino <jmmv@google.com>
@bors bors bot merged commit 886c76c into nix-rust:master Sep 30, 2018
4 checks passed
4 checks passed
bors Build succeeded
Details
buildbot/nix-rust/nix amd64_fbsd11 Build done.
Details
buildbot/nix-rust/nix i386_fbsd11 Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jmmv jmmv deleted the jmmv:utimes branch Sep 30, 2018
@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Oct 1, 2018

Ugh, I wasn't expecting the original message in the PR review thread would appear in the commit message :-( I guess I know for next time.

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Oct 1, 2018

And more: I just found out the hard way that futimens and utimesat are not available on macOS 10.12 or below. Not that I care much about supporting that old version, but I have to use this version on Travis as it's the only one that allows loading the FUSE kext.

What's the policy for nix regarding OS supported versions?

@asomers
Copy link
Member

@asomers asomers commented Oct 1, 2018

Nix doesn't attempt to guarantee consistent features across different OSes and versions, since its objective is simply to expose OS functionality. If a Nix function returns ENOSYS on older OSes, that's fine.

@jmmv
Copy link
Contributor Author

@jmmv jmmv commented Oct 1, 2018

But then, would it be OK to also implement utimes? This deprecated function is going to exist pretty much everywhere, and for my specific use case, it's sufficient. Otherwise, in those systems where these functions return ENOSYS, we need to fall back to raw libc calls...

@asomers
Copy link
Member

@asomers asomers commented Oct 1, 2018

Yeah, I think that's ok.

jmmv added a commit to jmmv/nix that referenced this pull request Oct 2, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.
@jmmv jmmv mentioned this pull request Oct 2, 2018
jmmv added a commit to jmmv/nix that referenced this pull request Oct 2, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.
bors bot added a commit that referenced this pull request Oct 4, 2018
Merge #946
946: Add a wrapper for utimes(2) r=asomers a=jmmv

PR #944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

Co-authored-by: Julio Merino <julio@meroh.net>
jmmv added a commit to jmmv/nix that referenced this pull request Nov 5, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

This should have been part of PR nix-rust#946, which added a wrapper for
utimes(2) following this same rationale, but missed lutimes(2) because
I simply didn't notice it existed.
@jmmv jmmv mentioned this pull request Nov 5, 2018
jmmv added a commit to jmmv/nix that referenced this pull request Nov 7, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

This should have been part of PR nix-rust#946, which added a wrapper for
utimes(2) following this same rationale, but missed lutimes(2) because
I simply didn't notice it existed.
jmmv added a commit to jmmv/nix that referenced this pull request Nov 7, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

This should have been part of PR nix-rust#946, which added a wrapper for
utimes(2) following this same rationale, but missed lutimes(2) because
I simply didn't notice it existed.
bors bot added a commit that referenced this pull request Nov 27, 2018
Merge #967
967: Add a wrapper for lutimes(2) r=asomers a=jmmv

PR #944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

This should have been part of PR #946, which added a wrapper for
utimes(2) following this same rationale, but missed lutimes(2) because
I simply didn't notice it existed.

Co-authored-by: Julio Merino <julio@meroh.net>
levex added a commit to levex/nix that referenced this pull request Dec 3, 2018
PR nix-rust#944 added wrappers for the more-modern futimens(2) and utimesat(2),
but unfortunately these APIs are not available on old-ish systems.

In particular, macOS Sierra and below don't implement them, making the
new APIs unusable.  Whether we should care about such "old" systems is
debatable, but the problem is that, at the moment, this is the only
macOS version usable on Travis to test kexts and, thus, to test FUSE
file systems.

This should have been part of PR nix-rust#946, which added a wrapper for
utimes(2) following this same rationale, but missed lutimes(2) because
I simply didn't notice it existed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants