Sunday, October 10, 2021

Re: devel/cargo: Support fetching crates as GitHub archive tarballs

Sebastien Marie <semarie@online.fr> 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.

No comments:

Post a Comment