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

Handle priority inversion for promoted bundled libraries #123

Merged
merged 2 commits into from
Mar 25, 2016

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Mar 4, 2016

Fixes arduino/Arduino#4064

The rationale behind this patch:

  • Compatible libraries are ordered by "importance": sketchbook, core, bundled
  • Bundled libraries can be updated via library manager, which brings them from the least important position to the most important
  • Bundled libraries are architecture independent
  • So we can use this information to promote again a core library if the one found in sketchbook is architecture independent

Example with Ethernet library:

  • bundled: version 1.0.5 triggers a library update
  • version 1.1.1 is downloaded to sketchbook
  • Compiling for Galileo, three libraries get discovered {sketchbook(1.1.1) -> core(1.0.0) -> bundled(1.0.5)}
  • When discovering the core version, check if the sketchbook one has arch=*
  • If so, push the core version to the top of the slice and use it

If a library gets updated upstream with the support for xxx architecture, the xxx core should simply remove the modified "fork" from its repo

Fixes arduino/Arduino#4064

Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
@facchinm
Copy link
Member Author

facchinm commented Mar 4, 2016

Adding binaries for all platforms, please replace the arduino-builder in your Arduino installation to test them

arduino-builder-pr123.zip

@damellis
Copy link

damellis commented Mar 4, 2016

To me, the real solution is that bundled libraries, when updated, should stay internal rather than being placed in the user's sketchbook. (Not necessarily in exactly the same folder as the original bundled library, but at an equivalent place in the priority stack.) Placing internal libraries in the user's sketchbook has issues besides just changing their place in the priority order.

Which isn't to say that I necessarily object to this particular patch, just that I don't think it solves the underlying problem.

@bperrybap
Copy link

facchinm,
I didn't understand the example, in particular why would a core library trump a users sketchbook library when the user sketchbook library supported that core?
This does not make sense, sketchbook libraries trump core libraries, right?
Was there a typo in the example?
Should the last few lines have read:

  • When discovering the core version, check if the sketchbook one supports the core
  • If the sketchbook one does not support the core, push the core version to the top of the slice and use it
  • If the sketchbook one does support the core, the default priority would be used and the sketchbook library would be used.

Meaning that a core library only trumps a user sketchbook library if the user sketchbook library does not support the core.
This seems like the desired behavior.
i.e. the order is ALWAYS: user sketchbook, core, bundled.
The only time you select a lower priority library is when the higher priority library does not exist or support the environment for the current build. (like if the sketchbook library does not support the core and there is a core library that does)

My overall comment is that when dealing with complex decision trees and implementation details like this, it is very important to document the intended behaviors before implementing it.
That way collaboration can happen prior to it being cast in code.

In this case, it appears that we are dealing with two sets of library selections/priorities.

  • how the library manager handles library updates & where it stores the updates
  • how the builder selects which library to use when there are multiple.

So I would be against merging in this patch until the full behavior is documented and discussed, especially given that as dave mentioned above that it does not appear to solve the underlying problem.

While I agree with dave's comment above about it not solving the underlying problem (I went into detail on this in the other issue), I do not believe it is as simple as just creating a separate IDE bundled library area either.

For example, I have many different IDEs installed so I can test with each of them.
So I'm looking at this from the perspective of working with multiple IDEs being installed.
I'm also looking at this from the perspective being able to update a 3rd party core version of a library independently from a bundled library.
Having the library manager using a single sketchbook area for updates creates real problems.
Separating the IDE bundled libraries into their own area, doesn't fully solve the issue either as while it solves the issue of clobbering user libraries, it doesn't solve the issue of each IDE having its own separate bundled library updates.

For example, it isn't just about keeping the bundled libraries separate, but also keeping each IDEs bundled libraries separate from each other.
i.e. if I update the bundled Ethernet library on IDE 1.6.8 it should not affect the bundled Ethernet library used with 1.6.7
Each IDE should be able to do its own bundled updates.
If you don't do it this way, there is no way to do regression testing since updating a library for a single IDE updates the library for all of the IDEs installed.

A lot of what makes this complicated, is that the library manager is looking at libraries as one big flat name space and we are actually dealing with multiple name spaces as well as library collisions.
For example, If there is a core library that is overriding a bundled library, then that is actually a library collision since the two libraries are not the same library but have the same name.
They are more than likely coming from different places (different "name spaces") and are being maintained by different entities. This will be true with 3rd party cores and libraries.
You also have the potential for a user to have installed a colliding by name, yet different, library in his sketchbook area that he is using to override a core or bundled library.

My view is that there are separate and distinct entities that each can have their own libraries:

  • user sketchbook
  • core specific
  • bundled with an IDE

The priority of selection is well defined. (user sketchbook, core, bundled)

The questions becomes how do you do handle updates?
And do you distinguish between the different types of library updates?
(user, core, bundled)

I would argue that the 3 need to be kept separate and there should be a way to independently update each.

One way to handle this kind of stuff and fully resolve all the issues would be to have the library .properties be able to indicate where the library repo is so that each library at each level can use the library manager to do its own library updates separately and independently from the others.

In other words, the library manager can initially point to "known" libraries in a single name space,
(just like it does now) but once a library is installed, the library's own .properties points the library manager to where to get updates.
This allows each library to maintain its own repo and that way you totally avoid any issues with priority or name collision.
For example the ethernet library in a core could point to a different repo than the bundled ethernet library for a given IDE.

A core library can update itself and not affect the bundled library for a given IDE. A user can install a library manually in his sketchbook and override core and bundled and even get updates for his manually installed library and not affect the core or bundled libraries.
All 3 can be updated independently and the priority is always the same:
user sketchbook, core, bundled.

The core library updates remain in the core area, the bundled updates remain in that particular IDE library area.
If a user has installed his own library and then removes it, the builder will then be using the latest library from the core or IDE bundled area.

Yes the library manager would work a bit differently in that it would get the repos from the .properties but it is a big win in that this keeps everything simple, clean and separate.
The only issue comes down to where to store the core and bundled updates.
My preference would be install them in place as it keep everything together, but there seems to be resistance against that.
They could be placed in the users arduino area or sketchbook area, but they would need to be in their own directories so that each installed IDE version would have its own updates that are separate from other versions of the IDE.

@PaulStoffregen
Copy link
Sponsor

David's proposal would be much better.

As the maker of a 3rd party core with several customized copies of the bundled libraries, of course I like a solution which solves the problem where incompatible copies installed from the library manager break my core's well tested libraries. This has been quite a problem.

I didn't understand the example, in particular why would a core library trump a users sketchbook library when the user sketchbook library supported that core?

Imagine you're using Teensy with a WIZ820io ethernet module stacked on top. It works great, because Teensy's core provides its own Ethernet library, which has code specifically to support that other ethernet shield which isn't supported by the normal Arduino bundled library.

Then one day, Arduino updates the bundled library with a couple changes, like a few lines to support their newest product: arduino-libraries/Ethernet@69e2575 and a minor change in the metadata wording: arduino-libraries/Ethernet@9a68147

The library managers display the popup which alters you to update to the latest version of the Ethernet library. So you do what most people do and click ok and complete the update.

Suddenly your project no works. Now Arduino's updated Ethernet library is now being used. The really bad part is it compiles and runs without any error, but simply does not communicate because it lacks the code to support the hardware you're using.

@per1234
Copy link
Contributor

per1234 commented Mar 5, 2016

Thanks very much facchinm for your work towards a solution to this issue!

I did a preliminary test of this using Arduino IDE 1.6.8 2016/03/04 03:33 with https://github.com/JChristensen/mighty-1284p/tree/v1.6.3 and https://github.com/MCUdude/MightyCore and the Arduino bundled versions of the libraries that are included with those cores installed to sketchbook. I am happy to test other hardware packages but I think this already demonstrate an issue that will require a change so I will wait to go further.

Issues:

  1. MightyCore Servo library and Mighty 1284P GSM library were incorrectly chosen because both libraries specify avr in library.properties.
  2. Mighty 1284P Ethernet, Firmata, and SD libraries were not correctly chosen but this was solved by changing architectures=* to architectures=avr in library.properties of the Mighty 1284P libraries which won't cause any problems and is an easy fix.
  3. Mighty 1284P Servo library was incorrectly chosen because both libraries specify avr in library.properties. This issue has been solved I believe by ATmega1284/1284P support being added to the Arduino bundled Servo library in Added to #define for 1284/1284p Timer 3 inclusion. Arduino#4547 so even though the wrong library will be chosen it should still work.

I think issue 2 could be solved by giving the core library higher include priority over the sketchbook library when they both have the same architecture value but that would prevent sketchbook libraries being able to purposefully override core libraries.

I also would prefer the approach of installing Library Manager updates of Arduino bundled libraries(any library that is found at {Arduino IDE install folder}/libraries) to the Arduino15/libraries folder and adjusting the include priorities to place that location above the Arduino IDE install folder and below all other include locations, however I am very glad to see that the Arduino developers consider this issue worth fixing and will be glad of any solution that is found.

Responses to @bperrybap's comments:

  • Documenting include priorities - agree, this is already confusing and will become more so with any fix to this issue. Once finalized I think https://github.com/arduino/Arduino/wiki/Build-Process would be the appropriate place for the information to be published.
  • 3rd party Core library updates - this can already be handled by doing a Boards Manager release of the core.
  • Keeping each IDEs bundled library updates separate - With my proposed solution of installing Arduino bundled library updates to Arduino15/libraries if you created a portable folder in each of your Arduino IDE install folders then the portable/libraries folders would be used for bundled library updates instead of Arduino15/libraries.

@bperrybap
Copy link

@PaulStoffregen It sounds like you have not understood my proposal.
I guess I shouldn't have said I didn't understand the example and instead said it doesn't make sense and that altering library priorities is not workable.

What you are describing is a re-ordering of the priority to
core, sketchbook, bundled. (which is the only way to put core above sketchbook)
and that is not what federico's example said was the priority in the very first bullet.
If a library in the users sketchbook area can work on the core, and the user sketchbook is the highest priority, then the sketchbook library will be used over the one in the core.
It makes no sense to have a library in the users sketchbook that says it works with a core be explicitly trumped by the core's library.
It is not possible to have it both ways where the priority is
sketchbook, core, bundled, and then allow a core library to ever trump a sketchbook library.

The Teensy example you described is EXACTLY the problem I brought up and described above as well as went into detail in the thread in the other issue.
(library manager breaking existing priorities by slamming an IDE bundled library update into the users sketchbook)
It further emphasizes the issues I brought up which can be solved by doing the things differently.
I was suggesting that the library manager keep each layer/entity's library updates separate so that the library manager can be used to update any layer/entity library separately without clobbering any other layer's/entity's library. The priority of how libraries are chosen NEVER has to change.

The real issue is how updates are handled and where library updates are stored NOT how libraries are prioritized.

The current priority is:
user sketchbook, core, bundled.
This existing priority does not ever need to be altered. The priority order isn't the problem. The problem is that the library manager is not doing updates to these areas separately.
The library manager is too simplistic in how it does its updates and where it installs those updates, in that it is attempting to use a single area (the users sketchbook) for all library updates as an attempt to avoid doing in place updates for certain libraries.

It needs to be smarter as to how it handles its updates.

There is simply no way to solve this by re-prioritization.
Any attempts to do this through re-prioritization quickly breaks down.
Re-prioritization is simply unworkable.

For example, in Paul's Teensy example above, it is desired that a core library trump the IDE bundled library, which it will before doing an update with the existing library manager.
It is then desired that core library trump the sketchbook library after the existing library manager installs an update into the users sketchbook This requires making core higher priority than sketchbook.
This is not workable since there is no way to know when core should be higher priority than user sketchbook.
user sketchbook is either higher priority than core or it isn't.
The user may be intentionally wanting to trump the core's library.
There is also a case where a core library may want to be able to update its core library through the library manager without having to update the full core.

The only way to solve this is to make the library manager smarter so that it can update libraries within each layer/entity separately.

And by smarter I mean that the library manager treats updates for each layer/entity library separately and the builder will also know where to find these updates.
You cannot have a single sketchbook area for library all updates.
There must be separate areas for each layer/entity.
By keeping them separate you fully solve the problem.

In my previous post I didn't suggest where to put any of the updates (like @per1234 did in Arduino15/libraries) as I believe that is an implementation detail and before details like that should be considered, the overall concepts need to be discussed and agreed upon.

Concept:

What I'm suggesting is that each layer/entity (user sketchbook, core, bundled) gets its own update area.
Where that location actually is can be determined later.
This also requires that each individual library in each layer have the ability to specify its own update repository. When updating libraries, the library manager must treat each library in each layer separately rather than essentially treating them as a single library with a single update repository.

So lets look back an Paul's teensy example and see how that works when you have separate update areas and a rigid fixed priority of: user sketchbook, core, IDE bundled.
Scenario:
User is using Paul's teensy core, which supplies core ethernet (WIZ820io) library.
teensy core ethernet library trumps IDE bundled ethernet library.

An update to the IDE bundled ethernet library becomes available.
The user installs the update, the update is installed in a location that overrides the IDE bundled ethernet library but does not affect any other core libraries, or the users libraries in his sketchbook.
Sketches built with the teensy core, will continue to use the teensy core ethernet library, since it still continues to trump/override the the Ethernet library that was just updated.

This works because the library priority is fixed and each library within each layer can be updated without affecting any other layer's/entity's libraries.

The concept is that there is not just one library. Each layer/entity can have its own version of a library with its own update repository that can be distinctly different and updated separately even through it has the same name as a library in another layer.

This ensures that f a lower layer's library is updated it will not affect a high priority library or it's use.

Detailed Examples:

In terms of some additional details. Having separately updateable libraries in each layer would potentially require that there be more library locations.

Existing locations and priority.

  • user sketchbook
  • per core
  • per IDE (bundled)

If the updates were done "in place" then the problems are solved, things are really simple, and no new library locations would be needed. Everything is kept separate and updates only affect the library being updated.
A library in a core would update that core's library in place inside its core, a bundled library would update a library in place inside its IDE install tree. To me this would be the ideal way to run as it keeps things really clean and simple.

If "in place" updates is not going to be allowed, then additional library locations would be needed.
This requires adding a library tree somewhere under the users area that could contain
an area for each core and each IDE.

If you add other core and IDE library areas for updates, then the new library locations would be:

  1. user libraries
  2. per core area for library updates
  3. per core area (included in core)
  4. per IDE bundled area for library updates
  5. included with IDE(bundled)

The priority is FIXED and NEVER changes.
This layering gives you the flexibility to update what you need as well preserving priorities.
In this layering, libraries in number 3 and number 5 are not altered when library updates are done and locations for number 3 and 5 do not have to be writeable.
Number 1, 2, and 4 are writeable by the user.
NOTE: Obviously number 5 is per installed IDE since those libraries are in the distribution install tree.
As such, number 4 should be per each installed IDE as well as it is a place holder location to perform "logical" updates to that IDE since instead of doing in place updates in the original IDE tree.
This means that any updates for a given IDE bundled libraries are lost when upgrading to a new IDE or using a different version of the IDE.
When a new IDE is installed the user can do any library updates for the IDE as needed/desired.
While this may sound extreme and a pain, not doing so creates many issues, only one of which being that it makes regression testing impossible.
Each IDE needs to manage its own bundled library updates.

NOTE: the example below is when in place library updates is not being allowed.
If in place updates were allowed, then everything works exactly the same other than numbers 2 and 3 collapse and numbers 4 and 5 collapse.

So now back to Paul’s example,
The IDE ships with an ethernet library bundled. It would be in 5 above.
The teensy core supplies its own ethernet library.
The teensy core ethernet library would be in location 3 above.
The teensy core ethernet library in 3 would have priority over the ethernet library in 5.
The Teensy library in 3 and the bundled library in 5 each have their own separate and different update repository.
If 5 has an update, the library manager would prompt that there is an IDE update for the ethernet library. The user could install it and it would go in 4
While this update would have priority over the original library in 5, the core library in 3 would still have priority over 4.

And, if paul had an update to his teensy core ethernet library he could publish that to the teensy core ethernet library repo and the library manager would prompt that there is a Teensy core update for the ethernet library.
(remember that each individual library can point to its own update repository and a core library would be pointing to a different repository than an IDE bundled library)
The user could install this teensy core ethernet library update and it would go in 2
This update would have priority over the original library in 5, as well any update from that IDE in 4 as well as the original teensy core library in 3
And yet still allow the user to override the teensy core library by installing it in 1 (his user library area) if he so desires.

By keeping the layers separate and allowing updates to each separately, you never have to muck with or deal with any priority issues. The priority is fixed and the library manager allows each layer to do its own updates.

Cores can update individual core libraries without having to update its entire core and without disturbing any IDE or user libraries.
IDEs can update their own libraries without disturbing core or user libraries.

Users can install their their own library and, if desired, override core or bundled libraries.

The overall concept is to treat each library update separately rather than try to treat updates to all libraries with the same name as equivalent with a single repository.

And to do that, you have support having a separate repo for each library in each layer and you have to either update libraries in place or provide alternate places for library updates that be used to be able to logically update the libraries at a given layer without disturbing the libraries in other layers.

One advantage of using alternate locations for core and IDE library updates vs in place updates, is that the library manager could have the ability to revert back to the original library that was supplied in that layer.

@facchinm
Copy link
Member Author

facchinm commented Mar 7, 2016

In my opinion, none of the proposed solutions will completely remove the problem.
The problem can be synthesised as:

  • We use a library which is incompatible with the target platform

Why are we doing it?

  • Because a library declares itself as compatible while it's not

These libraries are usually declared as "architecture independent" since they are based on some other library like Wire or SPI which abstract the underlying hardware.

Said so:

  • the main target should be ZERO breakage (not perfect library resolution)
  • libraries included in cores should never be updated independently (only core updates will bring updated core libraries)
  • bundled libraries can be updated and the updates are correctly placed inside the sketchbook (since they ARE libraries which could be shipped outside a core and also outside the IDE)

Possible solutions (always in my opinion):

  • In library.properties, add some fields or complete the architecture field to specialize the library and let the builder decide more wisely (eg. arduino-libraries/MIDIUSB@7b052b8)
  • If a name clash is encountered between sketchbook and core, only override the sketchbook library if it is architecture=* (this will let you perform regression tests, simply write explicitly the supported architectures you are testing against)

@PaulStoffregen
Copy link
Sponsor

Another even simpler solution is to never update the bundled libraries with the library manager. Just leave them as shipped with that version of the IDE and update with the next IDE release.

solves arduino/ArduinoCore-samd#80, which was caused by USBHost library not having a corresponding .h, thus bypassing the findBestLibraryWithHeader check
@facchinm
Copy link
Member Author

facchinm commented Mar 7, 2016

Adding another bunch of compiled versions with the USBHost issue fixed
arduino-builder-pr123-2.zip

@PaulStoffregen
Copy link
Sponsor

Ok, I just took the time to read Bill's extremely long message. I agree completely, though I do wish it could be stated more concisely.

As the maintainer of a 3rd party core, I believe architecture matching does not work. It's a beautiful ideal, much like Communism, which is simply incompatible with practical reality of human behavior. I publish one of the very widely uses 32 bit cores, and I must intentionally label it "avr" for compatibility. Numerous libraries that work perfectly fine are declared as "architecture=avr" or "architecture=avr,sam". If I create an architecture called "kinetis", my users suffer. Many libraries are unmaintained, or maintained by a busy individual, or maintained by small companies with limited resources who see no ROI in supporting anything over the 8 bit AVR. That is simply the reality of the Arduino world. It makes the architecture field pretty much worthless for any 3rd party boards/cores.

Stated another way, consider the practical effect of the ever-expanding number of architectures. For architecture matching to work, ALL library authors (who don't use "architecture=*") must bear the maintenance cost for updating for the ever expanding list of architectures. It's a system that simply does not scale.

@bperrybap
Copy link

@PaulStoffregen here is an attempt to boil it all down to something much more concise.
If the IDE is going to allow a system of library overrides using a priority that is based on the library's location and declared target architecture support, it cannot allow a library manager to try to simplify the update process to using a single repo for a library and storing those updates in a single location.
Attempting to simplify the updated process in this way, breaks many things and is unworkable.

One way to solve the problem, is to update the library manager to give each library in each priority location/layer the ability to update itself using its own repository. This completely solves the problem. Each library can update itself independently using its own repository. This is what allows the user, the core, and IDE to update their own version of the library and never break any other layers version of the library all while preserving the priority overrides.
When doing a library update, updating "in place" is the easiest way to handle this.
Each library at each layer can update itself "in place" which overwrites itself.
To avoid updating libraries "in place", requires adding additional "logical" library locations that override the potentially un-writable "in place" locations.

Another much simpler approach is what Paul just mentioned.
Cores and the IDE can NEVER use the library manager to update their individual libraries separately and must update their entire "blob".
I think that while this is workable, it is a bit of a heavy hammer to require updating an entire layer just to get out individual library bug fixes.

@facchinm
I think some of the different views to approaches for solutions are resulting from the person's vantage point.
I think that the arduino.cc team often has an unintentional blindness to 3rd party developer issues and needs when looking at solutions.
This was the case with changes from the pre 1.x to the 1.0 release, and was the case with the early 1.5x library specification.
i.e. a solution that may work fine for the IDE, its libraries, and arduino.cc's release methodology, may cause extreme pain or possibly not work at all for the 3rd party s/w.

I also believe that many of these types of discussions drag on because the "problem" is not being fully defined and written down. So, again, because of different vantage points, different people will view the "problem" and what needs to be solved differently.

In terms of the latest proposals:
I believe that the argument of selecting and using a library that is incompatible with a target because a library incorrectly declared its architecture compatibility is an entirely different issue. That seems to be a bug so it is an issue that can never be solved, it would be outside the scope of the problem, and should be ignored.

If a library only works with a given sub library within a particular core, then it should be declaring that it only works with that architecture. A library must properly declare its target architectures.

In my view your "Said so" bullets are creating unnecessary restrictions and propagating the existing library manager problem of using a single library repository that always stomps on the users library area to do updates.
Why is a core library any different than an IDE bundled library with respect to updates?
Why should the library manager and IDE attempt to work around the library manager issues differently for an IDE bundled library vs a core library?
The 2nd bullet in "Possible Solution" seems to be just bumping the priority of "core" above "sketchbook"

I believe that any complete solution should to be generic and work for both core and bundled libraries as the big picture issues an needs for each of the layers is the same.

The base problem here is that the library manager is too simplistic in the way it handles its updates.
It is attempting to use a single library repository for updates, and then it is stealing the users library area to install IDE bundled library updates.
If the IDE wants to update a library that was bundled with the IDE, it needs to do it somewhere else than the users library area.
If the library manager can't or won't do it "in place", then it needs to figure out a new location that can have priority over the original IDE location but be lower priority than the core and the user area.

One way to handle all that is to look at what I proposed, which is to allow each individual library to have its own repository and update location rather than try to use a single repository for a library with a single update location.
Doing this, solves the problem for doing updates for IDE bundled libraries as well as cores and still allows priority overrides.

When libraries and library selection is based on fixed prioritized layers where each library in each layer can manage its own version of the library, the problem goes away and you gain the flexibility that any library in any layer can update itself without affecting any other version of the library in another layer.

@bperrybap
Copy link

@PaulStoffregen
I think a lot of what you are facing is that you are doing some very specialized compatibility and emulation layers to make things that are hard coded for one architecture, or in some cases essentially broken or done incorrectly, continue to work with your add on cores.

This is an example where 3rd party s/w developers have to jump through hoops because the s/w that they have to integrate into or be compatible with cannot be easily updated or is somewhat "broken".

There is no easy answer to this.

While definitely related, I think it is a larger issue than fixing the update manager and builder better handle library updates.

@facchinm
Copy link
Member Author

facchinm commented Mar 8, 2016

@bperrybap ,
I agree with you, we absolutely need to define a clear priority strategy, write it down and then implement consequently.

As per my vantage point ( 😉 ), your view suffer from a problem: all libraries are created equal, and there is no way to differentiate two libraries beside their position in the filesystem

This PR could solve the great majority of the problems which rise from the actual "dummy" update system. Of course making the library updater smarter would only bring additional benefits but it would also bring a lot more complexity (and compatibility problems with existing installations/making IDE regression tests with the same sketchbook impossible etc etc)

@bperrybap
Copy link

@facchinm
I don't view this as a library prioritization issue. It is larger than that.
I don't believe that any amount of re-arranging priorities can solve the problem.
I believe that this is a library update issue.

I very much disagree with your assessment of my view and proposal.
The main concept is that if the IDE is going to allow for overriding a library with a replacement library in a different layer, then, for updates, the library manager needs to support the ability to update libraries in each layer separately and individually.
This means that the library manager becomes a tool that can be used by libraries to update themselves from a library specified repository and not a tool that updates libraries from centralized list of library repositories.
A centralized list and single update location does not allow each library in each layer to specify its own repository for updates and replace itself with an update.
This is a very big distinction.

I believe that priorities are easily handled by using library layers.
And while these "layers" might be implemented as physical filesystem locations, do not think of it in terms of specific physical filesystem locations but rather as logical library layers that can be extended and updated.
Example layers: User, core, IDE bundled.
I believe this is the way it works today, and this does not need to change.
These layers can have a fixed priority and there should be no need to ever change that priority.
A given library's priority relative to other libraries of the same name is determined based on which layer the library is installed into.
The IDE builder, simply picks the library in the highest priority layer that supports the target core.

I believe that the real issue that needs to be solved is how to do library updates.
What needs to be defined and changed is how/where to do the library updates so that each layer can update its own version of the library and not affect a version of the library in some other layer.

The current library manager with respect to updates takes a far too simplistic view:

  • there is one library that can be updated
  • there is a single repo for any given library
  • there is a single place to install updates for all libraries.

This is unworkable and that is the problem we are seeing.
Consider this example:
arduino.cc or a 3rd party core developer wants/needs to update a core library like Wire or SoftSerial.
More specifically, suppose arduino.cc wants to do an update to the SoftSerial library in AVR or SAM or both.
This cannot be done without having separate repos for each library in each core layer and storing the updates for each library in each core layer in different places.
With each individual library using its own repo and updates for each layer stored in different locations, each library in each core (layer), AVR, SAM, etc.. could update their individual libraries at any time and not impact the library in any other core (layer).

No matter how the library priorities are arranged and what overriding rules are in put in place, I see no way to handle the above example through just priorities using a single installation location for library updates.
Users, Cores and IDEs need to keep their libraries separated.
And that is the point I'm trying to stress.

The only way to fully solve the update issue is to provide the ability of each library in each layer to be updated individually.
Each library with its own repository allows updates to be pushed to those individual libraries.
And if "in place" updates are not allowed, the layer must be logically extended by using an additional location for that layer's updates.

@facchinm
Copy link
Member Author

@ArduinoBot build this please

@facchinm
Copy link
Member Author

Hi @bperrybap , sorry for the delay.
I wondered a lot about this topic and my conclusion is that, in order to break less things possible , we should never update a core library separately from the core itself (hence the problem you are pointing out with Wire is solved).
Also, core libraries are not listed in Library Manager (so they won't trigger an update).
The only problem is with bundled libraries (Ethernet, ...) and "fake" core libraries (like USBHost)
This PR should solve both this issues by assigning the right priority to core libraries.
It's of course a "best effort" approach, if someone is willing to code the "overlay" for libraries updates I'll be more than glad to review and merge it (I don't believe it will happen tomorrow 😄 )

@per1234
Copy link
Contributor

per1234 commented Mar 18, 2016

@facchinm please consider documenting the changes proposed in this PR with an updated version of the include priorities list I created at arduino/Arduino#4064 (comment). This will make it more easy to understand and also illustrate how much complexity is being added. I think an ideal solution would add the minimum possible number of items to the list while fixing the issue in all cases.

@PaulStoffregen
Copy link
Sponsor

@facchinm - Does this solve any problem other than Arduino updating the IDE-bundled libs between official IDE releases? If the libs provided with a core only update when the entire core is updated, why can't the same be true for the IDE's bundled libs? The bundled libs are very mature and stable, and Arduino releases a new IDE a few times a year? Is Library Manager updating of the IDE's bundled libs really needed?

My concern is the ever-increasing complexity of Arduino's build process, and the overly complicated design of most of the code added in the last year to 18 months. No single snowflake does much harm, but over time more and more can build up to an avalanche.

@bperrybap
Copy link

@facchinm
I'm not yet seeing how barring core library updates and mucking with priorities solves the problem.
What about these potential issues:

  • user has his own version of the library that he is working on in his sketchbook area
    (How does library manager not stomp on that and ensure that his library has priority?)
  • updates from one IDE for a bundled library affecting that library for another installed IDE
    (If so, regression testing becomes impossible, and a newer IDE installed can end up using an older IDEs "updated" bundled libraries.)

Consider the case where you have IDE X.X.X installed. There is a bundled library update for a library ZZ (dumped in the users sketchbook). Then, a newer IDE X.X.Y is installed that has a newer version of library ZZ with additional bug fixes or expanded capabilities.
Based on the sketchbook priority, the older ZZ library updated by IDE X.X.X would be selected instead of the newer ZZ library that was bundled with X.X.Y
I also see no way for this work when multiple IDEs are installed on the same machine since an update by any IDE affects all of them since there is only a single library update area.

My view is similar to Paul's in that from my perspective there really isn't any difference between a core and a bundled library update.
But I also believe that If updates to either of those layers are allowed, the update should not be allowed to affect any other layer nor should it impact any of the user's 3rd party libraries, nor personal libraries, nor other currently installed IDE's bundled libraries, nor future installed IDE's bundled libraries.

I share Paul's concern about complexity of the build process.
From looking from the outside in, it seems like many features are being hastily tossed in without a focus on a comprehensive overall design.

To me, at this point in time, the only viable option "to break less things possible", is to limit the library manager to only updating 3rd party libraries. (essentially, what Paul proposed about not supporting updating IDE bundled libraries)
This avoids and defers the issues related to core libraries and IDE bundled libraries until later.
Any use of the existing current library manager's single library model of using single repository and single update location simply cannot be made to work for IDE bundled libraries or core libraries.

For the longer term, I think the library manager needs a re-design.
The proposal that I made, solves all these update and priority issues between the various layers.
It treated each library individually as belonging to a logical layer and used a simple fixed priority for library selection by the builder, but still allowed each individual library within each layer to update itself using its own repository.
It added complexity to the library manager but even more importantly required no changes to the existing builder.

In my view this is the direction that the library updates needs to go.
Put as much of the complexity of handling library updates as possible into the library and keep it out of the builder.

@facchinm
Copy link
Member Author

@bperrybap your proposal is perfect, the table by @per1234 too, but we still miss the code for both Java IDE and builder and, above all, we still miss a complete series of regression tests for validating the new behaviour.
We need to tackle this soon and can't wait for the perfect solution to appear (which could take months and a lot of code and further discussion), so I'm going to merge this, let in staging for some weeks and then decide if we can keep it or if we should revert and apply a proper solution.

@facchinm facchinm merged commit b2f7c72 into arduino:master Mar 25, 2016
@soundanalogous
Copy link

With this change, does a library in sketchbook get trumped by a lib in core even if the lib in core also has arch=*? The reason I ask is I maintain Firmata, which is sort of an edge case here in that it is a 3rd party library that also happens to be included with the Arduino core libraries (it gets included somewhere in the Arduino build process before the IDE is available for download).

Arduino has a fork of Firmata and pulls from the main Firmata repo before releasing an new IDE. My concern is that with this new rule (if I'm understanding it correctly) that if a user updates Firmata via the Library Manager it will always be trumped by the (likely older) version of Firmata in the Arduino core. I typically update Firmata more frequently than the Arduino IDE release cycle so that is my concern.

@facchinm
Copy link
Member Author

Hi Jeff,
the fix only promotes a library if it's included in the core, not bundled (it would be completely nonsense 😄 )
AFAIK, no core bundles Firmata, and if they do they have specific reasons to do so (like not being included yet in mainline).

@soundanalogous
Copy link

Okay so the version of Firmata that ships with the Arduino IDE is considered "bundled" and therefore not affected by this update. SGTM then.

@bperrybap
Copy link

As @PaulStoffregen suggested, Arduino.cc could simply opt to stop supplying bundled library updates until the library manager is properly fixed by disabling and removing all the arduino.cc bundled libraries from the library manager.
This requires no code changes so it is an even easier solution.

It looks like this is a very simple temporary work around to simply bump the fixed priority order to make core libraries be the highest priority. So the new priority is now:

  • core
  • user
  • bundled

Which is what I said in my very first post.
Which is why I said the example didn't make sense given the initial post said that user library priority was above core libraries.
i.e. this is a simple patch of making core higher priority than the user library.
But this didn't agree with the stated priority in the very first post.

And so the problem of arduino.cc bundled lbrary update stomping on top of user libraries will still be a problem.
Could the library manager at least prompt for confirmation before it clobbers a non library managed user library?

This patch sounds like it will help in some cases but still break things in other cases.

@facchinm
Copy link
Member Author

Moving core priority above sketchbook has been discussed a lot and it received very negative feedback. We can't drop bundled libraries because people need this feature (for example in schools installations).
As I said, I consider the "overlay" proposal (every level updated in a different folder) to be the best around but it takes time to code and test.

@bperrybap
Copy link

But from my understanding, the patch you are proposing is moving core priority above sketchbook.
The algorithm for determining priority currently is that there is a fixed priority and you take the highest priority library that supports the target core.
(Perhaps the existing implementation is not properly looking at the target core, but I assumed that is how it should work, and was the way it was currently working)

In order to get a core library to trump a user sketchbook library in any situation, you have to re-arrange the priorities so that core is above user sketchbook.

I sounds like that is what this is being done in this patch.

That said, this proposal is silent on what happens in some situation.
(The example only showed a user sketchbook sketch for architecture=*, not for a specific core)
For example:
If the user sketchbook library specifies a specific core and there is also a core provided library.
I'm assuming that the core library would still trump the user's sketchbook library in this case as well.
If it doesn't, then you need to be very specific on how this is handled.
And what if the user sketchbook library is a "old" library with no .properties file?

And what about libraries that don't have the exact same name?
How is the library matching/promotion done?
Is it based on locating the library header files in directories or the actual library name?

I have great concerns about this for the broader picture and longer term.
It is the beginning of moving more library selection logic into build tool code rather than keeping it simple by being based on a simple location path list managed outside the build tool.
This will have implications on any non IDE build tools as they will also have update their code include whatever this new selection logic is.


For school installations, it would seem that "update in place" would be the ideal solution for bundled updates.
As that way the school, (or the teacher) could do the updates, and ensure that all students were using the proper versions of the libraries and more importantly the same libraries and a consistent IDE environment.
In place updates would require having the proper privileges to write in that area, which can also be a good thing a multi user environment as it ensures that users can't muck wit another user's build environment.
As is, the bundled updates using the user sketchbook area seems to be crazy in a student environment, as each student is left to doing the updates himself, so the build environment for each student can not be assured to be same.
Maybe the teachers are lazy and don't want to manage the library updates, but the existing method of using the user's sketchbook area for bundled udpates seems like a real mess for an academic environment.

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.

lib manager overrides core libraries included with 3rd party boards
6 participants