On Sun, Apr 5, 2015, at 01:31 PM, Stuart Henderson wrote: > On 2015-04-04, Landry Breuil <lan...@rhaalovely.net> wrote: > > On Sat, Apr 04, 2015 at 11:07:11PM +0200, Adam Wolk wrote: > >> Hi tech@ > >> > >> I'm the maintainer of www/otter-browser and I got caught while packaging > >> otter-browser 0.9.04. Upstream asked us to point at a different commit > >> then the tagged revision so we did: > >> > >> GH_TAGNAME = v0.9.04 > >> # This is the actual tagged revision > >> # GH_COMMIT = 869d29d19719b3057e137a79d4a10025d2c920f6 > >> # but we were asked by upstream to release from the following commit > >> # as it's considered an important fix affecting the majority of users > >> GH_COMMIT = 23d7ee6f9cd636e750687a01975b177c1c9c2e53 > >> > >> This port was reviewed with an ok by two people and underwent further > >> changes later on. > >> > >> I didn't notice that the port actually packaged GH_TAGNAME contents > >> instead of GH_COMMIT. > > > > GH_COMMIT is meaningless in terms of "package version", which expects a > > correctly structured version, hence GH_TAGNAME being usually used in > > combination with GH_PROJECT. > > Pretty much all ports with GH_TAGNAME are also setting GH_COMMIT, but > GH_COMMIT doesn't do anything there. I think we were hoping it would > fetch a specific commitid in case the tag was slided, but it doesn't > look like github supports that. >
I can confirm this. That's how I got burned with otter. Ports 'tell you' that they are downloading from that 'tag' pluus the GH_COMMIT you set making you think it's actually downloading the proper changeset. In reality, github does redirects behind the scene and points at the tagged revision. > I think we should remove the existing ones and make it an error to > specify both GH_TAGNAME and GH_COMMIT. Thoughts? If people think this > is a good idea I'll do the ports mop-up. > > Index: bsd.port.mk > =================================================================== > RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v > retrieving revision 1.1288 > diff -u -p -r1.1288 bsd.port.mk > --- bsd.port.mk 4 Jan 2015 05:47:07 -0000 1.1288 > +++ bsd.port.mk 5 Apr 2015 11:30:41 -0000 > @@ -1168,6 +1168,9 @@ MASTER_SITE_OVERRIDE ?= No > .endif > > .if !empty(GH_ACCOUNT) && !empty(GH_PROJECT) > +. if !empty(GH_COMMIT) && !empty(GH_TAGNAME) > +ERRORS += "Fatal: specifying both GH_TAGNAME and GH_COMMIT is invalid" > +. endif > . if ${GH_TAGNAME} == master > ERRORS += "Fatal: using master as GH_TAGNAME is invalid" > . endif > > I think it is a good idea. I would also suggest changing the docs to no longer suggest that it is good practice to set both. Index: bsd.port.mk.5 =================================================================== RCS file: /cvs/src/share/man/man5/bsd.port.mk.5,v retrieving revision 1.415 diff -u -p -r1.415 bsd.port.mk.5 --- bsd.port.mk.5 3 Apr 2015 10:19:22 -0000 1.415 +++ bsd.port.mk.5 5 Apr 2015 12:26:41 -0000 @@ -1701,8 +1701,7 @@ Yields a suitable default for Account name of the GitHub user hosting the project. .It Ev GH_COMMIT SHA1 commit id to fetch. -It is good practice to always specify -the commit id, even if ${GH_TAGNAME} was specified. +It is an error to specify ${GH_COMMIT} when ${GH_TAGNAME} is specified. .It Ev GH_PROJECT Name of the project on GitHub. .It Ev GH_TAGNAME