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

avoid file ownership requirement in touch #28819

Merged
merged 1 commit into from
Sep 28, 2018
Merged

avoid file ownership requirement in touch #28819

merged 1 commit into from
Sep 28, 2018

Conversation

tanmaykm
Copy link
Member

Using futimes and passing NULL for the times parameter sets the current and access file times, but does not require file ownwership (on unices).

From man page:

If times is NULL, the access and modification times are set to the current time. The caller must be the owner of the file, have permission to write the file, or be the superuser.

ref: #28815

@tanmaykm
avoid file ownership requirement in touch
9d6cd42 
Using `futimes` and passing NULL for the `times` parameter sets the current and access file times, but does not require file ownwership.

From man page:
> If times is NULL, the access and modification times are set to the current time. The caller must be the owner of the file, have permission to write the file, or be the superuser.
@tanmaykm
Copy link
Member Author

File tests have passed. CI failure is in LinearAlgebra/matmul and seems unrelated.

@tanmaykm tanmaykm changed the title WIP: avoid file ownership requirement in touch avoid file ownership requirement in touch Aug 22, 2018
@tanmaykm tanmaykm mentioned this pull request Aug 22, 2018
Closed
@Keno
Copy link
Member

Keno commented Aug 22, 2018

This seems fine, but maybe the proper solution is to fix libuv instead?

@tanmaykm
Copy link
Member Author

Yes, it may go in for a longer cycle though, which should be fine as long as we have the solution in Julia in the mean time.

It is arguably not a bug in libuv uv_fs_futime implementation, but just that it exposes a limited subset of the underlying apis capability. Maybe a more appropriate change to libuv would be to have a uv_fs_touch which does the equivalent of what we do in Julia's touch?

@tanmaykm
Copy link
Member Author

CI passing now (after a re-run)

@aviks
Copy link
Member

aviks commented Sep 27, 2018

Can this be merged now?

@JeffBezanson
Copy link
Member

futimes does not seem to accept a pointer as the second argument. Am I missing something?

@tanmaykm
Copy link
Member Author

It does... as per linux and osx man pages. Doesn't it?

@JeffBezanson
Copy link
Member

Ah, though the linux man page doesn't mention the NULL argument value.

@tanmaykm
Copy link
Member Author

It says it behaves similar to utimes and utimes man page does mention behavior when NULL is passed.

@nalimilan
Copy link
Member

I'd rather apply a temporary patch to our libuv fork than add platform-specific code in Julia.

@JeffBezanson
Copy link
Member

I agree it's not really a bug in libuv though; it would be adding a new function there. We should probably just merge this.

@JeffBezanson JeffBezanson reopened this Sep 28, 2018
@JeffBezanson JeffBezanson added the io Involving the I/O subsystem: libuv, read, write, etc. label Sep 28, 2018
@JeffBezanson JeffBezanson merged commit 3b3a163 into JuliaLang:master Sep 28, 2018
@aviks
Copy link
Member

aviks commented Nov 11, 2018

Hmnn.. .This does not seem to have made into 1.0.2, which is unfortunate since I keep hitting this. Added a backport pending label now.

@StefanKarpinski StefanKarpinski added triage This should be discussed on a triage call backport 1.0 and removed triage This should be discussed on a triage call labels Jan 31, 2019
@JeffBezanson JeffBezanson removed triage This should be discussed on a triage call triage backport pending 1.0 labels Jan 31, 2019
JeffBezanson pushed a commit that referenced this pull request Jun 6, 2019
@tanmaykm @JeffBezanson
avoid file ownership requirement in touch (#28819)
51495d3 
Using `futimes` and passing NULL for the `times` parameter sets the current and access file times, but does not require file ownwership.

From man page:
> If times is NULL, the access and modification times are set to the current time. The caller must be the owner of the file, have permission to write the file, or be the superuser.
(cherry picked from commit 3b3a163)
@KristofferC KristofferC mentioned this pull request Aug 26, 2019
Merged
55 tasks
KristofferC pushed a commit that referenced this pull request Aug 26, 2019
@tanmaykm @KristofferC
avoid file ownership requirement in touch (#28819)
375fc8b 
Using `futimes` and passing NULL for the `times` parameter sets the current and access file times, but does not require file ownwership.

From man page:
> If times is NULL, the access and modification times are set to the current time. The caller must be the owner of the file, have permission to write the file, or be the superuser.

(cherry picked from commit 3b3a163)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
@tanmaykm @KristofferC
avoid file ownership requirement in touch (#28819)
1959454 
Using `futimes` and passing NULL for the `times` parameter sets the current and access file times, but does not require file ownwership.

From man page:
> If times is NULL, the access and modification times are set to the current time. The caller must be the owner of the file, have permission to write the file, or be the superuser.

(cherry picked from commit 3b3a163)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tanmaykm @Keno @aviks @JeffBezanson @nalimilan @StefanKarpinski @KristofferC