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

cmd/snap-update-ns: fix mount rules for font sharing #4136

Merged
merged 6 commits into from Nov 3, 2017

Conversation

zyga
Copy link
Collaborator

@zyga zyga commented Nov 2, 2017

The mount rules should refer to directories, not files (the traling
slash is relevant).

Signed-off-by: Zygmunt Krynicki me@zygoon.pl

@mvo5 mvo5 added this to the 2.29 milestone Nov 2, 2017
zyga and others added 3 commits November 2, 2017 19:58
The mount rules should refer to directories, not files (the traling
slash is relevant).

Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Copy link

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

echo "The plug is connected by default"
snap interfaces | MATCH "$CONNECTED_PATTERN"

echo "Then the snap is able to desktop files and directories"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/able to/able to access/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is coming from a patch by @sergiocazzolato

@zyga zyga removed this from the 2.29 milestone Nov 2, 2017
@codecov-io
Copy link

codecov-io commented Nov 2, 2017

Codecov Report

Merging #4136 into master will increase coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4136      +/-   ##
==========================================
+ Coverage   75.62%   75.63%   +0.01%     
==========================================
  Files         435      435              
  Lines       37583    37613      +30     
==========================================
+ Hits        28421    28450      +29     
+ Misses       7173     7170       -3     
- Partials     1989     1993       +4
Impacted Files Coverage Δ
cmd/snap-update-ns/main.go 0% <0%> (ø) ⬆️
cmd/snap-update-ns/change.go 97.46% <100%> (ø) ⬆️
interfaces/builtin/serial_port.go 65.51% <0%> (-1.48%) ⬇️
overlord/ifacestate/helpers.go 60.26% <0%> (+0.66%) ⬆️
wrappers/desktop.go 78.94% <0%> (+6.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae0a488...dd810cd. Read the comment docs.

@mvo5 mvo5 mentioned this pull request Nov 3, 2017
This patch fixes a nasty bug that results in wrong list of made changes.
This is caused by how the for / range syntax operates in golang.

Before this patch the list of changes made contains N copies of the
first change. This seems to be caused by golang using a hidden temporary
variable that gets overwritten but has the same address. We add the
address of that variable to the list of changes.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@mvo5 mvo5 merged commit 2879d01 into snapcore:master Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants