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

Please install stuff in /usr/local/share by default, not /usr/share #479

Closed
volkertb opened this issue Jan 14, 2023 · 5 comments
Closed

Comments

@volkertb
Copy link

volkertb commented Jan 14, 2023

Hi,

First of all: awesome project!

Just one small annoyance I noticed when I ran the x11docker script with the --update option: I noticed it installing (or at least trying to install) stuff under /usr/share. That really ought to be /usr/local/share, IMO. After all, I'm manually installing something outside of the distro's package manager.

I'm specifically referring to these log lines in the console output:

x11docker note: Storing README.md, CHANGELOG.md and LICENSE.txt in
  /usr/share/doc/x11docker

x11docker note: Storing man page in /usr/share/man/man1/x11docker.1.gz

gzip: skipping: x11docker.man does not exist
x11docker note: Error storing man page.

The log didn't report anything else being installed under /usr other than /usr/share, but obviously, this would apply to anything being installed under /usr: it should be /usr/local by default, unless the software is being installed through a distro-specific package manager.

See also https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html

Thanks for considering.

@mviereck
Copy link
Owner

You are right that x11docker should use /usr/local following the specifications.
However, there is one issue why I decided against it: If x11docker is installed in /usr/local/bin, it is not within PATH for root. This is at least true for debian, likely for other distributions, too. But root should be able to run x11docker, in case one does not want to use insecure group docker.
So x11docker installs itself as /usr/bin/x11docker.
I could store at least the docs in /usr/local/share (not sure if at least man would find the man page); but having the script in /usr, but the rest in /usr/local is somewhat inconsistent.

@volkertb
Copy link
Author

volkertb commented Jan 17, 2023

Most people will use sudo (or doas) as a non-root user for operations that require root, in which case /usr/local/bin will be in the path. Only more experienced people will sometimes log in as root or use su to obtain a root shell, and those people tend to be aware of the absence of /usr/local/bin from the root path. They'll be savvy enough to prefix the path whenever they invoke x11docker as root. And even if they don't like prefixing the path, and don't mind their distro installation being cluttered, they'll be sure to specify --prefix=/usr during installation, to override the default.

So this does not seem enough of a justification to me to default to /usr/bin.

Or did I misunderstand your explanation?

@mviereck
Copy link
Owner

mviereck commented Jan 17, 2023

Or did I misunderstand your explanation?

You understand me right.

Most people will use sudo (or doas) as a non-root user for operations that require root, in which case /usr/local/bin will be in the path.

This seems to be true now. I've also checked su here an Debian bullseye, and it has /usr/local/bin in PATH, too.
It seems this has changed meanwhile. My observation of missing /usr/local/bin has been about 5 years ago, when development of x11docker started. It was back then when I decided to use /usr/bin.

So you are right, x11docker should use /usr/local in general for all files. I just have to write a clean update procedure that does not break anything and leaves no files behind.

However, there might still be some distributions around without /usr/local/bin in PATH for su/sudo.

mviereck pushed a commit that referenced this issue Jan 18, 2023
--remove-oldprefix: new option -479
@mviereck
Copy link
Owner

I've published release v7.6.0 to adress this issue: https://github.com/mviereck/x11docker/releases/tag/v7.6.0

I hope this works well and does not cause too much confusion for users.

@volkertb
Copy link
Author

Thanks for resolving this so quickly! I'm personally confident that this won't cause issues for the vast majority of users.

Thanks again for developing this tool and sharing it with the world. 🙂

By the way, sorry for creating a duplicate issue. I could have sworn I did a search through existing issues before I created this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants