-
Notifications
You must be signed in to change notification settings - Fork 21
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
Run go mod tidy
after go mod edit -replace
#39
Conversation
This lets you *not* specify a correctly formatted version for a replace but let you use branch names or `latest` or nothing (which is `latest`)
I did test with a forked extension and with local repo for the main k6 project and both worked fine after the update so hopefully nothing is broken 🤞 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hhmm this is breaking xk6 run
from within the xk6-browser repo:
Log
$ XK6_BROWSER_LOG=info ~/Projects/grafana/xk6/xk6 run -q examples/226-waitfornavigation-timeout.js
2022/02/17 10:51:26 [INFO] Temporary folder: /tmp/buildenv_2022-02-17-1051.1260111165
2022/02/17 10:51:26 [INFO] Writing main module: /tmp/buildenv_2022-02-17-1051.1260111165/main.go
2022/02/17 10:51:26 [INFO] Initializing Go module
2022/02/17 10:51:26 [INFO] exec (timeout=10s): /home/ivan/.local/go/bin/go mod init k6
go: creating new go.mod: module k6
go: to add module requirements and sums:
go mod tidy
2022/02/17 10:51:26 [INFO] Replace github.com/grafana/xk6-browser => /home/ivan/Projects/grafana/xk6-browser
2022/02/17 10:51:26 [INFO] exec (timeout=10s): /home/ivan/.local/go/bin/go mod edit -replace github.com/grafana/xk6-browser=/home/ivan/Projects/grafana/xk6-browser
2022/02/17 10:51:26 [INFO] exec (timeout=0s): /home/ivan/.local/go/bin/go mod tidy
go: finding module for package go.k6.io/k6/cmd
go: found github.com/grafana/xk6-browser in github.com/grafana/xk6-browser v0.0.0-00010101000000-000000000000
go: found go.k6.io/k6/cmd in go.k6.io/k6 v0.36.0
go: finding module for package gopkg.in/tomb.v1
go: found gopkg.in/tomb.v1 in gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
k6 imports
github.com/grafana/xk6-browser imports
go.k6.io/k6/js/common imports
github.com/serenize/snaker tested by
github.com/serenize/snaker.test imports
github.com/onsi/ginkgo imports
github.com/onsi/ginkgo/internal/remote imports
github.com/nxadm/tail imports
github.com/nxadm/tail/watch imports
github.com/fsnotify/fsnotify loaded from github.com/fsnotify/fsnotify@v1.4.7,
but go 1.16 would select v1.4.9
To upgrade to the versions selected by go 1.16:
go mod tidy -go=1.16 && go mod tidy -go=1.17
If reproducibility with go 1.16 is not needed:
go mod tidy -compat=1.17
For other options, see:
https://golang.org/doc/modules/pruning
2022/02/17 10:51:27 [INFO] Cleaning up temporary folder: /tmp/buildenv_2022-02-17-1051.1260111165
2022/02/17 10:51:27 [ERROR] exit status 1
Whereas it works on the current master
. 😕 I'm on go 1.17.3.
And unrelated to these changes, but I'm actually noticing the same error from here and here, except when running xk6 build v0.36.0 --output xk6-browser --with github.com/grafana/xk6-browser@main
(this happens both on the current xk6
master and on this branch). I wiped my cache just in case... 😕 EDIT: Hhmm for some reason v0.1.3 is always selected with @main
or @917b83ef14b3e2075f92c092163c47e359883a6a
, which is not compatible with k6 v0.36.0. Is this a regression or did it never work like this?
And a related comment: could you add some documentation for --replace
in the README, with some usage examples? Currently it's not clear when it should be used for or what it accomplishes.
This was done in #33 ? |
I know 😄 But coming back to it I'm lapsing on when it's useful, and have to go back and read #32, which probably means the README could explain when it's useful, along with some examples. But you're right, it's unrelated to these changes, we can do that in another PR. |
It is ... literally what |
I think this is because of https://go.dev/doc/go1.17#go-command . This does not happen if you run with go1.18 and is unrelated to this change AFAIK as well. I guess we can add |
Sure, but within the context of xk6, which is a wrapper around the go toolchain, we should explain when this is useful (e.g. when you want to replace a dependency of the extension without importing it), instead of "go see what
I wouldn't want to make 1.17 the minimum required version, but if that's the only way of getting this to work, then sure. In that case, let's update the README and |
This will now error out with
which is not great but I guess it's somewhat better than erroring out for everybody using the latest version (which I would argue are the majority of the people - especially new users). Unfortunately capturing the output and checking it will be ... a lot of work which arguably will be outdated in few months when 1.18 is more ubiqutious and we in general no longer support go1.16. Or even depend on 1.18 in k6 or an extension 🤷 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @<commit-ish>
works properly now 🎉
To avoid that obscure flag provided but not defined: -compat
error, should we just check go version
up front and error out with a friendlier message if it's <1.17?
Getting the output of a command seems to involve a lot of work which IMO will not really payoff given that 1.17 is now 6 months old and 1.18 is getting out soon ™️ . I would expect by the time people update to this version |
This lets you not specify a correctly formatted version for a replace
but let you use branch names or
latest
or nothing (which islatest
)