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

CI: add build jobs for macOS and Windows in travis.yml #214

Merged
merged 40 commits into from
Feb 1, 2024

Conversation

fkuehlein
Copy link
Collaborator

@fkuehlein fkuehlein commented Jan 12, 2024

I will have to do this step-wise to see what works, as Travis CI documentation does not really cover our specific case. Will start with the macOS job, add a Windows job after that, and finally add the Linux jobs back again, hopefully not needing too many builds along the way.

@fkuehlein fkuehlein linked an issue Jan 12, 2024 that may be closed by this pull request
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9779716) 60.72% compared to head (138a445) 60.72%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #214   +/-   ##
=======================================
  Coverage   60.72%   60.72%           
=======================================
  Files          44       44           
  Lines        6357     6357           
=======================================
  Hits         3860     3860           
  Misses       2497     2497           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- and comment out linux job matrix for now
- remove `sed` commands for macos build
- try refactoring job matrix, including linux jobs again
- remove linux jobs again, try finishing osx first
@fkuehlein
Copy link
Collaborator Author

fkuehlein commented Jan 12, 2024

  • macOS: building successfully, although wasn't yet able to combine with the linux jobs.

  • Windows: Silent install of miniconda.exe is not working somehow, just timing out.
    Tried going with choco install miniconda3 instead, but cannot find conda command afterwards. Maybe /AddToPath not working? Package does not appear to be maintained ...

    Will probably just go with choco install python instead.

- and remove install options
- as adding to build matrix not working somehow
- found a typo in the choco install, try that one again now
- cut `refrenshenv` command
- and put `path` in new line
@fkuehlein
Copy link
Collaborator Author

fkuehlein commented Jan 15, 2024

This PR is becoming a bit of a mess, but I've run into multiple issues here. @ntfrgl, sorry for bothering you with it at this stage, but maybe you have a quick idea on what direction to proceed? I feel like I'm acting pretty clumsy on this, have no experience whatsoever with windows and/or git bash, don't want to waste too much time and credits...

Relevant parts of Travis docs including examples: explicitly including jobs into build matrix, run travis on multiple os, the windows build environment

i. combining the linux and macos jobs

macOS builds are running fine, although taking about 20 minutes which could be sped up maybe. Only I could not figure out how to expand the linux build matrix with a single macOS build (see 5145e49 for my most recent try). Therefore, in f66d3e3 I just added all linux jobs explicitly, which is not very elegant but works.

ii. installing miniconda on windows vm

I tried installing miniconda two ways:

  • download the .exe file with wget or curl and do a silent install (/S flag) (see 36c107e and fe538c9) as proposed by Anaconda docs:

    $> curl https://repo.anaconda.com/miniconda/Miniconda3-latest-Windows-x86_64.exe -o miniconda.exe
    $> start /wait "" miniconda.exe /S

    Problem is the build will just time out with no response after 10m. I tried the same in cmd.exe on a Windows machine and it finished within a minute. Seems to me that the second command is run before the first one is finished and thus will never succeed (see build logs).

  • install via chocolatey:

    $> choco install miniconda3 --params="'/S /InstallationType=JustMe /AddToPath=1 /D=%UserProfile%\miniconda'"

    Problem is it won't find the conda command after that (see 46615ec), although the /AddToPath flag is set. Probably need to refresh environment variables but refreshenv command is not found either (see 8db98d0).

now an alternative would be to go without conda and just pip instead, of course, but would then need to rewrite the everything for just the windows case.

@fkuehlein
Copy link
Collaborator Author

Finally got the choco install miniconda3 to run in 7698e85 thanks to an example found here.

Now pyunicorn will install fine, but when trying to run tests I get this error from matplotlib.font_manager when importing igraph:

..\..\..\miniconda\envs\test-env\lib\site-packages\matplotlib\font_manager.py:215: in win32FontDirectory
    return os.path.join(os.environ['WINDIR'], 'Fonts')
..\..\..\miniconda\envs\test-env\lib\os.py:679: in __getitem__
    raise KeyError(key) from None
E   KeyError: 'WINDIR'

see build log for full error message. I checked for the $WINDIR environment variable in 147a406 and it is correctly set (see build log), don't understand why matplotlib won't find it and hence giving up for today. Don't think installing miniconda via choco is optimal anyway because it is limited to python=3.9 and does not appear to be maintained ...

@fkuehlein
Copy link
Collaborator Author

fkuehlein commented Jan 22, 2024

Same problem occurs without conda, just using pip (see commit above).

@fkuehlein fkuehlein added this to the Release 0.7 milestone Jan 31, 2024
- consolidate build matrix
- remove `language: python`
- macOS: force GNU `sed`
- Windows: pass `$WINDIR` into `tox`
- set env vars in `before_install` phase
- remove a layer of quoting in `sed` args
- Tox: skip package installation inside venv
  (packaging & Cython have matured)
- Travis: avoid `brew update`
- don't trust the documentation
@ntfrgl
Copy link
Member

ntfrgl commented Feb 1, 2024

@fkuehlein, thank you for getting all the tedious work done! This is a big step forward for the project.

In my commits above, I resolved the remaining issues you mentioned, and achieved some simple speedups.

@ntfrgl ntfrgl marked this pull request as ready for review February 1, 2024 05:59
@fkuehlein
Copy link
Collaborator Author

Thank you lots, @ntfrgl! Great to see all green checks on this ... with the travis.yml containing everything we need yet being boiled down to improved readability and fair speed, I'll merge this right away. :)

@fkuehlein fkuehlein merged commit 296045d into master Feb 1, 2024
3 checks passed
@fkuehlein fkuehlein deleted the 201-windows_macos branch February 2, 2024 08:52
@ntfrgl ntfrgl mentioned this pull request Feb 21, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: set up Travis CI for macOS and Windows builds
2 participants