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

CI/CD #35

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open

CI/CD #35

wants to merge 38 commits into from

Conversation

mibmo
Copy link
Contributor

@mibmo mibmo commented Jun 10, 2022

Automatic builds for the statically-linked binaries.

Checklist:

There are quite a few things I want to change, such as the artifact names and general structure, but this is a working first draft.
I'm thinking downloads will be available both through action artifacts & from the releases page (that requires another release be made, but that doesn't really have to happen till after this is completely done).

Note: I haven't really found a good workflow for incrementally developing actions, so you're likely to see a bunch of force-pushes to my repository. Not a whole lot I can do there, I'm afraid (act does exist, but I can't get it to run with Podman)

Resolves #13

they were kinda preventing macos from building correctly, so...
@mibmo
Copy link
Contributor Author

mibmo commented Jun 11, 2022

I'm struggling quite a bit with getting Mac builds to work (at all). Could someone knowledgeable perhaps pitch in?

@mibmo
Copy link
Contributor Author

mibmo commented Jun 11, 2022

I did get mac builds working once (as well as those locally on my machine), but that requires updating the pinned Tor version (the latest commit works just fine). How would you feel about that, @cretz? Seems like it'd be a good thing to update it anyway, as it's pretty out of date.

@cretz
Copy link
Owner

cretz commented Jun 11, 2022

requires updating the pinned Tor version (the latest commit works just fine)

What version? Can you make sure the integration test at the root of the repo (go test .) works with it? Hopefully that's not too challenging, but if it is we can discuss other options.

Also, once we merge master the workflows should start running (I have enabled actions on this repo). And we can remove the other CI stuff. Thanks for doing this!

@mibmo
Copy link
Contributor Author

mibmo commented Jun 11, 2022

What version?

Tor 0.4.7.7

Can you make sure the integration test at the root of the repo (go test .) works with it? Hopefully that's not too challenging, but if it is we can discuss other options.

Since tests rely on Bine, I think we'd need an additional PR to Bine to support the bump in version (process/embedded/tor-0.4.7). Looking at the internals of Bine, it seems like it'd fairly simple? You're the expert here though! :)

Updating the Tor version might end up being a bigger change (not sure though), so I've split it into a different PR, #36.

Also, once we merge master the workflows should start running (I have enabled actions on this repo). And we can remove the other CI stuff. Thanks for doing this!

Sounds good! :D

@mibmo mibmo changed the title WIP: automatic builds WIP: CI/CD Jun 11, 2022
@mibmo mibmo mentioned this pull request Jun 11, 2022
seems like they're the "standard", and this allows for flexibility in
specifying runner OS versions.
this is to allow for running tests afterwards.
gh actions don't support cross-file workflow dependencies yet.
also cleans up env a bit; some unnecessary envvars were leftover from
copy-pasting the action from another project
@milesrichardson
Copy link

I have a Mac! I tried the binary from your build and it worked. It connected to the Tor network (Bootstrapped 100%), no problem.

I had to remove the Mac quarantine attributes from the binary, but I believe that's only because I downloaded it via my browser from GitHub Actions. If you download the artifact via curl then they artifacts won't be there.

xattr -d com.apple.quarantine tor
xattr -d com.apple.macl tor

@cretz
Copy link
Owner

cretz commented Apr 10, 2023

@milesrichardson - Thanks!

@mibmo - This looks pretty good, shall I go ahead and merge?

@mibmo
Copy link
Contributor Author

mibmo commented Apr 10, 2023

@milesrichardson: thank you so much for testing! glad to hear it worked. :)

@cretz: almost! i'm gonna try getting windows to build first.

the whole caching the PROJECT_HASH with actions/cache and diffing it
is a bit jank but honestly i'd say that it's worth it.
@mibmo
Copy link
Contributor Author

mibmo commented Apr 10, 2023

@cretz: could we possible make tor-static into a go module? i.e. just go mod init? afaik it wouldn't change much except provide us with a lockfile (which would be very handy). if so, i'll open a pr real quick :) seems like i already did earlier in this pr, woops. anyhow, i'm updating it to 1.20 as per actions/runner-images#7276

@mibmo mibmo force-pushed the ci branch 2 times, most recently from cabffc4 to 1e68214 Compare April 11, 2023 14:59
@mibmo
Copy link
Contributor Author

mibmo commented Apr 11, 2023

@cretz: are you sure the windows guide in the readme is correct? i've a friend running windows following it step-by-step and they can't get it to build either? in any case, if this keeps posing an issue we could also just merge and do another PR on building for windows specifically.

@cretz
Copy link
Owner

cretz commented Apr 11, 2023

@mibmo - I am not sure, it may be outdated. What is your error? The important thing about Windows is that, like other CGo Windows dependencies, it has to be built with GCC. I support disabling Windows builds and doing in a separate PR if wanted. Can also look around to see how others build such libraries.

@mibmo
Copy link
Contributor Author

mibmo commented Apr 11, 2023

@cretz: i think it might be a bit outdated, e.g. it was missing instructions on installing go; we had to do that manually. go is running fine though,

output from running go run build.go build-all in msys2 terminal

$ go run build.go build-all
2023/04/11 19:26:47 *** Building all ***
2023/04/11 19:26:47 *** Building openssl ***
2023/04/11 19:26:47 Running in folder openssl: perl ./Configure --prefix=/C/MSYS2/home/rasmu/tor-sta
tic/openssl/dist --openssldir=/C/MSYS2/home/rasmu/tor-static/openssl/dist no-shared no-dso no-zlib m
ingw64
2023/04/11 19:26:52 Running in folder openssl: make depend
2023/04/11 19:26:52 Running in folder openssl: make -j32
2023/04/11 19:26:52 *** Done building openssl ***
2023/04/11 19:26:52 *** Done building all ***
2023/04/11 19:26:52 exit status 2
exit status 1

unless you know what's wrong and can help with a fix, i vote for just merging this and then we can open an issue for windows specifically — not having windows builds is really a shame though, it's the whole reason i started the pr, haha.

edit: it's the same error that the CI is spewing rn :) check the windows build

@mibmo mibmo changed the title WIP: CI/CD CI/CD Apr 12, 2023
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.

Consider distributing prebuilt static libs
3 participants