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.

Sign up for GitHub

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

Jump to bottom

fix: rust bindings build for macos #89

Merged
merged 11 commits into from
Oct 4, 2024
Merged

fix: rust bindings build for macos #89

merged 11 commits into from
Oct 4, 2024

Conversation

knst
Copy link

@knst knst commented Aug 27, 2024 •
edited by ogabrielides
Loading

That's some useful changes that blocked build, but it's not full final version yet.

Further included changes:

  • Relic Arch fix (also in fix: aarch64 is not supported, should set ARCH to RELIC_NONE #90)
  • Relic wasn't linked when building with feature "Apple"
  • GMP was bumped to 6.3.0 (Since 6.2.1 have issues on m1: also dropped the custom patches for 6.2.1)
  • Dropped apple.rust.sh and apple.rust.single.sh since only apple.rust.deps.sh is used.
  • Fix for passing correctly CC and CXX paths when building for Apple

kwvg
kwvg reviewed Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@kwvg kwvg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Consider cf62af3 for cleaned up history. Also, please change base branch to develop.

apple.rust.deps.sh Outdated Show resolved Hide resolved
rust-bindings/bls-signatures/src/lib.rs Outdated Show resolved Hide resolved
rust-bindings/bls-signatures/src/lib.rs Outdated Show resolved Hide resolved
kwvg
kwvg reviewed Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@kwvg kwvg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

EDIT: Disregard this comment message

@ogabrielides ogabrielides requested a review from kwvg October 1, 2024 14:13
@HashEngineering
Copy link

here are my results - use #89 along with my own changes in #88 to build my system to support Android. There are several layers:

  • Rust running on Mac (ARM-64) - Yes, no issues
  • Java running on Mac (ARM-64) - Yes, no issues
  • Java running on Android (ARM-64) - Yes, no issues

ogabrielides
ogabrielides previously approved these changes Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

ACK b4ca778

kwvg
kwvg reviewed Oct 4, 2024
View reviewed changes
apple.rust.deps.sh
@@ -143,7 +144,7 @@ CC="$CLANG" CFLAGS="$CFLAGS" CPPFLAGS="$CFLAGS" LDFLAGS="$CFLAGS" \
--host=${HOST} --prefix="${CURRENT_DIR}/gmplib-${PFX}" \
--disable-shared --enable-static --disable-assembly -v
EOF

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: whitespace

kwvg
kwvg approved these changes Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

Please change the base branch to develop.

utACK b4ca778

@ogabrielides ogabrielides changed the base branch from main to develop October 4, 2024 08:38
@ogabrielides ogabrielides dismissed their stale review October 4, 2024 08:38

The base branch was changed.

ogabrielides
ogabrielides approved these changes Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

ACK b4ca778

knst
knst commented Oct 4, 2024
View reviewed changes
Copy link
Author

@knst knst left a comment

Choose a reason for hiding this comment

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

LGTM b4ca778

but I can't really self-review my own PR on github

@UdjinM6 UdjinM6 changed the title fix: build bls for mac fix: rust bindings build for macos Oct 4, 2024
UdjinM6
UdjinM6 approved these changes Oct 4, 2024
View reviewed changes
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

@ogabrielides ogabrielides merged commit a181889 into dashpay:develop Oct 4, 2024
14 of 27 checks passed
@ogabrielides ogabrielides mentioned this pull request Oct 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwvg kwvg kwvg approved these changes

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@ogabrielides ogabrielides ogabrielides approved these changes

@pankcuf pankcuf Awaiting requested review from pankcuf

@PastaPastaPasta PastaPastaPasta Awaiting requested review from PastaPastaPasta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@knst @HashEngineering @ogabrielides @UdjinM6 @kwvg