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: |
|
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 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 (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 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.
Clean up summary 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 |
Created attachment 183921 [details] Patch for adding options for installing Bash, Fish and Zsh completions Add options for installing Bash, Fish and Zsh completions.