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

syscall: mksyscall_windows.go doesn't handle goroot correctly #16493

Closed
mattn opened this issue Jul 25, 2016 · 6 comments
Closed

syscall: mksyscall_windows.go doesn't handle goroot correctly #16493

mattn opened this issue Jul 25, 2016 · 6 comments
Assignees
Milestone

Comments

@mattn
Copy link
Member

@mattn mattn commented Jul 25, 2016

  1. What version of Go are you using (go version)?
    go version devel +1d2ca9e Mon Jul 18 21:07:34 2016 +0000 windows/amd64
  2. What operating system and processor architecture are you using (go env)?
    windows/amd64
  3. What did you do?
    set GOROOT=c:\dev\go
    set GOPATH=c:\dev\gopath
    go generate with //go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go
  4. What did you expect to see?
    mksyscall_windows.go doesn't put import internal/syscall/windows because it's not a standard package, or x/sys package.
  5. What did you see instead?
    I can see import internal/syscall/windows in the code.

func (src *Source) IsStdRepo() (bool, error) {
if len(src.Files) == 0 {
return false, errors.New("no input files provided")
}
abspath, err := filepath.Abs(src.Files[0])
if err != nil {
return false, err
}
goroot := runtime.GOROOT()
if runtime.GOOS == "windows" {
abspath = strings.ToLower(abspath)
goroot = strings.ToLower(goroot)
}
return strings.HasPrefix(abspath, goroot), nil
}

goroot is checked without delimiter.

@bradfitz bradfitz changed the title mksyscall_windows.go doesn't handle goroot correctly syscall: mksyscall_windows.go doesn't handle goroot correctly Jul 25, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 25, 2016

CL https://golang.org/cl/25165 mentions this issue.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 28, 2016

What did you do?
set GOROOT=c:\dev\go
set GOPATH=c:\dev\gopath
go generate with //go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go

What did you do? Did you run "go generate" command? Which directory?

I don't understand what the problem is. Please provide more details. Thank you.

Alex

@mattn
Copy link
Member Author

@mattn mattn commented Jul 28, 2016

Few days ago, I contributed some patches for https://github.com/tompng/pokemon-go .
I modified pokemon-go to use https://github.com/jteeuwen/go-bindata .

In my configuration, GOROOT and GOPATH have same prefix part C:\dev\go but not same directory as you can see in above. But mksyscall_windows.go handle those as same. So the code made by mksyscall_windows.go are generated as pkgStd because IsStdRepo doesn't check trailing path separator as delimiter.
If I have last backslash in GOPATH and GOROOT, it will works fine. But official installation instruction doesn't have trailing slash/backslash in the environment variables.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 28, 2016

Few days ago, I contributed some patches for https://github.com/tompng/pokemon-go .
I modified pokemon-go to use https://github.com/jteeuwen/go-bindata .

I checked github.com/tompng/pokemon-go package. It does not use mksyscall_windows.go. Why are you telling me about github.com/tompng/pokemon-go package?

In my configuration, GOROOT and GOPATH have same prefix part C:\dev\go but not same directory as you can see in above. But mksyscall_windows.go handle those as same. So the code made by mksyscall_windows.go are generated as pkgStd because IsStdRepo doesn't check trailing path separator as delimiter.

You keep repeating yourself. I understanding that you think there is a bug here. And I don't disagree with you. But I would like to see the bug by myself. Please provide some code or commands to demonstrate the bug. Show us some real computer output, and explain why that output is wrong in your opinion.

Thank you.

Alex

@mattn
Copy link
Member Author

@mattn mattn commented Jul 28, 2016

I checked github.com/tompng/pokemon-go package. It does not use mksyscall_windows.go. Why are you telling me about github.com/tompng/pokemon-go package?

Sorry much! I was confusing. True repository is my private for my job. Please check following code.

foo.go is below:

package foo

//go:generate go run $GOROOT/src/syscall/mksyscall_windows.go -output bar.go foo.go

//sys   GetLastError() (lasterr error)
C:\dev\godev\src\github.com\mattn\foo>go generate

and go generate. bar.go is following.

// MACHINE GENERATED BY 'go generate' COMMAND; DO NOT EDIT

package foo

import (
    "internal/syscall/windows/sysdll"
    "syscall"
    "unsafe"
)

var _ unsafe.Pointer

var (
    modkernel32 = syscall.NewLazyDLL(sysdll.Add("kernel32.dll"))

    procGetLastError = modkernel32.NewProc("GetLastError")
)

func GetLastError() (lasterr error) {
    r0, _, _ := syscall.Syscall(procGetLastError.Addr(), 0, 0, 0, 0)
    if r0 != 0 {
        lasterr = syscall.Errno(r0)
    }
    return
}

internal/syscall/windows/sysdll should not be generated.

P.S. I must say sorry again.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 29, 2016

Please check following code.

Thanks for the test.

Alex

@quentinmit quentinmit added this to the Go1.8 milestone Aug 1, 2016
@gopherbot gopherbot closed this in 5dc7525 Aug 17, 2016
@golang golang locked and limited conversation to collaborators Aug 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.