netbirdio / netbird

Connect your devices into a secure WireGuard®-based overlay network with SSO, MFA and granular access controls.
https://netbird.io
BSD 3-Clause "New" or "Revised" License
11.18k stars 515 forks source link

CONTRIBUTING.md not updated #1212

Closed Fantu closed 12 months ago

Fantu commented 1 year ago

Hi, today I did a fast try to build a windows client with updated ICE (https://github.com/M2Rbiz/netbird/commit/3b00ee9a4241b26f313a6cdfcef8d7891a4a7809) to see if solves https://github.com/netbirdio/netbird/issues/1195 I saw that CONTRIBUTING.md is not updated:

can someone tell me the updated and correct instruction please? I suggest to update also CONTRIBUTING.md with them thanks for any reply and sorry for my bad english

mlsmaycon commented 1 year ago

Thanks, @Fantu, for submitting this issue and for your intention to contribute. The PR #1226 is addressing your comments.

Fantu commented 1 year ago

@mlsmaycon thanks, why in updated CONTRIBUTING you put go 1.21 but go.mod have 1.20? in my build test I used 1.21 on debian unstable install I also use for debian packaging

about client build howto no other info are needed? I taken a time to found them in repo, was for missed Go knowledge while expert Go developers don't need it because it's basic?

mlsmaycon commented 1 year ago

We are preparing for an update to go 1.21, as they are compatible. Now that you mentioned it, I missed adding the explicit flag to disable CGO:

CGO_ENABLED=0 go build .

This should be it. I will update the guide.

Fantu commented 1 year ago

in first PR I also added a comment for a typo still present I also tried build netbird-ui following https://github.com/netbirdio/netbird/blob/main/.goreleaser_ui.yaml I added "CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc" but when I tried to use it have issue, probably something was missed on both command I also used "GOOS=windows GOARCH=amd64"

mlsmaycon commented 1 year ago

Is an issue preventing from starting the UI? Do you have a log message? Maybe opening from powershell will display the error.

Regarding the typo, can you share it again?

Fantu commented 1 year ago

the typo is in the only review comment done before your merge: https://github.com/netbirdio/netbird/pull/1226

now on the tests device I restored stable netbird binaries when there will be correct/full build procedure (if my was incomplete, I don't know) and/or will be other possible fix to "reconnect loop" I'll rebuild and redo the tests and I'll take and post details on netbird-ui issue (if still present)

mlsmaycon commented 1 year ago

Sorry, I believe we missed the comment. I see that it was just approved and merged with no other visible comments.

Fantu commented 1 year ago

build procedure I used for my previous test, did on debian unstable with go 1.21 from system package:

go install github.com/akavel/rsrc@v0.10.2
/home/fabio/go/bin/rsrc -arch amd64 -ico client/ui/netbird.ico -manifest client/manifest.xml -o client/resources_windows_amd64.syso
cd client/
env GOOS=windows GOARCH=amd64 go build
/home/fabio/go/bin/rsrc -arch amd64 -ico client/ui/netbird.ico -manifest client/ui/manifest.xml -o client/ui/resources_windows_amd64.syso
cd client/ui/
env CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc GOOS=windows GOARCH=amd64 go build

after I taken the generated binaries, renamed and replaced on windows devices used for tests (I closed netbird-ui and did "netbird down" before replace them)

Fantu commented 12 months ago

Thanks for the update and improvements to CONTRIBUTING.md https://github.com/netbirdio/netbird/pull/1323 will add also more detailed build instruction for windows client and installer, so I think this issue can be closed