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

The package cannot be vendored with vndr (regression in #137) #139

Closed
AkihiroSuda opened this issue Jan 9, 2020 · 9 comments · Fixed by LK4D4/vndr#87
Closed

The package cannot be vendored with vndr (regression in #137) #139

AkihiroSuda opened this issue Jan 9, 2020 · 9 comments · Fixed by LK4D4/vndr#87

Comments

@AkihiroSuda
Copy link
Member

vndr omits the go-systemd package when github.com/coreos/go-systemd is specified in vendor.conf, due to the regression in #137.

vendor.conf:

github.com/containerd/cgroups 918ed86e29ccef2bf3f0c70d76405781afc3bdf5
github.com/coreos/go-systemd v22.0.0
...

vndr output:

...
2020/01/09 16:35:37 WARNING: package github.com/coreos/go-systemd is unused, consider removing it from vendor.conf

If github.com/coreos/go-systemd/v22 is specified in vendor.conf instead, vndr fails as follows:

2020/01/09 16:38:59 WARNING: package github.com/coreos/go-systemd/v22 is not root import, should be github.com/coreos/go-systemd

vndr version: LK4D4/vndr@d87a917

@AkihiroSuda
Copy link
Member Author

cc @Zyqsempai @LK4D4

@thaJeztah
Copy link
Member

Shouldn't those upstream packages use a /vX subdirectory?

@AkihiroSuda
Copy link
Member Author

Seems they intentionally chose not to use subdir coreos/go-systemd#307 (comment)

@thaJeztah
Copy link
Member

I'd like not to have to move directories around for each breaking change (i.e. /vX -> /vY)

Yeah; they'd still have to update import paths with each release though. I guess go mod is gonna be a pain whichever way.

Curious though; was it a deliberate choice to use both v1 and v5 of the godbus/dbus package in this repository? I see that this file did not move to v5;

"github.com/godbus/dbus"

Also wondering if moving (containerd) to go.mod (and using the vXX import paths) is a breaking change, thus would mean containerd will have to bump major version to v2+ because of that

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Jan 9, 2020

thus would mean containerd will have to bump major version to v2+ because of that

Kubernetes didn't bump major

@thaJeztah
Copy link
Member

Does kubernetes actually use SemVer, or only "fake SemVer for the sake of go modules?" (as many packages)

AkihiroSuda added a commit to AkihiroSuda/vndr that referenced this issue Jan 9, 2020
Support vendoring github.com/foo/bar/v42 like the following:

`foo.go`:
```go
package foo

import (
        "github.com/coreos/go-systemd/v22/dbus"
)

func Foo() (*dbus.Conn, error) {
        return dbus.New()
}
```

`vendor.conf`:
```
github.com/coreos/go-systemd/v22 v22.0.0
github.com/godbus/dbus/v5 v5.0.3
```

Now `vndr` checks out the contents of `github.com/coreos/[email protected]` repo
into the `./vendor/github.com/coreos/go-systemd/v22` directory.

Note that `vndr` does not verify the actual version number written in `go.mod`.
So `vndr github.com/coreos/go-systemd/v23 v22.0.0` would vendor v22, not v23.

Fix containerd/cgroups#139

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/vndr that referenced this issue Jan 9, 2020
Support vendoring github.com/foo/bar/v42 like the following:

`foo.go`:
```go
package foo

import (
        "github.com/coreos/go-systemd/v22/dbus"
)

func Foo() (*dbus.Conn, error) {
        return dbus.New()
}
```

`vendor.conf`:
```
github.com/coreos/go-systemd/v22 v22.0.0
github.com/godbus/dbus/v5 v5.0.3
```

Now `vndr` checks out the contents of `github.com/coreos/[email protected]` repo
into the `./vendor/github.com/coreos/go-systemd/v22` directory.

Note that `vndr` does not verify the actual version number written in `go.mod`.
So `vndr github.com/coreos/go-systemd/v23 v22.0.0` would vendor v22, not v23.

Fix containerd/cgroups#139

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Member Author

Fix for vndr: LK4D4/vndr#87

AkihiroSuda added a commit to AkihiroSuda/vndr that referenced this issue Jan 9, 2020
Support vendoring `github.com/coreos/go-systemd/v22` as follows:

`foo.go`:
```go
package foo

import (
        "github.com/coreos/go-systemd/v22/dbus"
)

func Foo() (*dbus.Conn, error) {
        return dbus.New()
}
```

`vendor.conf`:
```
github.com/coreos/go-systemd/v22 v22.0.0
github.com/godbus/dbus/v5 v5.0.3
```

Now `vndr` checks out the contents of `github.com/coreos/[email protected]` repo
into the `./vendor/github.com/coreos/go-systemd/v22` directory.

Note that `vndr` does not verify the actual version number written in `go.mod`.
So `vndr github.com/coreos/go-systemd/v23 v22.0.0` would vendor v22, not v23.

Fix containerd/cgroups#139

Signed-off-by: Akihiro Suda <[email protected]>
@thaJeztah
Copy link
Member

Fix for vndr: LK4D4/vndr#87

🤗 LOL literally just now opened vndr in my IDE to see if I could fix it there 😂

LK4D4 pushed a commit to LK4D4/vndr that referenced this issue Jan 9, 2020
Support vendoring `github.com/coreos/go-systemd/v22` as follows:

`foo.go`:
```go
package foo

import (
        "github.com/coreos/go-systemd/v22/dbus"
)

func Foo() (*dbus.Conn, error) {
        return dbus.New()
}
```

`vendor.conf`:
```
github.com/coreos/go-systemd/v22 v22.0.0
github.com/godbus/dbus/v5 v5.0.3
```

Now `vndr` checks out the contents of `github.com/coreos/[email protected]` repo
into the `./vendor/github.com/coreos/go-systemd/v22` directory.

Note that `vndr` does not verify the actual version number written in `go.mod`.
So `vndr github.com/coreos/go-systemd/v23 v22.0.0` would vendor v22, not v23.

Fix containerd/cgroups#139

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Jan 10, 2020

This was fixed in vndr v0.1.0

AkihiroSuda added a commit to AkihiroSuda/containerd that referenced this issue Jan 13, 2020
Note: now vndr >= v0.10 is required (containerd/cgroups#139)

Signed-off-by: Akihiro Suda <[email protected]>
dims added a commit to dims/containerd that referenced this issue Mar 12, 2020
Same as e1221e6

Note: now vndr >= v0.10 is required (containerd/cgroups#139)

Signed-off-by: Davanum Srinivas <[email protected]>
tussennet pushed a commit to tussennet/containerd that referenced this issue Sep 11, 2020
Note: now vndr >= v0.10 is required (containerd/cgroups#139)

Signed-off-by: Akihiro Suda <[email protected]>
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 a pull request may close this issue.

3 participants
@thaJeztah @AkihiroSuda and others