Skip to content

Conversation

@h3xx
Copy link
Contributor

@h3xx h3xx commented Jul 11, 2023

Establishes some sane defaults:

  • architecture-dependent CMAKE_INSTALL_LIBDIR=lib or =lib64
  • CMAKE_INSTALL_BINDIR=bin (default)
  • CMAKE_INSTALL_DATADIR=share (default)

*_FULL* options also supported:

  • CMAKE_INSTALL_FULL_BINDIR=${CMAKE_INSTALL_PREFIX}/bin (default)

Establishes some sane defaults:
- architecture-dependent CMAKE_INSTALL_LIBDIR=lib or =lib64
- CMAKE_INSTALL_BINDIR=bin (default)
- CMAKE_INSTALL_DATADIR=share (default)

*_FULL* options also supported:
- CMAKE_INSTALL_FULL_BINDIR=${CMAKE_INSTALL_PREFIX}/bin (default)
@scottfurry
Copy link
Contributor

It's nice that Kitware has this functionality available in CMake. Unfortunately, using the GNUInstallDirs (or GNU Directory Variables ) is presumptive. There is also the XDG Base Directory Specification.

Pull Request being offered, thank you, I find is pushing us to pick a standard.

@justinclift
Copy link
Member

Meh, it doesn't seem to be actively hurting anything. Lets see if it passes our CI tests first. 😄

@scottfurry
Copy link
Contributor

It's the "pick-a-side" notion that bothers me. GNU is not exactly perfect. Freedesktop is an industry group for the most part. When confronted with either one my head goes back to clips of Pirates of the Caribbean - "...the code is more what you'd call "guidelines" than actual rules". Same can be said for some specifications sadly.

Sure, we could be using different install variables. We could be more aligned with a standard. And yet,... no one has complained or commented until now...

@justinclift
Copy link
Member

justinclift commented Jul 12, 2023

Yeah, I don't personally care either way either. 😄

That being said, this GNU based PR has turned up so maybe lets merge this can call it "first mover's advantage"? 😁

On second thought, I'd rather not. Let's just close this as "Nope".

@justinclift
Copy link
Member

@h3xx Thanks for the PR. But, we're probably not going to "pick a side" here, as @scottfurry was referring to it. 😉

@h3xx
Copy link
Contributor Author

h3xx commented Jul 12, 2023

Pull Request being offered, thank you, I find is pushing us to pick a standard.

The GNU vs. XDG debacle (first I've heard of it) is not even a choice you have to make. The "GNU" in GNUInstallDirs I believe was chosen arbitrarily, not out of some dismissal of XDG standard; there is no XDGInstallDirs CMake module, the one named with GNU is the only one like it.

The reason for merging this change would be simply to allow overriding the bindir name. E.g. -DCMAKE_INSTALL_BINDIR=libexec or =/opt/foo/binaries. With the current code, you can't do that. If you use "make install" to install it, the paths are hardcoded to prefix + bin, lib, share, etc.

The default paths provided by GNUInstallDirs are the same as what is hardcoded in the current code.

All I wanted to do with this PR is, in a standard way, allow packagers to override the install paths instead of having to manually move files around after the install.

@justinclift
Copy link
Member

Hmmm, that sounds reasonable to me. @scottfurry Thoughts?

@mgrojo
Copy link
Member

mgrojo commented Jul 12, 2023

GNUInstallDirs is called like that because it follows the GNU Coding Standards for installation directories. I don't find anything controversial there because it follows established Unix customs for installing software in the system (in fact our hardcode values are the same). XDG Base Directory is for another thing, without any traditional custom: the location for files under user home directories. They're for different things, so we are not choosing sides.

Since this allows for more flexibility and standardization, I'm in favor of merging this pull request.

@mgrojo mgrojo reopened this Jul 12, 2023
@justinclift justinclift merged commit fa0b235 into sqlitebrowser:master Jul 13, 2023
@justinclift
Copy link
Member

No worries @mgrojo. Thanks for this PR, and for clarifying info about it @h3xx. 😄

@scottfurry
Copy link
Contributor

Too late...

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.

4 participants