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

Fixes windows and FreeBSD ci failures #159

Merged
merged 7 commits into from
Mar 28, 2024

Conversation

EthanHeilman
Copy link
Contributor

@EthanHeilman EthanHeilman commented Mar 28, 2024

Fixes the two CI failures

Windows

Github Action for windows was failing because gcc was not installed on windows runner. This fixes this issue by installing mingw onto the windows runner, which will install gcc

Error that this fixes

C:\golang\src\github.com\awnumar\memguard>call env CGO_ENABLED=1 go test -race -v ./... 
# runtime/cgo
cgo: C compiler "gcc" not found: exec: "gcc": executable file not found in %PATH%
FAIL	github.com/awnumar/memguard [build failed]
FAIL	github.com/awnumar/memguard/core [build failed]
FAIL	github.com/awnumar/memguard/examples/casting [build failed]
FAIL	github.com/awnumar/memguard/examples/deadlock/x01 [build failed]
FAIL	github.com/awnumar/memguard/examples/socketkey [build failed]
?   	github.com/awnumar/memguard/examples/stdin	[no test files]
?   	github.com/awnumar/memguard/examples/streams	[no test files]
FAIL	github.com/awnumar/memguard/examples/stream [build failed]
FAIL

An additional issue is that even though mingw (gcc) is installed. Golang can't find it, because the associated environment variables that go looks at to find gcc aren't set. We borrow the approach from:
https://groups.google.com/g/golang-nuts/c/AlQvxD7wv30

Installing mingw to get gcc on the windows runner was suggested to me by chatgpt4.

FreeBSD

FreeBSD was failing because the version of FreeBSD we were attempting to use was too old. This PR updates the runner from freebsd-12-2-release-amd64 to freebsd-14-0-release-amd64-ufs

It seems like that we will run into this problem again in a few years when freebsd-14-0-release-amd64-ufs becomes obsolete. I suppose we could use FreeBSD latest, worth thinking about.

@awnumar
Copy link
Owner

awnumar commented Mar 28, 2024

To avoid the problem, I think removing -race and CGO_ENABLED for windows would fix it

@EthanHeilman
Copy link
Contributor Author

@awnumar I think it would fix it, but I've gotten -race running on windows before and I'm hoping if I can just get golang to see gcc, we should be good. Famous last words I know

The freebsd issue seems simple to fix as well. The package url just points to a version of FreeBSD that is no longer being hosted at that location: pkg: http://pkgmir.geo.freebsd.org/FreeBSD:12:amd64/quarterly/meta.txz: Not Found

http://pkgmir.geo.freebsd.org/FreeBSD:12:amd64/quarterly/meta.txz --> https://pkgmir.geo.freebsd.org/FreeBSD:15:amd64/base_weekly/meta.txz

@awnumar
Copy link
Owner

awnumar commented Mar 28, 2024

Thanks for the contribution. The echo you added isn't running because it fails in the install step, so try moving it before the crash

Might as well update the URL in this pr too?

@EthanHeilman
Copy link
Contributor Author

EthanHeilman commented Mar 28, 2024

Yeah, hopefully I can fix both windows and freebsd.

Any idea on why osx is failing in my PR #157

Edit: Nevermind, there is a data race

@awnumar
Copy link
Owner

awnumar commented Mar 28, 2024

There's a race condition with the GC when accessing finalizerCalled. A mutex or channel or atomic variable should fix it

@EthanHeilman EthanHeilman changed the title Fixes windows ci failure Fixes windows and FreeBSD ci failures Mar 28, 2024
@awnumar awnumar merged commit 763f8c7 into awnumar:master Mar 28, 2024
4 checks passed
@EthanHeilman
Copy link
Contributor Author

@awnumar I had a debugging statement:

    - echo $PATH
    - refreshenv
    - echo $PATH

Can you delete it from the workflow? I don't want to confuse anyone who reads it in the future.

@awnumar
Copy link
Owner

awnumar commented Mar 28, 2024

Okay, I'll remove it.

Thanks for the work on discovering and fixing this.

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