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

multi-architecture support #229

Closed
wants to merge 5 commits into from
Closed

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Sep 8, 2020

PR Details

Enables aarch64 support

#167

Description

Related Issue

Motivation and Context

Phones and Pi's now support vsys

How Has This Been Tested

rpi 4 and pinephone

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@ncying ncying left a comment

Choose a reason for hiding this comment

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

#167 we need seriously check and package updates. We can not directly cite some 3rd party dependency directly since everyone can upload their complied package to mvn-repo. I already noticed many related packages listed. But for some security problems, we still keep the original dependency first, since this support is not necessary for all devices.

And you should also need to test this update work or not. Build the jar package and run the node on an Android phone or Raspberry Pi.

@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

I think that I've chosen this package from a well-audited source, but if you have thoughts on another, please let me know.

Does travis provide build artifacts?

This builds now, because we have closed #226

what does the -all in a scala package refer to? Is it the instruction set architecture, or something else?

compiling on raspberry pi, if that works I'll put it on my PinePhone
Screen Shot 2020-09-09 at 11 52 30 AM

@codecov-commenter
Copy link

Codecov Report

Merging #229 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
- Coverage   63.70%   63.68%   -0.02%     
==========================================
  Files         199      199              
  Lines        5744     5744              
  Branches      297      297              
==========================================
- Hits         3659     3658       -1     
- Misses       2085     2086       +1     
Impacted Files Coverage Δ
src/main/scala/vsys/network/Handshake.scala 75.00% <0.00%> (-2.50%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d8cdd0...f73d2ad. Read the comment docs.

@ncying
Copy link
Member

ncying commented Sep 9, 2020

@faddat you should also check that package based on this new dependency can be well start in the aarch64 device.

@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

Screen Shot 2020-09-09 at 12 34 45 PM

So, seems I made it explode. Maybe library compatibility, I'll fiddle with it and see what I can do.

do you have a link to a good document that explains scala compilation? I'd like to refactor our build system to use mill, but that did not go so great for me so far.

ARM support confirmed.  Imported module is well tested.
@faddat faddat marked this pull request as ready for review September 9, 2020 07:48
@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

Screen Shot 2020-09-09 at 2 31 00 PM

Screen Shot 2020-09-09 at 2 32 46 PM

@faddat faddat mentioned this pull request Sep 9, 2020
@faddat
Copy link
Contributor Author

faddat commented Sep 12, 2020 via email

@faddat faddat changed the title aarch64 arm64 Nov 17, 2020
@faddat faddat changed the title arm64 multi-architecture support Nov 17, 2020
@faddat
Copy link
Contributor Author

faddat commented Nov 17, 2020

So I have reviewed this to the best of my knowledge. I do not think that it introduces any kind of security risk.

If we merge it, we gain hardware build targets for VSYS software.

@codecov-io
Copy link

Codecov Report

Merging #229 (12814d4) into master (7875b61) will decrease coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
- Coverage   66.42%   66.36%   -0.07%     
==========================================
  Files         199      199              
  Lines        5770     5770              
  Branches      277      277              
==========================================
- Hits         3833     3829       -4     
- Misses       1937     1941       +4     
Impacted Files Coverage Δ
src/main/scala/vsys/utils/Time.scala 75.00% <0.00%> (-5.00%) ⬇️
src/main/scala/vsys/network/Handshake.scala 75.00% <0.00%> (-2.50%) ⬇️
...ain/scala/vsys/api/http/TransactionsApiRoute.scala 53.92% <0.00%> (-0.99%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7875b61...12814d4. Read the comment docs.

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