Skip to content

Replace docker/pkg/fileutils with github.com/moby/patternmatcher#136

Merged
tonistiigi merged 4 commits intotonistiigi:masterfrom
thaJeztah:replace_docker
Sep 30, 2022
Merged

Replace docker/pkg/fileutils with github.com/moby/patternmatcher#136
tonistiigi merged 4 commits intotonistiigi:masterfrom
thaJeztah:replace_docker

Conversation

@thaJeztah
Copy link
Collaborator

@thaJeztah thaJeztah commented Sep 30, 2022

@thaJeztah thaJeztah mentioned this pull request Sep 30, 2022
Merged
@thaJeztah thaJeztah requested review from aaronlehmann and tonistiigi and removed request for aaronlehmann September 30, 2022 19:59
@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

@tonistiigi @aaronlehmann @cpuguy83 PTAL (I haven't tagged a release yet, but can do if we're all happy with the current state)

@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

hm.. something is failing; let me check

@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

Ah; linter is still running go 1.17; need to update github actions to go 1.18/1.19; let me open a PR

 go mod tidy: go.mod file indicates go 1.18, but maximum supported version is 1.17

This was referenced Sep 30, 2022
Closed
Merged
@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

opened #137

@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Sep 30, 2022

Thanks for doing this!

@thaJeztah thaJeztah mentioned this pull request Sep 30, 2022
Closed
@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

It kept dropping off my list to do. Not entirely out of the woods yet; see moby/moby#44227, but we can temporarily depend on the aliases I created in docker/docker for that (until the buildkit version we use there is compatible with the latest tonistiigi/fsutil.

@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

Ah, dang... and more to do; looks like we're still using io/ioutil in this repo;

#26 9.826 fs.go:6:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
#26 9.826 	"io/ioutil"
#26 9.826 	^

@thaJeztah
replace uses of deprecated io/ioutil
9814930 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
thaJeztah commented Sep 30, 2022
View reviewed changes
copy/copy_test.go
Comment on lines -160 to +146
t3, err := ioutil.TempDir("", "test")
require.NoError(t, err)
defer os.RemoveAll(t2)
t3 := t.TempDir()
Copy link
Collaborator Author

@thaJeztah thaJeztah Sep 30, 2022

Choose a reason for hiding this comment

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

I saw a couple of these, and I guess those were copy/paste errors; we create t3, but add a defer to remove t2 (I guess CI will show if it was not intentional 😂)

@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

Nice; all green now; I'll tag v0.5.0, and update this PR 👍

@thaJeztah
replace pkg/fileutils with github.com/moby/patternmatcher
258c13f 
The pkg/fileutils package was moved to a separate module / repository.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
bench: update docker to v20.10.18
a485dce 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Update Dockerfile syntax to latest stable
d38ad66 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Collaborator Author

thaJeztah commented Sep 30, 2022

done; should all be good now

@thaJeztah thaJeztah mentioned this pull request Sep 30, 2022
Merged
@tonistiigi tonistiigi merged commit 4638ad6 into tonistiigi:master Sep 30, 2022
@thaJeztah thaJeztah deleted the replace_docker branch September 30, 2022 23:01
@thaJeztah thaJeztah mentioned this pull request Sep 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

@aaronlehmann aaronlehmann aaronlehmann approved these changes

+1 more reviewer

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @aaronlehmann @tonistiigi @cpuguy83