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 chmod/chmodSync on *nix (and fix Cargo.toml) #1088

Merged
merged 2 commits into from
Oct 26, 2018

Conversation

kevinkassimo
Copy link
Contributor

Continuation from #673 (since no updates for quite a while and this is an important fs call)

Implements chmod and chmodSync on *nix. (On windows this is noop)

Prior art:

  • Go: os.Chmod(name string, mode FileMode) error
  • Node: fs.chmod(file, mode, callback);
  • Python: os.chmod(path, mode);
  • Rust: std::fs::set_permissions<P: AsRef>(path: P, perm: Permissions) -> Result<()>

lchmod is not implemented yet. It seems that Rust std::fs only support inspecting symlink metadata, but any set_permissions/set_mode calls would eventually set the permission of linked file/dir.

Also, add getopts entry in Cargo.toml (forgot to include this in the PR yesterday)

@kevinkassimo kevinkassimo force-pushed the fs/chmod branch 2 times, most recently from 83d758a to 9d624a6 Compare October 25, 2018 07:00
js/chmod_test.ts Outdated
testPerm({ write: false }, function chmodSyncPerm() {
let err;
try {
const filename = deno.makeTempDirSync() + "/test.txt";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think deno.makeTempDirSync() raise a PermissionDenied error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops. Thanks for pointing out

@ry
Copy link
Member

ry commented Oct 26, 2018

On windows there are a bunch of errors like this

C:/deno/js/chmod_test.ts:18:8 - error TS2339: Property 'chmodSync' does not exist on type 'typeof import("deno")'.
18   deno.chmodSync(filename, 0o777);
          ~~~~~~~~~

Otherwise looks good!

@ry
Copy link
Member

ry commented Oct 26, 2018

I suspect the Windows problem is similar to what we saw in the CheckJS #1090 patches - where Windows is not properly detecting changes to the bundle (maybe when a new file is added?) There were changes to this mechanism in fd68f85... maybe I introduced a bug.

@ry ry mentioned this pull request Oct 26, 2018
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@ry ry merged commit a99aaf5 into denoland:master Oct 26, 2018
@srijanreddy98
Copy link
Contributor

@ry @kevinkassimo can you suggest me what to do next.

ry added a commit to ry/deno that referenced this pull request Oct 27, 2018
- Add URLSearchParams (denoland#1049)
- Implement clone for FetchResponse (denoland#1054)
- Use content-type headers when importing from URLs. (denoland#1020)
- Use checkJs option, JavaScript will be type checked and users can
  supply JSDoc type annotations that will be enforced by Deno (denoland#1068)
- Add separate http/https cache dirs to DENO_DIR (denoland#971)
- Support https in fetch. (denoland#1100)
- Add chmod/chmodSync on unix (denoland#1088)
- Remove broken features: --deps and trace() (denoland#1103)
- Ergonomics: Prompt TTY for permission escalation (denoland#1081)
@ry ry mentioned this pull request Oct 27, 2018
ry added a commit that referenced this pull request Oct 27, 2018
- Add URLSearchParams (#1049)
- Implement clone for FetchResponse (#1054)
- Use content-type headers when importing from URLs. (#1020)
- Use checkJs option, JavaScript will be type checked and users can
  supply JSDoc type annotations that will be enforced by Deno (#1068)
- Add separate http/https cache dirs to DENO_DIR (#971)
- Support https in fetch. (#1100)
- Add chmod/chmodSync on unix (#1088)
- Remove broken features: --deps and trace() (#1103)
- Ergonomics: Prompt TTY for permission escalation (#1081)
@kevinkassimo kevinkassimo deleted the fs/chmod branch December 27, 2019 07:50
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.

4 participants