On Mon, Jul 11, 2022 at 11:17:55AM +0000, Klemens Nanni wrote:
> GH_* ports obviously set MASTER_SITES to MASTER_SITES_GITHUB which
> defaults github.com/${account}/${project}/archive/${commit-or-tag}.
> 
> If additional patches need to be fetched, e.g. pending PRs to fix the
> local port, additional MASTER_SITES0-9 must be defined which always
> duplicate the GH_* values:
> 
>       MASTER_SITES0 = https://github.com/account/project/pull/
>       PATCHFILES +=   many_fixes-{}number.patch:0
> or
>       MASTER_SITES0 = https://github.com/account/project/commit/
>       PATCHFILES +=   one_fix-{}id.patch:0
> or
>       MASTER_SITES0 = https://github.com/account/project/
>       PATCHFILES +=   many_fixes-{pull/}number.patch:0
>       PATCHFILES +=   one_fix-{commit/}id.patch:0
> 
> Not super important but a bit annoying, the automatically generated
> MASTER_SITES_GITHUB can't really be reused (without dirty make fiddling)
> so MASTER_SITES0 is always a duplicate and PATCHFILES need :0 to work.
> 
> 
> But all we need to make this reusable is move the "archive/" part from
> MASTER_SITES_GITHUB to GH_DISTFILE, i.e. internally do
> 
>       MASTER_SITES =  https://github.com/account/project/
>       DISTNAME =      account-project-{archive/}commit_or_tag.tar.gz
> instead of
>       MASTER_SITES =  https://github.com/account/project/archive/
>       DISTNAME =      account-project-{}commit_or_tag.tar.gz
> 
> since this way the aforementioned patch fetching can be simplified to
> 
>       PATCHFILES +=   many_fixes-{pull/}number.patch
> or
>       PATCHFILES +=   one_fix-{commit/}id.patch
> 
> without additional MASTER_SITES0-9.
> 
> It's a rather unimportant detail but juggling with WIP ports, own PRs,
> fixes, etc. gets a little easier with this since the Makefile stays a
> bit smaller.
> 
> For testing, I have fetched several ports, both using GH_COMMIT and
> GH_TAGNAME, without any failures or distinfo changes.
> 
> As for the logic in bsd.port.mk itself, this also lifts the quirky
> add-trailing-slash-to-MASTER_SITES_GITHUB-only-if-GH_TAGNAME-is-set
> since it now ends with a fixed string and the additional URL paths only
> come in where GH_TAGNAME or whatever follows is definitely set.

Here's the logically identical diff rebased on top of the two cleanups.

If we go this route, I'd like to commit this first and then proceed with
sthen's suggestion of
        MASTER_SITES_GITHUB = https://github.com/

in a separate diff, since
a) it's easier to review
b) I still miss a bit for to arrive at sthen's version


This diff also uses a new _GH_PATH helper which definitely helps
readability and also simplifies the follow-up diff.

Feedback? Objection? OK?


Index: bsd.port.mk
===================================================================
RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
retrieving revision 1.1578
diff -u -p -r1.1578 bsd.port.mk
--- bsd.port.mk 13 Jul 2022 12:54:26 -0000      1.1578
+++ bsd.port.mk 13 Jul 2022 13:06:49 -0000
@@ -613,8 +613,9 @@ GH_PROJECT ?=
 
 .if !empty(GH_PROJECT) && !empty(GH_TAGNAME)
 _GH_TAG_DIST = ${GH_TAGNAME:C/^(v|V|ver|[Rr]el|[Rr]elease)[-._]?([0-9])/\2/}
+_GH_PATH = archive/
 DISTNAME ?=   ${GH_PROJECT}-${_GH_TAG_DIST}
-GH_DISTFILE = ${GH_PROJECT}-${_GH_TAG_DIST}${EXTRACT_SUFX}
+GH_DISTFILE = ${GH_PROJECT}-{${_GH_PATH}}${_GH_TAG_DIST}${EXTRACT_SUFX}
 .endif
 
 PKGNAME ?= ${DISTNAME}
@@ -1262,7 +1263,7 @@ ERRORS += "Fatal: specifying both GH_TAG
 ERRORS += "Fatal: using master as GH_TAGNAME is invalid"
 .  endif
 MASTER_SITES_GITHUB += \
-       
https://github.com/${GH_ACCOUNT}/${GH_PROJECT}/archive/${GH_TAGNAME:S/$/\//}
+       https://github.com/${GH_ACCOUNT}/${GH_PROJECT}/
 
 MASTER_SITES ?= ${MASTER_SITES_GITHUB}
 HOMEPAGE ?= https://github.com/${GH_ACCOUNT}/${GH_PROJECT}
@@ -1291,7 +1292,8 @@ _warn_checksum += ;echo ">>> MASTER_SITE
 EXTRACT_SUFX ?= .tar.gz
 
 .if !empty(GH_COMMIT)
-GH_DISTFILE = 
${DISTNAME}-${GH_COMMIT:C/(........).*/\1/}{${GH_COMMIT}}${EXTRACT_SUFX}
+_GH_PATH = archive/${GH_COMMIT}
+GH_DISTFILE = 
${DISTNAME}-${GH_COMMIT:C/(........).*/\1/}{${_GH_PATH}}${EXTRACT_SUFX}
 DISTFILES ?= ${GH_DISTFILE}
 .elif defined(DISTNAME)
 DISTFILES ?= ${DISTNAME}${EXTRACT_SUFX}

Reply via email to