Fixes for GNU style Makefile #72
Conversation
mattiasb
force-pushed
the
feature/makefile-gnu
branch
from
d4005ad to
1d6900a
Compare
If sbindir is configurable, the service file needs to be generated as well.
Specifically ignore the generated `wondershaper.service`.
mattiasb
force-pushed
the
feature/makefile-gnu
branch
from
1d6900a to
ba5b802
Compare
|
Any news here? It's a bit sad that I didn't catch this issue immediately when I posted my previous MR. But the current state is that my MR was buggy. This MR fixes it. But only the previous MR was merged. :/ |
There was a problem hiding this comment.
Thanks for the additional code. I understand the issue, but I think it can be solved a bit simpler than your proposal. If you can either explain or revise, I'd be happy to merge (on short notice).
As discussed, I don't use wondershaper much myself anymore. So it's just random pings and notifications that alert me to the project. Sorry for any delays.
| @@ -0,0 +1,2 @@ | |||
| *~ | |||
| /wondershaper.service | |||
There was a problem hiding this comment.
Why is this added to the gitignore
There was a problem hiding this comment.
*~ is Emacs backup files. They should arguably not go in this MR (Emacs users should just add *~ (and #*) to their global ignores. Will add a fixup for this.
There was a problem hiding this comment.
/wondershaper.service is ignored since it is generated below (and hence we'd like to avoid committing it by accident).
| install: | ||
|
|
||
| wondershaper.service: wondershaper.service.in | ||
| sed 's|@sbindir@|$(sbindir)|' wondershaper.service.in > wondershaper.service |
There was a problem hiding this comment.
This seems pretty convoluted. I understand you're trying to be flexible, but wouldn't anyone who wants a different directory be able to edit both the makefile and the services file.
Can you point to any convention/precedent of this procedure with the .in and the usage of sed within the make?
That was a fast merge! Thanks!
I found an issue that I didn't have time to fix before you merged the previous PR though. :P
If
$(sbindir)is configurable, the service file needs to be generated as well. So do that.Also update
README.mdwith the new paths.