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

cmd/compile: cannot import "./package" on Windows #25693

Open
ysmolsky opened this Issue Jun 1, 2018 · 8 comments

Comments

Projects
None yet
7 participants
@ysmolsky
Member

ysmolsky commented Jun 1, 2018

Test cannot import dotted package which was compiled in local directory.

Reproduce in Windows:

cd $GOROOT/test
../bin/go run run.go -- fixedbugs/bug345.go

main.go:26: missing error "[\\w.]+[^.]/io|has incompatible type"
main.go:28: no match for `[\w.]+[^.]/io|has incompatible type` in:
	main.go:28:2: undefined: io
Unmatched Errors:
main.go:11:2: import path contains invalid character ':': "C:/Users/gopher/AppData/Local/Temp/368460658/io"
main.go:25:8: undefined: io
main.go:28:2: undefined: io

Specifically it can be reproduced by following sequence:

cd %TMPDIR%
go tool compile -e %GOROOT%/test/fixedbugs/bug345.dir/io.go
go tool compile -e %GOROOT%/test/fixedbugs/bug345.dir/main.go

I do not have an access to dev env in Windows so I cannot investigate any further.

@bradfitz Should I disable the test for Windows until the fix is found or should I revert the original CL completely?

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jun 1, 2018

@bradfitz Should I disable the test for Windows until the fix is found or should I revert the original CL completely?

We probably shouldn't have done cleanups like this during the freeze. Maybe @ianlancetaylor and I just really want to see this dependency gone.

How about we revert on Tuesday (US/Pacific) if it's not fixed before then?

@ysmolsky

This comment has been minimized.

Member

ysmolsky commented Jun 1, 2018

I'm okay with revert on that date.

Ping @alexbrainman, maybe he can lend me a hand and give a hint with this bug.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 1, 2018

Before the CL in question (https://golang.org/cl/115277) we did not run the test on Windows, so let's just go back to not running the test on Windows. The test is failing for reasons that are unrelated to the point of the test. There is nothing OS-specific about this test, so skipping it on Windows doesn't seem like a big deal.

@ysmolsky ysmolsky changed the title from cmd/compile: cannot import "./package" in Windows to cmd/compile: cannot import "./package" on Windows Jun 1, 2018

@gopherbot

This comment has been minimized.

gopherbot commented Jun 1, 2018

Change https://golang.org/cl/115857 mentions this issue: test: skip test/fixedbugs/bug345.go on windows

@ysmolsky

This comment has been minimized.

Member

ysmolsky commented Jun 1, 2018

I agree with @ianlancetaylor on this. Fixing this test might be not easy, and disabling it on Windows is not a regression too. Pushed the CL. This ticket is a TODO to enable on Windows.

gopherbot pushed a commit that referenced this issue Jun 1, 2018

test: skip test/fixedbugs/bug345.go on windows
Before the CL 115277 we did not run the test on Windows,
so let's just go back to not running the test on Windows.
There is nothing OS-specific about this test,
so skipping it on Windows doesn't seem like a big deal.

Updates #25693
Fixes #25586

Change-Id: I1eb3e158b322d73e271ef388f8c6e2f2af0a0729
Reviewed-on: https://go-review.googlesource.com/115857
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Jun 1, 2018

I agree with @ianlancetaylor on this.

Me too. :-)

This ticket is a TODO to enable on Windows.

I will investigate this issue when I have some free time. Thank you for making builders green.

Alex

@FiloSottile FiloSottile added this to the Go1.11 milestone Jun 5, 2018

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 30, 2018

@odeke-em

This comment has been minimized.

Member

odeke-em commented Dec 9, 2018

Hello there @ysmolsky @bradfitz @ianlancetaylor @alexbrainman, should we move
this to Go1.13 or do y'all think that there is more that we could do for Go1.12? Thank you.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Dec 9, 2018

Sorry, I forgotten about this issue.

If I make this change

diff --git a/test/fixedbugs/bug345.go b/test/fixedbugs/bug345.go
index b974a61ffb..04c28ce857 100644
--- a/test/fixedbugs/bug345.go
+++ b/test/fixedbugs/bug345.go
@@ -1,4 +1,3 @@
-// +build !windows
 // errorcheckdir -n

 // Copyright 2011 The Go Authors. All rights reserved.
@@ -6,5 +5,3 @@
 // license that can be found in the LICENSE file.

 package ignored
-
-// TODO(ysmolsky): Fix golang.org/issue/25693 to enable on Windows.

to f70bd91 and run this command

c:\Users\Alex\dev\go\src>go tool compile -e %GOROOT%\test\fixedbugs\bug345.dir\main.go
c:\users\alex\dev\go\test\fixedbugs\bug345.dir\main.go:11:2: import path contains invalid character ':': "c:/Users/Alex/dev/go/src/io"
c:\users\alex\dev\go\test\fixedbugs\bug345.dir\main.go:25:8: undefined: io
c:\users\alex\dev\go\test\fixedbugs\bug345.dir\main.go:28:2: undefined: io

c:\Users\Alex\dev\go\src>

I can see the error.

But I am not familiar enough with how compiler works. The source file does not have any imports containing : character.

c:\Users\Alex\dev\go\src>type c:\users\alex\dev\go\test\fixedbugs\bug345.dir\main.go
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main

import (
        "bufio"
        goio "io"

        "./io"
)

func main() {
        // The errors here complain that io.X != io.X
        // for different values of io so they should be
        // showing the full import path, which for the
        // "./io" import is really ..../go/test/io.
        // For example:
        //
        // main.go:25: cannot use w (type "/Users/rsc/g/go/test/fixedbugs/bug345.dir/io".Writer) as type "io".Writer in function argument:
        //      io.Writer does not implement io.Writer (missing Write method)
        // main.go:27: cannot use &x (type *"io".SectionReader) as type *"/Users/rsc/g/go/test/fixedbugs/bug345.dir/io".SectionReader in function argument

        var w io.Writer
        bufio.NewWriter(w) // ERROR "[\w.]+[^.]/io|has incompatible type"
        var x goio.SectionReader
        io.SR(&x) // ERROR "[\w.]+[^.]/io|has incompatible type"
}

c:\Users\Alex\dev\go\src>

So compiler must be replacing

        "./io"

with

        "c:/Users/Alex/dev/go/src/io"

Is that valid? If it is valid thing to do, then compiler should be able to handle Windows paths like c:/Users/Alex/dev/go/src/io.

Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment