Skip to content
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

Consider raising LP048 to error instead of warning #336

Open
alranel opened this issue Feb 13, 2022 · 2 comments
Open

Consider raising LP048 to error instead of warning #336

alranel opened this issue Feb 13, 2022 · 2 comments
Assignees
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement

Comments

@alranel
Copy link
Member

alranel commented Feb 13, 2022

LP048 checks whether all libraries mentioned in the depends field of library.properties exist in the Library Manager, however it is a warning and not an error.

The issue is that arduino-cli lib install will refuse to install any library whose dependencies are not in the Library Manager:

% arduino-cli lib install "OpenMV Arduino RPC"
Error installing OpenMV Arduino RPC: No valid dependencies solution found: dependency 'SoftwareSerial' is not available

I saw quite a few libraries mentioning built-in classes such as SoftwareSerial or EEPROM, which are not known to the Library Manager. When trying to install such libraries from the IDE 2.0, it will just refuse to install them without providing any error.

The easiest way to fix this asymmetry would probably be to raise this check to error level. Other fixes could be to implement ways to bypass this check at library installation time in CLI and IDE.

What do you think @per1234?

@per1234 per1234 self-assigned this Feb 13, 2022
@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Feb 13, 2022
@alranel
Copy link
Member Author

alranel commented Feb 14, 2022

Just for reference, the workaround when using the Arduino CLI is:

arduino-cli lib install "OpenMV Arduino RPC" --no-deps

(But then one must install the other valid dependencies manually)

@per1234
Copy link
Contributor

per1234 commented Mar 8, 2022

Considerations

There are some factors to consider related to enforcement of LP048:

Dependent libraries might be submitted in parallel

If LP048 was made a requirement for library registry submissions, the library maintainer would need to submit these separately, and they would need to be accepted in the correct order, with the correct timing between submissions to allow the previous submission to propagate into http://downloads.arduino.cc/libraries/library_index.json

So this would make the registry less friendly to the library maintainers, and more work for the registry maintainers.

LP048 is not run by arduino/libraries-repository-engine

For performance reasons, Arduino Lint is configured to skip the download of http://downloads.arduino.cc/libraries/library_index.json (#170) when arduino/libraries-repository-engine is using it to lint the library releases:

https://github.com/arduino/libraries-repository-engine/blob/e2f24550877f8429797961f4b417558dcb57c448/internal/libraries/lint.go#L76

Since LP048 relies on the data from library_index.json, it is disabled when Arduino Lint is in this mode (#173).

This means that there would be some technical challenges to enforcing LP048 on new releases added to Library Manager.

Previously valid depends configurations may be broken by external actions

Library maintainers are allowed to remove or rename their libraries. This breaks the depends fields of any libraries that referenced it.

Conclusion

The problems mentioned above center around the use of the rule by the Library Manager system. If we only want to more strongly encourage people to properly use and maintain the depends field, LP048 can be configured to be more strict, while keeping it at a warning for Library Manager submissions and disabled for Library Manager releases.


Since the Library Manager system uses Arduino Lint with the --compliance=permissive flag, this means there are two options for how strict to make Arduino Lint about it:

  • Error at both "specification" and "strict" compliance settings
  • Error only at the "strict" compliance setting

Even though the Arduino Library Specification does not explicitly state that the referenced libraries must be in Library Manager, I think it is implicit, and thus reasonable to consider an error even at the "specification" setting.


The issue of false positives during development of dependent libraries in preparation for submission to the registry could be mitigated by configuring LP048 as an error only when Arduino Lint is used with the [--library-manager=update](https://arduino.github.io/arduino-lint/dev/#library-manager-setting


When trying to install such libraries from the IDE 2.0, it will just refuse to install them without providing any error.

This is tracked at arduino/arduino-ide#621

Other fixes could be to implement ways to bypass this check at library installation time in CLI and IDE.

I think the current behavior of Arduino CLI in this situation is reasonable for manual usage. It might be problematic in some cases for automated operations. It could be made to adapt to that similar to how Arduino already handles automatic update checks and post-install script execution by arduino-cli core install:

https://github.com/arduino/arduino-cli/blob/8b53b85052918fe0d6e77eeed9bf3b5197da48fa/cli/arguments/post_install.go#L60

My opinion is that the current behavior of Arduino IDE 2.x in this situation is not acceptable.

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

No branches or pull requests

2 participants