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

GOPATH and GOBIN not set #12

Open
radu-matei opened this issue Aug 15, 2019 · 19 comments

Comments

@radu-matei
Copy link

commented Aug 15, 2019

Ref:

setup-go/src/installer.ts

Lines 105 to 115 in 8187235

const goPath: string = process.env['GOPATH'] || '';
const goBin: string = process.env['GOBIN'] || '';
// set GOPATH and GOBIN as user value
if (goPath) {
core.exportVariable('GOPATH', goPath);
}
if (goBin) {
core.exportVariable('GOBIN', goBin);
}
}

2019-08-15T22:05:36.9141345Z env:
2019-08-15T22:05:36.9141441Z   GOROOT: /opt/hostedtoolcache/go/1.12.0/x64
2019-08-15T22:05:36.9141482Z ##[endgroup]
2019-08-15T22:05:37.0161150Z LEIN_HOME=/usr/local/lib/lein
2019-08-15T22:05:37.0161985Z M2_HOME=/usr/share/apache-maven-3.6.1
2019-08-15T22:05:37.0162146Z BOOST_ROOT=/usr/local/share/boost/1.69.0
2019-08-15T22:05:37.0162236Z GOROOT_1_11_X64=/usr/local/go1.11
2019-08-15T22:05:37.0162330Z ANDROID_HOME=/usr/local/lib/android/sdk
2019-08-15T22:05:37.0162641Z JAVA_HOME_11_X64=/usr/lib/jvm/zulu-11-azure-amd64
2019-08-15T22:05:37.0162818Z LANG=C.UTF-8
2019-08-15T22:05:37.0162931Z INVOCATION_ID=e226194a393e43d59aba6bd4c85af030
2019-08-15T22:05:37.0163149Z JAVA_HOME_12_X64=/usr/lib/jvm/zulu-12-azure-amd64
2019-08-15T22:05:37.0163325Z ANDROID_SDK_ROOT=/usr/local/lib/android/sdk
2019-08-15T22:05:37.0163420Z RUNNER_TOOL_CACHE=/opt/hostedtoolcache
2019-08-15T22:05:37.0163674Z JAVA_HOME=/usr/lib/jvm/zulu-8-azure-amd64
2019-08-15T22:05:37.0163967Z RUNNER_TRACKING_ID=github_28b0e254-504e-46ce-b1ab-b1db282d32b4
2019-08-15T22:05:37.0164043Z GITHUB_ACTIONS=true
2019-08-15T22:05:37.0164178Z DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
2019-08-15T22:05:37.0164238Z USER=runner
2019-08-15T22:05:37.0164303Z GITHUB_HEAD_REF=
2019-08-15T22:05:37.0164521Z GITHUB_ACTOR=radu-matei
2019-08-15T22:05:37.0164594Z GITHUB_ACTION=run
2019-08-15T22:05:37.0164676Z GRADLE_HOME=/usr/share/gradle
2019-08-15T22:05:37.0164763Z PWD=/home/runner/work/signy/signy
2019-08-15T22:05:37.0164871Z HOME=/home/runner
2019-08-15T22:05:37.0164966Z GOROOT=/opt/hostedtoolcache/go/1.12.0/x64
2019-08-15T22:05:37.0165060Z JOURNAL_STREAM=9:26649
2019-08-15T22:05:37.0165324Z JAVA_HOME_8_X64=/usr/lib/jvm/zulu-8-azure-amd64
2019-08-15T22:05:37.0165421Z RUNNER_TEMP=/home/runner/work/_temp
2019-08-15T22:05:37.0165510Z CONDA=/usr/share/miniconda
2019-08-15T22:05:37.0165615Z BOOST_ROOT_1_69_0=/usr/local/share/boost/1.69.0
2019-08-15T22:05:37.0165753Z RUNNER_WORKSPACE=/home/runner/work/signy
2019-08-15T22:05:37.0166170Z GITHUB_REF=refs/heads/gh-actions
2019-08-15T22:05:37.0166286Z GITHUB_SHA=419522609132ac1c17aed7d67e84e855de049c5e
2019-08-15T22:05:37.0166418Z GOROOT_1_12_X64=/usr/local/go1.12
2019-08-15T22:05:37.0166504Z DEPLOYMENT_BASEPATH=/opt/runner
2019-08-15T22:05:37.0166638Z GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
2019-08-15T22:05:37.0166705Z RUNNER_OS=Linux
2019-08-15T22:05:37.0166811Z GITHUB_BASE_REF=
2019-08-15T22:05:37.0167013Z VCPKG_INSTALLATION_ROOT=/usr/local/share/vcpkg
2019-08-15T22:05:37.0167113Z PERFLOG_LOCATION_SETTING=RUNNER_PERFLOG
2019-08-15T22:05:37.0167581Z JAVA_HOME_7_X64=/usr/lib/jvm/zulu-7-azure-amd64
2019-08-15T22:05:37.0167657Z RUNNER_USER=runner
2019-08-15T22:05:37.0167712Z SHLVL=1
2019-08-15T22:05:37.0167844Z GITHUB_REPOSITORY=engineerd/signy
2019-08-15T22:05:37.0167932Z GITHUB_EVENT_NAME=push
2019-08-15T22:05:37.0168211Z LEIN_JAR=/usr/local/lib/lein/self-installs/leiningen-2.9.1-standalone.jar
2019-08-15T22:05:37.0168321Z RUNNER_PERFLOG=/home/runner/perflog
2019-08-15T22:05:37.0168438Z GITHUB_WORKFLOW=Go
2019-08-15T22:05:37.0168514Z ANT_HOME=/usr/share/ant
2019-08-15T22:05:37.0168631Z PATH=/opt/hostedtoolcache/go/1.12.0/x64/bin:/usr/share/rust/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin
2019-08-15T22:05:37.0168788Z GITHUB_WORKSPACE=/home/runner/work/signy/signy
2019-08-15T22:05:37.0168994Z CHROME_BIN=/usr/bin/google-chrome

@damccorm

This comment has been minimized.

Copy link
Collaborator

commented Aug 16, 2019

Since its not totally inferable what these should be set to, we leave them unless the user provides a value. What would your expectation be here?

@lmittmann

This comment has been minimized.

Copy link

commented Aug 16, 2019

I would expect to have GOPATH = $(go env GOPATH).

@radu-matei

This comment has been minimized.

Copy link
Author

commented Aug 16, 2019

I agree with @lmittmann here - provide sane defaults for $GOPATH and $GOBIN, and allow users to change them if needed.

Also, it would be really convenient if the source code was placed in $GOPATH/{repository} directly.

@radu-matei

This comment has been minimized.

Copy link
Author

commented Aug 16, 2019

FWIW, I don't think it's reasonable to expect users to setup a simple Go build that needs to run in the GOPATH in the following way:

    - name: Build
      run: |
        export GOPATH=$HOME/go
        export GOBIN=$(go env GOPATH)/bin
        export PATH=$PATH:$GOPATH
        export PATH=$PATH:$GOBIN
        mkdir -p $GOPATH/pkg
        mkdir -p $GOBIN
        mkdir -p $GOPATH/src/github.com/$GITHUB_REPOSITORY
        mv $(pwd)/* $GOPATH/src/github.com/$GITHUB_REPOSITORY
        cd $GOPATH/src/github.com/$GITHUB_REPOSITORY
@damccorm

This comment has been minimized.

Copy link
Collaborator

commented Aug 19, 2019

Ok, have put some more thought into this and I think I agree that we should provide defaults here. Here's my current wip plan:

  • GOPATH gets set to the current working directory. Unless you intentionally do something funny here (e.g. change the working-directory, this will be the root of your repository.
  • GOBIN gets set to $GOPATH/bin.
  • If the user scopes GOPATH or GOBIN into the step via the env block, those paths will be respected. So for example if your workflow looks like:
steps:
- uses: actions/setup-go@v1
  with:
    version: '1.12'
  env:
    GOPATH: $HOME

then GOPATH would be set to $HOME and GOBIN would be set to $HOME/bin

  • Both GOPATH and GOBIN are added to the path
  • $GOPATH/bin and $GOPATH/pkg are created if necessary.

Thoughts? Does that make sense?

@damccorm damccorm self-assigned this Aug 19, 2019

@radu-matei

This comment has been minimized.

Copy link
Author

commented Aug 19, 2019

This looks good to me, thanks!

Additional comments:

  • on any Go version that still relies on $GOPATH, the source code must be moved in $GOPATH/src/github.com/{org/user}/{repo}

  • $GOPATH/bin and $GOPATH/pkg must exist

Is there any chance all of these would be provided by default?

@damccorm

This comment has been minimized.

Copy link
Collaborator

commented Aug 19, 2019

$GOPATH/bin and $GOPATH/pkg must exist

That makes sense, updated my comment accordingly to create these if necessary.

on any Go version that still relies on $GOPATH, the source code must be moved in $GOPATH/src/github.com/{org/user}/{repo}

Want to make sure I understand correctly - you're suggesting that we rearrange the directory structure so that it looks like:

- $GOPATH
-- bin
-- pkg
-- src
--- github.com
---- {org/user}
----- {repo}
------ {repo contents}

I see why this is desirable, but I'm a little hesitant to do something that heavy/of that magnitude - to do that we'd need to move the entire repo and it would be hard to make it completely transparent to the end user what was happening. I'm more inclined to leave that for the user to do in a script step (though maybe we can provide some examples on the Readme).

Does that seem reasonable?

@radu-matei

This comment has been minimized.

Copy link
Author

commented Aug 19, 2019

I understand the hesitation - that being said, samples in the readme showing what needs to be done for $GOPATH and non-$GOPATH scenarios would be really helpful.

Thanks!

@kjk

This comment has been minimized.

Copy link

commented Aug 22, 2019

  • GOPATH gets set to the current working directory. Unless you intentionally do something funny here (e.g. change the working-directory, this will be the root of your repository.

I think it's better to keep GOPATH outside of the checked out code i.e. the current value of /home/runner/go seems fine.

Let's assume that my kjk/notionapi repo is checked out under /home/runner/work/notionapi.

If GOPATH is also /home/runner/work/notionapi, if I do go get honnef.co/go/tools/cmd/staticcheck to install staticcheck (to run it as part of workflow), the binary will be installed in my repo /home/runner/work/notionapi/bin/staticcheck and go.mod will be modified.

This is probably fine most of the time but I bet that it'll break tests for some people because they didn't expect the checkout to be polluted by artifacts of go get.

This also allows relatively easy work-around for people still relying on GOPATH as opposed to modules.

Assuming that my kjk/notionapi repo relies on GOPATH, it seems I could use path arg to actions/checkout and set it to /home/runner/go/src/github.com/kjk/notionapi and everything else should just work.

When GOPATH is the same as checkout / working directory, more gymnastics will be needed to move the source around.

@crazy-max

This comment has been minimized.

Copy link

commented Aug 25, 2019

Hi guys,

I also encountered this problem and I managed to solve it by adding this step. Works on all environments:

      -
        name: Set GOPATH
        # temporary fix
        # see https://github.com/actions/setup-go/issues/14
        run: |
          echo "##[set-env name=GOPATH;]$(dirname $GITHUB_WORKSPACE)"
          echo "##[add-path]$(dirname $GITHUB_WORKSPACE)/bin"
        shell: bash
@vcabbage

This comment has been minimized.

Copy link

commented Aug 25, 2019

I modified my fork to add the bin directories to PATH based on the output of go env. This will use the default locations or the env var if set. I'd be happy to contribute these changes if desired. (I also added support for installing "tip", which is a requirement for me. I can separate that logic out.)

Few notes:

  • GOPATH/{pkg,src,bin} directories will be created by the toolchain as needed, no need to create them explicitly.
  • I agree that setting GOPATH to the workspace directory is probably not wanted in many cases.
  • Another complication that arises from attempting to place the checkout in the proper GOPATH location is vanity imports. For example, though my repo is github.com/vcabbage/amqp it uses vanity imports and must be placed at GOPATH/src/pack.ag/aqmp.
@nathany

This comment has been minimized.

Copy link

commented Aug 30, 2019

GOPATH is being slowly phased out.

I'm adopting this approach for tools like golint via the Go Wiki.

Even a matrix build on the past few versions of Go (go 1.11 through go 1.13) could work without GOPATH if using modules. An example in the README of checking out code to the GOPATH would be useful for projects that aren't yet using Go modules, but I don't think GOPATH should be set (to the workspace).

Setting GOBIN to $PWD/bin or similar, and adding GOBIN to to the front of the PATH would be handy, though I don't know if it's possible for environment variable changes in setup-go to persist to other steps? I think just including golint in the docs would help people out.

- name: Lint
  run: |
    GOBIN=$PWD/bin go install golang.org/x/lint/golint
    ./bin/golint ./...
@radu-matei

This comment has been minimized.

Copy link
Author

commented Aug 30, 2019

Of course people will invariably have opinions about this.
Regardless, a combination of documentation and configuration (user input that determines which paths should be set, for example) should be enough to cover all preferences.

@mvdan

This comment has been minimized.

Copy link

commented Sep 3, 2019

I don't think setup-go should be setting GOPATH at all. Recent Go versions already deafult this to $HOME/go, and I agree with others that the current directory should not be inside GOPATH.

The same applies to GOBIN; it already defaults to $GOPATH/bin. Why do we need to set it explicitly? I've only seen it used like $GOBIN/some-binary when PATH isn't being set up correctly, but we can just fix that instead.

@kjk

This comment has been minimized.

Copy link

commented Sep 3, 2019

I don't think setup-go should be setting GOPATH at all. Recent Go versions already default this to $HOME/go,

I think it would be good GOPATH to be consistent across Go versions.

When it's set by default by Go toolchain to $HOME/go, leave it alone.

If it's not set by default, do set it.

Otherwise people will have to do it themselves in scripts as long as they use Go versions that don't set it by default.

@mvdan

This comment has been minimized.

Copy link

commented Sep 3, 2019

Fair enough, I guess it's fine to set GOPATH=$HOME/go for older Go versions. Noone should run into any inconsistencies, as long as they use $(go env GOPATH) and not $GOPATH directly.

@kjk

This comment has been minimized.

Copy link

commented Sep 3, 2019

I think $(go env GOPATH) wouldn't work on Windows.

@mvdan

This comment has been minimized.

Copy link

commented Sep 3, 2019

I keep forgetting that the default Windows shell is cmd :) I don't think it matters much as long as #14 is fixed.

@crazy-max

This comment has been minimized.

Copy link

commented Sep 3, 2019

As stipulated in doc:

For convenience, add the workspace's bin subdirectory to your PATH:

$ export PATH=$PATH:$(go env GOPATH)/bin

The scripts in the rest of this document use $GOPATH instead of $(go env GOPATH) for brevity. To make the scripts run as written if you have not set GOPATH, you can substitute $HOME/go in those commands or else run:

$ export GOPATH=$(go env GOPATH)

So maybe this part of code should be changed: https://github.com/actions/setup-go/blob/master/src/installer.ts#L112-L121

gesellix added a commit to gesellix/couchdb-prometheus-exporter that referenced this issue Sep 19, 2019
gesellix added a commit to gesellix/couchdb-prometheus-exporter that referenced this issue Sep 19, 2019
gesellix added a commit to gesellix/couchdb-prometheus-exporter that referenced this issue Sep 20, 2019
add ci action workflow (#50)
add action workflow

We should keep following actions/setup-go#12 to reduce custom GOPATH configs and PATH adjustments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
You can’t perform that action at this time.