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

path/filepath: Split should trim trailing path separator else a Split stalemate occurs. #9928

Closed
odeke-em opened this issue Feb 19, 2015 · 7 comments

Comments

@odeke-em
Copy link
Member

  1. What version of Go are you using (go version)?
  2. What operating system and processor architecture are you using?
go version go1.2.1 linux/386
Linux emmanuel-HP-G50-Notebook-PC 3.13.0-39-generic #66-Ubuntu SMP Tue Oct 28 13:31:23 UTC 2014 i686 athlon i686 GNU/Linux
go version go1.3 darwin/amd64
Darwin 14.1.0 Darwin Kernel Version 14.1.0: Mon Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64

filepath.Split returns a dir and base when given a path 'p'. If the path has no trailing separator, it behaves as expected producing dir and base. If p has a trailing path separator e.g 'p/', filepath.Split should return 'p' as the dir, and '' as the base. However, filepath.Split always returns the original value as the dir and '' as the base value, instead of 'p', '' e.g in http://play.golang.org/p/fswqGij8Hu

This behaviour causes an infinite loop when walking up a path. My use case is trying to find the least non existant path starting from the end of a path. In the code excerpt below:

  • The first function is my work-around to solve the problem by always trimming the excess '/'.
  • The second function illustrates the issue and never exits since it is stuck at the original path being the dir.
  • Assumption is that a "" signifies no existant path was found.
package main

import (
    "fmt"
    "os"
    "path/filepath"
    "strings"
)

var PathSeparator = fmt.Sprintf("%c", os.PathSeparator)

func leastNonExistantRootPatched(p string) string {
    last := ""
    for p != "" {
        fInfo, _ := os.Stat(p)
        if fInfo != nil {
            break
        }
        last = p
        p, _ = filepath.Split(strings.TrimRight(p, PathSeparator))
        fmt.Println("Still spinning: ", p)
    }
    return last
}

func leastNonExistantRootInfinite(p string) string {
    last := ""
    for p != "" {
        fInfo, _ := os.Stat(p)
        if fInfo != nil {
            break
        }
        last = p
        p, _ = filepath.Split(p)
        fmt.Println("Still spinning: ", p)
    }
    return last
}

func main() {
    leastNonExistantRootPatched("/mnt/ghosting/symver/projects/gophers/alophering/change.go")
    leastNonExistantRootInfinite("/mnt/ghosting/symver/projects/gophers/alophering/change.go/")
}

Please contrast this with the behaviour of Python

>>> import os
>>> os.path.split('/mnt/ghosting/gophers/alophering/change.go/')
('/mnt/ghosting/gophers/alophering/change.go', '') # Notice the trailing slash was removed
>>> os.path.split('/mnt/ghosting/gophers/alophering/change.go')
('/mnt/ghosting/gophers/alophering', 'change.go')
@minux
Copy link
Member

minux commented Feb 19, 2015

If you want to walk a path, use filepath.Dir.

Note that the document of filepath.Split makes the behavior very clear:
Split splits path immediately following the final Separator, separating it into a directory and file name component. If there is no Separator in path, Split returns an empty dir and file set to path. The returned values have the property that path = dir+file.

Note the last sentence.

@minux minux closed this as completed Feb 19, 2015
@odeke-em
Copy link
Member Author

So what happens when I want to extract a dir as well as file from a path that may contain a trailing '/'? Would I have to first TrimRight for every string or instead perform two separate calls?

dir, file := filepath.Dir(p), filepath.Base(p)

I see your response directly helps with walking but the last sentence doesn't fit my issue which is that the dir and file should then be extracted. In the original examples, if I wanted dir, file, I would still be stuck in that infinite loop, something a user that hasn't investigated this issue would not be aware of unless they performed a trim or two separate function calls.

@mikioh mikioh changed the title path/filepath.Split should trim trailing path separator else a Split stalemate occurs. path/filepath: Split should trim trailing path separator else a Split stalemate occurs. Feb 19, 2015
@adg
Copy link
Contributor

adg commented Feb 19, 2015

We cannot change the semantics of filepath.Split now. https://golang.org/doc/go1compat

@mattn
Copy link
Member

mattn commented Feb 20, 2015

Call filepath.Clean before filepath.Split to remove last slash.

http://play.golang.org/p/bHy-iM-zM-

@odeke-em
Copy link
Member Author

@adg alright, but if there was a way of letting some folks know about it. I realize that it isn't Go's behavior to make the transition from other languages to using Go intuitive, and probably change semantics just for that case since Go has its own docs and specs. However, it took me a lot of hours and a couple of bugs reported in a program that I am using this for, for me to investigate it. I was expecting filepath.Split to be deterministic the way other languages have it, regardless of whether there was a trailing '/' or not.
@mattn sounds like a plan. I was actually doing a strings.Trim*.

Correction: * Go's responsibility instead of Go's behavior.

@adg
Copy link
Contributor

adg commented Feb 20, 2015

I sent a change to add an example for the Spit function to make its behaviour more obvious:
https://go-review.googlesource.com/5416

@odeke-em
Copy link
Member Author

Thank you @adg. At least this example will leave a hint.

adg added a commit that referenced this issue Feb 20, 2015
Fixes #9928

Change-Id: Iab37051078755a132f211ad48e756422f7c55a39
Reviewed-on: https://go-review.googlesource.com/5416
Reviewed-by: Minux Ma <minux@golang.org>
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants