jszczerbinsky/lwp

Linux installation

mid-kid opened this issue · 3 comments

Hi, I've stumbled across this tool, and it works really well for what it does. Thanks a lot for making it, there's a serious lack of good wallpaper engines on linux in general.

However, I have a bit of a problem with the installation instructions. I know distribution on linux is complicated, especially for people who aren't too familiar with the ecosystem, but there's a few rules that should be kept in mind, and I hope you don't mind me explaining.

Primarily, unzipping files straight into / is just a bad idea. This is for the same reason that running things like sudo pip install is: The /usr structure is supposed to only be managed by the system's package manager, and installing files that could conflict with, or overwrite files managed by it can lead to breakage, and long nights spent troubleshooting. This program is relatively innocuous, as it doesn't install things in key directories that change system behavior, and finding its files for removal isn't too difficult, but there's designated places to install user-provided software not managed by the package manager, such as /opt and /usr/local, so it'd be better to make use of those. More and more distributions are making /usr read-only for this reason.

Aside from that, I've noticed that both /etc and $HOME/.config are hardcoded. While these locations are conventional, there's conventions to provide methods of modifying them, as well. For /etc it's usually a sysconfdir variable in makefiles, -DCMAKE_INSTALL_SYSCONFDIR in cmake, and --sysconfdir in both autotools and meson. Doing so would allow someone to use /usr/local/etc to install the default config file and keep this package completely separate. For $HOME/.config, there's the relatively small XDG base directory specification, which recommends trying $XDG_CONFIG_HOME before falling back to $HOME/.config (it's completely OK, and even preferable to use getenv("HOME") for this).

And lastly, a side-note about the current makefile build system, it's generally recommended to use pkg-config to query information about linker paths and filenames, for example through pkg-config --cflags --libs sdl2. This is due to the fact that libraries might be installed in non-standard locations, and require certain compiler options to function correctly. Additionally, hardcoding gcc is discouraged as the user may use a different compiler, or even try to cross-compile, and make already initializes the $(CC) variable properly, which you should use. There's this and many other similar subtleties that may make a project more difficult to compile or package in linux distributions.

Thankfully, modern build systems take care of all of this these days, so I've written a simple meson.build script that solves all of the concerns above, and allows you to use the SYSCONFDIR define to find the default configuration file, and very likely works on windows without modifications:

project('lwp', 'c',
  version : '1.5')

deps = []
deps += dependency('sdl2')
deps += dependency('x11')

c_args = []
c_args += '-DSYSCONFDIR=' + (get_option('prefix') / get_option('sysconfdir'))

executable('lwp',
  'main.c',
  'wallpaper.c',
  'window.c',
  'parser.c',
  'debug.c',
  c_args : c_args,
  dependencies : deps,
  install : true)

install_data('default.cfg',
  rename : 'lwp.cfg',
  install_dir : get_option('sysconfdir'))
install_subdir('wallpapers',
  install_dir : get_option('datadir') / meson.project_name())

By default, this installs everything to /usr/local, but allows anyone to pick an installation location through the meson setup --prefix parameter. The big advantage of having this is that meson provides a high guarantee that software built with it can be packaged effortlessly for different distributions, as they commonly have well supported presets for meson projects. Of course, CMake works too, if you prefer something that's better supported in the windows world.

If you use configure_file() you may even generate the correct wallpaper path inside lwp.cfg at build time.

(disregard the bug label, this is more of a suggestion)

Thanks for Your advise and great explanation @mid-kid, I will apply Your suggestions in next update

Hey, thanks a lot for considering the suggestions! The changes look good.
That said, I made -DSYSCONFDIR a compiler define in order to store the configuration option in the compiled binary. While I'd love for such an environment variable to become standard, it currently really isn't, so most programs opt to "hardcode" it at compile time like that or provide a program-specific env var for it.
Ultimately it's up to you, though, I won't bother you further.