gamemode icon indicating copy to clipboard operation
gamemode copied to clipboard

Refactor attempt while providing upstream downstream

Open Kreyren opened this issue 5 years ago • 7 comments

DO_NOT_MERGE: Work in progress

Currently it is painful to build this project due to the lack of code quality assurance in bootstrap.sh and seemingly hard dependency on systemd, this is an attempt to resolve it while providing upstream downstream for paludis made for Exherbo Linux and Exheredrey.

Kreyren avatar Jul 05 '20 03:07 Kreyren

@FeralInteractive Please provide upstream-way of building (ideally with explanation) for this binary so that i can make a proper logic, thank you ^-^

EDIT: I know how to build it, but having more informations for in-code docs and logic for known issues would be appreciated

Kreyren avatar Jul 05 '20 03:07 Kreyren

I'm not sure I understand the goals here or what's missing in the current repo - could you explain a little more?

mdiluz avatar Jul 05 '20 09:07 mdiluz

@mdiluz I want to change following

  • bootstrap.sh, because it's not passing shellcheck and is seemingly designed to work only on specified systems with lack of sanitization
  • Add CI to capture these code quality issues
  • openrc support so that i can use this on my system (which doesn't seem to be supported atm, but seems to be easily implemented?)

I would also like to add exherbo and devuan downstream to upstream if you want it otherwise i will keep it in my fork, because keeping this on upstream repos seems to make it easier to work with. ^-^

Assuming it being welcomed?

Kreyren avatar Jul 05 '20 09:07 Kreyren

Currently it is painful to build this project due to the lack of code quality assurance in bootstrap.sh and seemingly hard dependency on systemd

First of all, I don't think that there is a lack of "QA" for this tiny bootstrap script. And gamemode it doesn't have a hard dependency on systemd either. It does require libsystemd, aka sd-bus, which btw doesn't require to have systemd as an init system. Anyway, you probably want to use elogind for this, and there is a Meson build option for this.

openrc support so that i can use this on my system (which doesn't seem to be supported atm, but seems to be easily implemented?)

So in this case, you might not want to run bootstrap.sh but rather write a custom script just for that purpose. I guess it doesn't make much sense to write a build script covering every possible option, so it only works for the majority of people. Distributions like Debian for example don't even use that script since they simply build with Meson, and set up the rest themselves. Basically you want to do

meson builddir -Dwith-sd-bus-provider=elogind --prefix=/usr
meson compile -C builddir
sudo meson install -C builddir

And restart polkit somehow. That's it, no need for 400+ loc. Put that in your build files and distribute the package.

stephanlachnit avatar Jul 05 '20 22:07 stephanlachnit

I largely agree with @stephanlachnit here, there's no reason to put downstream stuff into the upstream repo. I also want to comment a little:

bootstrap.sh, because it's not passing shellcheck and is seemingly designed to work only on specified systems with lack of sanitization

This is a reason to delete it, not add 500 lines of code.

And maybe this is simply a reminder to move the travis stuff over to github actions now that those are allowed for free.

mdiluz avatar Jul 07 '20 11:07 mdiluz

I guess it should simply be pointed out that bootstrap.sh is not to be used for packaging. I usually avoid such scripts even for a simple one-shot installation directly from source, because such scripts often make wrong assumptions about my system, i.e. they use sudo to clutter and clobber my system, overwriting files in /usr/lib or simply putting files not tracked by the package manager all over the system folders.

In this instance, it will install into /usr prefix by default. That should be /opt/gamemode or /usr/local probably. I'd prefer one-shot installations to live in their own namespace below /opt so I could uninstall by just deleting it. An unmanaged installer should never install to /usr directly.

As such, the script should probably be removed, or the defaults should change. In short term, the install instruction should put a clear message that bootstrap.sh is not to be used by packagers, maybe by introducing a new "Packaging" section in the README file showing how to use meson/ninja and how to manage the systemd/elogind dependencies (these are not properly documented yet, this is the opportunity).

kakra avatar Sep 24 '20 22:09 kakra

I guess it should simply be pointed out that bootstrap.sh is not to be used for packaging. I usually avoid such scripts even for a simple one-shot installation directly from source, because such scripts often make wrong assumptions about my system, i.e. they use sudo to clutter and clobber my system, overwriting files in /usr/lib or simply putting files not tracked by the package manager all over the system folders.

In this instance, it will install into /usr prefix by default. That should be /opt/gamemode or /usr/local probably. I'd prefer one-shot installations to live in their own namespace below /opt so I could uninstall by just deleting it. An unmanaged installer should never install to /usr directly.

As such, the script should probably be removed, or the defaults should change. In short term, the install instruction should put a clear message that bootstrap.sh is not to be used by packagers, maybe by introducing a new "Packaging" section in the README file showing how to use meson/ninja and how to manage the systemd/elogind dependencies (these are not properly documented yet, this is the opportunity).

Yeah I guess it's a good idea to make it more clear that the scripts is only intended if you install from source. Switching to /usr/local is probably a good idea, but it will break gamemoded --test because that path is hardcoded, something that should be fixed for that.

stephanlachnit avatar Sep 26 '20 13:09 stephanlachnit