-
Notifications
You must be signed in to change notification settings - Fork 567
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
Conversation
There was a problem hiding this 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
disable-mnt | ||
private-cache | ||
private-dev | ||
private-tmp |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
# Uncomment the following line (or put it in your links.local) …
- On Fedora:
ll /bin/links
:/bin/links -> /etc/alternatives/links
- xlinks will need
fonts
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this 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.
disable-mnt | ||
private-cache | ||
private-dev | ||
private-tmp |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
For links.profile I came to this. |
There was a problem hiding this 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.
* 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
@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 |
I almost always use squash, because I don't like ugly commit-messages like |
@jose1711 @rusty-snake New idea... since xlinks shares a lot with links, why not just re-direct?
|
But now
|
@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. |
@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.
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 |
xlinks redirects to links Add basic private-etc line and a commented, extended private-etc
I added some changes. @jose1711 @rusty-snake review por favor. |
Don't know how badly |
There was a problem hiding this 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
.
Merged, Thanks. |
No description provided.