Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Do not use filepath.Join if constructing LinuxKit paths #143

Merged
merged 1 commit into from
Aug 14, 2017

Conversation

justincormack
Copy link
Collaborator

This will do the wrong thing on Windows, and construct paths with .

fix #142

Signed-off-by: Justin Cormack justin.cormack@docker.com

@justincormack
Copy link
Collaborator Author

cc @rn

@ijc
Copy link
Collaborator

ijc commented Aug 14, 2017

I think you just want to use path.Join everywhere, unlike filepath.Join that uses / everywhere regardless of platform.

Copy link
Member

@rn rn left a comment

Choose a reason for hiding this comment

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

It might be cleaner to call the function filepathJoinUnix or something more explicit

src/moby/util.go Outdated

// we are largely handling Linux paths, so do not want to use filepath.Join()
func filepathJoin(paths ...string) string {
return strings.Join(paths, "/")
Copy link
Member

Choose a reason for hiding this comment

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

filepath.Join() also does a filepath.Clean(). This may not be relevant here...

@justincormack
Copy link
Collaborator Author

@ijc oh yes.

@justincormack justincormack force-pushed the filepathjoin branch 2 times, most recently from 9fd7d03 to 4fd019a Compare August 14, 2017 14:36
This will do the wrong thing on Windows, and construct paths with \.

fix moby#142

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
@justincormack
Copy link
Collaborator Author

Ok, reworked using path.Join which is the slash only only version

@justincormack justincormack merged commit d9546ee into moby:master Aug 14, 2017
@justincormack justincormack deleted the filepathjoin branch August 14, 2017 16:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'moby build' of a 'efi-iso' does not start any system containers
3 participants