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.

Sign up for GitHub

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

Jump to bottom

Modules not working (canonical imports wrong?) #127

Closed
thockin opened this issue Sep 12, 2021 · 22 comments · Fixed by #128
Closed

Modules not working (canonical imports wrong?) #127

thockin opened this issue Sep 12, 2021 · 22 comments · Fixed by #128

Comments

@thockin
Copy link

thockin commented Sep 12, 2021 

thockin-glaptop4 go-build-template 6d246 master /tools$ go get -d github.com/estesp/manifest-tool
go get: github.com/codegangsta/[email protected]: parsing go.mod:
	module declares its path as: github.com/urfave/cli
	        but was required as: github.com/codegangsta/cli

thockin-glaptop4 go-build-template 6d246 master /tools$ go get -d github.com/estesp/[email protected]
go get: github.com/codegangsta/[email protected]: parsing go.mod:
	module declares its path as: github.com/urfave/cli
	        but was required as: github.com/codegangsta/cli
@estesp estesp mentioned this issue Sep 14, 2021
Merged
@estesp
Copy link
Owner

estesp commented Sep 14, 2021

Hmm; I partially solved this, but just realized I walked right into the fun v2/2.0.0 "problem" with modules. I'm going to need to move everything to a v2/ and tag another v2 beta with those changes so you can actually go get the v2.0.0 (main dev branch) content instead of the go tools picking v1.0.3 (which doesn't have the redirect fix for urfave).

@thockin
Copy link
Author

thockin commented Sep 14, 2021 via email

@estesp
Copy link
Owner

estesp commented Sep 15, 2021

So, I think I can do a v1.0.4 although there are things that are still broken in the 1.x manifest-tool. Maybe a better question (after spending a few hours fighting go module stuff) is what do you want to be the result of go get? I've recently moved up to 1.17 and go get doesn't install binaries, and go install has been purposefully "broken" to not work if the go.mod has replace directives, which I can't remove because of the original problem for which you opened the issue. :)

I did make it possible for go get github.com/estesp/manifest-tool/[email protected] to work; but I'm not even sure if that is doing what you want since it no longer installs binaries.

@thockin
Copy link
Author

thockin commented Sep 15, 2021 via email •
edited
Loading

@marcosnils
Copy link

marcosnils commented Sep 15, 2021 •
edited
Loading

👋 wrote an answer that I later deleted since I missed the tools.go detail. Just pushed an updated example here (https://github.com/marcosnils/test) where you can clone that repo and perform the go install "hack" against the manifest-tool repo successfully. The "problem" here is that since replace directives are not being taken into account, go chooses the v1.22.2 version of the urfave dependency which contains a regression that still hasn't been fixed upstream.

In any case, hope this could be useful somehow 🤷‍♂️

@marcosnils
Copy link

marcosnils commented Sep 15, 2021 •
edited
Loading

Update: Interestingly, if you add the same replace directive in your repo, the go install works as intended. Just verified that the resulting manifest-tool binary has the correct replacement with go version -m $(which manifest-tool).

Just updated my sample repo to reflect this.

@thockin
Copy link
Author

thockin commented Sep 15, 2021

It still doesn't work:

$ go get -d github.com/estesp/[email protected]
go get: github.com/codegangsta/[email protected]: parsing go.mod:
	module declares its path as: github.com/urfave/cli
	        but was required as: github.com/codegangsta/cli

I added replace github.com/urfave/cli => github.com/urfave/cli v1.22.1 to go.mod.

:(

@marcosnils
Copy link

marcosnils commented Sep 15, 2021 •
edited
Loading

$ go get -d github.com/estesp/[email protected]

1.0.3 doesn't have any go.mod definitions, hence that's why you're getting dependency errors.

this works go get -v -d github.com/estesp/manifest-tool/v2@latest. That's what I tried before in my previous comment.

@estesp
Copy link
Owner

estesp commented Sep 15, 2021

If you are wanting to rely on 1.0.x I can try and work up a go.mod there and cut a v1.0.4 release; as @marcosnils said, so far I was focused on trying to fix it for the main branch which is now the v2 codebase.

The v1 code was still using vndr and never supported Go modules properly as that was several years ago.

@thockin
Copy link
Author

thockin commented Sep 15, 2021 via email

@thockin
Copy link
Author

thockin commented Sep 15, 2021 via email

@thockin
Copy link
Author

thockin commented Sep 15, 2021

Hmm... The go get -d worked:

$ go get -d github.com/estesp/manifest-tool/v2
go: downloading github.com/estesp/manifest-tool/v2 v2.0.0-beta.1
go: downloading golang.org/x/net v0.0.0-20210226172049-e18ecbb05110
go get: added github.com/estesp/manifest-tool/v2 v2.0.0-beta.1
go get: upgraded golang.org/x/net v0.0.0-20191119073136-fc4aabc6c914 => v0.0.0-20210226172049-e18ecbb05110
go get: upgraded golang.org/x/sys v0.0.0-20191119060738-e882bf8e40c2 => v0.0.0-20210426230700-d19ff857e887

But I am still missing something - how do I build it?

$ grep estesp go.mod 
	github.com/estesp/manifest-tool/v2 v2.0.0-beta.1

$ go build -o ../bin/tools github.com/estesp/manifest-tool
no required module provides package github.com/estesp/manifest-tool; to add it:
	go get github.com/estesp/manifest-tool

$ go build -o ../bin/tools github.com/estesp/manifest-tool/v2
no required module provides package github.com/estesp/manifest-tool/v2; to add it:
	go get github.com/estesp/manifest-tool/v2

$ go build -o ../bin/tools github.com/estesp/manifest-tool@v2
package github.com/estesp/manifest-tool@v2: can only use path@version syntax with 'go get' and 'go install' in module-aware mode

$ GOBIN=`pwd`/../bin/tools go install github.com/estesp/manifest-tool@v2
go install: github.com/estesp/manifest-tool@v2: no matching versions for query "v2"

$ GOBIN=`pwd`/../bin/tools go install github.com/estesp/manifest-tool
no required module provides package github.com/estesp/manifest-tool; to add it:
	go get github.com/estesp/manifest-tool

$ GOBIN=`pwd`/../bin/tools go install github.com/estesp/manifest-tool/v2
no required module provides package github.com/estesp/manifest-tool/v2; to add it:
	go get github.com/estesp/manifest-tool/v2

Other (non-v2) tools work:

$ grep licenses go.mod
	github.com/google/go-licenses v0.0.0-20210816172045-3099c18c36e1

$ GOBIN=`pwd`/../bin/tools go install github.com/google/go-licenses
$

$ go build -o ../bin/tools github.com/google/go-licenses
$

@marcosnils
Copy link

marcosnils commented Sep 15, 2021 •
edited
Loading

But I am still missing something - how do I build it?

go install github.com/estesp/manifest-tool/v2/cmd/manifest-tool

^ if you run that command inside your tools project, it'll use the version you have defined in your go.mod and put the manifest-tool binary in your $GOPATH/bin directory.

@thockin
Copy link
Author

thockin commented Sep 15, 2021 via email

@estesp
Copy link
Owner

estesp commented Sep 15, 2021

I feel dumb. THanks.

At least I'm not the only one. 😂 Thanks @marcosnils for your help and example here!

@marcosnils
Copy link

I feel dumb. THanks.
At least I'm not the only one. Thanks @marcosnils for your help and example here!

We all feel the same 🤗

@thockin
Copy link
Author

thockin commented Sep 15, 2021

I will repeat, so it doesn't get lost: Is there a root cause for this conflicting import name which could be merged back to eliminate this replace ?

@marcosnils
Copy link

marcosnils commented Sep 15, 2021 •
edited
Loading

Is there a root cause for this conflicting import name which could be merged back to eliminate this replace

hey Tim, the replace directive is not there to prevent the conflicting import name, it prevents a regression on the urfave/cli library that hasn't yet been fixed upstream as indicated in the go.mod comment (urfave/cli#1092), Seems like the project kind of stalled and it was never fixed.

The main reason that you're getting the import name error when doing go get is because v1 and v2 are basically different packages. So basically to get v2 it's important to specify the proper /v2 suffix in the go get command.

AFAIK the right way to get rid of that replace directive is to either bump urfave/cli to v2 (as long as it fixes the original regressing), wait them to craft a new v1, or just fork the lib and fix it there.

@thaJeztah
Copy link

It's complicated; think the main issue is that codegansta/cli added a go.mod with the new name (urfave/cli) without updating the major version.

Versions until v1.20 had no go.mod, so could be found under the old name; https://pkg.go.dev/github.com/codegangsta/cli?tab=versions

However, because those versions didn't have a go.mod, it appears they're also indexed under the new name; https://pkg.go.dev/github.com/urfave/cli?tab=versions

Because go modules require SemVer (and consider versions to be SemVer if it can be parsed as such), it looks for "latest v1.x", but the urfave/cli introduced a breaking change; it added a go.mod declaring the new name for the project in the go.mod.

Changing the module name is a breaking change so requires a major version update (v2), and the import paths to be updated accordingly.

That ship has sailed for the urfave/cli project, as there's already a v2 (and v1 releases with a go.mod)

So yes, I think the only way to allow using the v1.x versions of manifest-tool with go modules enabled, is to rename the import paths in a v1.0.x release.

Or use the v2 (which should work with go modules) (go get -d github.com/estesp/manifest-tool/v2@latest).

@estesp
Copy link
Owner

estesp commented Jul 26, 2022

With v2.0.5 (just released), you can now install the manifest-tool command with:

$ go install github.com/estesp/manifest-tool/v2/cmd/[email protected]
...
$ ~/go/bin/manifest-tool -version
/go/bin/manifest-tool version 2.0.5 (commit: )

You can also create a new go.mod for a new project and properly do go get github.com/estesp/manifest-tool/v2 and it works without any replace rules as well.

@thockin
Copy link
Author

thockin commented Jul 26, 2022

Weird:

go get github.com/estesp/manifest-tool/[email protected]
go: github.com/deislabs/[email protected] requires
	github.com/docker/[email protected]: invalid version: unknown revision 000000000000

@thockin
Copy link
Author

thockin commented Jul 26, 2022

Huh, blowing away go.mod and starting fresh makes it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add replace directive for urfave/cli estesp/manifest-tool
4 participants
@estesp @marcosnils @thaJeztah @thockin