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

Add profile for links and xlinks #2734

Merged
merged 10 commits into from
Jun 2, 2019
Merged

Conversation

jose1711
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Fred-Barclay Fred-Barclay left a comment

Choose a reason for hiding this comment

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

Looks really good! I had a few comments on the links profile -- can you check them and also try them for the xlinks profile?
Cheers!
Fred

etc/xlinks.profile Outdated Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Outdated Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
disable-mnt
private-cache
private-dev
private-tmp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also try this please: private-etc ca-certificates,crypto-policies,resolv.conf,ssl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is often too limiting for associated programs (e. g. mpv was unable to start)

Copy link
Collaborator

@SkewedZeppelin SkewedZeppelin Jun 1, 2019

Choose a reason for hiding this comment

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

we usually don't write profiles that allow other programs to work
see #1718 (comment)
and
#1770 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better: private-etc ca-certificates,crypto-policies,nsswitch.conf,pki,resolv.conf,ssl and if e.g. exists /etc/links also links.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we usually don't write profiles that allow other programs to work
see #1718 (comment)
and
#1770 (comment)

hmm... if this is the case then the whole novideo, noaudio, x11 discussion is pointless, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rusty-snake how about doing this:

private-etc ca-certificates,crypto-policies,nsswitch.conf,pki,resolv.conf,ssl
# Uncomment the following line to allow external media players
# private-etc alsa,asound.conf,alternatives,machine-id,openal,pulse

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Fred-Barclay sound good, but

  1. # Uncomment the following line (or put it in your links.local) …
  2. On Fedora: ll /bin/links: /bin/links -> /etc/alternatives/links
  3. xlinks will need fonts

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rusty-snake I don't understand 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added private-etc + comment on how to enable media players

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Fred-Barclay we need alternatives, because on some system is links a symlink to /etc/alternativs/links

etc/links.profile Outdated Show resolved Hide resolved
Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

Additional nitpicking.

I'm also think that is it better to write always "media player" instead of "audio player"/"video player" because the most options are needed of both.

etc/links.profile Outdated Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Show resolved Hide resolved
etc/links.profile Outdated Show resolved Hide resolved
disable-mnt
private-cache
private-dev
private-tmp

This comment was marked as outdated.

etc/links.profile Outdated Show resolved Hide resolved
etc/xlinks.profile Outdated Show resolved Hide resolved
etc/links.profile Outdated Show resolved Hide resolved
etc/links.profile Outdated Show resolved Hide resolved
@rusty-snake
Copy link
Collaborator

rusty-snake commented Jun 1, 2019

For links.profile I came to this.

etc/xlinks.profile Outdated Show resolved Hide resolved
Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

After long discussions... the time has come.

@Fred-Barclay ?

* Create allow-INTERPETER.inc

 * allow-lua.inc
 * allow-perl.inc
 * allow-python2.inc
 * allow-python3.inc

* Create allow-java.inc

* Update profiles to use new allow-INTERPRETER.inc includes

* Update profiles to use new allow-INTERPRETER.inc includes 2/x

* Fix order of allow-INTERPRETER.inc includes

* Update profiles to use new allow-INTERPRETER.inc includes 3/x

* Fixup comment about allow-java.inc

netblue30#2736 (comment)

* Add Arch Linux specific paths to allow-perl.inc
@Fred-Barclay
Copy link
Collaborator

Fred-Barclay commented Jun 1, 2019

@rusty-snake just need one update on private-etc. Then whichever of us gets to this first should be good to merge! 😄

EDIT: if you merge this, please choose squash and merge

@rusty-snake
Copy link
Collaborator

I almost always use squash, because I don't like ugly commit-messages like Merge pull request #ABCD from USER/BRANCH 😉 .

@Fred-Barclay
Copy link
Collaborator

@jose1711 @rusty-snake New idea... since xlinks shares a lot with links, why not just re-direct?
I.e.:


# Firejail profile for xlinks
# Description: Text WWW browser (X11)
# This file is overwritten after every install/update
# Persistent local customizations
include xlinks.local
# Persistent global definitions
include globals.local

noblacklist /tmp/.X11-unix
private-bin xlinks

# Redirect
include links.profile

@rusty-snake
Copy link
Collaborator

But now

# added by included profile
#include globals.local

@Fred-Barclay
Copy link
Collaborator

@rusty-snake I think it's ok to include globals.inc here and also in links.profile. Other redirect profiles (like for google chrome beta) do the same.

@glitsj16
Copy link
Collaborator

glitsj16 commented Jun 1, 2019

@Fred-Barclay If I follow this thread correctly I do think @rusty-snake has a fair point in wanting to avoid including globals.local twice. See #2006 and #2010 for reference.

@rusty-snake I think it's ok to include globals.inc here and also in links.profile. Other redirect profiles (like for google chrome beta) do the same.

google-chrome-beta.profile does include globals.local because its redirect profile (chromium-common.profile) does not. Your proposed xlinks redirect profile needs to have include globals.local commented because its redirect profile (links.profile) already takes care of including that file. Hope this helps to clear up any confusion.

xlinks redirects to links

Add basic private-etc line and a commented, extended private-etc
@Fred-Barclay
Copy link
Collaborator

I added some changes. @jose1711 @rusty-snake review por favor.

@jose1711
Copy link
Contributor Author

jose1711 commented Jun 2, 2019

I added some changes. @jose1711 @rusty-snake review por favor.

Don't know how badly xlinks wants access to fonts but maybe we still want private-etc fonts in its profile?

Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

Just add alternativs to private-etc, because on fedora /usr/bin/links is a symlink to /etc/altenatives/links.

etc/links.profile Outdated Show resolved Hide resolved
etc/links.profile Outdated Show resolved Hide resolved
@rusty-snake rusty-snake merged commit cdc2347 into netblue30:master Jun 2, 2019
@rusty-snake
Copy link
Collaborator

Merged, Thanks.

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.

5 participants