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

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

Closed
ysmolski opened this issue Jun 1, 2018 · 10 comments
Closed

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

ysmolski opened this issue Jun 1, 2018 · 10 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@ysmolski
Copy link
Member

ysmolski 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?

@ysmolski ysmolski added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 1, 2018
@bradfitz
Copy link
Contributor

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?

@ysmolski
Copy link
Member Author

ysmolski 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
Copy link
Contributor

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.

@ysmolski ysmolski changed the title cmd/compile: cannot import "./package" in Windows cmd/compile: cannot import "./package" on Windows Jun 1, 2018
@gopherbot
Copy link

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

@ysmolski
Copy link
Member Author

ysmolski 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
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
Copy link
Member

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
Copy link
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
Copy link
Member

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

@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@iwdgo
Copy link
Contributor

iwdgo commented Oct 30, 2019

While investigating another . related issue on Windows, it appeared that this test is now successful.

$GOPATH\golang\go\test>ver

Microsoft Windows [Version 10.0.18362.418]

$GOPATH\golang\go\test>go version
go version go1.13.3 windows/amd64

$GOPATH\golang\go\test>go run run.go -- fixedbugs/bug345.go

$GOPATH\Google\golang\go\test>goissue run run.go -- fixedbugs/bug345.go

$GOPATH\golang\go\test>

goissue is on tip.

@gopherbot
Copy link

Change https://go.dev/cl/395258 mentions this issue: test: compile source files as if from "test" module

@golang golang locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

10 participants