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 File.chown and File.chmod #3358

Merged
merged 1 commit into from
Sep 28, 2016

Conversation

ysbaddaden
Copy link
Contributor

Implements File.chown(path, uid, gid) as well as File.chmod(path, mode).

I followed the POSIX definitions which made more sense in the context of File, instead of following the Ruby definitions that behave like the UNIX CLI tools: chown uid, gid, *paths and chmod mode, *paths. These last forms would be better into FileUtils (which is expected to behave like a shell).

I didn't:

  • implement the File#chown and File#chmod counterparts (targeting a file descriptor, not a path). I'm not sure they are that useful (I never needed fchown nor fchmod).
  • write specs for chown because it required special privileges or a particular computer setup. I manually tested it behaved as documented instead.

@asterite
Copy link
Member

Looks good to me, thank you!

I think we can add typeof specs for chown, so at least we test that it compiles and keeps compiling after changes. What do you think? Something like:

typeof(File.chown("foo", uid: 100, gid: 200, follow_symlinks: true))

@drosehn
Copy link

drosehn commented Sep 27, 2016

Aside:
fchown() and fchmod() are important to use to avoid race conditions, especially when a program is running as root. See, for instance, table 4-1 in the section Working with Publicly Writable Files Using POSIX Calls in this Apple documentation:

https://developer.apple.com/.../Articles/RaceConditions.html

or google for: chown vs race conditions
or: chmod vs race conditions

I work with several programs that run as root, so I'm pretty familiar with this detail !

@ysbaddaden
Copy link
Contributor Author

@asterite done.

@drosehn good point, but that may come later on.

@asterite asterite merged commit c2cabff into crystal-lang:master Sep 28, 2016
@ysbaddaden ysbaddaden deleted the std-file-chmod-chown branch September 28, 2016 12:18
@asterite asterite added this to the 0.19.3 milestone Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants