-
Notifications
You must be signed in to change notification settings - Fork 76
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
feat(pack): automatically install non-matching charmlibs #1918
Conversation
e434ac7
to
94d86dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied verbatim from the spec, which believe we are not following with the change as described
The library fetching code can now always be run as part of charmcraft pack. There are three possible options during the pack process:
- The required libraries are not in the source tree, and are fetched as part of the packing process
- There exist libraries in the source tree that satisfy the requirements
- There exist libraries in the source tree that conflict with charmcraft.yaml requirements (or are not specified at all)
It is important to note that while charmcraft pack will now be looking at the store, the charmcraft pack should never mutate the libraries on the filesystem, except in the first case. The interaction with the store during the pack process is only to provide warnings or information about the libraries in use (such as the availability of updates). The inability to reach the store should not make the pack process fail (except in the first case).
In the second and third cases, the fetch code is still run - if there are newer versions of the library available in the store, or the libraries do not match the constraints specified, the user should be notified as part of the packing process, but it should only be a warning/info notice, and the pack process should carry on using the in-tree libraries.
c408ed0
to
6616890
Compare
6616890
to
7fff8c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, have some minor comments and perhaps an expectation mismatch on fetching on pack (I thought it would be ephemeral and not persisted if I ran charmcraft clean
)
8528ccf
to
0aafde7
Compare
If
charmcraft.yaml
specifies charmlibs, pack will now automatically upgrade libraries on disk that don't match the specifications in the yaml file.