Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add check for symlinks in library #56

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
per1234 merged 1 commit into main from per1234/symlink-check
Nov 25, 2020
Merged

Add check for symlinks in library #56

per1234 merged 1 commit into main from per1234/symlink-check
Nov 25, 2020

Conversation

Copy link
Contributor

@per1234 per1234 commented Nov 24, 2020

No description provided.

@per1234 per1234 merged commit e8ca1c2 into main Nov 25, 2020
@per1234 per1234 deleted the per1234/symlink-check branch November 25, 2020 10:59
Copy link

I think this feature might be broken: I just tested version 1.1.4 with a repository that contains a symlink, but it didn't detect it.

Copy link
Contributor

Hi @bblanchon can you tell us which repo so we can test it?

Copy link

bblanchon commented Jul 15, 2021
edited
Loading

Hi @silvanocerza,

It was ArduinoJson, but I removed the symlink a few hours ago, so you'll have to check out the tag v6.18.1.
I doubt that this issue is specific to ArduinoJson's repo; you can probably reproduce the issue with any library.
I looked at the test cases and there doesn't seem to be one covering this feature.

Copy link
Contributor

I tested it a bit.

Seems like rule LS005 passes if the symlink points to a directory, so it worked for you since it was ArduinoJson -> ../../../... Instead creating a symlink to a file makes it fail.

Probably we need to clarify the rule or make it work for directories too. @per1234 what do you think?

Copy link
Contributor Author

per1234 commented Jul 15, 2021

what do you think?

We prohibit any type of symlink in libraries, so this must be fixed:
https://github.com/arduino/library-registry/blob/main/FAQ.md#what-are-the-requirements-for-a-library-to-be-added-to-library-manager

Thanks for the report @bblanchon!

Copy link

You're welcome, @per1234.
ArduinoJson 6.18.1 is missing from the Library Manager; I'm assuming it's due to the symlink.

Copy link
Contributor Author

per1234 commented Jul 15, 2021

ArduinoJson 6.18.1 is missing from the Library Manager; I'm assuming it's due to the symlink.

That is correct. We now provide a dedicated web page with the indexer logs for each of the Library Manager libraries. You can see the logs for ArduinoJson here:
https://downloads.arduino.cc/libraries/logs/github.com/bblanchon/ArduinoJson/
Even though Arduino Lint dropped the ball on this one, there is an additional symlink check built in to arduino/libraries-repository-engine that caught the symlink before the library could be added to the index:

2021年07月15日 16:47:17 Error syncing library: Error while zipping library: Symlink not allowed: /home/arduino/workspace/workspace/Librariesv2/gitclones/github.com/bblanchon/ArduinoJson/extras/ci/espidf/components/ArduinoJson -> ../../../..
bblanchon reacted with thumbs up emoji

Copy link

Thanks, @per1234. I didn't know the log files were publicly accessible; this is awesome!

@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Sep 29, 2021
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rsora rsora Awaiting requested review from rsora

1 more reviewer

@silvanocerza silvanocerza silvanocerza approved these changes

Reviewers whose approvals may not affect merge requirements
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /