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

Build more wheels for Linux, macOS and Windows #251

Merged
merged 4 commits into from
Feb 12, 2024

Conversation

Schamper
Copy link
Contributor

@Schamper Schamper commented Feb 3, 2024

I've previously used this to build my own wheels for yara-python, but since there's now already a GitHub Actions contributed, it seems fitting to add these!

Copy link

google-cla bot commented Feb 3, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@HydraDragonAntivirus
Copy link

Thanks for wheels I installed yara based on your wheels.

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to parametrize the name of the artifact, because you're adding a matrix, so several artifacts will be produced.

      - name: Store the distribution packages
        uses: actions/upload-artifact@v4
        with:
          name: python-package-distributions-${{ matrix.os }}-${{ matrix.arch }}
path: dist/*.whl

Copy link
Contributor

Choose a reason for hiding this comment

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

And it's seems that the --output-dir dist/ attribute is loose 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I've added the workflow, but I should have made sure that it could be run on a pull_request event.

Copy link
Contributor Author

@Schamper Schamper Feb 5, 2024

Choose a reason for hiding this comment

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

I didn't touch the triggers yet because I'm not sure what the preference of the team is, but I've proposed something that I personally like.

Thanks for the tip on the artifact name. I hadn't used it like that before, but I believe I also made the correct changes to the download-artifact step to have it work as expected.

I also believe dist is the default output directory, but I've re-added it for completeness.

Since the QEMU run is quite expensive, I've also added a concurrency limit that I used in my own CI for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can add tags to the push event, so the job publish-to-pypi will work, otherwise LGTM

on:
  push:
    branches:
      - master
    tags:
  pull_request:
  workflow_dispatch:

@plusvic plusvic merged commit ad0ec37 into VirusTotal:master Feb 12, 2024
8 checks passed
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