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

Reorganize and another test for directory-permissions handling #4286

Merged
merged 18 commits into from
Mar 11, 2020

Conversation

dubiousjim
Copy link
Contributor

This is part of a series of PRs towards #4017.

This PR moves the handling of default permissions for op_mkdir to Rust (where this also is/will be handled for related operations), improves some debug! calls for directory-permissions related functions, and adds a test verifying that the mode argument supplied to mkdir is subject to the process's umask.

cli/js/mkdir_test.ts Outdated Show resolved Hide resolved
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.

Looks good- just one comment.

* denoland/master:
  use Object instead of Map for promise table (denoland#4309)
  reorg: move js runtime tests to cli/js/tests/ (denoland#4250)
  upgrade: dprint 0.8.0 (denoland#4308)
  reorg: move JS ops implementations to cli/js/ops/, part 3 (denoland#4302)
  test: add actual error class to fail message (denoland#4305)
  upgrade: typescript 3.8.3 (denoland#4301)
  reorg: move JS ops implementations to cli/js/ops/, part 2 (denoland#4283)
  feat(std/node) add appendFile and appendFileSync (denoland#4294)
  disable test_raw_tty (denoland#4282)
This was referenced Mar 10, 2020
@dubiousjim dubiousjim mentioned this pull request Mar 10, 2020
* denoland/master:
  feat (std/encoding): add binary module (denoland#4274)
  refactor(cli/js/net): Cleanup iterable APIs (denoland#4236)
  Add Deno.umask (denoland#4290)
  refactor: Cleanup options object parameters (denoland#4296)
  refactor: uncomment tests broken tests, use skip (denoland#4311)
  Add global "quiet" flag (denoland#4135)
* github/filesystem8d:
  fix
  undo typescript changes
@dubiousjim
Copy link
Contributor Author

This could be ready to go, but as with #4287, I implemented it by throwing on Windows if an explicit mode was provided. For two reasons: 1. it's easier to reverse that and switch to noop than the other way, and 2. the recent umask(mode) PR went with throwing on Windows, and as I said in #4306, I hoped for a consistent policy, to guide how to proceed when implementing new functionality. But it'd be easy, and I'm happy, to change this to make WIndows silently ignore an explicit mode if that's what's wanted.

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.

This patch includes unrelated changes to deno_typescript/typescript.

@dubiousjim
Copy link
Contributor Author

Pushed commits undoing the deno_typescript stuff (sorry, not used to working with submodules), and made the mode option be ignored in Windows rather than throwing (to synch with what you said on #4289).

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 a28fa24 into denoland:master Mar 11, 2020
dubiousjim added a commit to dubiousjim/deno that referenced this pull request Mar 13, 2020
* denoland/master:
  Remove doc strings from cli/js TS files (denoland#4329)
  upgrade: Rust 1.42.0 (denoland#4331)
  Enable std tests in debug mode (denoland#4332)
  fix: Node polyfill fsAppend rework (denoland#4322)
  v0.36.0
  Add waker to StreamResource to fix hang on close bugs (denoland#4293)
  reorg: Deno global initialization (denoland#4317)
  move compiler API tests to integration tests (denoland#4319)
  support permission mode in mkdir (denoland#4286)
  Stricter permissions for Deno.makeTemp* (denoland#4318)
  reorg: remove dispatch.ts, move signals, factor out web utils (denoland#4316)
  reorg: cli/js/compiler/, move more API to cli/js/web/ (denoland#4310)
  Improve dprint config (denoland#4314)
  doc(cli/flags): Reduce empty lines in help messages (denoland#4312)
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.

2 participants