Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

When building on M1, inside of depends, you should use make NO_PROTOBUF=1 NO_OPENSSL=1 -j8

@PastaPastaPasta PastaPastaPasta added this to the 18 milestone Feb 9, 2022
Munkybooty
Munkybooty previously approved these changes Feb 10, 2022
Copy link

@Munkybooty Munkybooty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

it should be mentioned that you need to run

./configure --prefix=`pwd`/depends/aarch64-apple-darwin21.2.0 --disable-bip70

otherwise you'll be met with linker errors:

clang: error: linker command failed with exit code 1 (use -v to see invocation)
ld: symbol(s) not found for architecture arm64

make check finished in 1:55 and functional tests are still sluggish

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
@ogabrielides
Copy link

Compilation works on m1.
make check successful along with functional tests.

ogabrielides
ogabrielides previously approved these changes Feb 11, 2022
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to disable bip70 when there is no openssl (cause it's a dependency) but it feels weird to disable openssl when we simply don't want bip70 imo. How about smth like e308bef15b7214572ccac5a32868173b435ef251?

@Munkybooty
Copy link

OpenSSL unfortunately is the main blocker for Native M1 building. I believed when I had #4612 open that we may finally be able to build on M1 because protobuf being optional gave me the assumption that without bip70 we would have no issue. We realized upon building that that's when the OpenSSL was the true blocker. We use version 1.0.1 and M1 support exists in 1.0.2 and 1.1.1. When @kittywhiskers tried to bump the version we had some nontrivial errors come up iirc. That mostly is the motivation behind disabling it

@UdjinM6
Copy link

UdjinM6 commented Feb 11, 2022

@Munkybooty yes, I get it. My point is that disabling bip70 should not disable openssl cause it doesn't make sense in general (and esp. for non-M1 systems). With the patch I'm proposing you can either build depends with NO_OPENSSL=1 or specify --without-openssl in configure - both options will result in openssl AND bip70 disabled.

@Munkybooty
Copy link

@UdjinM6 Very fair point, I am in favor of your patch, it makes the most sense logically, I was hoping to clear any confusion if there was any. I see what you mean though

@PastaPastaPasta
Copy link
Member Author

What is the use case for including openssl if you're not including bip70? @UdjinM6

@UdjinM6
Copy link

UdjinM6 commented Feb 11, 2022

@PastaPastaPasta OpenSSL removal in bitcoin bitcoin#17265 was delayed until the merge of the replacement for RNG state initializers PR bitcoin#17270. I feel like we shouldn't drop openssl that easy too. It might be ok for edge cases like M1 native builds (i.e. "I want it now and I don't care!!11" use case :) ) but I'd rather not make it a common practice for other builds. Basically, you should disable openssl explicitly (presumably) understanding all the consequences, it should NOT be disabled silently because of some internal non-obvious logic.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@UdjinM6 UdjinM6 merged commit b26eaf6 into dashpay:develop Feb 13, 2022
Copy link

@Munkybooty Munkybooty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post tACK

@PastaPastaPasta PastaPastaPasta deleted the m1-build-pr branch February 15, 2022 15:49
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Nov 9, 2023
…builds (dashpay#4683)

* build: allow building without openssl, enables native m1 development builds

* Update configure.ac, according to review

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

* introduce `--with-openssl`

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
gades pushed a commit to piratecash/pirate that referenced this pull request Dec 9, 2023
…builds (dashpay#4683)

* build: allow building without openssl, enables native m1 development builds

* Update configure.ac, according to review

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

* introduce `--with-openssl`

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
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.

4 participants