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

snap-confine: Ensure lib64 biarch directory is respected #3980

Merged
merged 1 commit into from Sep 28, 2017

Conversation

@ikeydoherty
Copy link
Contributor

ikeydoherty commented Sep 27, 2017

This fixes a 2.28+ regression whereby snap-update-ns no longer works
due to the strict apparmor rules.

Signed-off-by: Ikey Doherty ikey@solus-project.com

This fixes a 2.28+ regression whereby snap-update-ns no longer works
due to the strict apparmor rules.

Signed-off-by: Ikey Doherty <ikey@solus-project.com>
Copy link
Contributor

Conan-Kudo left a comment

If you're actually adding support for biarch locations and you don't have a dedicated libexec dir, then you should probably add all of the multilib directories to this.

@ikeydoherty

This comment has been minimized.

Copy link
Contributor Author

ikeydoherty commented Sep 27, 2017

In Solus the "dedicated libexecdir" is "named subdir of libdir". I'm not sure how many distros do this, and I'm kinda doubtful we'd see lib32, libx32 variants on top of that. But, if they exist, then sure. This one is a bit less important than my other PRs anyway.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 27, 2017

Codecov Report

Merging #3980 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3980      +/-   ##
==========================================
+ Coverage   75.97%   75.97%   +<.01%     
==========================================
  Files         423      423              
  Lines       36505    36505              
==========================================
+ Hits        27734    27735       +1     
+ Misses       6833     6832       -1     
  Partials     1938     1938
Impacted Files Coverage Δ
cmd/snap/cmd_aliases.go 95% <0%> (+1.66%) ⬆️

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 1d7d79c...aad4f90. Read the comment docs.

@Conan-Kudo

This comment has been minimized.

Copy link
Contributor

Conan-Kudo commented Sep 27, 2017

@ikeydoherty As it's trivial and Yocto and Gentoo allow this as an option when building them, I would suggest going for it.

@zyga
zyga approved these changes Sep 28, 2017
Copy link
Contributor

zyga left a comment

+1

@zyga zyga requested a review from jdstrand Sep 28, 2017
@mvo5

This comment has been minimized.

Copy link
Collaborator

mvo5 commented Sep 28, 2017

Thanks, I will cherry pick this into release/2.28 - where do we have the regressions? solus?

@mvo5 mvo5 merged commit f96badd into snapcore:master Sep 28, 2017
1 of 7 checks passed
1 of 7 checks passed
artful-amd64 autopkgtest finished (failure)
Details
artful-i386 autopkgtest finished (failure)
Details
xenial-amd64 autopkgtest finished (failure)
Details
xenial-i386 autopkgtest finished (failure)
Details
xenial-ppc64el autopkgtest finished (failure)
Details
zesty-amd64 autopkgtest finished (failure)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mvo5

This comment has been minimized.

Copy link
Collaborator

mvo5 commented Sep 28, 2017

Aha, this is only breaking in master, right?

Copy link
Contributor

jdstrand left a comment

Was asked to review, but already merged....

This LGTM.

@ikeydoherty

This comment has been minimized.

Copy link
Contributor Author

ikeydoherty commented Sep 28, 2017

Yeah only cropped up on Solus - but ya, cheers for merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.