patchcontrib/persistent-https: update ldflags syntax for Go 1.7+

From: Parker Moore <parkrmoore@gmail.com>

This fixes contrib/persistent-https builds for Go v1.7+ and is compatible with Go v1.5+.

Running `make all` in `contrib/persistent-https` results in a failure on Go 1.7 and above.

Specifically, the error is:

    go build -o git-remote-persistent-https \
   -ldflags "-X main._BUILD_EMBED_LABEL 1468613136"
    # _/Users/parkr/github/git/contrib/persistent-https
    /usr/local/Cellar/go/1.7rc1/libexec/pkg/tool/darwin_amd64/link: -X
flag requires argument of the form importpath.name=value
    make: *** [git-remote-persistent-https] Error 2

This `name=value` syntax for the -X flag was introduced in Go v1.5 (released Aug 19, 2015):

- release notes: https://golang.org/doc/go1.5#link
- commit: https://github.com/golang/go/commit/12795c02f3d6fc54ece09a86e70aaa40a94d5131

In Go v1.7, support for the old syntax was removed:

- release notes: https://tip.golang.org/doc/go1.7#compiler
- commit: https://github.com/golang/go/commit/51b624e6a29b135ce0fadb22b678acf4998ff16f

This patch includes the `=` to fix builds with Go v1.7+.
---
 contrib/persistent-https/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

[+cc Shawn, who participated in the original discussion, as I don't
  think Colby really works on git any more]

On Fri, Jul 15, 2016 at 01:44:14PM -0700, Parker Moore wrote:

With the disclaimer that I have very little experience with Go, this seems like a good, well-explained change. My only question would be whether people still use pre-v1.5 versions of Go, since it sounds like this would adversely affect them if they do. (If it does, it seems the Go people did not give a very good deprecation period for the syntax change, if people are using both the pre-new-syntax and post-old-syntax versions simultaneously). I'm not sure what the alternative is, beyond perhaps checking the version of Go dynamically in the Makefile.

-Peff

Jeff King <peff@peff.net> writes:

Yeah, you get something like this:

    $ ./git-remote-persistent-https --print_label
    2016/07/18 13:34:33 unlabeled build; build with "make" to label

which is probably not the end of the world. The label does not even identify the version of the source in any way, so I am not sure how people are depending on that feature anyway ;-)

Would it be a better solution simply to remove this build flag? Alternatively, if Git wished to support Go v1.5 and below, I would be more than happy to send a patch with a dynamic lookup in the Makefile based on the output of `go version`. I would be more than happy to submit either patch.

On Mon, Jul 18, 2016 at 9:32 PM, Parker Moore <parkrmoore@gmail.com> wrote:

I think we could remove that BUILD_LABEL entirely. Colby liked having a marker so he knows what "version" a user is running, but without any correlation to source here it just isn't that useful.

Shawn Pearce <spearce@spearce.org> writes:

Inside an organization where people use it as a tool supplied by somebody else, who is the designated supplier of it, build-stamp may be sufficient to identify what "version" a user is running. If we wanted to do a "here is the source that was built from", the logical place to pull that information from would be ../../GIT-VERSION-FILE, but the mechanism to embed the information would still be -X var=val (or "-X var val" for older Go).

So unless the "dynamic lookup in the Makefile" turns out to be too gross, we would want to keep the mechanism and just make it usable for versions before 1.5 and also after 1.7, I would guess.

I agree. It would make more sense to build this to a specific version or git revision rather than a time. Perhaps that would be a different patch?

A dynamic lookup of the go version would look for go 1.0, 1.1, 1.2, 1.3, 1.4 and 1.5.0. These versions would be incompatible with the `X var=val` syntax. I am not too familiar with Makefile syntax for numerical comparison, but I believe this would be fairly simple. Would you like me to whip up a patch for it?

Parker Moore <parkrmoore@gmail.com> writes:

It would definitely be a separate (and optional) patch and must come after "do we use = in between or send var/val as two separate arguments?" patch. That was what I meant to say with "... would be version file, but the mechanism to embed it would be the same as today".

$ go version go version go1.3.3 linux/amd64

is what I seem to be locally getting, so I'd imagine it would be something like this perhaps?

git-remote-persistent-https:
	case $$(go version) in \
	"go version go"1.[0-5].*) EQ=" " ;; *) EQ="=" ;; esac && \
	go build -o git-remote-persistent-https \
		-ldflags "-X main._BUILD_EMBED_LABEL$${EQ}$(BUILD_LABEL)"

Surely, and thanks.