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 alternate way to install gotip #5618

Merged
merged 12 commits into from
Jun 15, 2024
Merged

Conversation

EraKin575
Copy link
Contributor

@EraKin575 EraKin575 commented Jun 13, 2024

Which problem is this PR solving?

#5601

Description of the changes

Fixed gotip workflow according to proposal

How was this change tested?

tested through https://github.com/EraKin575/test-project/actions/runs/9495245139/job/26167103130

Checklist

@EraKin575 EraKin575 requested a review from a team as a code owner June 13, 2024 06:06
@EraKin575 EraKin575 closed this Jun 13, 2024
@EraKin575 EraKin575 reopened this Jun 13, 2024
@EraKin575 EraKin575 changed the title Fixed Gotop workflow Fixed Gotip workflow Jun 13, 2024
echo "GOROOT=$(go env GOROOT)" >> $GITHUB_ENV
echo "GOTIP_BIN=$(go env GOPATH)/bin/gotip" >> $GITHUB_ENV

- name: Fail if Go Tip is not installed
Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary, we will see if the previous steps fail.

.github/actions/setup-go-tip/action.yml Outdated Show resolved Hide resolved
@yurishkuro
Copy link
Member

Please demonstrate how it was tested

EraKin575 and others added 2 commits June 13, 2024 22:36
Signed-off-by: EraKin575 <tejaskumar574@gmail.com>
@EraKin575
Copy link
Contributor Author

Please demonstrate how it was tested

I tested it by making a workflow of my own and then running the actions file: https://github.com/EraKin575/test-project

@yurishkuro yurishkuro changed the title Fixed Gotip workflow Add alternate way to install gotip Jun 13, 2024
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Jun 13, 2024
@yurishkuro
Copy link
Member

I tested it by making a workflow of my own and then running the actions file

in your test repo, can you add a step go version, to verify that the go command is now taken from gotip? The manual part in our action also sets PATH, but your additional step does not do that.

go install golang.org/dl/gotip@latest
gotip download
echo "GOROOT=$(go env GOROOT)" >> $GITHUB_ENV
echo "GOTIP_BIN=$(go env GOPATH)/bin/gotip" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

I see this in your run https://github.com/EraKin575/test-project/actions/runs/9504391060/job/26196935810#step:7:66

Success. You may now run 'gotip'!
Run echo "Active Go version:"
Active Go version:
go version go1.22.4 linux/amd[64](https://github.com/EraKin575/test-project/actions/runs/9504391060/job/26196935810#step:7:67)

This isn't gotip. And I was expecting that, because you're not setting up env vars the same way as we do in L38-41. For example, you do GOROOT=$(go env GOROOT) - that's just default Go version, nothing to do with gotip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
Someting like this?

Copy link
Member

Choose a reason for hiding this comment

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

Note - we could still make it work, I suppose, but then we need to change how the tests are run, e.g. make test GO=gotip

Copy link
Member

Choose a reason for hiding this comment

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

Something like this?

I don't like "designing by screenshots" - you should test it out. This is why we have the final step to print go version

EraKin575 and others added 3 commits June 14, 2024 23:52
@yurishkuro yurishkuro merged commit 3d421b7 into jaegertracing:main Jun 15, 2024
37 checks passed
@yurishkuro
Copy link
Member

I fixed it. After the manual build step, the gotip toolchain look good: https://github.com/jaegertracing/jaeger/actions/runs/9530915022/job/26271312690#step:5:125

Building packages and commands for linux/amd64.
---
Installed Go for linux/amd64 in /home/runner/sdk/gotip
Installed commands in /home/runner/sdk/gotip/bin
Success. You may now run 'gotip'!
Run echo Setup Go environment
Setup Go environment
go version devel go1.23-fe36ce6 Fri Jun 14 22:40:29 2024 +0000 linux/amd64
Run echo Check Go Version
Check Go Version
GOPATH=/home/runner/gotip
GOROOT=/home/runner/sdk/gotip
which go:
/home/runner/sdk/gotip/bin/go
/usr/bin/go
/bin/go
Active Go version:
go version devel go1.23-fe36ce6 Fri Jun 14 22:40:29 2024 +0000 linux/amd64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:ci Change related to continuous integration / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants