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

Simple implementation of chmod(path, mode) #7590

Merged
merged 2 commits into from
Aug 11, 2014
Merged

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented Jul 13, 2014

currently only integer modes, can do string modes like +x later
fixes #7574

Interestingly on Windows, it looks like libuv ignores everything you ask for except the 0o200 bit, and the returned filemode() ends up either 0o444 or 0o666.

@@ -164,6 +165,11 @@ end
@windowsxp_only symlink(p::String, np::String) =
error("WindowsXP does not support soft symlinks")

function chmod(p::String, mode::Integer)
err = ccall(:jl_fs_chmod, Int32, (Ptr{Uint8}, Cint), bytestring(p), mode)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to call bytestring here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

utf8 filename?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, there you go. Was going off the example of every other ccall in base/fs.jl that takes a string, looks like those could all be cleaned up and remove the bytestring then.

Copy link
Member

Choose a reason for hiding this comment

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

Why would bytestring not be needed? The parameter type is String!

@porterjamesj
Copy link
Contributor

💯 been wanting this for a while

@tkelman
Copy link
Contributor Author

tkelman commented Jul 26, 2014

Rebased on latest master, and added a @windows_only call to chmod during rm. Since Julia's rm is more like unlink than rm on Unix in terms of (not) checking file permissions before deleting, I'm making the Windows behavior consistent with the Unix behavior here. An alternative would be to do a permissions sanity check for all OS'es, and add a force keyword argument.

@staticfloat
Copy link
Sponsor Member

This looks great to me. Thanks for tackling this, @tkelman.

@jakebolewski it might be interesting to try out this branch and see if it fixes your libgit2 issues before merging, but for the record, I think this is ready to merge.

tkelman added a commit that referenced this pull request Aug 11, 2014
Simple implementation of chmod(path, mode)
@tkelman tkelman merged commit edab6bc into JuliaLang:master Aug 11, 2014
@tkelman
Copy link
Contributor Author

tkelman commented Aug 11, 2014

I'm hitting the button on this now that master's open for 0.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chmod() missing Julia-side
5 participants