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
