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

chmod doesn't work on Windows #4357

Open
cknight opened this issue Mar 13, 2020 · 6 comments
Open

chmod doesn't work on Windows #4357

cknight opened this issue Mar 13, 2020 · 6 comments
Labels
feat new feature (which has been agreed to/accepted) public API related to "Deno" namespace in JS windows Related to Windows platform

Comments

@cknight
Copy link
Contributor

cknight commented Mar 13, 2020

In Node, the following is observed on Windows:

const fs = require('fs');
fs.chmodSync('test_file.txt', 0o000);  //makes the file read-only
fs.chmodSync('test_file.txt', 0o777);  //makes the file writable

However, in Deno (v0.36.0) no file mode I tested changed the read-only attribute in any way (including the above two file modes).

From the Node docs:

on Windows only the write permission can be changed, and the distinction among the permissions of group, owner or others is not implemented.

Windows is not mentioned in the Deno docs.

It would be good if Deno could manipulate the read-only attribute of Windows files and specifically call out Windows quirks in the documentation for chmod.

@dubiousjim
Copy link
Contributor

Sounds reasonable.

Should presumably also apply to the soon-to-be added mode option on open (#4289, should be merged soon) and truncate (#4288, waiting on some other PRs). Perhaps also the mode option on mkdir, but I've got little Windows experience so I don't know if they accept/honor the read-only attribute on directories.

Should the API be that 0o000 and 0o777 are required to change these attributes, and other values are ignored? Or that other values throw an error? Or that only the user-write bit (0o200) has an effect, and other bits are ignored?

@dubiousjim
Copy link
Contributor

dubiousjim commented Mar 14, 2020

Of course, calling truncate and open with a read-only mode are guaranteed to either be ignored (if the file already exists) or fail (because you can't write to a file you're creating with read-only permissions). Still if the mode arguments are interpreted a given way on Windows for chmod, they should be interpreted the same way here too.

I guess it shouldn't be an error to open(path, {read:true, write:false, create:true, mode:0o400}). But if the file is created, it's going to be an empty read-only file, so this won't generally be useful.

@cknight
Copy link
Contributor Author

cknight commented Mar 16, 2020

Should the API be that 0o000 and 0o777 are required to change these attributes, and other values are ignored? Or that other values throw an error? Or that only the user-write bit (0o200) has an effect, and other bits are ignored?

I think the way node handles this is that only the user write-bit is honored. In my example 0o000 and 0o777 were arbitrary. 0o077 and 0o007 both behave as per 0o000 for example. Caveat, my access to a windows test box is limited, so I'm going off memory here.

dubiousjim added a commit to dubiousjim/deno that referenced this issue Mar 20, 2020
Currently chmod is a noop on Windows, while analogous functions like
chown and umask throw (see denoland#4306). Symlink also currently throws, but
that can be implemented on Windows, it just hasn't been done yet
(see denoland#815, an implementation was crafted but there were issues that
haven't been resolved yet).

It's possible to implement chmod in Windows to a limited degree: Rust
lets one make Windows files read-only. But the API for how this should
work hasn't been settled (see denoland#4357). Until we do implement chmod on
Windows, perhaps we should make it throw like the other
Windows-unimplemented functions in /cli/ops/fs.rs do?

If so, this PR provides an implementation, and fixes tests accordingly.

Note that the current implementation of writeFile applies its `mode`
option to the file regardless of whether the file was created (and does
so using chmod, so it has to be touched by this PR). A forthcoming PR in
the denoland#4017 series will make writeFile instead use the new `mode` option
in open for new files, and so no longer rely on chmod.
dubiousjim added a commit to dubiousjim/deno that referenced this issue Mar 20, 2020
Currently chmod is a noop on Windows, while analogous functions like
chown and umask throw (see denoland#4306). Symlink also currently throws, but
that can be implemented on Windows, it just hasn't been done yet
(see denoland#815, an implementation was crafted but there were issues that
haven't been resolved yet).

It's possible to implement chmod in Windows to a limited degree: Rust
lets one make Windows files read-only. But the API for how this should
work hasn't been settled (see denoland#4357). Until we do implement chmod on
Windows, perhaps we should make it throw like the other
Windows-unimplemented functions in /cli/ops/fs.rs do?

If so, this PR provides an implementation, and fixes tests accordingly.

Note that the current implementation of writeFile applies its `mode`
option to the file regardless of whether the file was created (and does
so using chmod, so it has to be touched by this PR). A forthcoming PR in
the denoland#4017 series will make writeFile instead use the new `mode` option
in open for new files, and so no longer rely on chmod.
@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2021
@TradeIdeasPhilip
Copy link

I see similar issues in Deno.writeTextFile(). I'm setting the mode to 0o444 (read only). The file is created and filled correctly. But the permissions are wrong. They are "-rw-r--r--" (644) according to git bash, and all permissions for the current use according to Windows file explorer.

I should have created a read only file. Barring that I should have gotten an exception or at least seen more details in the documentation.

@stale stale bot removed the stale label Jan 10, 2021
@kitsonk kitsonk added public API related to "Deno" namespace in JS suggestion suggestions for new features (yet to be agreed) labels Jan 10, 2021
@martin-braun
Copy link

It would be nice if Deno.chmod would allow to change read permissions on Windows, so Deno tests could include file access tests on Windows as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted) public API related to "Deno" namespace in JS windows Related to Windows platform
Projects
None yet
Development

No branches or pull requests

6 participants