gobuffalo / buffalo

Rapid Web Development w/ Go
http://gobuffalo.io
MIT License
8.08k stars 577 forks source link

Invalid case-insensitive filepath handling #982

Closed egonelbre closed 6 years ago

egonelbre commented 6 years ago

Some FileSystems are case-insensitive, mainly Windows, which means using strings.TrimPrefix to find the relative directory or path, will eventually fail. Something like this, will be probably more successful in most cases:

func ImportPath(gopaths []string, targetpath string) (string, error) {
    var err error
    var rel string
    for _, gopath := range gopaths {
        rel, err = filepath.Rel(gopath, targetpath)
        if err == nil {
            return filepath.ToSlash(rel), nil
        }
    }
    return "", err
}

Example in Windows with a case-mismatch:

f:\Go\src\sandbox\buffalo>echo %GOPATH%
F:\Go

f:\Go\src\sandbox\buffalo>buffalo new example
      create  .buffalo.dev.yml
      create  assets/images/logo.svg
      create  assets\css\application.scss
<<< SNIP >>>
      create  public\robots.txt
      create  templates\_flash.html
      create  templates\application.html
      create  templates\index.html
         run  go get -t ./...
can't load package: package sandbox/buffalo/example:
main.go:6:3: invalid import path: "f:/Go/src/sandbox/buffalo/example/actions"
can't load package: package sandbox/buffalo/example/actions:
actions\app.go:11:3: invalid import path: "f:/Go/src/sandbox/buffalo/example/models"
can't load package: package sandbox/buffalo/example/grifts:
grifts\init.go:5:2: invalid import path: "f:/Go/src/sandbox/buffalo/example/actions"
Usage:
  buffalo new [name] [flags]
<<< SNIP >>>
ERRO[0038] Error: exit status 1

And the main.go, app.go, Dockerfile... contain import paths such as:

package main

import (
  "log"

  "f:/Go/src/sandbox/buffalo/example/actions"
)

func main() {
  app := actions.App()
  if err := app.Serve(); err != nil {
    log.Fatal(err)
  }
}

I suspect there are other places that may contain similar mistakes.

markbates commented 6 years ago

A PR would be very welcome!


Mark Bates

On Mar 22, 2018, 6:21 AM -0400, Egon Elbre notifications@github.com, wrote:

Some FileSystems are case-insensitive, mainly Windows, which means using strings.TrimPrefix to find the relative directory or path, will eventually fail. Something like this, will be probably more successful in most cases: func ImportPath(gopaths []string, targetpath string) (string, error) { var err error var rel string for _, gopath := range gopaths { rel, err = filepath.Rel(gopath, targetpath) if err == nil { return filpath.ToSlash(rel), nil } } return nil, err } Example in Windows with a case-mismatch: f:\Go\src\sandbox\buffalo>echo %GOPATH% F:\Go

f:\Go\src\sandbox\buffalo>buffalo new example create .buffalo.dev.yml create assets/images/logo.svg create assets\css\application.scss <<< SNIP >>> create public\robots.txt create templates_flash.html create templates\application.html create templates\index.html run go get -t ./... can't load package: package sandbox/buffalo/example: main.go:6:3: invalid import path: "f:/Go/src/sandbox/buffalo/example/actions" can't load package: package sandbox/buffalo/example/actions: actions\app.go:11:3: invalid import path: "f:/Go/src/sandbox/buffalo/example/models" can't load package: package sandbox/buffalo/example/grifts: grifts\init.go:5:2: invalid import path: "f:/Go/src/sandbox/buffalo/example/actions" Usage: buffalo new [name] [flags] <<< SNIP >>> ERRO[0038] Error: exit status 1 And the main.go, app.go, Dockerfile... contain import paths such as: package main

import ( "log"

"f:/Go/src/sandbox/buffalo/example/actions" )

func main() { app := actions.App() if err := app.Serve(); err != nil { log.Fatal(err) } } I suspect there are other places that may contain similar mistakes. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

egonelbre commented 6 years ago

Tried my best.

I wasn't sure what the policy of error handling is, but I took the following approach:

  1. When there was error handling around, returned the error
  2. When there was no error handling, then fell back to strings.TrimPrefix results.

I'm not sure what the appropriate action should be for paths that are outside of GoPath, or is that handled somewhere further above? In those cases the result from filepath.Rel can give a result such as ../../x/y/z.

egonelbre commented 6 years ago

PS: I'm not sure whether I caught them all. There were few strings.TrimPrefix-es that I wasn't sure about, and there might be some dependencies outside of gobuffalo that are still affected.

egonelbre commented 6 years ago

Also there might be a better place to put that version of RelativePath with convenient fallback, but I couldn't find a place where to put it.

markbates commented 6 years ago

Thank you for this!