search

go-python / gopy

gopy generates a CPython extension module from a go package.
BSD 3-Clause "New" or "Revised" License
2.05k stars 113 forks source link

Fix: Broken with Go 1.20 - quoted path #312

Closed mlange-42 closed 1 year ago

mlange-42 commented 1 year ago

I.e., go was called as go build -v path (with "path" instead of the actual path).

Broke package extraction in Go 1.20. I have no idea why it worked at all in Go 1.19.

Fixes #311

EDIT: Realized that the actual problem was NeedsDeps missing from the package load mode. I guess this made the difference between Go 1.19 and 1.20.

rcoreilly commented 1 year ago

thanks for the fix -- do we need to up the go.mod version of packages?

mlange-42 commented 1 year ago

thanks for the fix -- do we need to up the go.mod version of packages?

Works for me as is. Quite new to Go, so not sure what the implications would be.

rcoreilly commented 1 year ago

Just remembered that the path has to be quoted so spaces in path names (common on windows) still work. So you'll want to revert that one.

rcoreilly commented 1 year ago

I can fix the go.mod when I pull it and run tests etc -- go get -u <package> is the command to get the updated latest version.

mlange-42 commented 1 year ago

Just remembered that the path has to be quoted so spaces in path names (common on windows) still work. So you'll want to revert that one.

Ah, ok... but not quoted as it was, which just added path as argument. We want "<path>". Will fix it.

Tests aren't in the CI? Looks so to me... 🤔

mlange-42 commented 1 year ago

Quotes are not supported here:

go build -v "github.com/mlange-42/gopy_ecs"
panic: path "\"github.com/mlange-42/gopy_ecs\"" not in error "invalid import path \"\\\"github.com/mlange-42/gopy_ecs\\\"\"" [recovered]

But thinking a bit further, they are also not required. An array of arguments is passed, which already considers the spaces/quotes that would come from the command line. I.e. the path is one element in the array, not multiple due to splitting by spaces.

rcoreilly commented 1 year ago

good point! Yeah our CI is broken -- @sbinet was using that appveryor thing -- eventually I'll transition to just use the github one which should work..

mlange-42 commented 1 year ago

Looks like some Python 2 stuff is failing to install (in GH Actions, not appveyor). Also, the Install Linux packages step looks quite complicated. Is this really all necessary? And is Python 2 support really still required?

tamirfri commented 1 year ago

@rcoreilly don't you ashamed? the project is broken. and the awesome guy need to wait more than a month to get this damn hotfix in. meanwhile I just forked the repo of @mlange-42. shame on you and on the sleepy maintainers

sbinet commented 1 year ago

@tamirfri I am not sure your comment is really trying to be constructive. we (well, at least, I, when I was sending some of my free cycles this way) are all working on this "pro-bono". we are all stretched thin.

perhaps a simple, polite, ping would have been better suited to reinject momentum in this PR.

rcoreilly commented 1 year ago

@tamirfri it does take a bit of time to test the updates locally and make the release version, and I had some major deadlines. I know it is frustrating and I've been waiting for other maintainers to do merges too, so I can see both sides of this..

rcoreilly commented 1 year ago

@mlange-42 it would make sense to remove support for py2 at this point.. a PR would be welcome.. I haven't been able to use gopy until it supports generics..