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

support pre-generate bindings for aarch64 #430

Merged
merged 13 commits into from
Mar 5, 2020
Merged

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Feb 20, 2020

follow #356 this pr support pre generated binding for aarch64, so we can get rid of depending on newer version of clang.

Signed-off-by: glorv glorvs@163.com

@BusyJay
Copy link
Member

BusyJay commented Feb 20, 2020

Thanks for your contribution. But I'm worry about the maintenance of the feature. On one hand, the bindings should be tested by CI to ensure the generated bindings actually work. On the other hand, because we don't have machines that are aarch64 based. It will be broken when we are trying to upgrade grpc c core.

@glorv
Copy link
Contributor Author

glorv commented Feb 20, 2020

Thanks for your contribution. But I'm worry about the maintenance of the feature. On one hand, the bindings should be tested by CI to ensure the generated bindings actually work. On the other hand, because we don't have machines that are aarch64 based. It will be broken when we are trying to upgrade grpc c core.

  • we can add test for arm64 either in travis-ci or in our ci, and if necessary, we can add extra test cases to make sure all the bindings are properly generated.
  • we already had aarch64 machine now, and we have done many tests on aarch64 machine provided by Huawei, so it is not hard to dev or run test on aarch64 arch machine if needed. From v4.0, we will start release aarch64 tidb package, so we will also run consistent test on aarch machines.

@glorv glorv force-pushed the aarch-bindgen branch 5 times, most recently from 7c62542 to 16106e3 Compare March 3, 2020 06:31
glorv added 3 commits March 3, 2020 17:41
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
glorv added 6 commits March 3, 2020 18:29
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@glorv
Copy link
Contributor Author

glorv commented Mar 5, 2020

@BusyJay PTAL

addons:
apt:
update: true
packages:
Copy link
Member

Choose a reason for hiding this comment

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

generating bindgen needs LLVM support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. It seems the default CI environment already install llvm tool chains. On the other hand, there are no llvm apt repo available on aarch64/linux, thus made it hard to install llvm tool chain on this platform, so just use the system default packages maybe a good idea currently.

BusyJay
BusyJay previously approved these changes Mar 5, 2020
Signed-off-by: glorv <glorvs@163.com>
glorv added 2 commits March 5, 2020 13:51
Signed-off-by: glorv <glorvs@163.com>
Signed-off-by: glorv <glorvs@163.com>
@glorv
Copy link
Contributor Author

glorv commented Mar 5, 2020

@hunterlxt PTAL again~

hunterlxt
hunterlxt previously approved these changes Mar 5, 2020
Copy link
Member

@hunterlxt hunterlxt left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

Signed-off-by: glorv <glorvs@163.com>
@hunterlxt hunterlxt merged commit 0bb4c60 into tikv:master Mar 5, 2020
hunterlxt added a commit that referenced this pull request Mar 5, 2020
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.

3 participants