On Sun, Apr 5, 2015, at 01:31 PM, Stuart Henderson wrote:
> On 2015-04-04, Landry Breuil <[email protected]> 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
