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.

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.
-- 
Sebastien Marie

Reply via email to