Skip unused broken symlinks #1201
Conversation
|
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 to potentially unexpected results. It is probably better to just error out and let the user sort out the problem instead. Additionally, this prepares for improving the handling of broken symlinks in the next commit.
Previously, SketchLoad would error out on *any* error during file enumeration, including broken symlinks (where `os.Stat` returns `ErrNotExist`), even when that symlink would not be loaded (i.e. no valid extension) anyway. Now, `ErrNotExist` errors are ignored and a broken symlink is processed as normal. This changes the code to assume broken symlinks are not directories 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 the existing `os.Open()` call fails (and since the previous commit, returns an error like before for all broken symlinks).
matthijskooijman
force-pushed
the
skip-unused-broken-symlinks
branch
from
dadb144 to
021b449
Compare
|
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 |
|
Closing in favor of #1438. |
per1234
added
topic: code
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 (see this post for some suggestions if not)?