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

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

Signed-off-by: Ikey Doherty [email protected]

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

Signed-off-by: Ikey Doherty <[email protected]>
Copy link
Contributor

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor

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

Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

+1

@zyga zyga requested a review from jdstrand September 28, 2017 09:21
@mvo5 mvo5 added this to the 2.28 milestone Sep 28, 2017
@mvo5
Copy link
Contributor

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 canonical:master Sep 28, 2017
@mvo5 mvo5 removed this from the 2.28 milestone Sep 28, 2017
@mvo5
Copy link
Contributor

mvo5 commented Sep 28, 2017

Aha, this is only breaking in master, right?

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.

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

This LGTM.

@ikeydoherty
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants