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

Mockery fails to generate anything if import "C" is present on Winodws #358

Open
targodan opened this issue Jan 8, 2021 · 7 comments
Open

Comments

@targodan
Copy link

targodan commented Jan 8, 2021

My yapscan project makes use of CGO and mockery. (currently only the develop branch is affected, in case you want to try it with my project)

I installed mockery version 2.5.1 via go get and tried to generate my mocks via go generate, which calls mockery --inpackage --testonly --name ".*".

This behaves differently on Linux and Windows.

On Linux

It kind of works. It seems that any file which contains import "C" is ignored entirely but no error is emitted and the interfaces in files without import "C" are generated.

I can work with this, by just getting any interfaces I need to mock out of the file with import "C".

On Windows

It doesn't work at all. The output looks somewhat like this (sorry for not being precise, couldn't copy-paste from my Windows VM):

INF Starting mockery
INF Walking
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
ERR Error parsing file <path_to_my_project>\\cgo.go:4:8 could not import C (no metadata for C)
FTL Unable to find '.*' in any go files onder this path

Note that it's always the same file and line, although only one file in this directory contains an import "C". The row and colum 4:8 points to the opening " of the import "C".

I guess a valid solution would be to just ignore any imports of the special package "C" (but not the file that contains the import), as that's just a meta package for interfacing with cgo.

@LandonTClipp
Copy link
Contributor

C is a hard package to deal with because it's not a real Golang package so the typical tools you use to gather metadata on it won't work.

A PR to explicitly ignore C would be welcome.

@echocrow
Copy link

echocrow commented Jan 11, 2021

It kind of works. It seems that any file which contains import "C" is ignored entirely but no error is emitted and the interfaces in files without import "C" are generated.

Experiencing the same issue here on macOS & mockery v2.5.1, e.g. when running mockery --all --keeptree. (However, the exact command does not seem to matter.)

Example package & command:

package foo

import "C"

//go:generate mockery --name=Foo
type Foo interface {
	Bar() string
}
go generate ./...
INF Starting mockery dry-run=false version=0.0.0-dev
INF Walking dry-run=false version=0.0.0-dev
FTL Unable to find 'Foo' in any go files under this path dry-run=false version=0.0.0-dev
foo.go:5: running "mockery": exit status 1

I'm guessing right now this is "intended behavior" due to potential complications when C functions are at play. Still, is there any way to opt out of import "C" file ignoring? In the example above, mockery would have no issues mocking Foo.

@LandonTClipp
Copy link
Contributor

I don't believe there is any way to opt out of this. There needs to be extra code that is capable of blacklisting certain special imports like this.

@echocrow
Copy link

I don't believe there is any way to opt out of this. There needs to be extra code that is capable of blacklisting certain special imports like this.

Yeah I get that this needs to be the default due to the complexities involved with the C package. The feature request might be a way to manually and explicitly opt out of this silent file-skipping behavior (e.g. via a CLI flag), at one's own risk that generation may fail if C is involved in the interface. (Not sure if that's what you were referring to in your previous post?) Alternatively, an approach that blacklists only types tied to the C package (instead of full files importing C) might also solve this, but I'm guessing would even further up the complexity.
I'm happy to look into a PR for this myself. After a quick search through the repo, though, I frankly have no idea where to even start with this, or where/how flies with import "C" are currently being skipped. Any hint or idea where this is currently happening?

Until (if ever) this gets improved, is the general recommendation to move interfaces & func types into separate files? This may be contradictory the as-simple-as-possible mantra, but is not the end of the world.

I'd been pulling my hair for a fair bit wondering why some interfaces seem to just not exist for mockery, until I recognized the pattern. If this is not addressable in the foreseeable future, any possibility to add a warning log to the command (e.g. Ignoring "foo.go": Files importing "C" package are not supported.), or a mention in the README.md? (Happy to look into the former, but still clueless as to how these files are currently being skipped.)

(PS: Sorry for high-jacking this issue. This was the only related result I could find yesterday. Should I open a new issue, if you do think there is room for improvement?)

@targodan
Copy link
Author

I had a quick glance at the code too and couldn't figure out where one could skip resolving of the import "C". I'd be fine as well with any interfaces depending on the "C" package (i.e. in parameters or return types). That feels dirty anyway. 😉

It would be nice though if not the whole file would be skipped (again, this behaviour even seems OS dependent).

@LandonTClipp
Copy link
Contributor

It's possible that the issue lies rather with the Golang-provided code parser. I haven't taken a close look at it myself but my suspicion is that our problem is really with how golang.org/x/tools/go/packages handles this.

I found that the packages.Load accepts config: https://pkg.go.dev/golang.org/x/tools/go/packages#Config

It seems there is an optional ParseFile function you can define that will modify the parsing behavior. That might be worth looking into.

@aymericDD
Copy link

Any updates about this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants