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: Test GMT dev version on Windows by building from source #2773

Merged
merged 4 commits into from
Oct 29, 2023

Conversation

seisman
Copy link
Member

@seisman seisman commented Oct 27, 2023

Description of proposed changes

In the "GMT Dev Tests" workflow, the way we get the GMT dev version is different on Linux/macOS and Windows. We build the GMT master branch source code on Linux/macOS, but on Windows, we install the dev version from the conda-forge/dev channel. The conda-forge/dev channel is updated every few weeks to months, so it's not always update-to-date. We choose to use the conda-forge/dev channel for Windows because building GMT on Windows is difficult.

However, it turns out that building GMT on Windows is much easier than I initially though, as long as we install all GMT dependencies via conda.

In this PR, I update the workflow to build GMT from source code on Windows, so that we're always using the same GMT dev version on all three platforms.

Supersedes #756

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman seisman marked this pull request as draft October 27, 2023 14:10
@seisman seisman marked this pull request as ready for review October 27, 2023 14:11
@seisman seisman added the maintenance Boring but important stuff for the core devs label Oct 27, 2023
@seisman seisman added this to the 0.11.0 milestone Oct 27, 2023
@seisman seisman marked this pull request as draft October 27, 2023 14:14
@seisman seisman marked this pull request as ready for review October 27, 2023 14:14
@seisman seisman added the needs review This PR has higher priority and needs review. label Oct 27, 2023
@weiji14 weiji14 marked this pull request as draft October 29, 2023 01:59
@weiji14 weiji14 marked this pull request as ready for review October 29, 2023 01:59
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Nice! The compilation on Windows worked at https://github.com/GenericMappingTools/pygmt/actions/runs/6680586107/job/18153844748#step:8:405. I'll approve this since the build works, but I have one comment about setting more build flags (which can be done separately if needed).

mkdir build
cd build
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
cmake -G Ninja .. -DCMAKE_INSTALL_PREFIX=${{ env.GMT_INSTALL_DIR }} -DCMAKE_BUILD_TYPE=Release -DCMAKE_PREFIX_PATH=${{ env.MAMBA_ROOT_PREFIX }}\envs\pygmt\Library
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

We set the CMAKE_PREFIX_PATH flag to tell CMake where to find these libraries, so these flags are not necessary:

      -D FFTW3_ROOT=%LIBRARY_PREFIX% ^
      -D GDAL_ROOT=%LIBRARY_PREFIX% ^
      -D NETCDF_ROOT=%LIBRARY_PREFIX% ^
      -D PCRE_ROOT=%LIBRARY_PREFIX% ^
      -D ZLIB_ROOT=%LIBRARY_PREFIX% ^
      -D CURL_ROOT=%LIBRARY_PREFIX% ^

Parallelization support. Can be enable if necessary.

      -D GMT_ENABLE_OPENMP=TRUE ^
      -D GMT_USE_THREADS=TRUE ^

Not necessary:

      -D GMT_LIBDIR=lib ^

GSHHG and DCW data are not installed, but they can be automatically downloaded when needed.

      -D DCW_ROOT=%DCW_DIR% ^
      -D GSHHG_ROOT=%GSHHG_DIR% ^
      -D COPY_GSHHG=TRUE ^
      -D COPY_DCW=TRUE ^

Unnecessary flags for our "GMT Dev Tests":

      -D GMT_INSTALL_TRADITIONAL_FOLDERNAMES=FALSE ^
      -D GMT_INSTALL_MODULE_LINKS=FALSE ^

Copy link
Member

Choose a reason for hiding this comment

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

Parallelization support. Can be enable if necessary.

Adding this would be good!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in c6b1baa.

@seisman
Copy link
Member Author

seisman commented Oct 29, 2023

It's unclear to me why we have a dvc error in this branch, which lead to ~200 failures

ERROR: unexpected error - failed to load directory ('4d', 'd0ad31844cb0b0b451648cda314e2a.dir'): [Errno 2] No such file or directory: 'D:\\a\\pygmt\\pygmt\\.dvc\\cache\\files\\md5\\4d\\d0ad31844cb0b0b451648cda314e2a.dir'

The scheduled runs two days ago don't have the issue https://github.com/GenericMappingTools/pygmt/actions/runs/6661401871/job/18104207392.

@seisman seisman marked this pull request as draft October 29, 2023 04:04
@seisman seisman marked this pull request as ready for review October 29, 2023 04:04
@seisman
Copy link
Member Author

seisman commented Oct 29, 2023

/test-gmt-dev

@seisman seisman merged commit 7922882 into main Oct 29, 2023
@seisman seisman deleted the build-windows branch October 29, 2023 13:11
@seisman seisman removed the needs review This PR has higher priority and needs review. label Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants