makefile improvements #60
Conversation
| MKDIR ?= mkdir | ||
| prefix ?= /usr/local | ||
| bindir ?= $(prefix)/bin | ||
| sysconfdir ?= $(prefix)/etc |
There was a problem hiding this comment.
Is this standard? I do not see it in the FHS list.
There was a problem hiding this comment.
Honestly I just copied it from Jon who usually tracks these conventions closely: https://github.com/NVIDIA/enroot/blob/v3.4.0/Makefile#L5
But yes, in this case it does look like it's standard:
‘sysconfdir’
The directory for installing read-only data files that pertain to a single machine–that is to say, files for configuring a host. ...
https://www.gnu.org/software/make/manual/html_node/Directory-Variables.html
There was a problem hiding this comment.
I suppose since we're looking at that file I should point out that files like this should really be under /usr/share/ rather than /etc/ (see the definition of 'datarootdir'), but I don't personally care to make that breaking change today.
There was a problem hiding this comment.
The problem with that breaking change is that there are many customers who mount default-counters.csv into /etc/dcgm-exporter/ location via docker -v command-line argument. Such change will break their use cases as they will have to either change where the file is mounted inside the container or change the dcgm-exporter command-line arguments. I would consider both scenarios as a breaking change that needs a major version release at least.
There was a problem hiding this comment.
Right, yes, I agree. But we're choosing not to make the breaking change, right? I was just making a pedantic point. I'm not arguing that we make that change now or even ever.
e.g. package maintainers may choose to use this to build position indepent code with `make GOFLAGS=-buildmode=pie binary`
There are no dependencies on the 'binary' target, so it will always recompile. The README suggests running 'make' and then 'sudo make install', but since the install target depends on the binary target, this causes the build to be redone as root, which is both a waste of compute time and a nuisance since it pollutes the source tree with files owned by root. A better fix for this would be to fill in the appropriate dependencies for the binary target. This change is the quick+easy fix.
lukeyeager
force-pushed
the
build-improvements
branch
from
cb4f876
to
c6a36b4
Compare
|
The PR is stale because there hasn't been any activity for the last two years. |
NOTE: this makes two breaking changes to the 'install' target:
sudo make prefix=/usr sysconfdir=/etc install.make install. This could be fixed with additional work.