Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_cli): Allow to run on read-only file systems #3851

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

realtimetodie
Copy link
Contributor

Fixes an issue where Rome could not process files on read-only file systems during traversal, where write access to the file system was not required.

This is required to execute Rome in isolated environments (sandboxes).

Example - Error message when running the check command on a read-only file system

$ rome check .
/test.js internalError/io ━━━━━━━━━━

  × Read-only file system (os error 30)
  

Checked 0 file(s) in 939µs
Skipped 1 file(s)
Rome encountered an unexpected error

This is a bug in Rome, not an error in your code, and we would appreciate it if you could report it to https://github.com/rome/tools/issues/ along with the following information to help us fixing the issue:

Source Location: crates/rome_cli/src/traversal.rs:210:8
Thread Name: main
Message: attempt to subtract with overflow

References

@netlify
Copy link

netlify bot commented Nov 24, 2022

Deploy Preview for docs-rometools ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ac18d62
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6387bec0ae1b3e00080bbc42
😎 Deploy Preview https://deploy-preview-3851--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@realtimetodie realtimetodie changed the title fix(rome_fs): Allow to run on isolated read-only file systems fix(rome_cli): Allow to run on isolated read-only file systems Nov 24, 2022
@realtimetodie
Copy link
Contributor Author

The additional variant increased the complexity of the implementation. I am working on tests for this. Please do not merge yet.

@realtimetodie realtimetodie requested a review from a team as a code owner November 26, 2022 22:06
@realtimetodie realtimetodie force-pushed the rome-fs-read-only branch 4 times, most recently from ec5b03e to 410f191 Compare November 26, 2022 22:26
@realtimetodie realtimetodie changed the title fix(rome_cli): Allow to run on isolated read-only file systems fix(rome_cli): Allow to run on read-only file systems Nov 26, 2022
@realtimetodie
Copy link
Contributor Author

I added integration tests for the check and format commands.

@ematipico ematipico requested a review from leops November 30, 2022 16:13
@ematipico ematipico added the A-CLI Area: CLI label Nov 30, 2022
@ematipico ematipico added this to the 11.0.0 milestone Nov 30, 2022
@realtimetodie
Copy link
Contributor Author

I replaced the std::fs tests with the MemoryFileSystem in read-only mode. There was another bug, where unnecessary write access to the underlying file system was required to read the rome.json configuration file. It is good that we have the tests.

@MichaReiser MichaReiser merged commit 40f10a4 into rome:main Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CLI Area: CLI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants