Bug 204772 - Added GH_TUPLE that allows to add many GitHub projects in a more succinct form
Summary: Added GH_TUPLE that allows to add many GitHub projects in a more succinct form
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mathieu Arnold
URL: https://reviews.freebsd.org/D4514
Keywords: dogfood, feature, needs-qa, patch
Depends on:
Blocks: 204932 205053 204578 204906 204907 205282 205699
  Show dependency treegraph
 
Reported: 2015-11-24 00:55 UTC by Yuri Victorovich
Modified: 2016-03-01 20:23 UTC (History)
2 users (show)

See Also:
koobs: exp-run?


Attachments
patch (1.15 KB, patch)
2015-11-24 00:55 UTC, Yuri Victorovich
no flags Details | Diff
patch (1.09 KB, patch)
2015-11-29 09:33 UTC, Yuri Victorovich
no flags Details | Diff
patch (2.21 KB, patch)
2015-12-04 22:40 UTC, Yuri Victorovich
no flags Details | Diff
patch (2.22 KB, patch)
2015-12-11 20:11 UTC, Yuri Victorovich
no flags Details | Diff
patch with .for cycle and fix for extra-elements (2.26 KB, patch)
2015-12-12 00:31 UTC, Yuri Victorovich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2015-11-24 00:55:44 UTC
Created attachment 163461 [details]
patch

It allows to replace this

> GH_ACCOUNT=    feross              jhiesey:mp   jonschlinkert:ca
> GH_PROJECT=    webtorrent          mp4box.js:mp center-align:ca
> GH_TAGNAME=    ${DISTVERSIONFULL}  7630c6b:mp   5c5fab5:ca

with this

> GH_PROJECTS= faross:webtorrent:${DISTVERSIONFULL} jhiesey:mp4box.js:7630c6b:mp jonschlinkert:center-align:5c5fab5:ca

NodeJS projects tend to have a lot of GitHub dependencies, and it is very good to be able to write them in as brief form as possible.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-24 07:03:19 UTC
This will need:

* Documentation updates (in the respective .mk file) for usage
* A review (on phabricator) is highly recommended
* An exp-run testing for regressions
Comment 2 Jan Beich freebsd_committer freebsd_triage 2015-11-25 03:33:37 UTC
Comment on attachment 163461 [details]
patch

>+# GH_PROJECTS   - column-separated account, project, tagname and id

s/column/colon/ and plural names may attract typos. How about the following?

  # GH_TUPLE      - above shortened to account:project:tagname[:group]

>+.  if defined(GH_PROJECTS)
>+GH_ACCOUNT+=	${GH_PROJECTS:@ENTRY@${ENTRY:C,^([^:]*):([^:]*):([^:]*)((:[^:]*)?),\1\4,}@}
>+GH_PROJECT+=	${GH_PROJECTS:@ENTRY@${ENTRY:C,^([^:]*):([^:]*):([^:]*)((:[^:]*)?),\2\4,}@}
>+GH_TAGNAME+=	${GH_PROJECTS:@ENTRY@${ENTRY:C,^([^:]*):([^:]*):([^:]*)((:[^:]*)?),\3\4,}@}
>+.  endif

:@ loops are neither supported by fmake (on 9.x systems) nor required here.

  GH_ACCOUNT+=	${GH_PROJECTS:C@^([^:]*):([^:]*):([^:]*)((:[^:]*)?)@\1\4@}
  GH_PROJECT+=	${GH_PROJECTS:C@^([^:]*):([^:]*):([^:]*)((:[^:]*)?)@\2\4@}
  GH_TAGNAME+=	${GH_PROJECTS:C@^([^:]*):([^:]*):([^:]*)((:[^:]*)?)@\3\4@}
Comment 3 Yuri Victorovich freebsd_committer freebsd_triage 2015-11-29 09:33:15 UTC
Created attachment 163645 [details]
patch

Jan,
Thanks for your suggestions.
Comment 4 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-04 22:40:04 UTC
Created attachment 163857 [details]
patch

Added two useful targets related to GH_TUPLE:
gh-get-latest-revisions - finds the latest revisions of every project listed in GH_TUPLE
gh-get-changed-revisions - finds all projects which revisions changed on GitHub

These are useful for ports that have a lot of GitHub projects as distfiles, which update in a rolling release fashion.
Comment 5 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-11 10:32:39 UTC
You still can't use the @var@ loop thing.
Comment 6 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-11 10:40:21 UTC
Ok, @var@ loop thing is only for the developer helper targets. They can be left out for now. The main thing is GH_TUPLE.
Comment 7 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-11 11:14:13 UTC
Added a review for a working version.
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-11 20:11:18 UTC
Created attachment 164127 [details]
patch

Fixed the bug: need to allow subsequent elements like this :xxx[:yyy[...]] for convenience of other uses.
Comment 9 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-12 00:31:34 UTC
Created attachment 164140 [details]
patch with .for cycle and fix for extra-elements

So far I couldn't make it work with _ACCOUNT/_PROJECT variables. So posting the working patch with .for cycle for now.
Comment 10 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-12 00:40:20 UTC
When I add
> _GROUP:=${_TUPLE_TMP:C@^([^:]*):([^:]*):([^:]*)((:[^:]*)?).*@\4@:C@:@@:C@-@_@g}
and use ${_GROUP}, every group evaluates to the same, last value in cycle.

I don't know if there is any workaround for this.
Comment 11 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-13 12:13:43 UTC
Here is the short testcase:

TT+=lx:x
TT+=cx:y

.for T in ${TT}
X:=${T:C@:.*@@}
G+=X${X:C@x@@}
.endfor
.error "G=${G}"

It evaluates to Xc Xc instead of Xl Xc

I am not sure if it is practical to attempt to find a solution in this case.

Because it otherwise works fine, just nor very human-readable.
Comment 12 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-13 12:33:05 UTC
You are using the := at the wrong place, it should read:


X=${T:C@:.*@@}
G:=${G} X${X:C@x@@}
Comment 13 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-13 12:53:45 UTC
yes, I get the same now, and got this before too. It looks like https://go.googlesource.com produces the different file for the same URL after a while.

Temporary workaround: rm /usr/ports/distfiles/* && make makesum && make
Comment 14 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-13 12:56:44 UTC
Sorry, wrong thread.
Comment 15 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-15 21:13:02 UTC
When Makefile has this:
X:=${X} $$ABC
X:=${X} $$ABC

$ make -VX
 BC BC

Do you know how to print the dollar sign without it being expanded?
Comment 16 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-16 01:00:50 UTC
It's why I rewrote the whole thing to not use make.
Comment 17 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-16 01:15:46 UTC
You mean you rewrote the whole ports system?
Comment 18 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-16 04:45:26 UTC
If you are saying that that this is impossible to represent the dollar sign like that, this needs to be just fixed.
Comment 19 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-16 14:10:31 UTC
What does need to be fixed ?

Say you have:

$ cat foo
.for a in bar baz
FOO:= ${FOO} $$${a}
.warning ${FOO}
.endfor
$ make -f foo
make: "/home/mat/work/freebsd/ports/tata" line 3: warning:  ar
make: "/home/mat/work/freebsd/ports/tata" line 3: warning:  ar az

You're trying to coerce make(1)'s loop mechanism into generating shell code, it can't work easily.  It's much simpler to do all that in shell, which is what I did.
Comment 20 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-16 14:33:54 UTC
I see, I didn't notice, no notifications from phabricator.

I will also add the ability to set them based on options:
SOMEOPT_GH_TUPLE+= ...
Comment 21 Mathieu Arnold freebsd_committer freebsd_triage 2015-12-16 15:11:04 UTC
Please, stop updating the patch here, I'm not looking at it any more, put comments on the code review.

Also, you don't need OPT_GH_TUPLE, you can use OPT_VARS= GH_TUPLE+=foo:bar:baz
Comment 22 Yuri Victorovich freebsd_committer freebsd_triage 2015-12-29 11:58:29 UTC
The latest version is here: https://reviews.freebsd.org/D4603
Comment 23 Yuri Victorovich freebsd_committer freebsd_triage 2016-03-01 17:15:23 UTC
This can be committed without gh-* targets (GH_TUPLE itself). There were some questions asked, ex "why the hashtags are clipped at 7 characters?" that need to be resolved.
Comment 24 commit-hook freebsd_committer freebsd_triage 2016-03-01 20:23:37 UTC
A commit references this bug:

Author: mat
Date: Tue Mar  1 20:22:46 UTC 2016
New revision: 409898
URL: https://svnweb.freebsd.org/changeset/ports/409898

Log:
  Introduce GH_TUPLE.

  GH_TUPLE allows one to put all the GH_{ACCOUNT,PROJECT,TAGNAME} into one
  variable, in the form of account:project:tagname[:group].  It is helpful
  when there are many submodules.

  PR:		204772
  With hat:	portmgr
  Sponsored by:	Absolight
  Differential Revision:	https://reviews.freebsd.org/D4514

Changes:
  head/CHANGES
  head/Mk/bsd.sites.mk
Comment 25 commit-hook freebsd_committer freebsd_triage 2016-03-01 20:23:39 UTC
A commit references this bug:

Author: mat
Date: Tue Mar  1 20:23:18 UTC 2016
New revision: 48308
URL: https://svnweb.freebsd.org/changeset/doc/48308

Log:
  Add a bit of documentation about the upcoming GH_TUPLE.

  PR:		204772
  Reviewed by:	wblock
  Sponsored by:	Absolight
  Differential Revision:	https://reviews.freebsd.org/D5509

Changes:
  head/en_US.ISO8859-1/books/porters-handbook/makefiles/chapter.xml