Change directory.open_file() to use readonly open #2697
Conversation
On non-Linux and non-BSD platforms the direction.open_file() logic defaults to a named path base open, rather than using `openat` calls. However, this was using the `File.create(...)` call which opens files for read/write, rather than just readonly. This in turn fails since the `FileWrite` capability is explicitly dropped. This commit simply switches to using `File.open(...)` which opens the file in a readonly mode. This fixes: ponylang#2695
|
Is there a test that can be added to protect against regression, (a test which would fail on OSX CI without this change)? |
mfelsche
added
the
changelog - fixed
|
Sure, no problem, I will add a unit test. |
Note, care needs to be taken not the mask the `error` in a: ``` try ... then ... end ``` that does not have an `else` block.
|
I've added an extra unit test for the Directory.open_file case. |
|
Awesome! FYI: I've created tests after this pattern too and just found out about: https://stdlib.ponylang.org/ponytest--index#tear-down which makes state setup and tear_down much easier and the test-code itself more clean. |
packages/files/_test.pony
Outdated
| created.dispose() | ||
|
|
||
| // open a file (ro) | ||
| let readonly:File = dir.open_file("created")? |
There was a problem hiding this comment.
For compliance with the standard library style guide, can you add a space here after the :?
Same comment for line 154 above.
Space between : and type.
|
@jemc I've submitted the style guide related tweak. Please let me know if this was what you meant? Thanks, |
|
Yeah, that's what I meant - I didn't merge yet because I wasn't sure if @mfelsche wanted to see the refactor to use |
* Change directory.open_file() to use readonly open On non-Linux and non-BSD platforms the direction.open_file() logic defaults to a named path base open, rather than using `openat` calls. However, this was using the `File.create(...)` call which opens files for read/write, rather than just readonly. This in turn fails since the `FileWrite` capability is explicitly dropped. This commit simply switches to using `File.open(...)` which opens the file in a readonly mode. This fixes: ponylang#2695 * Introduce a unit test Directory.open_file Note, care needs to be taken not the mask the `error` in a: ``` try ... then ... end ``` that does not have an `else` block. * Tweak to match stdlib style guide. Space between : and type.
On non-Linux and non-BSD platforms the direction.open_file() logic
defaults to a named path base open, rather than using
openatcalls.However, this was using the
File.create(...)call which opens filesfor read/write, rather than just readonly. This in turn fails since
the
FileWritecapability is explicitly dropped.This commit simply switches to using
File.open(...)which opens thefile in a readonly mode.
This fixes: #2695