Improve 'make install' for distros.

[orbea]

Fixes #70.

The main goal of this is a more standard make install and make uninstall which will work much better with distros. I only touched the linux/bsd path and left both windows and osx alone since I am not knowledgeable of what those platforms expect.

More specifically this does:

1. Removes the check disabling the install and uninstall checks when run as root. Some distros like Slackware build and create packages as root and there are tools like sandbox (https://wiki.gentoo.org/wiki/Project:Sandbox) designed to catch programs that install out of the expected paths.
2. This adds DESTDIR which by default is unset. When set it allows for staged installs. For example everything gets installed into a fake root directory which is then used to create a distro package.
3. This follows the FHS defaults for installed paths where applicable.
4. This unhardcodes /usr/share/ when DATADIR is defined during the build so that it correctly respects the installed path.

For 3 the default paths are:

• prefix = /usr/local
• bindir = $(prefix)/bin
• datarootdir = $(prefix)/share
• datadir = $(datarootdir)

These can be overridden with setting these variables as make arguments.

For example on Slackware I would use:

make install \
  prefix=/usr \
  bindir=/usr/games \
  datadir=/usr/share/games \
  DESTDIR=$PKG

It is important to have both datarootdir and datadiras the former is for files like icons and desktop files while the latter is data files specific for bsnes. On Slackware I would install the former to /usr/share/ and the latter gets installed to /usr/share/games/bsnes.

Please review to make sure there are not any hardcoded paths I missed.

Some things I did not include.

• Out of tree builds, it is helpful to be able to compile bsnes without touching the actual source tree. I will look into this later depending on the outcome of this PR.
• Variables like prefix or bindir may be more standard if written as PREFIX or BINDIR, but its not really important.

[ghisvail]

That would also benefit the Flatpak packaging.

[orbea]

I added one more commit to take care of out of tree builds.

Example:

mkdir -p /tmp/build
cd /tmp/build
make install -f /path/to/bsnes/bsnes/GNUmakefile

This adds dependencies for the obj and out directories which may not exist in the build directory so they are automatically created before any compilation.

It may need to abstract mkdir similar to delete for rm, but someone else will need to tell me the proper windows command for this to work.

[Screwtapello]

Sorry it's taken such a long time to get back to you!

Most of these changes look pretty straightforward, and in my limited experience trying to write RPM spec files at a former job, I understand why they're useful. However, I'm less sure about supporting out-of-tree builds — that's the biggest change of all (67 insertions, 55 deletions) and I don't think it's strictly needed - I can imagine it being useful in occasional, niche situations, but I don't think it's going to be used regularly enough to make the change worthwhile.

I'm happy to merge the other changes as-is right now, but if you have a common use-case for out-of-tree builds I'm willing to hear it before I decide.

[ghisvail]

I believe out-of-tree builds are an orthogonal concern to the original topic of this PR, i.e. proper support for a make install target.

Perhaps it is best to merge the changes until the last commit, submit the latter as a separate PR and continue the discussion there?

[orbea]

I removed the out of tree commit from this PR and pushed it to this branch for now

https://github.com/orbea/bsnes/tree/out_of_tree

However, I'm less sure about supporting out-of-tree builds — that's the biggest change of all (67 insertions, 55 deletions) and I don't think it's strictly needed - I can imagine it being useful in occasional, niche situations, but I don't think it's going to be used regularly enough to make the change worthwhile.

This is a very understandable concern and when I made similar changes for other projects (i.e. mupen64plus) it was not nearly as big of a diff. This is because each file has its own make rule which is a strange design. It would be much better if they are listed in variables with a few make rules to control them all. Perhaps if this was done first it would be a far more straight forward and less intrusive commit.

but if you have a common use-case for out-of-tree builds I'm willing to hear it before I decide.

Its very useful for source based distros like what is used in gentoo as it keeps a clean distinction from the build files and the actual source tree. It is also helpful in cases where the source directory may not have write permissions or if the user wishes to keep the source on their filesystem while only building in tmpfs.

I also have a personal simple build repo which depends on out of tree builds working so I have a strong desire for it.

https://notabug.org/orbea/build/src/master/games/bsnes

[orbea]

I think the freebsd build issue was introduced after I rebased to master...

[ghisvail]

@orbea you might want to rebase onto master to benefit from this update.

[Screwtapello]

Thank you very much for making that change. I've reviewed this PR, I like everything in it, and I'd be happy to merge it except that I've just noticed it wouldn't work. When bsnes looks for a resource — the game database, shaders, firmware files, etc. — it calls a function called locate():

bsnes/bsnes/target-bsnes/bsnes.cpp

Lines 8 to 17 in be0ee76

	auto locate(string name) -> string {
	string location = {Path::program(), name};
	if(inode::exists(location)) return location;
	location = {Path::userData(), "bsnes/", name};
	if(inode::exists(location)) return location;
	directory::create({Path::userSettings(), "bsnes/"});
	return {Path::userSettings(), "bsnes/", name};
	}

It checks beside the executable, it checks the "user data" path (~/.local/share/bsnes on POSIX), and then it assumes the file is in the "user config" path (~/.config/bsnes on POSIX). It doesn't fallback to /usr/local/, it doesn't respect $DATADIR, and it definitely doesn't respect $XDG_DATA_DIRS.

Probably the simplest thing to do now would be to add a fallback to DATADIR or /usr/local to that function, but probably at some point we should clean up all bsnes' resource-loading and make it more XDG-compliant. Heck, I might as well file an issue for that now.

[Screwtapello]

Somehow I overlooked that such an issue already exists: #111.

[orbea]

@Screwtapello Thanks for pointing that out, I failed to notice that sharedData() is not actually used. I updated the last commit, I think it should be better now?

[ghisvail]

@Screwtapello indeed, that's what we observed back then with my FHS compliance PR on Higan which then triggered issue #111 for further discussion. I did a (hopefully) comprehensive review of path queries throughout the code base and the results are available here.

The last commit from @orbea is similar to the patch applied to the Debian packaging to support FHS shared data directory.

[carmiker]

From my perspective, this PR looks good to be merged.