Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support #10

tianon · 2014-09-15T21:19:19Z

Fixes #4
Fixes #9

Also, update.sh will make sure it's properly copied into each version subfolder, like we've done with other official images.

yosifkit · 2014-09-15T21:34:32Z

LGTM. cc @proppy

tianon · 2014-09-15T21:40:31Z

For those watching "from the couch" (so to speak), this is based on @proppy's short go-build script, which was the inspiration for the whole change. ❤️

proppy · 2014-09-15T21:40:48Z

1.2.1/go-wrapper

+fi
+
+case "$cmd" in
+	get|build)


what about install?

build technically is install, since it really just does go get (where go-wrapper get is really go get -d, ie, download only)

what about naming it go-wrapper download, because I kinda expected go-wrapper get to be go get :)

I think that's definitely fair, and makes sense 👍

Should we also rename build to be install ?

yes, actually build is install as currently implemented.

oups, install is really get.

Sorry for the missunderstanding.

What about going for go-wrapper get and just passing the arg verbatim.

That way people using the wrapper wouldn't have to rely on the usage.

My problem with that is that it was never clear to me (as a layman "go
tool" user) that a raw "go get" is really "download + build + install", and
so I hate perpetuating that verbiage choice.

Ok, then I think we need different name not to confuse user.

go download is great

what about go compile?

What if we switch it to actually be "go install", and keep the install
name? (and thus assume you did a "download" beforehand)

👍 ❤️

proppy · 2014-09-15T21:44:14Z

@tianon Thanks for building up on my "naive" implementation :)

proppy · 2014-09-15T21:51:12Z

1.2.1/onbuild/Dockerfile


 ONBUILD COPY . /go/src/app
-ONBUILD RUN go get -d -v ./...
-ONBUILD RUN go build -v ./...
+ONBUILD RUN go-wrapper get


I wonder if separate script would make this friendlier, when calling it from the non-onbuild image:

go-run go-get go-build ``

Probably friendlier to users yes, but does blow up the number of scripts we copy around our repo, which is what I was trying to avoid. Maybe we just have the script itself support being symlinked to go-<cmd> names that imply the command bit?

This is actually technically a Docker limitation, but that's a digression. 😄

So, my problem with that is that it makes them seem more official than they really are, ie, go-install will now be confusingly similar to go install but ever so slightly different, which is what I was trying to avoid by keeping everything in the go-wrapper script directly (so it's clearly separated).

tianon · 2014-09-15T22:08:02Z

So, here's the "usage" text with my latest force push:

# go-wrapper 
usage: /usr/local/bin/go-wrapper command [args]
   ie:
       /usr/local/bin/go-wrapper download
          (equivalent to "go get -d <package>")
       /usr/local/bin/go-wrapper download -u
          (equivalent to "go get -d -u <package>")

       /usr/local/bin/go-wrapper install
          (equivalent to "go get <package>")
          (in other words, "go get -d && go build && go install")

       /usr/local/bin/go-wrapper run
          (assumes "install" has run)
       /usr/local/bin/go-wrapper run -app -specific -arguments

proppy · 2014-09-15T22:29:36Z

1.2.1/go-wrapper

+		echo
+		echo "       $0 install"
+		echo '          (equivalent to "go get <package>")'
+		echo '          (in other words, "go get -d && go build && go install")'


tianon · 2014-09-15T22:34:55Z

Ok, updated usage:

$ ./go-wrapper
usage: ./go-wrapper command [args]

Available Commands:

       ./go-wrapper download
       ./go-wrapper download -u
          (equivalent to "go get -d [args] [godir]")

       ./go-wrapper install
       ./go-wrapper install -race
          (equivalent to "go install [args] [godir]")

       ./go-wrapper run
       ./go-wrapper run -app -specific -arguments

proppy · 2014-09-15T22:50:57Z

1.2.1/go-wrapper

+if [ -f .godir ]; then
+	goDir="$(cat .godir)"
+	goBin="$(basename "$goDir")"
+	goPath="${GOPATH%%:*}"


maybe add a comment that is used to get the first entry of the GOPATH
Add a line note

tianon · 2014-09-15T23:26:11Z

Ok, comment about GOPATH added, and much more verbose usage text:

$ ./go-wrapper

usage: go-wrapper command [args]

This script assumes that is is run from the root of your Go package (for
example, "/go/src/app" if your GOPATH is set to "/go").

The main feature of this wrapper over the native "go" tool is that it supports
the addition of a ".godir" file in your package directory which is a plain text
file that is the import path your application expects (ie, it would be something
like "github.com/jsmith/my-cool-app").  If this file is found, the current
package is then symlinked to /go/src/<.gopath> and the commands then use that
full import path to act on it.  This allows for applications to be universally
cloned into a path like "/go/src/app" and then automatically built properly so
that inter-package imports use the existing source instead of downloading it a
second time.  This also makes the final binary have the correct name (ie,
instead of being "/go/bin/app", it can be "/go/bin/my-cool-app"), which is why
the "run" command exists here.

Available Commands:

  go-wrapper download
  go-wrapper download -u
    (equivalent to "go get -d [args] [godir]")

  go-wrapper install
  go-wrapper install -race
    (equivalent to "go install [args] [godir]")

  go-wrapper run
  go-wrapper run -app -specific -arguments
    (assumes "GOPATH/bin" is in "PATH")

$ ./go-wrapper some-unknown-command
error: unknown command: some-unknown-command

usage: go-wrapper command [args]

This script assumes that is is run from the root of your Go package (for
example, "/go/src/app" if your GOPATH is set to "/go").

The main feature of this wrapper over the native "go" tool is that it supports
the addition of a ".godir" file in your package directory which is a plain text
file that is the import path your application expects (ie, it would be something
like "github.com/jsmith/my-cool-app").  If this file is found, the current
package is then symlinked to /go/src/<.gopath> and the commands then use that
full import path to act on it.  This allows for applications to be universally
cloned into a path like "/go/src/app" and then automatically built properly so
that inter-package imports use the existing source instead of downloading it a
second time.  This also makes the final binary have the correct name (ie,
instead of being "/go/bin/app", it can be "/go/bin/my-cool-app"), which is why
the "run" command exists here.

Available Commands:

  go-wrapper download
  go-wrapper download -u
    (equivalent to "go get -d [args] [godir]")

  go-wrapper install
  go-wrapper install -race
    (equivalent to "go install [args] [godir]")

  go-wrapper run
  go-wrapper run -app -specific -arguments
    (assumes "GOPATH/bin" is in "PATH")

proppy · 2014-09-15T23:34:00Z

1.2.1/go-wrapper

+	goPath="${GOPATH%%:*}" # this just grabs the first path listed in GOPATH, if there are multiple (which is the detection logic "go get" itself uses, too)
+	goDirPath="$goPath/src/$goDir"
+	mkdir -p "$(dirname "$goDirPath")"
+	if [ ! -L "$goDirPath" ]; then


I wonder if it's worth checking that the symlink point to the same location.

proppy · 2014-09-15T23:34:52Z

LGTM, just made 2 last comments about the symlinks and default on the go tool when the command not found.

But feel free to ignore those suggestions if you think they are not appropriate.

tianon · 2014-09-15T23:47:47Z

Passing straight through to "go" makes errors in our stuff less obvious -
like when I switched the script over to "go-wrapper download", I got an
immediate message about "get" being an unknown command, which was useful
and let me know I needed to fix something. The implicit pass-through would
also break some of our ".godir" magic depending on the command you use, so
I'm still -1 on that.

On the symlink though, that's probably a good point, although in that case
we could just wipe it out with "ln -sf", so I think I'll take the easy way
out and go with that.

…o accommodate ".godir" support

tianon · 2014-09-15T23:49:16Z

Ok, updated again. ;)

yosifkit · 2014-09-16T17:29:54Z

LGTM

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support

tianon force-pushed the go-wrapper branch 2 times, most recently from de04e23 to 9643ee8 Compare September 15, 2014 21:33

yosifkit mentioned this pull request Sep 15, 2014

golang: custom import paths docker-library/docs#24

Closed

proppy reviewed Sep 15, 2014
View reviewed changes

tianon force-pushed the go-wrapper branch from 9643ee8 to a8395fc Compare September 15, 2014 22:07

proppy reviewed Sep 15, 2014
View reviewed changes

tianon force-pushed the go-wrapper branch 2 times, most recently from b943e81 to 5535527 Compare September 15, 2014 22:34

proppy reviewed Sep 15, 2014
View reviewed changes

tianon force-pushed the go-wrapper branch from 5535527 to f88febb Compare September 15, 2014 23:21

proppy reviewed Sep 15, 2014
View reviewed changes

Add simple "go-wrapper" script for building and running, especially t…

9ff2ccc

…o accommodate ".godir" support

tianon force-pushed the go-wrapper branch from f88febb to 9ff2ccc Compare September 15, 2014 23:49

yosifkit added a commit that referenced this pull request Sep 16, 2014

Merge pull request #10 from infosiftr/go-wrapper

5a66872

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support

yosifkit merged commit 5a66872 into docker-library:master Sep 16, 2014

yosifkit deleted the go-wrapper branch September 16, 2014 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support #10

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support #10

tianon commented Sep 15, 2014

yosifkit commented Sep 15, 2014

tianon commented Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

proppy Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

proppy Sep 15, 2014

proppy commented Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

proppy Sep 15, 2014

tianon Sep 15, 2014

tianon commented Sep 15, 2014

proppy Sep 15, 2014

tianon commented Sep 15, 2014

proppy Sep 15, 2014

tianon commented Sep 15, 2014

proppy Sep 15, 2014

proppy commented Sep 15, 2014

tianon commented Sep 15, 2014

tianon commented Sep 15, 2014

yosifkit commented Sep 16, 2014

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support #10

Add simple "go-wrapper" script for building and running, especially to accommodate ".godir" support #10

Conversation

tianon commented Sep 15, 2014

yosifkit commented Sep 15, 2014

tianon commented Sep 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy commented Sep 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tianon commented Sep 15, 2014

Choose a reason for hiding this comment

tianon commented Sep 15, 2014

Choose a reason for hiding this comment

tianon commented Sep 15, 2014

Choose a reason for hiding this comment

proppy commented Sep 15, 2014

tianon commented Sep 15, 2014

tianon commented Sep 15, 2014

yosifkit commented Sep 16, 2014