feat: make libtor on by default for nix builds #6060
Conversation
ghpbot-tari-project
added
P-acks_required
brianp
force-pushed
the
build-feat-libtor-default
branch
from
1c2f0e4 to
e98cf7a
Compare
| @@ -52,7 +52,7 @@ tonic = { version = "0.6.2", features = ["tls", "tls-roots" ] } | |||
| tari_metrics = { path = "../../infrastructure/metrics", optional = true, features = ["server"] } | |||
|
|
|||
| [features] | |||
| default = ["metrics"] | |||
| default = ["metrics", "libtor"] | |||
There was a problem hiding this comment.
this will break windows builds
There was a problem hiding this comment.
This is what we're testing.
Including tari_libtor shouldn't cause any kind of error, and then inside tari_libtor, libtor is actually compiled out on windows. So in theory it shouldn't break because it never really gets included. We also specify not to include tari_libtor for our windows builds. So let's see what happens!
Don't worry we're not merging until we validate.
There was a problem hiding this comment.
nice, I don't actually care about windows - but I sweated a lot trying to get it to work on there 😩
(the tor project itself doesn't even compile natively on windows, they cross compile from linux with msys2/mingw)
There was a problem hiding this comment.
they cross compile from linux with msys2/mingw)
That's an interesting note. Not that I'm going to dive into it, but that our pre-compiled windows bin could actually contain libtor possibly, as long as we cross compile.
Compiling natively would never have it, but it's an interesting option.
There was a problem hiding this comment.
oh that's definitely interesting, good point.
Test Results (Integration tests) 2 files + 2 11 suites +11 12m 27s ⏱️ + 12m 27s For more details on these failures, see this check. Results for commit 15741ca. ± Comparison against base commit 3396168. ♻️ This comment has been updated with latest results. |
ghpbot-tari-project
removed
the
P-reviews_required
stringhandler
merged commit b5e0d06
into
tari-project:development
Description
This changes the defaults to include libtor for the MinoTari Node and Console wallet by default. Removing the need to run tor independently.
Closes #6050
Motivation and Context
Make setup a smidge easier
How Has This Been Tested?
Locally: Ran the node and wallet without having a tor session started on the machine. Node and wallet used the built in libtor no problem.
Changed
use_libtortofalsein the config. Started the node and received aExitError { exit_code: TorOfflineerror as expected.Turned on local tor session, and restarted node with no issue. Node is now running on provided tor, and not built in.
Windows built without error in CI: https://github.com/brianp/tari/actions/runs/7274136545
@Cifko did the windows testing for me. He could compile natively as usual without any libtor related problems.
He also ran the installed package from the test builds with no issue.
This confirms that us setting a libtor default for *nix systems is not currently interfering with windows compilation or builds.
Breaking Changes