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

Fix/w32 modules ref #11

Closed
wants to merge 2 commits into from
Closed

Conversation

kfsone
Copy link

@kfsone kfsone commented Feb 25, 2021

Prototype references github.com/gonutz/w32 which, with modules, fetches the v1.0.0 version that does not build. This change updates prototype to build with go 1.15 and 1.16 in a fresh environment.

Testing:

  • reimaged a vm, win 10 2020 h2
  • installed git, go,
  • clone github.com/kfsone/prototype into $env:GOPATH/src/github.com/gonutz/prototype and checked out the fix/w32-modules-ref branch
  • execute the following steps:
    $nutzdir = Join-Path $env:GOPATH src/github.com/gonutz
    $srcurl = "https://raw.githubusercontent.com/gonutz/prototype/master/samples/worm/worm.go"

    cd (mkdir (join-path $nutzdir proto.before))
    invoke-webrequest $srcurl -outfile worm.go  # iwr for short
    go run worm.go

    # go 1.16 fails because no modules stuff
    go mod init ; go mod tidy
    go run worm.go

fails:

# github.com/gonutz/prototype/draw
..\..\..\..\pkg\mod\github.com\gonutz\prototype@v0.0.0-20210219081453-295a54f2a887\draw\window_windows.go:93:8: undefined: w32.UnregisterClassAtom
..\..\..\..\pkg\mod\github.com\gonutz\prototype@v0.0.0-20210219081453-295a54f2a887\draw\window_windows.go:442:7: undefined: w32.WM_MOUSEHWHEEL
..\..\..\..\pkg\mod\github.com\gonutz\prototype@v0.0.0-20210219081453-295a54f2a887\draw\window_windows.go:515:10: cannot use style & ^w32.WS_OVERLAPPEDWINDOW (type int32) as type uint32 in argument to w32.SetWindowLong

then I used go mod edit to switch to using the local on-disk copy of prototype:

    cd (mkdir (Join-Path $testdir proto.after))
    iwr $srcurl -outfile worm.go

    go mod init
    # --vv--
    go mod edit -replace github.com/gonutz/prototype=../../gonutz/prototype  ## <<
    # --^^--
    go mod tidy
    go run worm.go

which works

image

@gonutz
Copy link
Owner

gonutz commented Feb 26, 2021

Wow, you put in a lot of effort, thank you! The thing is, I am still thinking about what to do with these dependencies. The w32 one is under my control and I will use v2 like I did everywhere else now. But the glfw, gl and sdl2 ones are only my forks (backup-copies really) of those repos. I am thinking to update them. While the gl and glfw have not changed very much, there has a lot been going on in the veandco sdl2 repo. I will have to experiment a little to see if I want to update this dependency. Then I will probably still do it in my own fork, if ever the authors decide to pull their repo, go get will fail new users of my library.

@gonutz
Copy link
Owner

gonutz commented Feb 28, 2021

Hey, thanks again for the contribution. I have now go mod'ed all the dependencies and made new versions of them. They all are now in my gonutz account (so other people cannot pull it off the internet over night) and I made all of them at least version 1.0.0 so we do not have those ugly v0.0.0-abdeb345234commithash123423b2fas in the go.mod file.

Please give it a try and let me know if anything else is broken for you. I will check it again later on my Linux desktop but so far it all seems to work.

Sorry for not merging your PR, I hate to make people feel like they did work in vain. Which in this case you actually did not do as this issue made me handle this repo earlier than I would have and I hope you see that this is now in overall better shape with all of the dependencies under control :-)

I actually like doing this, it makes you consider your dependencies. I saw the veandco/go-sdl2 in the mod file in your PR and was wondering where that came from. I found it, it was in a submodule in the go-sdl2 repo that contained the test cases. I removed this from my fork and a couple of other things to reduce the dependencies. This is a good thing in my opinion, fewer and known dependencies are nice.

@gonutz gonutz closed this Feb 28, 2021
@kfsone
Copy link
Author

kfsone commented Feb 28, 2021

No offense taken: most of the steps were just me seeing if I'd screwed up my go environment, and the PR was just "if it helps" :)

@kfsone kfsone deleted the fix/w32-modules-ref branch March 4, 2021 00:29
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

Successfully merging this pull request may close these issues.

None yet

2 participants