Remove hard-coded paths

[evelikov]

Earlier commit added the full path into the systemd service. That's the correct thing to do, although it did not consider that the path is can be selected at build time - see grep SBIN Makefile.

For this task, one should audit the whole repo for SBIN and other hard-coded paths and sed ... s|#FOO#|$FOO| replace them, like the others in the Makefile

[evelikov]

Essentially look at all the FOO = $(DESTDIR)/tada instances, applying distribution backed data if the variable is ever overwritten. If so, convert the code/docs/scripts to use actual user-data. Otherwise see if it makes sense to remove the variable or instead use :=.

[evelikov]

Exhibit A: https://github.com/dell/dkms/pull/335

Namely: not only are we embedding the incorrect path, but we also conflate merge DESTDIR into the respective directories. At a glance I see 3 options, in order of increasing preference:

• the bad idea: simply drop the DESTDIR merging
  • effectively a silence breakage, make install will work w/o errors and files are misplaced
• reasonable compromise : deprecate the old ones, error if set and introduce new
  • make install will error with SBIN is deprecated, use sbindir ensuring it does not contain DESTDIR or alike
• ideal solution: if DESTDIR is present within the SBIN/etc paths - warn and strip it
  • don't think I can pull that off, likely to be convoluted and/or fragile if we manage to :frowning_face:

Currently leaning toward the second option. Unless someone knows how to get the last one working, or has other suggestions.

Happy to hear the input of @anbe42, @scaronni and others.

[anbe42]

I would drop $(DESTDIR) from all variable definitions, add it to the install targets instead and deliberately break everyone relying on the old broken behavior. Since nobody filed a bug about it, nobody seems to use it.

[evelikov]

Are you suggesting the first option - silent breakage? Debian does not set KCONF and friends, but others like Arch and Fedora do.

[AndrewAmmerlaan]

Are you suggesting the first option - silent breakage? Debian does not set KCONF and friends, but others like Arch and Fedora do.

Then don't make it silent, keep it simple and go for option 1 but communicate this change in CHANGELOG.md and send an email to all downstream maintainers (you can find a list on repology.org). It should be trivial to accommodate this change in downstream install specifications or scripts.

[evelikov]

Then don't make it silent

Any concrete suggestions how to do that apart from the CHANGELOG? People (maintainers and others) rarely read the announcements :cry:

[AndrewAmmerlaan]

Then don't make it silent

Any concrete suggestions how to do that apart from the CHANGELOG? People (maintainers and others) rarely read the announcements 😢

But they do read their email. This I think is the best way of reaching downstream maintainers to announce potentially breaking changes.

[anbe42]

untested, this could be used to throw an error:

ifneq (,$(DESTDIR))
$(if $(filter $(DESTDIR)%,$(VAR)),$(error DESTDIR is a prefix of VAR))
endif

[evelikov]

On a quick test it works like a charm, thank you.

If anyone wants to beat me to it an open a PR (even if it just removes the DESTDIR embedding) that would be appreciated. Alternatively I will try to get to it late this/early next week. That alone and the fixup from Andrew will be enough for a release IMHO.