+
Skip to content
This repository was 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
Merged

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

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 •
edited
Loading

✅ 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
leops
leops reviewed Nov 25, 2022
View reviewed changes
@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.

@leops leops mentioned this pull request Nov 29, 2022
Merged
leops
leops reviewed Nov 29, 2022
View reviewed changes
@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.
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@leops leops leops approved these changes

@ematipico ematipico Awaiting requested review from ematipico

Assignees
No one assigned
Labels
A-CLI Area: CLI
Projects
Rome 2022
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@realtimetodie @MichaReiser @leops @ematipico
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载