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

Remove remaining uses of pkg/homedir #2143

Closed
wants to merge 1 commit into from
Closed

Remove remaining uses of pkg/homedir #2143

wants to merge 1 commit into from

Conversation

SamWhited
Copy link

@SamWhited SamWhited commented Oct 18, 2019

This PR was created to address build failures when updating buildkit to vendor a more recent version of docker which is not currently usable by this package.

- What I did

Remove uses of "github.com/docker/docker/pkg/homedir".Get.

- How I did it

Replaced it with os.UserHomeDir which was added in Go 1.12 and is already in use in other places in this package.

- How to verify it

Run the tests and ensure everything builds. Comments explain why errors were unhandled to prevent confusion later.

Previous behavior can be found here.

- Description for the changelog

n/a

@thaJeztah
Copy link
Member

This was removed in #2101, but later reverted in #2111 (see the description there for reasons why)

/cc @tiborvass @tonistiigi

@SamWhited
Copy link
Author

This is still removed from pkg/homedir in more recent versions of Docker, should removing that function be reverted there too? That would also solve my problem with updating docker in buildkit.

@tonistiigi
Copy link
Member

I don't understand this. The function exists in moby master https://github.com/moby/moby/blob/master/pkg/homedir/homedir_unix.go#L21 . I think you just need to vendor the correct version.

@SamWhited
Copy link
Author

That's odd, I could swear I was looking at master, and the commit appears to be right. I'll experiment and try to figure out what I did to cause the wrong version to be vendored. Thanks.

@SamWhited SamWhited closed this Oct 18, 2019
@SamWhited
Copy link
Author

Turns out that Travis was picking up the "Linux" build constraint, but not "Unix" so the file where Get was defined was not being included. There was a comment that said that the tag osusergo was required when building statically, so I've added that. Unsure why it was working before, but not now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants