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 umask command #681

Closed
hanche opened this issue May 26, 2018 · 18 comments · Fixed by #949
Closed

Add umask command #681

hanche opened this issue May 26, 2018 · 18 comments · Fixed by #949

Comments

@hanche
Copy link
Contributor

hanche commented May 26, 2018

The title speaks for itself, I think. Especially on shared systems, it can sometimes be useful to be able to change the process's umask value.

@xiaq
Copy link
Member

xiaq commented May 31, 2018

It is an oversight.

On the other hand, adding a builtin to Elvish is really easy; you can take a look at eval/builtin_fn.go and see some existing builtins in eval/builtin_fn_*.go.

One thing that makes this slightly more tricky than other builtins is that umask does not exist on Windows, so you will need a build tag. For that you can see daemon/sock_unix.go for an example.

@hanche
Copy link
Contributor Author

hanche commented Jun 3, 2018

Okay, I may have a look. But it could take weeks, possibly months. Got two really hectic weeks coming up, followed by one vacation week.

Meanwhile, for the record: There is no reason why we have to mimic the umask command of other shells, I think. It might instead be tempting to make it a magic variable somewhat like pwd, so that I can say

umask=700 touch foo

and thereby create foo with mode 600.

Also, what about symbolic modes? We might wish to support those too, although designing the interface will require some thought.

@xiaq
Copy link
Member

xiaq commented Jun 3, 2018

Aha. Having umask as a variable sounds like a great idea.

Re symbolic mode, it can get interesting for a variable. If you do umask = u=rwx,g=,o=, is the value of $umask that string or 700? Can you do umask = g+wx? Let's start without symbolic mode first.

@notramo
Copy link

notramo commented Jun 4, 2018

What about adding a command for symbolic mode? (similar to set-env and get-env)

@hanche
Copy link
Contributor Author

hanche commented Jun 4, 2018

Yeah, if we want to go all out, here is my suggestion: The umask variable is displayed and set in octal, end of that story. Then add a umask command with all sorts of bells and whistles to set or display the value. It could have an option to display the current value symbolically, for example.

@zzamboni
Copy link
Contributor

zzamboni commented Jun 4, 2018

Interesting discussion, and nice that we can depart from "the traditional way" of doing things like umask and look for better models.

I like the variable idea. How about making the variable take only a numeric value, and have a builtin to convert symbolic to numeric specifications? E.g.:

~> umask-sym go-w
0022
~> umask=0022 touch foo
~> umask=(umask-sym go-w) touch foo # equivalent

For completeness, there should be a way to convert numeric-to-symbolic masks, like the umask -S command.

Note: Please remember that the umask value is the complement of the desired permission - i.e. the bits in the mask are disabled. So just for correctness, in @xiaq's example above, the numeric value of u=rwx,g=,o=, is 077, not 700. Reference: https://en.wikipedia.org/wiki/Umask#cite_note-UNIXeighth-1

@zzamboni
Copy link
Contributor

zzamboni commented Jun 4, 2018

@hanche heh, seems we had a very similar idea exactly at the same time (42 minutes ago) ;)

image

@krader1961
Copy link
Contributor

FWIW, I think this is a good example of a feature that belongs in the new os: name space (still waiting for review and merge, see PR #927 :-)

@hanche
Copy link
Contributor Author

hanche commented Mar 17, 2020

Could be, although I notice that the os: name space is claimed to be for os-specific constants, which this isn't. AFAIK, Windows doesn't have any notion of umask at all, whereas it works identically on all unix versions. To me, this doesn't qualify it for the os: namespace the way it seems to be envisioned. The only thing about umask that would vary between OSs, is whether or not it exists at all?

(Side note: I started implementing this shortly after posting the issue, then realised I didn't know either the go language, the appropriate library/ies, nor xiaq's coding conventions well enough to pull it off. Then life intervened before I could proceed to learning what I needed to figure out. By now, I can't even recall what exact problems I ran into, so I'd have to start from scratch. Oh well. It shouldn't have to be more than a dozen lines of code.)

@krader1961
Copy link
Contributor

the os: name space is claimed to be for os-specific constants

It's really for anything that cannot be done in an OS agnostic fashion in addition to OS specific constants. It's just that the PR which establishes the os: namespace didn't include any functions because I couldn't think of a good example to include in that PR.

Assuming Windows doesn't have an analogous concept, or we can't think of a way to support both UNIX and Windows in an agnostic fashion, then I would argue os:umask makes more sense conceptually than builtin:umask. Other builtins we might want to add that are inherently OS specific include things like chmod.

@krader1961
Copy link
Contributor

I'm going to take a stab at implementing a $os:umask variable that only accepts, and outputs, a numeric representation of the OS umask. With just an hour of effort (by basing my work on pkg/eval/pwd.go) I already have something that works; albeit without unit tests. What I don't have working yet, since I don't normally use non-UNIX systems, is making this build and behave sensibly on such systems.

@krader1961
Copy link
Contributor

FWIW, it shouldn't be necessary to have adjunct commands to support symbolic mode. Ideally the umask "get" and "set" methods would implicitly handle the conversion. Having that happen on the "set" method is easy. Having it happen on the "get" method is harder since we need to determine if the context requires emitting a number, implicit string (in which case an octal literal), or an explicit string as a result of the context being to-string $umask.

@hanche
Copy link
Contributor Author

hanche commented Mar 21, 2020

This is great news! Personally, I have lived a long and happy life with just octal umasks, so I don't see any need for symbolic ones.

As for unit tests, one test would be to set the umask to some value and then ask again, to see that the change sticks. And perhaps to run /bin/sh -c umask and check that the returned value matches what is expected. Both macos and ubuntu seem to return a four digit umask with leading zeros, but I think some unixes may strip the leading zeros, so that requires a bit of care.

How to make the code compile on windows, I have no idea.

If you'd like to share your code and aren't ready yet to put it here on github, feel free to email me. I just added my work email to my profile.

@xiaq
Copy link
Member

xiaq commented Mar 21, 2020

Re which module this should be in:

When thinking about "OS functionalities", there are two different kinds:

  • Functionalities that have different implementations across OS'es, but compatible API.
  • Functionalities that have incompatible API across OS'es, or may even exist in some OS but not exist in others.

Using the prior art of Python and Go, their os packages are both for the first type of functionalities.

The second type of functionalities are best exposed via different modules. Go got it wrong in the standard library by implementing a syscall package that has different API on different platforms, but corrected the mistake in golang.org/x/sys, which has a unix subpackage and a windows subpackage.

So the umask functionality (whether as a variable or function) should live in a new unix module.

@xiaq
Copy link
Member

xiaq commented Mar 21, 2020

Re support for symbolic modes: I'd rather that be implemented separately as a command, rather than building the conversion into the variable. My primary concern is that this could be injecting too much magic into the behavior of variable assignment, which should have straightforward semantics.

In any case, start with a version that only supports octal masks.

@krader1961
Copy link
Contributor

So the umask functionality (whether as a variable or function) should live in a new unix module.

I agree. Working on an implementation forced me to think about how to provide sane behavior on multiple platforms. I concluded it isn't possible. I initially thought it should be a simple no-op on non-UNIX systems in the hope of making a feature like $os:umask usable in portable Elvish code. But I agree that isn't realistic and more importantly should be handled not by elvish but by the person writing the elvish code using the possibly nonsensical feature on a particular platform.

My primary concern is that this could be injecting too much magic into the behavior of variable assignment, which should have straightforward semantics.

I 100% agree on further reflection. Even if the umask stringifier function could easily determine if it was in a "simple" string context and should therefore emit the octal value, or in a (to-string ...) context and should therefore emit the symbolic value, that subtle difference in behavior will cause more problems than it solves.

@krader1961
Copy link
Contributor

Also, given how seldom people seem to use symbolic umask values, there doesn't seem to be any justifiable reason to provide support for symbolic umask values. The umask functionality is something almost no shell users think about in my experience. Not even experienced software engineers who work on UNIX. Mostly people just accept the default provided by their environment. Those who need to customize the value can be expected to know about the bit representation and how that relates to operations like creating a file.

@krader1961
Copy link
Contributor

If you have a strong stomach read the POSIX specification for the umask command: https://pubs.opengroup.org/onlinepubs/007908799/xcu/umask.html

Pay particular attention to phrases such as

When using the symbolic mode form on a regular file, it is implementation-dependent whether or not:

As found in the chmod document that umask links to regarding the interpretation of a umask value. What the POSIX specification is saying is that your program might work on platform "X" but not "Y'. Even though both are POSIX compliant. That POSIX simply codified extant behavior, among several incompatible implementations, is the most significant problem with the standard.

My implementation is going to limit the legal umask values to exclude any bits outside the range [0 .. 0o777]. That is, it will disallow specifying bits such as setuid and setgid in the umask. It will only allow masking the user/group/other permission sets.

krader1961 added a commit to krader1961/elvish that referenced this issue Mar 25, 2020
krader1961 added a commit to krader1961/elvish that referenced this issue Mar 27, 2020
krader1961 added a commit to krader1961/elvish that referenced this issue Mar 27, 2020
krader1961 added a commit to krader1961/elvish that referenced this issue Mar 31, 2020
@xiaq xiaq closed this as completed in #949 Apr 3, 2020
xiaq pushed a commit that referenced this issue Apr 3, 2020
* Introduce a unix module and implement $unix:umask

Resolves #681

* Address review feedback

* More review feedback tweaks

* goimports reformat
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 a pull request may close this issue.

5 participants