o/snapstate: install components and snaps from file #14095
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewphelpsj
requested review from
alfonsosanchezbeato and
pedronis
and removed request for
pedronis and
alfonsosanchezbeato
andrewphelpsj
force-pushed
the
install-component-from-file
branch
4 times, most recently
from
0651aa2
to
fdfee01
Compare
andrewphelpsj
force-pushed
the
install-component-from-file
branch
from
fdfee01
to
03f2877
Compare
pedronis
requested changes
Jun 26, 2024
overlord/snapstate/target.go
Outdated
| func installableComponentsFromPaths(info *snap.Info, components map[*snap.ComponentSideInfo]string) ([]ComponentSetup, error) { | ||
| setups := make([]ComponentSetup, 0, len(components)) | ||
| for csi, path := range components { | ||
| compInfo, _, err := backend.OpenComponentFile(path, info, csi) |
There was a problem hiding this comment.
shouldn't this do something more similar to validatedInfoFromPathAndSideInfo ?
There was a problem hiding this comment.
Yes, resolved in 88890b7.
…g from ephemeral files
andrewphelpsj
force-pushed
the
install-component-from-file
branch
from
301c185
to
88890b7
Compare
| // if we're removing the snap file and we are mounting a component for the | ||
| // firs time, then we know that the component also must be coming from an | ||
| // emphemeral file. in that case, remove it. | ||
| if snapsup.RemoveSnapPath { |
There was a problem hiding this comment.
Maybe we should add the flags from componentInstallFlags to ComponentSetup and check that instead?
There was a problem hiding this comment.
but this is code still the same? what am I missing
There was a problem hiding this comment.
Sorry, missed that in the commit.
pedronis
reviewed
Jun 28, 2024
| // if we're removing the snap file and we are mounting a component for the | ||
| // firs time, then we know that the component also must be coming from an | ||
| // emphemeral file. in that case, remove it. | ||
| if snapsup.RemoveSnapPath { |
There was a problem hiding this comment.
but this is code still the same? what am I missing
alfonsosanchezbeato
approved these changes
Jul 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows us to install snaps and components from files at the same time. Based on #14092, first commit is 12afed2.