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

Update CI to support ARM build #76

Closed
wants to merge 1 commit into from
Closed

Conversation

WhiteSte
Copy link
Contributor

According to #61 (comment) I've tried to modify the CI in order to build the natives also for mac os arm64 architecture. The service flyci-macos-large-latest-m1 should be free on public repos for 500minutes/month, that should be quite enough as a temporary solution.

I'm not an expert of github actions, how can i test the pipeline if it works?

Comment on lines +77 to +81
- uses: actions/upload-artifact@v3
if: matrix.os == 'flyci-macos-large-latest-m1' && matrix.architecture == 'arm64'
with:
name: macos-arm64
path: native/target/native/arm64-darwin/bin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for this PR!

We also need modifying the publish section: https://github.com/PDAL/java/blob/main/.github/workflows/ci.yml#L132-L135

'upload' uploads built artifacts and then we 'download' them to pack into the result package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really practise, should we add

 - uses: actions/download-artifact@v3
        with:
          name: macos-ARM
          path: native/target/native/arm64-darwin/bin

Something like that after L 135?

@pomadchin
Copy link
Collaborator

I'm not an expert of github actions, how can i test the pipeline if it works?

I think the best way to test is configuring and testing CI in your fork. Sadly there is not really good way to do it.

Thanks for making a PR!

@WhiteSte
Copy link
Contributor Author

sounds like the new m1 runner is also building x86 arch and the old mac os one is building also arm arch . I will revise that

@WhiteSte WhiteSte marked this pull request as draft January 18, 2024 23:48
@pomadchin
Copy link
Collaborator

Interesting, these runners were not even allocated 🤔

@WhiteSte
Copy link
Contributor Author

umh maybe @kgantchev can give us some clarifications

@kgantchev
Copy link

Thanks, I'll look over the thread in a bit and I'll get back to you with more info ASAP.

@kgantchev
Copy link

Interesting, these runners were not even allocated

BTW, what do you mean here?

@WhiteSte
Copy link
Contributor Author

Interesting, these runners were not even allocated

BTW, what do you mean here?

the runners are not available

@kgantchev
Copy link

kgantchev commented Jan 19, 2024

the runners are not available

Got it. We had an issue yesterday which we resolved. I'll check with our team to see if you might have been affected. My apologies if you experienced an interruption!

@kgantchev
Copy link

@WhiteSte we don't see an app installation for this GitHub account. Please make sure that you have installed the FlyCI GitHub app on the PDAL account.

@WhiteSte you can also fork the repo, install the FlyCI app, and run the actions in your account to validate that everything is working properly before making the contribution here (I assume the maintainers will be the happiest with that approach).

@WhiteSte WhiteSte closed this Jan 20, 2024
@WhiteSte WhiteSte deleted the patch-2 branch January 20, 2024 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants