Skip to content

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.

wking added 4 commits December 8, 2017 13:52
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@wking wking force-pushed the windows-clean branch 2 times, most recently from 3252cbd to 2f21180 Compare December 8, 2017 22:46
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 <[email protected]>
@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.

3 participants