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-confine: Support bash as base runtime entry #4197

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

ikeydoherty
Copy link
Contributor

This allows bash that makes use of readline + ncurses to correctly map
and execute within strict apparmor confinement, specifically required
for enabling bash-based base runtimes (such as Solus).

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

This allows bash that makes use of readline + ncurses to correctly map
and execute within strict apparmor confinement, specifically required
for enabling bash-based base runtimes (such as Solus).

Signed-off-by: Ikey Doherty <[email protected]>
@codecov-io
Copy link

codecov-io commented Nov 9, 2017

Codecov Report

Merging #4197 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4197   +/-   ##
=======================================
  Coverage   75.57%   75.57%           
=======================================
  Files         436      436           
  Lines       37830    37830           
=======================================
  Hits        28590    28590           
  Misses       7245     7245           
  Partials     1995     1995

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 5c066a8...ec4f3c0. Read the comment docs.

@zyga
Copy link
Contributor

zyga commented Nov 10, 2017

I'm not supper happy about this. Ideally we will remove the only reason this is needed (snappy-app-dev) CC @jdstrand

If we cannot avoid it then +1 but I'd like to really remove that shell script and not have to have extra permissions here.

@zyga zyga requested a review from jdstrand November 10, 2017 07:50
Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

@zyga second opinion?

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. +1

As for removing snappy-app-dev shell and using a helper, I will be doing that as part of my work to fix dynamic hotplugging.

@zyga zyga merged commit e683782 into canonical:master Nov 10, 2017
joebonrichie pushed a commit to solus-packages/snapd that referenced this pull request Aug 15, 2023
These changes are undergoing review upstream and are required to support
a Solus derived runtime, for the LSI efforts and more.

 - canonical/snapd#4197
 - canonical/snapd#4199

Effectively this introduces support for biarch multilib drivers, i.e. the
32-bit NVIDIA drivers, within the snap. Additionally it adds support for
using bash as an entry point as opposed to dash.

Signed-off-by: Ikey Doherty <[email protected]>
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