Bug 220361 - textproc/ripgrep: Add Bash, Fish and Zsh completions
Summary: textproc/ripgrep: Add Bash, Fish and Zsh completions
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Tobias Kortkamp
URL: https://reviews.freebsd.org/D11509
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-06-29 18:44 UTC by Petteri Valkonen
Modified: 2017-07-08 11:29 UTC (History)
1 user (show)

See Also:


Attachments
Patch for adding options for installing Bash, Fish and Zsh completions (1.84 KB, patch)
2017-06-29 18:44 UTC, Petteri Valkonen
no flags Details | Diff
Revised patch for adding options for installing Bash, Fish and Zsh completions (2.53 KB, patch)
2017-07-06 16:11 UTC, Petteri Valkonen
no flags Details | Diff
Revised patch for adding options for installing Bash, Fish and Zsh completions (1.74 KB, patch)
2017-07-06 18:46 UTC, Petteri Valkonen
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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