Sebastien Marie <[email protected]> writes:
> 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. I have run into two or three apps that have (may be had at this point :P - it was a while ago) git repos in the Cargo.toml stuff. I basically didn't port them because there wasn't a clear cut way to do exactly what this stuff solves. IMO it's worth having. > > 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 > > he released psst using forked crates (using git). but does psst author > is fully maintaining forked crates ? > > >> 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. > >> 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. > > >> 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 ? > >> 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. > >> # 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. > >> # 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 ? > >> >> # 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 > >> +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. > >> 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 > >> + { 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.
