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

Fix some commands crashing when an installed library has invalid version #1189

Merged
merged 5 commits into from
Feb 18, 2021

Conversation

silvanocerza
Copy link
Contributor

@silvanocerza silvanocerza commented Feb 18, 2021

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • What kind of change does this PR introduce?

Fixes a bug that caused several commands to crash.

  • What is the current behavior?

Some commands crash with a nil pointer dereference when an installed library has an invalid or empty version in its library.properties file.

  • What is the new behavior?

Installed libraries with invalid or empty version in library.properties file won't cause those commands to crash with a nil pointer deference. Also if a library version can't be determined we assume it's outdated.

No breaking changes here.

  • Other information:

This PR includes commits from #1177, I had to cherry-pick them to a separate branch since they were commit to the fork's master branch.

Fixes #1176.


See how to contribute

@silvanocerza silvanocerza self-assigned this Feb 18, 2021
@silvanocerza silvanocerza changed the title Scerza/fix lib nil version Fix some commands crashing when an installed library has invalid version Feb 18, 2021
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @r10r and @silvanocerza! I was able to reproduce #1176 before and everything is working nicely after.

I think you found a nice balance between providing resiliency while not going to unnecessary lengths to support non-compliant projects (e.g., creating an interpreter for versions so badly formed they can't even pass the very lenient "relaxed semver" requirement).

@silvanocerza silvanocerza merged commit 550179e into master Feb 18, 2021
@silvanocerza silvanocerza deleted the scerza/fix-lib-nil-version branch February 18, 2021 13:30
silvanocerza added a commit that referenced this pull request Feb 25, 2021
…ion (#1189)

* librariesindex: Fix nil pointer. Refs #1176

Let the library index return the latest known version,
if a library without a version is found.

Signed-off-by: Ruben Jenster <[email protected]>

* Remove logging statement from FindLibraryUpdate.

Signed-off-by: Ruben Jenster <[email protected]>

* Add a small comment to the lib.Version nil check.

Signed-off-by: Ruben Jenster <[email protected]>

* Fix some commands failing when an installed library has invalid version

* [skip changelog] Add integration tests

Co-authored-by: Ruben Jenster <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arduino-cli lib install PacketSerial: nil pointer dereference
3 participants