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

Add JSON output of go env and some env as strings #334

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucacome
Copy link

@lucacome lucacome commented Feb 16, 2023

Description:
Additional outputs are:

  • GOPATH as go-path string
    - GOROOT as go-root string
  • GOMOD as go-mod string
  • GOCACHE as go-cache string
  • GOMODCACHE as go-mod-cache string
  • go env as go-env JSON

Related issue:
Closes #54

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@lucacome
Copy link
Author

@marko-zivic-93 could you approve the CI run again? 🙏

@lucacome
Copy link
Author

Not really sure why it failed basic validation on windows...maybe a flaky test?

@lucacome lucacome force-pushed the add-env branch 2 times, most recently from 7d76f7a to 2c81452 Compare March 14, 2023 01:02
@mvdan
Copy link

mvdan commented Mar 27, 2023

action.yml appears to be missing go-cache, FYI. It would be nice to include all the other values from go env as well. For example, GOOS, GOARCH, GOPROXY, and GOAMD64 may be useful. We could carefully maintain a list of which ones are interesting or useful, but I would personally just expose all 39 of them. I imagine we could auto-generate the list to include in action.yaml.

I'm also thinking that it would be nicest if we could use the exact same names that go env uses. Otherwise I have to remind myself how each one of them was transformed, because there's a rather arbitrary choice about where the dashes are inserted. For example, would GOSUMDB be go-sumdb or go-sum-db? Some of its names, like CGO_ENABLED, already use underscores as well - so either cgo-enabled or cgo_enabled would be weird.

I would personally expose each one of them as-is, uppercase and all, like steps.setup-go.outputs.GOCACHE. Or, if we want them to have a common prefix to clarify that they come from go env, perhaps steps.setup-go.outputs.env_GOCACHE, though I don't think it's necessary. The fact that they are uppercase should be enough of a signal.

@lucacome
Copy link
Author

Thanks for catching that @mvdan I've added go-cache to action.yml.

As for the naming, I was using the same pattern used in the other variables, but I'm happy to change them if people think that's a better approach. As for exposing all of them, I didn't want to pollute the outputs and I'm not sure how we could auto-generate them... 😅

Some guidance from the maintainers would be appreciated 🙏

@lucacome
Copy link
Author

lucacome commented Jun 6, 2023

is there anything I can do to get this PR reviewed?

@mvdan
Copy link

mvdan commented Jun 6, 2023

cc @dmitry-shibanov :)

@Erictorrm
Copy link

Duplicate of #

@IvanZosimov IvanZosimov self-requested a review July 17, 2023 07:33
@dsame dsame linked an issue Aug 1, 2023 that may be closed by this pull request
5 tasks
core.setOutput('go-root', goEnvJson['GOROOT']);
core.setOutput('go-cache', goEnvJson['GOCACHE']);
core.setOutput('go-mod-cache', goEnvJson['GOMODCACHE']);
core.setOutput('go-env', goEnvJson);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @lucacome

Can you please remove the most outputs except these 3 ones

    core.setOutput('go-version', parsedGoVersion);
    core.setOutput('go-cache', goEnvJson['GOCACHE']);
    core.setOutput('go-mod-cache', goEnvJson['GOMODCACHE']);

The others does not seem relate to any real-world use cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dsame

I'm personally interested in go-path so I would like to keep that one as well 😅
Are you saying to also remove core.setOutput('go-env', goEnvJson); ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still of the opinion that the entire go env -json should be exposed in its entirety - I find most of the vars useful.

Additional outputs are:
- GOPATH as `go-path` string
- GOMOD as `go-mod` string
- GOCACHE as `go-cache` string
- GOMODCACHE as `go-mod-cache` string
- `go env` as `go-env` JSON
@lucacome
Copy link
Author

What needs to be done to get this merged?

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

Successfully merging this pull request may close these issues.

Support for Outputting or Exporting Go Environment Variables
4 participants