Skip to content

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Dec 28, 2023

Please check if the PR fulfills these requirements

See how to contribute

  • 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)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

This PR aims to make all library-related commands behave correctly when used in high-concurrency environments.

What is the current behavior?

No behavior changes (in theory...).

What is the new behavior?

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

Should fix #2457

The presence of the Index* fileds inside LibraryManager didn't give any
functional benefit.
It does make things more complicated without any actual benefit.
The original LibrariresManager has been split into three objects with
specific goals:
* The Builder object is used to construct a new LibrariesManager. It has
  methods to add librarires directories and to build the LibrariesManager.
* The Explorer object is used to query the LibrariesManager about
  installed libraries.
* The Installer object is used to rescan installed libraries and to
  install and uninstall.
@cmaglie cmaglie added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Dec 28, 2023
@cmaglie cmaglie self-assigned this Dec 28, 2023
Copy link

codecov bot commented Dec 28, 2023

Codecov Report

Attention: 80 lines in your changes are missing coverage. Please review.

Comparison is base (fb1b9a0) 67.55% compared to head (e5180b1) 67.61%.
Report is 2 commits behind head on master.

Files Patch % Lines
commands/lib/install.go 61.90% 7 Missing and 9 partials ⚠️
commands/internal/instances/instances.go 67.56% 8 Missing and 4 partials ⚠️
...ino/libraries/librariesmanager/librariesmanager.go 82.35% 10 Missing and 2 partials ⚠️
commands/instances.go 79.16% 3 Missing and 7 partials ⚠️
commands/lib/download.go 46.66% 3 Missing and 5 partials ⚠️
commands/lib/upgrade.go 58.82% 4 Missing and 3 partials ⚠️
commands/lib/resolve_deps.go 70.00% 4 Missing and 2 partials ⚠️
commands/lib/list.go 85.00% 2 Missing and 1 partial ⚠️
commands/version.go 75.00% 1 Missing and 1 partial ⚠️
...nternal/arduino/libraries/librariesresolver/cpp.go 91.30% 1 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2480      +/-   ##
==========================================
+ Coverage   67.55%   67.61%   +0.06%     
==========================================
  Files         207      205       -2     
  Lines       20759    20844      +85     
==========================================
+ Hits        14023    14094      +71     
- Misses       5590     5597       +7     
- Partials     1146     1153       +7     
Flag Coverage Δ
unit 67.61% <78.72%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmaglie cmaglie merged commit 1911448 into arduino:master Jan 2, 2024
@cmaglie cmaglie deleted the transactional_libmanager branch January 2, 2024 10:01
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.

[grpc]: error: concurrent map iteration and map write when executing lib list
2 participants