On Sun, Feb 18, 2018 at 10:30:29PM +0100, Klemens Nanni wrote:
> On Sun, Feb 18, 2018 at 07:27:36PM +0100, Sebastien Marie wrote:
> > On Sun, Feb 18, 2018 at 12:21:43PM +0100, Klemens Nanni wrote:
> > > MODULES = devel/cargo
> > > +BUILD_DEPENDS = lang/rust>=1.20 \
> > > + textproc/asciidoc
> >
> > lang/rust isn't necessary as it is automatically added in BUILD_DEPENDS
> > when you have devel/cargo in MODULES.
> I'm aware of that. >=1.20 is important here as it specifies the minimal
> version required to compile ripgrep.
>
> > > +MAKE_ENV = RUSTFLAGS="-C debuginfo=0"
> > [...]
> > > Index: patches/patch-Cargo_toml
> > > ===================================================================
> > > RCS file: /cvs/ports/textproc/ripgrep/patches/patch-Cargo_toml,v
> > > retrieving revision 1.3
> > > diff -u -p -r1.3 patch-Cargo_toml
> > > --- patches/patch-Cargo_toml 1 Nov 2017 20:27:22 -0000 1.3
> > > +++ patches/patch-Cargo_toml 18 Feb 2018 11:18:35 -0000
> > > @@ -3,7 +3,7 @@ Don't use debug on release.
> > > Index: Cargo.toml
> > > --- Cargo.toml.orig
> > > +++ Cargo.toml
> > > -@@ -60,4 +60,4 @@ simd-accel = [
> > > +@@ -75,4 +75,4 @@ simd-accel = [
> > > ]
> > >
> > > [profile.release]
> >
> > The patch hasn't been removed. And after testing, passing RUSTFLAGS in
> > MAKE_ENV is ineffective.
> My bad, will be more careful next time. Obviously this would still build
> without debug symbols giving a false positive here.
>
> > Your RUSTFLAGS setting is overrided by cargo.port.mk. But the module was
> > designed in a way that RUSTFLAGS is a user parameter (not sure it was a
> > good thing: the purpose was to pass RUSTFLAGS from command-line to test
> > things). But as it, it shouldn't be set directly (module variable should
> > have MODxxx name). So some changes in cargo.port.mk would be necessary.
> >
> > The following diff takes care of that:
> > - use MODCARGO_RUSTFLAGS as port variable to set RUSTFLAGS in cargo
> > invocation
> >
> > - while here, correct (a bit) the license detection (bug reported by
> > kpcyrd <kpcyrd AT rxv.cc> some time ago).
> >
> >
> > With it, adding
> >
> > MODCARGO_RUSTFLAGS += -C debuginfo=0
> >
> > to ripgrep's Makefile, makes RUSTFLAGS variable correctly setted and
> > passed to cargo.
> Sounds good to me, thanks for the work.
>
> > Please note, I am still unsure if strip(1) is need in post-install.
> Stripping results half-sized binaries here.
>
> As everything except the binary itself gets installed from post-install,
> we can simply turn it into do-install, as this will result in using
> INSTALL_PROGRAM which strips binaries by default.
>
> > Index: cargo.port.mk
> > ===================================================================
> > RCS file: /cvs/ports/devel/cargo/cargo.port.mk,v
> > retrieving revision 1.6
> > diff -u -p -r1.6 cargo.port.mk
> > --- cargo.port.mk 31 Jul 2017 13:16:08 -0000 1.6
> > +++ cargo.port.mk 18 Feb 2018 18:03:35 -0000
> > @@ -13,6 +13,9 @@ MODCARGO_FEATURES ?=
> > # Used to override a dependency with newer version.
> > MODCARGO_CRATES_UPDATE ?=
> >
> > +# RUSTFLAGS variable to pass to cargo.
> > +MODCARGO_RUSTFLAGS ?=
> > +
> > # Name of the local directory for vendoring crates.
> > MODCARGO_VENDOR_DIR ?= ${WRKSRC}/modcargo-crates
> >
> > @@ -126,7 +129,7 @@ MODCARGO_ENV += \
> > CARGO_TARGET_DIR=${MODCARGO_TARGET_DIR} \
> > RUSTC=${LOCALBASE}/bin/rustc \
> > RUSTDOC=${LOCALBASE}/bin/rustdoc \
> > - RUSTFLAGS="${RUSTFLAGS}"
> > + RUSTFLAGS="${MODCARGO_RUSTFLAGS}"
> >
> > # Helper to shorten cargo calls.
> > MODCARGO_CARGO_RUN = \
> > @@ -211,7 +214,7 @@ modcargo-gen-crates: extract
> > # modcargo-gen-crates-licenses will try to grab license information from
> > downloaded crates.
> > modcargo-gen-crates-licenses: configure
> > @find ${WRKSRC}/modcargo-crates -name 'Cargo.toml' -maxdepth 2 \
> > - -exec grep -H '^license' {} \; \
> > + -exec grep -H '^[ ]*license' {} \; \
> > | sed \
> > -e 's|^${WRKSRC}/modcargo-crates/|MODCARGO_CRATES += |' \
> > -e 's|/Cargo.toml:license.*= *"| # |' \
> We can use a proper variable here, pass multiple files to grep and
> simplify the first sed expression by cd'ing into the directory first.
>
> Updated diffs below for cargo.port.mk and ripgrep in that order. I
> slightly reordered the Makefile according to template.
>
> More feedback?
I completely missed the cargo.port.mk diff, so here it is.
ripgrep 0.8.1 came out four days ago, I have an updated diff for that as
well but would like to settle the RUSTFLAGS bits first.
Feedback? OK?
Index: cargo.port.mk
===================================================================
RCS file: /cvs/ports/devel/cargo/cargo.port.mk,v
retrieving revision 1.6
diff -u -p -r1.6 cargo.port.mk
--- cargo.port.mk 31 Jul 2017 13:16:08 -0000 1.6
+++ cargo.port.mk 24 Feb 2018 15:09:01 -0000
@@ -13,6 +13,9 @@ MODCARGO_FEATURES ?=
# Used to override a dependency with newer version.
MODCARGO_CRATES_UPDATE ?=
+# RUSTFLAGS variable to pass to cargo.
+MODCARGO_RUSTFLAGS ?=
+
# Name of the local directory for vendoring crates.
MODCARGO_VENDOR_DIR ?= ${WRKSRC}/modcargo-crates
@@ -126,7 +129,7 @@ MODCARGO_ENV += \
CARGO_TARGET_DIR=${MODCARGO_TARGET_DIR} \
RUSTC=${LOCALBASE}/bin/rustc \
RUSTDOC=${LOCALBASE}/bin/rustdoc \
- RUSTFLAGS="${RUSTFLAGS}"
+ RUSTFLAGS="${MODCARGO_RUSTFLAGS}"
# Helper to shorten cargo calls.
MODCARGO_CARGO_RUN = \
@@ -210,10 +213,10 @@ modcargo-gen-crates: extract
# modcargo-gen-crates-licenses will try to grab license information from
downloaded crates.
modcargo-gen-crates-licenses: configure
- @find ${WRKSRC}/modcargo-crates -name 'Cargo.toml' -maxdepth 2 \
- -exec grep -H '^license' {} \; \
+ @cd ${MODCARGO_VENDOR_DIR} && find . -maxdepth 2 -name Cargo.toml \
+ -exec grep -H '^[ ]*license' {} + \
| sed \
- -e 's|^${WRKSRC}/modcargo-crates/|MODCARGO_CRATES += |' \
+ -e 's|^\./|MODCARGO_CRATES += |' \
-e 's|/Cargo.toml:license.*= *"| # |' \
-e 's|"$$||g'
