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

filepath/clean: Add Windows support #539

Merged
merged 5 commits into from
Dec 13, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Dec 8, 2017

Also fix the Linux Clean handling for a/.. and /.. and add logic to compare the GOOS Clean and IsAbs results with the stdlib.

This is an alternative to #536.

This catches us up with how the Linux stdlib implementation handles
this case:

  $ cat test.go
  package main

  import (
    "fmt"
    "path/filepath"
  )

  func main() {
    fmt.Println(filepath.Clean("a/.."))
  }
  $ go run test
  .

by implementing the rule mentioned in [1].

[1]: https://golang.org/pkg/path/filepath/#Clean

Signed-off-by: W. Trevor King <wking@tremily.us>
And add tests to protect us from that in the future.

Also throw in a /.. test with a non-.. tail for good measure.

Signed-off-by: W. Trevor King <wking@tremily.us>
For the test cases that match the current GOOS.  This ensures we stay
consistent (at least for platforms where we run tests).

Also drop an unecessary trailing newline from the old error message.

Signed-off-by: W. Trevor King <wking@tremily.us>
For the test cases that match the current GOOS.  This ensures we stay
consistent (at least for platforms where we run tests).

Also drop an unecessary trailing newline from the old error message.

I don't compare for Abs, because the stdlib doesn't have a
configurable cwd.  We could check the current working directory
[1,2,...], but I don't want to use the frozen syscall package and am
not interested enough to import lots of per-OS x/sys packages.  Also,
the odds of the test working directory matching one of the cwd values
(or even of those cwd values actually existing on the host) are small,
and it would be even more work to temporarily change to (and possibly
create) those directories per test.

[1]: https://golang.org/pkg/syscall/#Getcwd
[2]: https://godoc.org/golang.org/x/sys/unix#Getcwd

Signed-off-by: W. Trevor King <wking@tremily.us>
And once we have Windows support for Clean, we can remove the guard
from Abs.

I don't have a Windows machine around to test with, so the expected
values are my best guesses.

Signed-off-by: W. Trevor King <wking@tremily.us>
@liangchenye
Copy link
Member

liangchenye commented Dec 13, 2017

LGTM

Approved with PullApprove

1 similar comment
@Mashimiao
Copy link

Mashimiao commented Dec 13, 2017

LGTM

Approved with PullApprove

@Mashimiao Mashimiao merged commit d1751c1 into opencontainers:master Dec 13, 2017
@wking wking deleted the windows-clean branch December 13, 2017 17:05
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.

None yet

3 participants