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

CONTRIBUTING.md: Instructions to get the source code should be updated for new version of Go #648

Closed
mhansen opened this issue Sep 22, 2021 · 15 comments · Fixed by #673
Closed
Assignees

Comments

@mhansen
Copy link
Contributor

mhansen commented Sep 22, 2021

What operating system and processor architecture are you using?

Google's defualt workstation golang binary:

$ uname -a
Linux markhansen.syd.corp.google.com 5.10.40-1rodete2-amd64 #1 SMP Debian 5.10.40-1rodete2 (2021-06-22) x86_64 GNU/Linux
$ which go
/usr/lib/google-golang/bin/go
$ go version
go version devel +b7a85e0003 linux/amd64

What did you do?

Followed steps in CONTRIBUTING.md to get the source code. My GOPATH is empty and defaults to ~/go (so I think you could maybe remove that bit of the CONTRIBUTING.md too?)

$ echo $GOPATH

$ go get github.com/google/pprof
go get: installing executables with 'go get' in module mode is deprecated.
        Use 'go install pkg@version' instead.
        For more information, see https://golang.org/doc/go-get-install-deprecation
        or run 'go help get' or 'go help install'.
# net
cgo: C compiler "clang" not found: exec: "clang": executable file not found in $PATH
# crypto/internal/boring
cgo: C compiler "clang" not found: exec: "clang": executable file not found in $PATH

I have some errors due to the new changes around go modules.

Then running the commands to run the tests:

$ cd $GOPATH/src/github.com/google/pprof
$ go test -v ./...

I tried going into ~/go/src/ but there is no src dir, only ~/go/bin/ and ~/go/pkg/. The code seems to be in pkg/mod:

$ ls ~/go/pkg/mod/github.com/google
'[email protected]'/  '[email protected]'/

What did you expect to see?

No errors, and able to run the tests with instructions given.

What did you see instead?

Errors and the tests weren't in the directory given.

@mhansen
Copy link
Contributor Author

mhansen commented Sep 22, 2021

(this isn't particularly blocking me -- I'll just clone the repo -- but thought I'd file the bug in case this is affecting new contributors)

@chavey
Copy link

chavey commented Oct 7, 2021

Mark,

Are you using a go version >= 1.8?

for go version < 1.8, GOPATH needs to be set for go get github.com/foo/bar to check out the github.com/foo/bar repo. Setting the GOPATH is required.

for go version >= 1.18 this is not require as by default GOPATH is set to $HOME/go.

In your case, if you are using a go version >= 1.8 and did not set GOPATH, you should have a check out of the repo under $HOME/go/src/github.com/google/pprof.
I you are using a go version < 1.8, you will not have a src directory.

The code that you see in ~/go/pkg/mod/github.com/google/[email protected]/ is not the checkout repo (it does not have a .git directory). The directory is set to readonly/execute.

@mhansen
Copy link
Contributor Author

mhansen commented Oct 7, 2021 via email

@mhansen
Copy link
Contributor Author

mhansen commented Oct 7, 2021

Actually, let me check when I'm at my workstation tomorrow, the above is off-the-cuff and may be wrong.

@mhansen
Copy link
Contributor Author

mhansen commented Oct 8, 2021

for go version < 1.18, GOPATH needs to be set

Is 1.18 a typo? I thought GOPATH was only required before Go 1.8? https://rakyll.org/default-gopath/

@mhansen
Copy link
Contributor Author

mhansen commented Oct 8, 2021

Let me maybe be a bit clearer on what I'm asking:

  • I think instead of saying go get github.com/google/pprof perhaps it should say go install github.com/google/pprof@latest because you're installing a binary rather than just getting a package, to silence that deprecation warning.
  • I think GOPATH has been implicit since Go 1.8, which is pretty ancient now (from 2016) so you might be able to remove the GOPATH stuff from your guide?

@aalexand
Copy link
Collaborator

aalexand commented Oct 8, 2021

Hmm, I am not sure go install github.com/google/pprof@latest works that well for the development case, as I think it will download the sources into a versioned directory under ~/go/pkg/ ?

@mhansen
Copy link
Contributor Author

mhansen commented Oct 8, 2021 via email

@aalexand
Copy link
Collaborator

aalexand commented Oct 8, 2021

I think with Go modules it can actually be just

$ cd $(mktemp -d)

$ git clone [email protected]:google/pprof.git
Cloning into 'pprof'...
Bremote: Enumerating objects: 3429, done.
remote: Counting objects: 100% (158/158), done.
remote: Compressing objects: 100% (108/108), done.
remote: Total 3429 (delta 66), reused 103 (delta 44), pack-reused 3271
Receiving objects: 100% (3429/3429), 2.95 MiB | 5.93 MiB/s, done.
Resolving deltas: 100% (2257/2257), done.

$ cd pprof
/tmp/tmp.d4vkfHmtp6/pprof

$ go test -v ./...
go: downloading github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639
go: downloading github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e
?   	github.com/google/pprof	[no test files]
?   	github.com/google/pprof/driver	[no test files]
=== RUN   TestParseData
--- PASS: TestParseData (0.00s)
PASS
...
...

@mhansen
Copy link
Contributor Author

mhansen commented Oct 8, 2021 via email

@chavey
Copy link

chavey commented Oct 8, 2021

for go version < 1.18, GOPATH needs to be set

Is 1.18 a typo? I thought GOPATH was only required before Go 1.8? rakyll.org/default-gopath

Yes, sorry for the typo, it should read 1.8. (https://golang.org/doc/go1.8#gopath)

@mhansen
Copy link
Contributor Author

mhansen commented Oct 8, 2021 via email

@chavey
Copy link

chavey commented Oct 8, 2021

This older thread about 'go get' golang/go#31529 (comment), suggests using git clone.

$ cd /tmp
$ git clone [email protected]:google/pprof.git
$ cd /tmp/pprof
$ go install # to get all dependencies as specified by go.mod

@chavey
Copy link

chavey commented Oct 12, 2021

Should we had a section about using git clone OR are we ok with the current documentation?

@aalexand
Copy link
Collaborator

Since go get is deprecated for installing things, we should replace the go get usage in both README.md and CONTRIBUTING.md with the git clone based flow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants