dirs: fix classic support detection #3940
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
`dirs.SupportsClassicConfinement()` was (mostly) just checking
dirs.SnapMountDir == "/snap"
without looking at `dirs.GlobalRootDir`, meaning that any and all
tests that (properly) called `dirs.SetRootDir(c.MkDir())`,
`dirs.SupportsClassicConfinement()` was returning `false`.
This was hidden by the fact that most tests that cared would simply
return on getting `false`, instead of skipping, so they quietly passed
without testing anything.
This PR addresses both of these issues.
chipaca
added a commit
to chipaca/snappy
that referenced
this pull request
Sep 19, 2017
…thing Currently, if a snap is installed with any flags, refreshed (preserving those flags), and reverted, the flags are lost. This means that a classic snap suddenly finds itself confined in strict mode (ditto for devmode; ditto again for jailmode). The current reply to this was "revert needs explicit flags", which is surprising to most users. The reasoning being that a snap can be classic at revision N, and strict at revision M, and as flags are per snap and not per revision we can't make a sensible decision. > The right fix is to make flags per revision, obviously (in retrospect) While we find time to implement the right fix, this PR changes the behaviour so that explicit flags are needed to _change_ the flags that the snap is installed with; if nothing is specified, flags are preserved. This will break the aforementioned flip-flopping snaps, but preserve the apparently more common case. So the breakage is moved from a bigger corner case to a smaller one, which is a win. This builds on canonical#3940.
mvo5
approved these changes
Sep 19, 2017
Codecov Report
@@ Coverage Diff @@
## master #3940 +/- ##
=========================================
+ Coverage 75.86% 75.9% +0.03%
=========================================
Files 416 417 +1
Lines 36130 36194 +64
=========================================
+ Hits 27410 27472 +62
- Misses 6794 6798 +4
+ Partials 1926 1924 -2
Continue to review full report at Codecov.
|
chipaca
added a commit
that referenced
this pull request
Sep 29, 2017
…thing (#3941) * overlord/snapstate: prefer a smaller corner case for doing the wrong thing Currently, if a snap is installed with any flags, refreshed (preserving those flags), and reverted, the flags are lost. This means that a classic snap suddenly finds itself confined in strict mode (ditto for devmode; ditto again for jailmode). The current reply to this was "revert needs explicit flags", which is surprising to most users. The reasoning being that a snap can be classic at revision N, and strict at revision M, and as flags are per snap and not per revision we can't make a sensible decision. > The right fix is to make flags per revision, obviously (in retrospect) While we find time to implement the right fix, this PR changes the behaviour so that explicit flags are needed to _change_ the flags that the snap is installed with; if nothing is specified, flags are preserved. This will break the aforementioned flip-flopping snaps, but preserve the apparently more common case. So the breakage is moved from a bigger corner case to a smaller one, which is a win. This builds on #3940. * add comments; fix spreads * skip the jailmode check on distros with no strict confinement
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.
dirs.SupportsClassicConfinement()was (mostly) just checkingwithout looking at
dirs.GlobalRootDir, meaning that any and alltests that (properly) called
dirs.SetRootDir(c.MkDir()),dirs.SupportsClassicConfinement()was returningfalse.This was hidden by the fact that most tests that cared would simply
return on getting
false, instead of skipping, so they quietly passedwithout testing anything.
This PR addresses both of these issues.