Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Makefile): Make bash completion honor PREFIX #155

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Stop using pkg-config to get the bash completion dir
Per [the bash_completion documentation](https://github.com/scop/bash-completion/blob/main/README.md), this is the preferred style as of new versions. It's also generally much simpler and more consistent with other options, so let's see if we can just switch over.
  • Loading branch information
nelhage authored Jan 15, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit e59ca8bc2b391a0c1113fdee0498d75bcc598f57
8 changes: 3 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ test/victim: override LDFLAGS := $(VICTIM_LDFLAGS)
clean:
rm -f reptyr $(OBJS) test/victim.o test/victim $(DEPS)

BASHCOMPDIR ?= $(shell $(PKG_CONFIG) --variable=completionsdir bash-completion 2>/dev/null | sed -e 's,/usr,$(PREFIX),')
BASHCOMPDIR ?= $PREFIX/share/bash-completion/completions/

install: reptyr
install -d -m 755 $(DESTDIR)$(BINDIR)
@@ -50,10 +50,8 @@ install: reptyr
install -m 644 reptyr.1 $(DESTDIR)$(MANDIR)/man1/reptyr.1
install -d -m 755 $(DESTDIR)$(MANDIR)/fr/man1
install -m 644 reptyr.fr.1 $(DESTDIR)$(MANDIR)/fr/man1/reptyr.1
bashcompdir=$(BASHCOMPDIR) ; \
test -z "$$bashcompdir" && bashcompdir=/etc/bash_completion.d ; \
install -d -m 755 $(DESTDIR)$$bashcompdir ; \
install -m 644 reptyr.bash $(DESTDIR)$$bashcompdir/reptyr
install -d -m 755 $(DESTDIR)$(BASHCOMPDIR) ; \
install -m 644 reptyr.bash $(DESTDIR)$(BASHCOMPDIR)/reptyr
Comment on lines +53 to +54
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does $(DESTDIR) come from? I know that autoconf works with PREFIX and DESTDIR, but I didn't see an autoconf setup in your build system, just the Makefile. What happens if someone sets the DESTDIR to e.g. foobar? Does line 54 expand to install -m 644 reptyr.bash foobar/usr/local/share/bash-completions/reptyr? That seems a little odd to me.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make allows referencing undefined variables, and they'll just expand to an empty string. I just cargo-culted the existing pattern we have, which seems to match the convention recommended by GNU Make.


.PHONY: PHONY

Loading