Skip to content

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
merged 1 commit into from
Nov 25, 2020
Merged

Add check for symlinks in library #56

merged 1 commit into from
Nov 25, 2020

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Nov 24, 2020

No description provided.

@per1234 per1234 requested a review from silvanocerza November 25, 2020 06:38
@per1234 per1234 force-pushed the per1234/symlink-check branch from 3e80dfb to bd49d86 Compare November 25, 2020 08:10
@per1234 per1234 requested a review from rsora November 25, 2020 09:18
@per1234 per1234 force-pushed the per1234/symlink-check branch from bd49d86 to 7a62024 Compare November 25, 2020 10:32
@per1234 per1234 merged commit e8ca1c2 into main Nov 25, 2020
@per1234 per1234 deleted the per1234/symlink-check branch November 25, 2020 10:59
@bblanchon
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.

@silvanocerza
Copy link
Contributor

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

@bblanchon
Copy link

bblanchon commented Jul 15, 2021

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.

@silvanocerza
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?

@per1234
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!

@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.

@per1234
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
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
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants