Bug 220361

Summary: textproc/ripgrep: Add Bash, Fish and Zsh completions
Product: Ports & Packages Reporter: Petteri Valkonen <petteri.valkonen>
Component: Individual Port(s)Assignee: Tobias Kortkamp <tobik>
Status: Closed FIXED    
Severity: Affects Only Me CC: tobik
Priority: --- Keywords: patch
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D11509
Attachments:
Description Flags
Patch for adding options for installing Bash, Fish and Zsh completions
none
Revised patch for adding options for installing Bash, Fish and Zsh completions
none
Revised patch for adding options for installing Bash, Fish and Zsh completions koobs: maintainer-approval+

Description Petteri Valkonen 2017-06-29 18:44:14 UTC
Created attachment 183921 [details]
Patch for adding options for installing Bash, Fish and Zsh completions

Add options for installing Bash, Fish and Zsh completions.
Comment 1 Tobias Kortkamp freebsd_committer freebsd_triage 2017-07-05 16:13:57 UTC
Yes, completion files would be great :)

A couple of nitpicks below.

I added the Unlicense to bsd.licenses.db.mk in ports r445082 which means
your patch does not currently apply anymore.

The license block can now be simplified to:

LICENSE=	MIT UNLICENSE
LICENSE_COMB=	dual
LICENSE_FILE=	${WRKSRC}/COPYING
LICENSE_FILE_MIT=	${WRKSRC}/LICENSE-MIT
LICENSE_FILE_UNLICENSE=	${WRKSRC}/UNLICENSE

+.include <bsd.port.options.mk>

Please remove this again.  It's not needed if you replace all `.if
${PORT_OPTIONS:MXXX}` with extra targets like post-install-XXX-on.
See [1].

${WRKDIR}/target/release/build/ripgrep-*/out should be changed to
${CARGO_TARGET_DIR}/*/build/ripgrep-*/out since it might not be
'release' but 'debug' when you compile the port WITH_DEBUG=yes.

Is there maybe a way to coerce the build to output the completion
files to another directory?

[1] https://www.freebsd.org/doc/en/books/porters-handbook/makefile-options.html#options-targets
Comment 2 Petteri Valkonen 2017-07-06 16:11:22 UTC
Created attachment 184134 [details]
Revised patch for adding options for installing Bash, Fish and Zsh completions

Attached is a revised patch which addresses your review comments (thanks!):

- the license block has been simplified
- the PORT_OPTION conditionals are replaced with post-install-XXX-on targets
- Cargo has been coerced to output the completion files to ${WRKDIR}/cargo-out

Cargo sets the OUT_DIR environment variable [1], and the build script (build.rs) generates the completion files in the directory specified as its value. I couldn't find a clean way to override it, so I resorted to overwriting it during do-patch.

[1] https://github.com/rust-lang/cargo/blob/master/src/doc/environment-variables.md
Comment 3 Tobias Kortkamp freebsd_committer freebsd_triage 2017-07-06 16:53:12 UTC
(In reply to petteri.valkonen from comment #2)
Great, thanks! LGTM.

But I can't help and wonder if we can't drop the patch in files/ and
instead set an environment variable that Cargo doesn't set itself.
Something like this should work I think (untested):

RIPGREP_OUTDIR=	${WRKDIR}/cargo-out
CARGO_ENV=	RIPGREP_OUTDIR=${RIPGREP_OUTDIR}

post-patch:
	@${REINPLACE_CMD} 's|OUT_DIR|RIPGREP_OUTDIR|' ${WRKSRC}/build.rs
Comment 4 Petteri Valkonen 2017-07-06 18:46:18 UTC
Created attachment 184138 [details]
Revised patch for adding options for installing Bash, Fish and Zsh completions

Yes, setting a new environment variable works like a charm, and simplifies the patching process quite nicely.

A new, and hopefully final, patch is attached.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-07 03:52:09 UTC
Clean up summary
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-07-08 11:28:26 UTC
A commit references this bug:

Author: tobik
Date: Sat Jul  8 11:27:55 UTC 2017
New revision: 445321
URL: https://svnweb.freebsd.org/changeset/ports/445321

Log:
  Add options for installing Bash, Fish and Zsh completions.

  - Fix license block

  PR:		220361
  Submitted by:	petteri.valkonen@iki.fi (maintainer)
  Approved by:	lme (mentor)
  Differential Revision:	https://reviews.freebsd.org/D11509

Changes:
  head/textproc/ripgrep/Makefile