Skip to content

Fix go.mod; simplify pkg/mount.Mounted#724

Merged
rhatdan merged 2 commits into
containers:masterfrom
kolyshkin:go-mod+mounted
Sep 28, 2020
Merged

Fix go.mod; simplify pkg/mount.Mounted#724
rhatdan merged 2 commits into
containers:masterfrom
kolyshkin:go-mod+mounted

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

Carry of #720
Closes: #723

1. go.mod: fix go version

Commit 1a28477 (inadvertently?) changed go version in go.mod from 1.13
to 1.15.

A "go " line in go.mod sets minimum go version required for the
package (see [1]). I don't think containers/storage suddenly requires go
1.15+.

Set it to 1.14, which is currently the oldest supported release.

[1] https://golang.org/ref/mod#go-mod-file-go

2. pkg/mount: rm path normalization from Mounted

Since moby/sys/mountinfo v0.2.0, the path does not have to be
normalized, so drop our wrapper.

Note this slightly change the call semantics: previously, a non-existing
path resulting in an error (wrapped ENOENT), while now it will return
"not mounted". The change is intentional -- the call is not to check if
the path exists, it is to check whether it is mounted.

Commit 1a28477 (inadvertently?) changed go version in go.mod from 1.13
to 1.15.

A "go <version>" line in go.mod sets minimum go version required for the
package (see [1]). I don't think containers/storage suddenly requires go
1.15+.

Set it to 1.14, which is currently the oldest supported release.

[1] https://golang.org/ref/mod#go-mod-file-go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since moby/sys/mountinfo v0.2.0, the path does not have to be
normalized, so drop our wrapper.

Note this slightly change the call semantics: previously, a non-existing
path resulting in an error (wrapped ENOENT), while now it will return
"not mounted". The change is intentional -- the call is not to check if
the path exists, it is to check whether it is mounted.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin mentioned this pull request Sep 24, 2020
Copy link
Copy Markdown
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Sep 25, 2020

@vrothberg
Copy link
Copy Markdown
Member

vrothberg commented Sep 25, 2020

While I am okay to revert the go version in the module I think we need to agree what it actually means. To my understanding, it does not set a lower bound of which versions of go can compile the code. However, it allows for using new features of go tooling. For instance, go >= 1.14 will default to -mod=vendor if a ./vendor dir is present. Such features seem useful to me.

Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Conditional LGTM :^)

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Sep 28, 2020

LGTM

@rhatdan rhatdan merged commit 9881e1c into containers:master Sep 28, 2020
@kolyshkin
Copy link
Copy Markdown
Contributor Author

To my understanding, it does not set a lower bound of which versions of go can compile the code.

AFAIK this is about both tooling features as well as the code (I barely remember seeing some code-related error, alas I don't remember what it was).

@kolyshkin kolyshkin deleted the go-mod+mounted branch September 28, 2020 15:01
rhatdan added a commit that referenced this pull request Sep 29, 2020
Fix go.mod; simplify pkg/mount.Mounted

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
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.

3 participants