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

Move x86 tests from Travis to GHA, add aarch64 wheel build to Travis #3026

Merged
merged 6 commits into from
Jan 30, 2021
Merged

Move x86 tests from Travis to GHA, add aarch64 wheel build to Travis #3026

merged 6 commits into from
Jan 30, 2021

Conversation

janaknat
Copy link
Contributor

Move Travis x86 tests to Github Actions. Also, add Travis-CI config to build aarch64 wheels.

@janaknat
Copy link
Contributor Author

@mpenkov I moved the x86 tests to GHA to preserve Travis credits.

pull_request:
branches: [ develop ]
schedule:
- cron: '0 0 * * sun,wed'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this here? These should run on each PR, not on a schedule.

- name: Enable core dumps
run: ulimit -c unlimited -S # enable core dumps
- name: Run tox tests
run: tox -e ${{ matrix.tox }} -vv
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is vv necessary here? Tox will spew lots of output that nobody really ever reads.

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 following the existing tests in .travis.yml. I can remove it.

.travis.yml Outdated
dist: trusty
branches:
only:
- develop
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this file actually do? It looks to me like it will build a wheel on every commit, which isn't what we want, right?

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 guess not. I can build on every release. Is that done through master?

Copy link
Collaborator

@mpenkov mpenkov Jan 22, 2021

Choose a reason for hiding this comment

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

Releases are done on each tag that matches the regex v\d+\.\d+\.\d+.

But we also want to build wheels twice a week (like with the other github workflow that you implemented).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can create a cron job from the UI. Can't find any documentation about setting it up through the .travis.yml file. https://docs.travis-ci.com/user/cron-jobs/

.travis.yml Outdated
- build_wheel $REPO_DIR $PLAT
script:
- install_run $PLAT
after_success:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are after_success and after_failure steps exactly the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove the after_failure? I followed what gensim-wheels was doing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If the two are identical, then yes. More specifically:

  • remove after_failure
  • make after_success another step (the last step in the workflow)
  • ensure that all steps in the workflow run, even if preceding steps fail

@mpenkov mpenkov changed the title Change Travis-CI config move x86 tests from Travis to GHA, add aarch64 wheel build to Travis Jan 17, 2021
@janaknat
Copy link
Contributor Author

@mpenkov I've made the changes.

@mpenkov
Copy link
Collaborator

mpenkov commented Jan 20, 2021

@janaknat Thank you.

In the future, can you please avoid force-pushing? It makes it more difficult to review your PR as a maintainer.

@janaknat
Copy link
Contributor Author

@mpenkov I've made the changes.

@@ -1,6 +1,6 @@
branches:
only:
- master
- /v\d+\.\d+\.\d+/
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to match to a tag name, not a branch name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on https://docs.travis-ci.com/user/customizing-the-build/#safelisting-or-blocklisting-branches I believe having a regex in branches.only will only build tagged commits.

Tagged commit build: https://travis-ci.com/github/janaknat/gensim/builds/214207365

@janaknat
Copy link
Contributor Author

@mpenkov Is there anything in the PR that I should address?

@janaknat
Copy link
Contributor Author

@mpenkov Ping.

@mpenkov
Copy link
Collaborator

mpenkov commented Jan 30, 2021

Sorry, I was busy with other projects during the week. Thank you for your patience.

Let's merge this and see how it plays out in the wild.

@mpenkov mpenkov merged commit 01cffc4 into piskvorky:develop Jan 30, 2021
@mpenkov
Copy link
Collaborator

mpenkov commented Jan 30, 2021

Looks like there are some problems, @janaknat please investigate.

https://github.com/RaRe-Technologies/gensim/runs/1796402952?check_suite_focus=true

@janaknat
Copy link
Contributor Author

@mpenkov Are there any caches being used? The errors look like issues with trying to download tox, gdb and the IP not being accessible.

@mpenkov
Copy link
Collaborator

mpenkov commented Jan 30, 2021

Not that I'm aware of. The github actions stuff is all very new, from memory, you're one of the first people to contribute to it with gensim.

@janaknat
Copy link
Contributor Author

@mpenkov I've got a PR that solved the issue for me. Looks like the repos are out of date. An apt update was required. #3035

@janaknat
Copy link
Contributor Author

@mpenkov Any dates on when the aarch64 wheels show up in PyPI?

@piskvorky
Copy link
Owner

For my part, I plan to do an open source sprint in February to finish up my tasks for the 4.0.0 Milestone.

Which of course doesn't mean we cannot do another beta / alpha release before that – up to @mpenkov .

@janaknat
Copy link
Contributor Author

@piskvorky @mpenkov Any updates on when the aarch64 wheels will get released?

@piskvorky
Copy link
Owner

piskvorky commented Mar 10, 2021

We're planning to release 4.0.0rc1 (release candidate) this week. And then the "full" 4.0.0 right after :)

@janaknat
Copy link
Contributor Author

That's great. aarch64 is on track to get wheels this time around?

@piskvorky
Copy link
Owner

Hm, I thought that's what this PR was about. @mpenkov will any other steps be necessary to get from here to wheels on PyPI?

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 15, 2021

We'll look at it as part of the release process. If there's any additional work required to get the wheels into PyPI once they're built, we'll take care of it then.

@piskvorky piskvorky changed the title move x86 tests from Travis to GHA, add aarch64 wheel build to Travis Move x86 tests from Travis to GHA, add aarch64 wheel build to Travis Mar 21, 2021
@janaknat
Copy link
Contributor Author

Hey @mpenkov @piskvorky I see gensim has a new release. I don't see aarch64 wheels. Any issues?

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 27, 2021

I couldn't get the pipeline working when making the release. I'll debug the issue.

Thank you for reminding me.

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 27, 2021

@janaknat
Copy link
Contributor Author

@mpenkov You need to migrate to Travis-ci.com from travis-ci.org. Arm64 runners are only available in .com.

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 27, 2021

OK, so this is related to #3083.

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