Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork436
Skip unused broken symlinks#1201
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Skip unused broken symlinks#1201
Uh oh!
There was an error while loading.Please reload this page.
Conversation
matthijskooijman commentedApr 19, 2021
Any chance of merging this? It's a fairly straightforward bugfix :-) |
When any sketch file that would normally be compiled is unreadable,SketchLoad would previously just silently skip it, leading topotentially unexpected results. It is probably better to just error outand let the user sort out the problem instead.Additionally, this prepares for improving the handling of brokensymlinks in the next commit.
Previously, SketchLoad would error out on *any* error during fileenumeration, including broken symlinks (where `os.Stat` returns`ErrNotExist`), even when that symlink would not be loaded (i.e. novalid extension) anyway.Now, `ErrNotExist` errors are ignored and a broken symlink is processedas normal. This changes the code to assume broken symlinks are notdirectories and to use the `path` value instead of the `info.Name()`,since the latter is not available when the `Stat()` call failed.If, based on the name of the broken symlink, the file would be skipped,processing continues as normal. If the file would be loaded, then theexisting `os.Open()` call fails (and since the previous commit, returnsan error like before for all broken symlinks).
dadb144 to021b449Comparematthijskooijman commentedApr 19, 2021
Hm, I just rebased this on top of latest master, and discovered that his fix no longer works entirely as expected due to#1235. That PR added support for .json files, so this means that a broken symlink to I'm not directly sure what a good approach for this is, though. Maybe add an explicit ignore for |
matthijskooijman commentedSep 15, 2021
Closing in favor of#1438. |
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)Bug fix
.ino,.cpp, etc.) is unreadable, it is silently skipped when loading the sketch..ino,.cpp, etc type of file)..ino,.cpp, etc.) is unreadable (i.e. no permission or otherwise unreadable), an error is thrown..ino,.cpp, etc type of file).titled accordingly?
Sketches that contain unreadable files that are not actually needed can no longer be compiled (they show an error instead of silently skipping the unreadable file), but I would consider such sketches an unsupported corner case.
The trigger for this PR was that I had a symlink in my sketch to the
compile_commands.jsonin the build folder, so my editor would find that file and use it for code completion. However, after a reboot, the symlink would become broken, andarduino-cli compile(even with--only-compilation-databasewould fail, requiring me to remove the symlink and then recreate it afterwards.The first commit was not strictly required for this problem, but it seemed like a good idea to not silently ignore unreadable files, and raising this error simplifies the second commit, while still raising an error as before on symlinks that would be loaded.
I've omitted testcases from this PR, since a significant of the code paths to test would result in
os.Exit, which I think is not supported directly in the test framework. Are there any strategies in place for handling this already (seethis post for some suggestions if not)?