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 macOS build #2661

Merged
merged 2 commits into from
Dec 10, 2022
Merged

CI: add macOS build #2661

merged 2 commits into from
Dec 10, 2022

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Nov 23, 2022

Add a macOS build with tests on the CI.

@nilason nilason added enhancement New feature or request macOS macOS specific CI Continuous integration labels Nov 23, 2022
@nilason nilason added this to the 8.3.0 milestone Nov 23, 2022
@neteler
Copy link
Member

neteler commented Nov 23, 2022

"macOS / macOS build (pull_request) Successful in 17m"

Superb 💯 (also compared to the Ubuntu CI "speed" here)

@wenzeslaus
Copy link
Member

...compared to the Ubuntu CI "speed" here

Notably, this is currently without tests unlike the Ubuntu CI. For speed, compare with GCC C/C++ standards check instead.

Comment on lines 1 to 5
# This file may be used to create an environment using:
# $ conda create --name <env> --file <this file>
# platform: osx-64
@EXPLICIT
https://conda.anaconda.org/conda-forge/osx-64/pandoc-2.19.2-h694c41f_1.tar.bz2
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with this type of file, so I'm wondering why this file and not an environment.yml file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is also an environment file, only with explicit packages. I took it from https://github.com/nilason/grass-conda for speed and predictability. Turned out an ordinary type of file doesn't make much difference in time. If predictability will be an issue we'll return to this question. I now changed to standard file, easier to maintain.

@nilason
Copy link
Contributor Author

nilason commented Nov 24, 2022

...compared to the Ubuntu CI "speed" here

Notably, this is currently without tests unlike the Ubuntu CI. For speed, compare with GCC C/C++ standards check instead.

Compile time is decent, nothing spectacular. Testing is not particularly fast.
It would have been good to have an Apple ARM based platform. GitHub doesn't have any unfortunately. Maybe https://cirrus-ci.org could be an alternative?

@nilason
Copy link
Contributor Author

nilason commented Nov 24, 2022

I have now opened this for review.

Following tests fails:

r.in.pdal

  • test_r_in_pdal_binning
  • test_r_in_pdal_selection

r.sun

  • test_rsun

Testreport available as artefact.

@nilason
Copy link
Contributor Author

nilason commented Nov 28, 2022

Now added using cache for deps, saving ~3 min.

@wenzeslaus
Copy link
Member

As an alternative to test success percentage, you could also disable specific tests, but at this point you would have to duplicate the whole test configuration in the .gunittest.cfg file because the system can't combine two configurations.

@nilason
Copy link
Contributor Author

nilason commented Nov 28, 2022

Oops, this installs grass into environment before caching, thus includes initial build of grass into cache. Not so good. Either remove or different install dir...

@wenzeslaus
Copy link
Member

I don't quickly see why is that, but the 3 mins may not be worth the trouble given the total time (the time fluctuation of GCC seems about 3 mins or so).

@nilason
Copy link
Contributor Author

nilason commented Nov 29, 2022

I don't quickly see why is that, but the 3 mins may not be worth the trouble given the total time (the time fluctuation of GCC seems about 3 mins or so).

See log extract:

ERROR: Module built against version 87ba3f2 but trying to use version
[1806](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1807)
       771e1f0. You need to rebuild GRASS GIS or untangle multiple
[1807](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1808)
rm test.raster3d.lib.tmp.html
[1808](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1809)
       installations.
[1809](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1810)
/usr/bin/install -c  -m 644 N_gwflow.h /Users/runner/work/grass/grass/dist.x86_64-apple-darwin13.4.0/include/grass/N_gwflow.h
[1810](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1811)
make[3]: *** [test.raster3d.lib.tmp.html] Error 1
[1811](https://github.com/OSGeo/grass/actions/runs/3567806784/jobs/5995944634#step:8:1812)

Now I added separate install dir. Let's see how it works out. (No trouble on my side :)

@nilason
Copy link
Contributor Author

nilason commented Nov 29, 2022

As an alternative to test success percentage, you could also disable specific tests, but at this point you would have to duplicate the whole test configuration in the .gunittest.cfg file because the system can't combine two configurations.

How would I duplicate the .gunittest.cfg file (where to and to what name)? And how to reference to it for the tests on Mac?

@wenzeslaus
Copy link
Member

How would I duplicate the .gunittest.cfg file (where to and to what name)? And how to reference to it for the tests on Mac?

The test executor has a --config parameter:

> python -m grass.gunittest.main --help
...
  --config CONFIG       Path to a configuration file (default: .gunittest.cfg)

The file could be in .github/workflows/ like the CI scripts or in macosx/ if that seems to be general limitation on macOS right now, named, e.g., gunittest_macos.cfg.

@nilason
Copy link
Contributor Author

nilason commented Nov 29, 2022

How would I duplicate the .gunittest.cfg file (where to and to what name)? And how to reference to it for the tests on Mac?

The test executor has a --config parameter:

> python -m grass.gunittest.main --help
...
  --config CONFIG       Path to a configuration file (default: .gunittest.cfg)

The file could be in .github/workflows/ like the CI scripts or in macosx/ if that seems to be general limitation on macOS right now, named, e.g., gunittest_macos.cfg.

Aha, thanks!

@nilason
Copy link
Contributor Author

nilason commented Nov 30, 2022

Remains three tests which often fails on CI, but passes ! locally:

  • raster/r.terraflow -- test_r_terraflow.py
  • scripts/g.extension -- test_addons_download.py
  • vector/v.db.select -- test_v_db_select.py

The r.terraflow test exceeds the time limit of 5 min, g.extension fail to download json file ("rate limit exceeded") and v.db.select fails for me unknown reason. Any suggestions (other than exclude the tests)?

(See test report)

@nilason
Copy link
Contributor Author

nilason commented Nov 30, 2022

The v.db.select test exits with:

ERROR: testGroup (__main__.SelectTest.testGroup)
Testing v.db.select with group option
----------------------------------------------------------------------
Traceback (most recent call last):
  File "vector/v.db.select/testsuite/test_v_db_select.py", line 210, in testGroup
    sel.run()
  File "etc/python/grass/pygrass/modules/interface/module.py", line 823, in run
    self.wait()
  File "etc/python/grass/pygrass/modules/interface/module.py", line 844, in wait
    raise CalledModuleError(
grass.exceptions.CalledModuleError: Module run `v.db.select map=geology layer=1 columns=GEO_NAME group=GEO_NAME format=plain -c` ended with an error.
The subprocess ended with a non-zero return code: -11. See errors above the traceback or in the error output.

"code: -11" is likely a segmentation fault.

@nilason
Copy link
Contributor Author

nilason commented Dec 7, 2022

I am inclined to merge this now. Better to have this running as is, even with the somewhat limited test range.

The following test files are added to the content of the main exclusion list (.gunittest.cfg), but in separate config file:

  • raster/r.in.pdal/testsuite/test_r_in_pdal_binning.py
  • raster/r.in.pdal/testsuite/test_r_in_pdal_selection.py
  • raster/r.sun/testsuite/test_rsun.py
  • raster/r.terraflow/testsuite/test_r_terraflow.py
  • scripts/g.extension/testsuite/test_addons_download.py
  • vector/v.db.select/testsuite/test_v_db_select.py

The last three only fails on the CI for some strange reason(s).

@nilason
Copy link
Contributor Author

nilason commented Dec 9, 2022

This now runs with -Wall -Werror on Clang (now version 14.0.6), i.e. it fails if new compiler warnings are detected.
If there are no objections, I'll merge this shortly.

@nilason nilason merged commit a7ee9e4 into OSGeo:main Dec 10, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@nilason nilason deleted the ci_add_mac_build branch May 15, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration enhancement New feature or request macOS macOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants