makefile: use full package name when building

[cyphar]

While ./cmd/... works in the canonical Go setup, I have a very
interesting Go setup involving symlinks within my GOPATH. It turns out
that Go doesn't like this very much and the only way to solve it is to
either stop using Go's inbuilt vendoring or build everything using the
full Go import.

The sort of errors you get are that the same type cannot be used in a
function call because the function signature was imported from a
different path.

Signed-off-by: Aleksa Sarai [email protected]

[wking]

On Tue, Oct 25, 2016 at 02:41:51AM -0700, Aleksa Sarai wrote:

While ./cmd/... works in the canonical Go setup, I have a very interesting Go setup involving symlinks within my GOPATH. It turns out that Go doesn't like this very much and the only way to solve it is to either stop using Go's inbuilt vendoring or build everything using the full Go import.

Can you provide enough details to reproduce your error? I have:

~/src/opencontainers $ ls -l image-tools
lrwxrwxrwx 1 wking wking 45 Sep 15 21:55 image-tools -> go/src/github.com/opencontainers/image-tools/
~/src/opencontainers $ alias cdcwd
alias cdcwd='cd $(realpath "${PWD}")'
~/src/opencontainers/image-spec $ cdcwd 
~/src/opencontainers/go/src/github.com/opencontainers/image-spec $ source ~/src/opencontainers/go/BASH.RC 
~/src/opencontainers/go/src/github.com/opencontainers/image-spec $ env | grep GO
GOBIN=/home/wking/bin
GOPATH=/home/wking/src/opencontainers/go/src/github.com/opencontainers/runc/Godeps/_workspace:/home/wking/src/opencontainers/go

and image-tools compiles fine with the current relative path.

[cyphar]

This doesn't happen with the current state of the repo (it only happens with #5 applied). But this is an issue common to all Go repos which have vendor directories and are "complicated enough". This is how my setup is set up:

% mkdir ~/.local
% export GOPATH=~/.local
% ln -s .. ~/.local/src
% mkdir ~/src/github.com
% ln -s .. ~/src/github.com/opencontainers
% cd ~/src; git clone [email protected]:opencontainers/image-tool.git
% cd ~/src/image-tool

If I apply #5 and try to build without this patch applied I get this:

% make tools
go build -ldflags "-X main.gitCommit=4dfe06f9cd6c3e0dfb245ddcc12d039722626540" ./cmd/oci-cas
go build -ldflags "-X main.gitCommit=4dfe06f9cd6c3e0dfb245ddcc12d039722626540" ./cmd/oci-create-runtime-bundle
go build -ldflags "-X main.gitCommit=4dfe06f9cd6c3e0dfb245ddcc12d039722626540" ./cmd/oci-image-init
go build -ldflags "-X main.gitCommit=4dfe06f9cd6c3e0dfb245ddcc12d039722626540" ./cmd/oci-image-validate
go build -ldflags "-X main.gitCommit=4dfe06f9cd6c3e0dfb245ddcc12d039722626540" ./cmd/oci-refs
# image-tools/cmd/oci-refs
../../.local/src/image-tools/cmd/oci-refs/list.go:69: cannot use state.printName (type func("image-tools/vendor/golang.org/x/net/context".Context, string) error) as type refs.ListNameCallback in argument to engine.List
../../.local/src/image-tools/cmd/oci-refs/put.go:80: cannot use &descriptor (type *"image-tools/vendor/github.com/opencontainers/image-spec/specs-go".Descriptor) as type *"github.com/opencontainers/image-tools/vendor/github.com/opencontainers/image-spec/specs-go".Descriptor in argument to engine.Put
make: *** [Makefile:33: oci-refs] Error 2
make tools  14.49s user 0.89s system 222% cpu 6.917 total

Trust me when I say this happens quite a lot to me. For example I was playing around with runV and it also fails to build with my setup. It's nice that I've actually figured out how to fix it though ...

[wking]

On Tue, Oct 25, 2016 at 01:46:58PM -0700, Aleksa Sarai wrote:

% mkdir /.local
% export GOPATH=/.local
% ln -s .. ~/.local/src
% mkdir ~/src/github.com
% ln -s .. ~/src/github.com/opencontainers

This makes less sense to me than having a symlink-less Go tree which
~/src/opencontainers, etc. link into (to make cd'ing in easier, see my
cdcwd alias 1). If we go this way, you'll also need to update the
‘test’ target, and possibly others.

I guess I'm not strongly opposed, but it seems like a work-around for
your slightly-broken local setup, and I'd prefer just fixing your
local setup ;).

[cyphar]

@wking

This makes less sense to me than having a symlink-less Go tree which ~/src/opencontainers, etc. link into (to make cd'ing in easier, see my cdcwd alias [1]).

I don't agree. I also don't like the fact that Go makes this a problem (why on earth should a programming language mandate what my home directory's layout is?!).

I guess I'm not strongly opposed, but it seems like a work-around for your slightly-broken local setup, and I'd prefer just fixing your local setup ;).

It's not a work-around. It's a fix that makes it that we no longer make assumptions about the current directory, and makes no difference in the "common" Go usecase...

[wking]

On Tue, Oct 25, 2016 at 05:35:50PM -0700, Aleksa Sarai wrote:

This makes less sense to me than having a symlink-less Go tree
which ~/src/opencontainers, etc. link into (to make cd'ing in
easier, see my cdcwd alias [1]).

I don't agree. I also don't like the fact that Go makes this a
problem (why on earth should a programming language mandate what my
home directory's layout is?!).

I agree that Go's package management is not how I'd design it, but I'd
rather not tilt at that windmill here ;).

I guess I'm not strongly opposed, but it seems like a work-around
for your slightly-broken local setup, and I'd prefer just fixing
your local setup ;).

It's not a work-around. It's a fix that makes it that we no longer
make assumptions about the current directory, and makes no
difference in the "common" Go usecase...

I think we're entitled to make assumptions about the current working
directory in a Makefile ;). This PR hard-codes that path, which makes
forking and any future project renames slightly harder. Not much
harder (and the Go imports already hard-code this path too), and I
expect both forking and renames aren't very likely. But I don't see a
need for impeding either any more than they already are, when you
could address the issue locally by turning your symlinks around and
working from the resulting real path (at
$GOPATH_ENTRY/github.com/opencontainers/image-tools).

[cyphar]

@wking If you want I can switch to using GOPATH=vendor go build .... Both of them can work (and it's hilarious that Go decided to go with an incompatible vendor system).

This PR hard-codes that path, which makes
forking and any future project renames slightly harder. Not much
harder (and the Go imports already hard-code this path too), and I
expect both forking and renames aren't very likely. But I don't see a
need for impeding either any more than they already are,

sed doesn't care how many files it has to act upon...

when you
could address the issue locally by turning your symlinks around and
working from the resulting real path (at
$GOPATH_ENTRY/github.com/opencontainers/image-tools).

But then cloning is twice as painful as it should be. I don't get why this is such an issue -- it doesn't affect anyone who has the stock Go setup and it fixes my setup. Programming tools are not meant to fight the user.

[wking]

On Wed, Oct 26, 2016 at 09:04:19AM -0700, Aleksa Sarai wrote:

@wking If you want I can switch to using GOPATH=vendor go build ....

Does this really work? How does it find image-tools?

sed doesn't care how many files it has to act upon...

True. I still think this is a local-setup issue, but I agree that
it's not a big weight on folks forking or renaming the repo. So no +1
from me, but not enough resistance for me to -1 it either ;).

[cyphar]

@wking

It works if you do something like this (I remember playing with this a while ago, there's some symlink trickery):

% ln -sf . vendor/src
% ln -sf $(pwd) vendor/src/github.com/opencontainers/image-tools

It's the same trickery we do for runC, but instead you also have vendor. To be honest, I have a bad feeling about the ./cmd stuff (in runC we use . and that appears to work). But it could work, I'd have to play around with it.

[vbatts]

While this LGTM, It does not guarantee that the go build is in fact building the current directory $(PACKAGE), as there is no GOPATH check or set up.
For me, this is how I have my code cloned out, but that is not true everywhere. I think this change requires a bit more trickery to ensure the current directory is in the GOPATH as expected.