Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/internal/obj: -trimpath help text is misleading #13616

Open
binarycrusader opened this issue Dec 14, 2015 · 7 comments
Open

cmd/internal/obj: -trimpath help text is misleading #13616

binarycrusader opened this issue Dec 14, 2015 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation
Milestone

Comments

@binarycrusader
Copy link
Contributor

Tested with Go 1.5.2.

The -trimpath option is documented as follows:

$ go tool asm -help 2>&1|grep -A1 trimpath
  -trimpath string
        remove prefix from recorded source file paths
...
$ go tool compile -help|grep -A1 trimpath
  -trimpath prefix
        remove prefix from recorded source file paths

However, it would appear that it mistakenly removes the leading '/' from the resulting path:

$ export PROTO_DIR=/builds/go-proto-area
$ cd $(PROTO_DIR)/usr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish
$ go install -asmflags -trimpath=$PROTO_DIR -gcflags -trimpath=$PROTO_DIR
$ strings ../../../../../pkg/solaris_amd64/golang.org/x/crypto/blowfish.a | grep 'lib\/'
xusr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish/block.go
xusr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish/block.go
...

If I add a terminating slash, then it fails to trim entirely:

$ export PROTO_DIR=/builds/go-proto-area/
$ cd $(PROTO_DIR)/usr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish
$ go install -asmflags -trimpath=$PROTO_DIR -gcflags -trimpath=$PROTO_DIR
$ strings ../../../../../pkg/solaris_amd64/golang.org/x/crypto/blowfish.a | grep 'lib\/'
/builds/go-proto-area/usr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish/block.go
/builds/go-proto-area/usr/lib/gocode/1.5/src/golang.org/x/crypto/blowfish/block.go
...

This appears to be due to a off-by-one error (or bad assumption) made in src/cmd/internal/obj/obj.go in setFile():

        // Remove leading TrimPathPrefix, or else rewrite $GOROOT to $GOROOT_FINAL.
        if h.TrimPathPrefix != "" && hasPathPrefix(abs, h.TrimPathPrefix) {
                if abs == h.TrimPathPrefix {
                        abs = ""
                } else {
                        abs = abs[len(h.TrimPathPrefix)+1:]
                }
@ianlancetaylor
Copy link
Contributor

This is working as expected. Maybe we need to fix the docs. It's a path prefix, not a string prefix. You give it a path, and it strips the path, leaving you a relative path.

Do you have a reason to want it to work some other way?

@ianlancetaylor ianlancetaylor changed the title asm/compile: -trimpath trimming seems broken cmd/internal/obj: -trimpath trimming seems broken Dec 15, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Dec 15, 2015
@binarycrusader
Copy link
Contributor Author

If it's a relative path, then what is it relative to? I don't think I want a relative path as the result, and certainly the help output doesn't imply it will be.

Also, confusing is that other times, when trimpath is applied, it leaves the leading '/' on, I'll try to find a reproducible test case for that.

Ultimately, what I'm trying to do is this:

I want to pre-build the supplementary packages provided with Go in a proto area, but later, install them from a package into the system root. So the recorded paths should be an absolute path referring to the system root, not relative to some other area if I understand correctly.

@ianlancetaylor
Copy link
Contributor

The idea of -trimpath is that you are building in /tmp/iant-12345/ and you want to strip that prefix because it's not useful. The result is intended to be a relative path--it's relative to GOROOT or GOPATH, depending.

I want to pre-build the supplementary packages provided with Go in a proto area, but later, install them from a package into the system root. So the recorded paths should be an absolute path referring to the system root, not relative to some other area if I understand correctly.

That sounds more like GOROOT_FINAL than -trimpath. But GOROOT_FINAL is specific to GOROOT. I don't think we have a feature like that in the toolchain.

@binarycrusader
Copy link
Contributor Author

Well, in my case, I could strip the path so it would be relative to GOPATH, and that would work for my purposes (since GOROOT_FINAL already fixes all the paths for the standard Go library), but I guess I don't understand why -trimpath is supposed to produce a relative result, at least, that was unexpected to me. But I guess that's because as you said, it's a path not a string.

The help text should probably make it clearer that the result of applying trim is always a relative path.

I'll try to reproduce the other inconsistency I found with trimpath tomorrow and report back.

@binarycrusader binarycrusader changed the title cmd/internal/obj: -trimpath trimming seems broken cmd/internal/obj: -trimpath help text is misleading Dec 15, 2015
@binarycrusader
Copy link
Contributor Author

Ok, so after further experiment, I can't reproduce the behaviour I saw at one point, where trimpath behaved differently when I used a trailing slash.

Regardless, I find trimpath confusing for three reasons:

  • it says "removes prefix from path" implying that prefix is treated as a string (to me anyway)
  • it always produces a relative path result (which isn't clear from help text)
  • if trimpath is really supposed to treat the input as a path to remove, then why does it care if I use a trailing slash or not? I find the behaviour of basically ignoring my input if I use a trailing slash to be surprising and unexpected.

Personally, it would be more useful to me if -trimpath produced a relative path if I used a trailing slash, and an absolute path if I did not. That would at least (to me) make some logical sense and would be easy to explain.

@ianlancetaylor
Copy link
Contributor

I suppose I personally don't mind if you want to change it, but the truth is that we don't really expect people to invoke -trimpath themselves. It's there for the Go tool to use, and it's documented for completeness.

@binarycrusader
Copy link
Contributor Author

Ok, if there are no objections then I'd be happy to contribute such a change. -trimpath makes it possible to easily distribute prebuilt Go packages so is pretty valuable for an OS distribution. I'll do some testing and further evaluation first and then propose something specific. Thanks for your patience and kind response.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Documentation
Projects
None yet
Development

No branches or pull requests

3 participants