Rewrite import paths?

[iangudger]

It would be really cool if packages in this repo were go get-able. Maybe rewrite the import path to github.com/tonistiigi/gvisor?

[tonistiigi]

I'd like to avoid the situation where something written with gvisor dependency would need to choose if it can only be built with this mirror or upstream. Also, maybe upstream will switch away in the future and we could just kill this mirror.

[amscanne]

I think we're unlikely to switch from bazel, but I'm very curious how you're building this repository. I think we'd be interesting in having some standardized upstream synthetic mirror (that is buildable and maybe even 'go get'-able). This is essentially what netstack is.

[tonistiigi]

@amscanne The builder is in the converter branch and runs in travis.

[amscanne]

The upstream repository should now support this: https://github.com/google/gvisor/tree/master#using-go-get

I'm afraid recent commits may require minor updates to the converter branch scripts. The canonical go module name has changed to gvisor.dev/gvisor (in order to directly support go get, in a limited capacity).

The mechanism is actually similar, but I hadn't dug into yours yet. Why do you need to do the assembly rewrite bits? I don't understand why I didn't hit something similar.

[tonistiigi]

in a limited capacity

what's the limited capacity?

Where is the code for the conversion bot that this uses?

Why do you need to do the assembly rewrite bits? I don't understand why I didn't hit something similar.

Assembly files are needed for the go:linkname directives (iirc). It is possible that bazel gopath now generates them as well, or maybe just these packages have added .s files now.

[amscanne]

what's the limited capacity?

The branch will be maintained in a best effort capacity. I mean, we have presubmit checks and other things to ensure that it keeps working, but we're just noting that it's "unofficial".

Where is the code for the conversion bot that this uses?

It's in the main repository.

• cloudbuild/go.yaml
• tools/go_branch.sh
• BUILD <- the :gopath target

Assembly files are needed for the go:linkname directives (iirc). It is possible that bazel gopath now generates them as well, or maybe just these packages have added .s files now.

Are you sure? Where is that coming from?

The linkname directives are used here without assembly files: https://github.com/google/gvisor/tree/go/third_party/gvsync

(And nothing at all would work if that package were broken.)

[tonistiigi]

The linkname directives are used here without assembly files:

That package does import _ "unsafe" for the same effect. Would need to dig up some old commits to find the case that refused to build without them.

Anyway, I'll move my own stuff over to that branch when I have some free time. If all goes painlessly will shut down my travis and archive this repo.

[tonistiigi]

One nice thing I have is that the history is still clean without merge commits.

Also, noticed this sorting issue in go_path https://github.com/google/gvisor/commit/dacee8d694b063ece50e6b3cc5f227dcc1c2f2e9 again that we both have. Would be nice to fix that.

[tiborvass]

Looks like this linkname issue was partially fixed in Go 1.12: https://github.com/golang/go/commit/ca3749230b5a7d43b3292226fdb2b6f3de5d653b

If linkname is used to link in a different package then the local one (extremely rare), then the remote package still needs an assembly file. Example: https://github.com/golang/go/blob/master/src/os/signal/sig.s

[amscanne]

The _ "unsafe" in that package is not to fix assembly linking, it is because the go:linkname directive is only supported in files that import the unsafe package. I think you might be some confusion between assembly functions and linkname'ed functions here, pure assembly functions in the same package don't require the directive.

If you could pull up the thing that was breaking, that would be really useful in understanding the issue.

Re: the merge commits, I actually constructed things that way intentionally. I wanted to be able to have a mechanism whereby the linear diff could be viewed (notwithstanding the annoying ordering issue that we seem to be both have), but I also wanted to link it to the canonical commit.

[tonistiigi]

The _ "unsafe" in that package is not to fix assembly linking, it is because the go:linkname directive is only supported in files that import the unsafe package.

Where did I claim that? I think we are talking about exactly the same thing.

If you could pull up the thing that was breaking, that would be really useful in understanding the issue.

@tiborvass already found that it is a change between go1.11 and go1.12 that doesn't require it anymore(triggered by the -complete flag on compile).

Re: the merge commits, I actually constructed things that way intentionally. I wanted to be able to have a mechanism whereby the linear diff could be viewed (notwithstanding the annoying ordering issue that we seem to be both have), but I also wanted to link it to the canonical commit.

I'm not sure what you mean by the diff. You can still diff against upstream (although the diffs are big in both versions), and of course against the rewritten commits as well. Every commit in this mirror has a link to the original upstream commit if you want to switch to the same position in another branch.

[tonistiigi]

If you want you can check the go:linkname behavior with:

cat <<EOT > main.go
package main

import _ "unsafe"

//go:linkname throw runtime.throw
func throw(string)

func main() {
}
EOT

cat <<'EOT' > Dockerfile
arg GOVER=1.12

from golang:$GOVER-alpine
workdir /go/src/github.com/tonistiigi/test
copy . .
run go build .
EOT

export DOCKER_BUILDKIT=1
docker build --build-arg GOVER=1.12 .
docker build --build-arg GOVER=1.11 .

[tonistiigi]

archiving repo