On Sun, Oct 10, 2021 at 08:29:20PM +0200, Sebastien Marie wrote:
> On Sun, Oct 10, 2021 at 05:49:15PM +0000, Klemens Nanni wrote:
> > On Sun, Oct 10, 2021 at 02:59:06PM +0000, Klemens Nanni wrote:
> > > Crates can come from various sources, we only support the registry which
> > > is fine for most ports, but especially during development/porting it is
> > > required to overwrite crates with patched versions.
> > >
> > > Such crates often live as forks on GitHub and can be downloaded as usual
> > > so let's support this.
> > >
> > > I need this for an upcoming port (which is still in early development)
> > > using feature branches of certain crates -- I followed the same process
> > > in order to add OpenBSD support.
> > >
> > > Neither existing nor new users of the cargo module have to do anything;
> > > Cargo.lock `source' lines are parsed as before except that more types
> > > are recognised.
> > >
> > > Thanks to sthen for pointing at www/nginx wrt. MASTER_SITES handling.
> > >
> > > Feedback? OK?
>
> I am a bit reserved. Supporting git repository could make sense, but
> no real usage has been found for now.
Well, now I have one :-)
> if I correctly understand psst, the author forked several crates,
> proposed PR, and PR stalled due to portability issues:
> https://github.com/linebender/druid/pull/1701
Such is development, some things take time and people hack on this for
fun not profit.
> he released psst using forked crates (using git). but does psst author
> is fully maintaining forked crates ?
I can't speak for them.
> > Correct diff this time. I can successfully run all the modcargo-* with
> > this.
> >
> > As one of the comments states, there are git repositories which serve
> > multiple crates, i.e. distinfo can have
> >
> > SHA256 (cargo/druid-0.7.0.tar.gz) = S1kjeVt44CpL8eteZSfzCvFpRuAJI9KZJZdnyTeSkJ4=
> > SHA256 (cargo/druid-derive-0.4.0.tar.gz) = S1kjeVt44CpL8eteZSfzCvFpRuAJI9KZJZdnyTeSkJ4=
> > SHA256 (cargo/druid-shell-0.7.0.tar.gz) = S1kjeVt44CpL8eteZSfzCvFpRuAJI9KZJZdnyTeSkJ4=
>
> here I disagree: if we support random git repositories, it usually
> means it isn't a true "druid-0.7.0" version, but druid-0.7.0 + some
> patches. so it will conflict at some point when a true druid-0.7.0
> will be used: sha256 will not be the same.
>
> similary, if one port is using "druid-0.7.0 + patch1" and another
> "druid-0.7.0 + patch2", the sha256 will be different.
>
> port infrastructure will not work with distfiles like that.
>
> filename should embed a part of the git commit to disambiguate.
Right, DISTFILES must be unique, thus I added the commit ID in known
GH_COMMIT fashion.
> > and I therefore link to the extracted repo from modcargo-crates/ instead
> > of moving it in there multiple times.
> >
> > I'm not too familiar with the rust/cargo ecosystem; is that something
> > which only occurs once Cargo.toml dictates fetching crates from anything
> > other than a registry?
> >
> > In case it helps, I am porting https://github.com/jpochyla/psst for
> > which a WIP port requiring these devel/cargo changes can be found at
> > https://github.com/jasperla/openbsd-wip/tree/master/audio/psst .
>
> reading the port for audio/psst, it seems you used MODCARGO_CRATE_GIT
> for doing patching ? usually it is done in the port itself. we also
> support fetching external patchfiles with PATCHFILES.
Yes, but I've done this regardless of our port, that is to port psst in
the first place: `cargo build` from a psst checkout, handle failure,
checkout crates, fix things, add `[patch]` section to psst's Cargo.toml
to point at my fixed/ported crate... rinse and repeat.
I guess I could've done all this through regular ports patches or so,
but the work via forks (read: MODCARGO_CRATE_GIT) had to be done for
upstreaming stuff anyway, so I might as well use my fork as a starting
point for a "clean" OpenBSD port without local noise/patches.
The goal is still to get everything upstream and switch to a regular
release eventually; but in the meantime, being able to test psst via
ports/packages seems benefical and worth doing. I think supporting git
git crates in devel/cargo makes testing and distribution much easier.
> > Index: cargo.port.mk
> > ===================================================================
> > RCS file: /cvs/ports/devel/cargo/cargo.port.mk,v
> > retrieving revision 1.24
> > diff -u -p -r1.24 cargo.port.mk
> > --- cargo.port.mk 11 Sep 2021 07:13:13 -0000 1.24
> > +++ cargo.port.mk 10 Oct 2021 17:21:28 -0000
> > @@ -42,10 +42,14 @@ _MODCARGO_DIST_SUBDIR = ${MODCARGO_DIST_
> > .endif
> >
> > # Use MASTER_SITES9 to grab crates by default.
> > -# Could be changed by setting MODCARGO_MASTER_SITESN.
>
> any reason to remove it ?
It states the obvious and I didn't want to repeat it for _GITN.
> > MODCARGO_MASTER_SITESN ?= 9
> > MASTER_SITES${MODCARGO_MASTER_SITESN} ?= ${MASTER_SITES_CRATESIO}
> >
> > +# Use MASTER_SITES8 to grab crates as GitHub tarballs by default.
> > +MODCARGO_MASTER_SITES_GITN ?= 8
> > +# XXX this assumes that all git crates come from the same site.
> > +MASTER_SITES${MODCARGO_MASTER_SITES_GITN} ?= https://github.com/
> > +
>
> Please avoid XXX. just saying the limitation is fine.
Done.
> > # per crates options
> > MODCARGO_CRATES_SQLITE3_BUNDLED ?= No
> >
> > @@ -54,6 +58,13 @@ MODCARGO_CRATES_SQLITE3_BUNDLED ?= No
> > DISTFILES += ${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}.tar.gz{${_cratename}/${_cratever}/download}:${MODCARGO_MASTER_SITESN}
> > .endfor
> >
> > +.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
> > +# strip leading master site and turn link into downloadble path,
> > +# e.g. https://github.com/author/project#hash -> author/project/archive/hash
> > +MODCARGO_CRATE_PATH_${_cratename} = ${_crateurl:${MASTER_SITES${MODCARGO_MASTER_SITES_GITN}}%=:S/#/\/archive\//}
> > +DISTFILES += ${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}{${MODCARGO_CRATE_PATH_${_cratename}}}.tar.gz:${MODCARGO_MASTER_SITES_GITN}
> > +.endfor
> > +
>
> if a variable isn't expected to be changed by the port maintainer,
> please name it _MODCARGO_CRATE_PATH_${_cratename} (with _ at start of
> name). else if it might be changed by user, please use ?= to let the
> user to override the value in the Makefile of the port.
Done, `_VARIABLE =' is used as those are purely internal.
> > # post-extract target for preparing crates directory.
> > # It will put all crates in the local crates directory.
> > MODCARGO_post-extract = \
> > @@ -63,6 +74,15 @@ MODCARGO_post-extract = \
> > MODCARGO_post-extract += \
> > mv ${WRKDIR}/${_cratename}-${_cratever} ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
> > .endfor
> > +.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
> > +# strip author and turn path into directory name, e.g. author/project/archive/hash -> project-hash
> > +MODCARGO_CRATE_DIR_${_cratename} = ${MODCARGO_CRATE_PATH_${_cratename}:C/^[^\/]+\///:S/\/archive\//-/}
> > +# do not move a commit based directory as it may contain multiple crates, e.g.
> > +# druid-d815bc... contains druid-0.7.0, druid-derive-0.4.0 and druid-shell-0.7.0
> > +MODCARGO_post-extract += \
> > + ${ECHO_MSG} "[modcargo] ${MODCARGO_CRATE_DIR_${_cratename}} got linked to instead" ; \
> > + ln -s ${WRKDIR}/${MODCARGO_CRATE_DIR_${_cratename}} ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
> > +.endfor
>
> does it will work if a some project uses druid-0.7.0 and git fork of druid-0.7.0 ?
I don't *think* so and I'm not fluent enough in Rust/Cargo.toml to
easily test that.
But I also think this shouldn't be a hard blocker. The diff allows more
porting without breaking existing code and new ports actually running
into such multi-versioned dependencies can be used to improve the cargo
module when needed.
> > # post-extract target to provide clean environment for specific crates
> > # in order to avoid rebuilding libraries from source behind us.
> > @@ -179,6 +199,13 @@ MODCARGO_post-patch += \
> > ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
> > .endfor
> >
> > +.for _cratename _cratever _crateverUNUSED in ${MODCARGO_CRATES_GIT}
>
> it should be _crateurlUNUSED and not _crateverUNUSED
Done.
> > +MODCARGO_post-patch += \
> > + ${LOCALBASE}/bin/cargo-generate-vendor \
> > + ${FULLDISTDIR}/${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}.tar.gz \
> > + ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
> > +.endfor
> > +
> > # configure hook. Place a config file for overriding crates-io index by
> > # local source directory, and set compilation options (based on release).
> > # Enabled by use of "CONFIGURE_STYLE=cargo".
> > @@ -347,12 +374,19 @@ _modcargo-metadata:
> > @${MODCARGO_post-patch}
> >
> > # modcargo-gen-crates will output crates list from Cargo.lock file.
> > +#
> > +# XXX: for crate git tarballs, the number sign (#) must be replaced here,
> > +# otherwise make/sh trips and it disappears incl. the trailing hash.
>
> if # cause problem with shell, you could escape it with \#. and if you
> already cope with it, the comment could be retired.
I zapped XXX but left a comment and didn't escape it (for now).
Escaping it once made in work in one but not all places and this stuffs
makes my head hurt more than appreciated now -- we can still improve it
later on.
> > modcargo-gen-crates: extract
> > @echo '# run: make modcargo-gen-crates-licenses'
> > @awk ' /^name = / { n=$$3; gsub("\"", "", n); } \
> > /^version = / { v=$$3; gsub("\"", "", v); } \
> > /^source = "registry\+https:\/\/github.com\/rust-lang\/crates\.io-index"/ \
> > - { print "MODCARGO_CRATES += " n " " v; }' \
> > + { print "MODCARGO_CRATES += " n " " v; } \
> > + /^source = .git\+https:\/\/github.com./ \
>
> the pattern is a bit odd, maybe something like:
> /^source = "git\+https:\/\/github\.com/
>
> - the first '.' should be "
> - the '.' inside github.com should be escaped
> - I don't understand the last '.' , so I might be wrong to remove it
Agreed (mix of copied over and temporary development tweaks).
I've now also adjusted the registry regex for consistency.
> > + { s=$$3; gsub("\"", "", s); \
> > + sub("git\\+", "", s); sub("(\\?.*)?#", "/archive/", s); \
> > + print "MODCARGO_CRATES_GIT += " n " " v " " s; }' \
> > <${MODCARGO_CARGOTOML:toml=lock}
> >
> > # modcargo-gen-crates-licenses will try to grab license information from downloaded crates.
> > @@ -360,6 +394,11 @@ modcargo-gen-crates-licenses: configure
> > @echo '# $$Open''BSD$$'
> > .for _cratename _cratever in ${MODCARGO_CRATES}
> > @echo -n "MODCARGO_CRATES += ${_cratename} ${_cratever} # "
> > + @sed -ne '/^license.*=/{;s/^license.*= *"\([^"]*\)".*/\1/p;q;};$$s/^.*$$/XXX missing license/p' \
> > + ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever}/Cargo.toml
> > +.endfor
> > +.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
> > + @echo -n "MODCARGO_CRATES_GIT += ${_cratename} ${_cratever} ${_crateurl} # "
> > @sed -ne '/^license.*=/{;s/^license.*= *"\([^"]*\)".*/\1/p;q;};$$s/^.*$$/XXX missing license/p' \
> > ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever}/Cargo.toml
> > .endfor
>
> Thanks.
Sure!
Index: cargo.port.mk
===================================================================
RCS file: /cvs/ports/devel/cargo/cargo.port.mk,v
retrieving revision 1.24
diff -u -p -r1.24 cargo.port.mk
--- cargo.port.mk 11 Sep 2021 07:13:13 -0000 1.24
+++ cargo.port.mk 10 Oct 2021 20:02:03 -0000
@@ -42,10 +42,15 @@ _MODCARGO_DIST_SUBDIR = ${MODCARGO_DIST_
.endif
# Use MASTER_SITES9 to grab crates by default.
-# Could be changed by setting MODCARGO_MASTER_SITESN.
MODCARGO_MASTER_SITESN ?= 9
MASTER_SITES${MODCARGO_MASTER_SITESN} ?= ${MASTER_SITES_CRATESIO}
+# Use MASTER_SITES8 to grab crates as GitHub tarballs by default.
+MODCARGO_MASTER_SITES_GITN ?= 8
+# This assumes that all git crates come from the same site
+# or that all sites support the same URL/download API.
+MASTER_SITES${MODCARGO_MASTER_SITES_GITN} ?= https://github.com/
+
# per crates options
MODCARGO_CRATES_SQLITE3_BUNDLED ?= No
@@ -54,6 +59,14 @@ MODCARGO_CRATES_SQLITE3_BUNDLED ?= No
DISTFILES += ${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}.tar.gz{${_cratename}/${_cratever}/download}:${MODCARGO_MASTER_SITESN}
.endfor
+.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
+# strip leading master site, e.g. https://github.com/author/project/archive/hash -> author/project/archive/hash
+_MODCARGO_CRATE_PATH_${_cratename} = ${_crateurl:${MASTER_SITES${MODCARGO_MASTER_SITES_GITN}}%=}
+# keep hash in filename (from bsd.port.mk's GH_COMMIT code)
+_MODCARGO_CRATE_COMMIT_${_cratename} = ${_MODCARGO_CRATE_PATH_${_cratename}:T:C/(........).*/\1/}
+DISTFILES += ${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}-${_MODCARGO_CRATE_COMMIT_${_cratename}}{${_MODCARGO_CRATE_PATH_${_cratename}}}.tar.gz:${MODCARGO_MASTER_SITES_GITN}
+.endfor
+
# post-extract target for preparing crates directory.
# It will put all crates in the local crates directory.
MODCARGO_post-extract = \
@@ -63,6 +76,15 @@ MODCARGO_post-extract = \
MODCARGO_post-extract += \
mv ${WRKDIR}/${_cratename}-${_cratever} ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
.endfor
+.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
+# strip author and turn path into unique directory name, e.g. author/project/archive/hash -> project-hash
+_MODCARGO_CRATE_DIR_${_cratename} = ${_MODCARGO_CRATE_PATH_${_cratename}:C/^[^\/]+\///:S/\/archive\//-/}
+# do not move a commit based directory as it may contain multiple crates, e.g.
+# druid-d815bc... contains druid-0.7.0, druid-derive-0.4.0 and druid-shell-0.7.0
+MODCARGO_post-extract += \
+ ${ECHO_MSG} "[modcargo] ${_MODCARGO_CRATE_DIR_${_cratename}} got linked to instead" ; \
+ ln -s ${WRKDIR}/${_MODCARGO_CRATE_DIR_${_cratename}} ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
+.endfor
# post-extract target to provide clean environment for specific crates
# in order to avoid rebuilding libraries from source behind us.
@@ -179,6 +201,13 @@ MODCARGO_post-patch += \
${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
.endfor
+.for _cratename _cratever _crateurlUNUSED in ${MODCARGO_CRATES_GIT}
+MODCARGO_post-patch += \
+ ${LOCALBASE}/bin/cargo-generate-vendor \
+ ${FULLDISTDIR}/${_MODCARGO_DIST_SUBDIR}${_cratename}-${_cratever}.tar.gz \
+ ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever} ;
+.endfor
+
# configure hook. Place a config file for overriding crates-io index by
# local source directory, and set compilation options (based on release).
# Enabled by use of "CONFIGURE_STYLE=cargo".
@@ -347,12 +376,17 @@ _modcargo-metadata:
@${MODCARGO_post-patch}
# modcargo-gen-crates will output crates list from Cargo.lock file.
+# number signs (#) in MODCARGO_CRATES_GIT are problematic, replace here!
modcargo-gen-crates: extract
@echo '# run: make modcargo-gen-crates-licenses'
@awk ' /^name = / { n=$$3; gsub("\"", "", n); } \
/^version = / { v=$$3; gsub("\"", "", v); } \
- /^source = "registry\+https:\/\/github.com\/rust-lang\/crates\.io-index"/ \
- { print "MODCARGO_CRATES += " n " " v; }' \
+ /^source = "registry\+https:\/\/github\.com\/rust-lang\/crates\.io-index"$$/ \
+ { print "MODCARGO_CRATES += " n " " v; } \
+ /^source = "git\+https:\/\/github\.com\/.+"$$/ \
+ { s=$$3; gsub("\"", "", s); \
+ sub("git\\+", "", s); sub("(\\?.*)?#", "/archive/", s); \
+ print "MODCARGO_CRATES_GIT += " n " " v " " s; }' \
<${MODCARGO_CARGOTOML:toml=lock}
# modcargo-gen-crates-licenses will try to grab license information from downloaded crates.
@@ -360,6 +394,11 @@ modcargo-gen-crates-licenses: configure
@echo '# $$Open''BSD$$'
.for _cratename _cratever in ${MODCARGO_CRATES}
@echo -n "MODCARGO_CRATES += ${_cratename} ${_cratever} # "
+ @sed -ne '/^license.*=/{;s/^license.*= *"\([^"]*\)".*/\1/p;q;};$$s/^.*$$/XXX missing license/p' \
+ ${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever}/Cargo.toml
+.endfor
+.for _cratename _cratever _crateurl in ${MODCARGO_CRATES_GIT}
+ @echo -n "MODCARGO_CRATES_GIT += ${_cratename} ${_cratever} ${_crateurl} # "
@sed -ne '/^license.*=/{;s/^license.*= *"\([^"]*\)".*/\1/p;q;};$$s/^.*$$/XXX missing license/p' \
${MODCARGO_VENDOR_DIR}/${_cratename}-${_cratever}/Cargo.toml
.endfor
No comments:
Post a Comment