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 GMT master instead of using conda-forge #261

Merged
merged 24 commits into from
Jan 21, 2019
Merged

Conversation

seisman
Copy link
Member

@seisman seisman commented Jan 16, 2019

The conda-forge development package creates all sorts of problems for us with
mixtures of different channels, etc. It's a hassle to setup and we end up falling behind
new features added to GMT 6 (which is what we support).

Replace the conda-forge package with GMT built from the master branch on the CI
services. Update the install instructions to tell users to build GMT from source.

.travis.yml Outdated
@@ -13,6 +13,7 @@ branches:
- master
# Regex to build tagged commits with version numbers
- /\d+\.\d+(\.\d+)?(\S*)?$/
- /test-*/
Copy link
Member Author

Choose a reason for hiding this comment

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

This line needs to be removed.

Copy link
Member

Choose a reason for hiding this comment

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

Why is it there? Travis should build this branch when the PR is opened so we don't really need it.


git clone --depth=1 https://github.com/GenericMappingTools/gmt "${HOME}/gmt-master"
cd "${HOME}/gmt-master"
bash ci/travis-setup.sh
Copy link
Member

Choose a reason for hiding this comment

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

@seisman this might be a problem because it uses the Ubuntu packages to compile GMT. So it won't work on Mac. Ideally, we should try to use the conda installed packages but this has causes a lot of trouble for me in the past.

Copy link
Member Author

Choose a reason for hiding this comment

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

@leouieda Currently, the master branch is only built on Linux with Python 3.6 installed. If you really want to test the GMT master branch on Mac, we should use homebrew.

Copy link
Member

Choose a reason for hiding this comment

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

OK, do you have any idea how that works? If we can get that setup, I would be willing to ditch the conda-force based jobs in favor of this. It's a lot easier to control and would actually help test GMT itself.

Copy link
Member

Choose a reason for hiding this comment

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

I'll try to get this setup in a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

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

@leouieda See this PR GenericMappingTools/gmt#146.

The only problem is it's too slow, (3 minutes for Linux builds, 13 minutes for Mac builds).
That's why this PR is not merged into GMT master branch.

If you think it's acceptable, I can add it to gmt-python.

@seisman
Copy link
Member Author

seisman commented Jan 16, 2019

@leouieda Three of the six build jobs fail. The GMT master branch fails because of some incompatible changes of psbasemap in the latest GMT. We can open a new PR to fix these tests failures.

@leouieda
Copy link
Member

Perhaps we should outsource this to a continuous-integration repo on this org so we can share the same scripts as base GMT?

@seisman
Copy link
Member Author

seisman commented Jan 16, 2019

Since we always need to clone the main GMT repo to build the latest GMT, it makes more sense to put these scripts in the main GMT repo.

@leouieda
Copy link
Member

Since we always need to clone the main GMT repo to build the latest GMT, it makes more sense to put these scripts in the main GMT repo.

Right, I forgot about this. That makes sense.

leouieda added a commit to GenericMappingTools/gmt that referenced this pull request Jan 21, 2019
Travis allows caching of certain directories. For GMT/Python, we're
using the build script from this repo to build the latest GMT master
branch and test against it. But we cache the `build` directory to
speedup the build process (GenericMappingTools/pygmt#261).
seisman pushed a commit to GenericMappingTools/gmt that referenced this pull request Jan 21, 2019
Travis allows caching of certain directories. For GMT/Python, we're
using the build script from this repo to build the latest GMT master
branch and test against it. But we cache the `build` directory to
speedup the build process (GenericMappingTools/pygmt#261).
@leouieda
Copy link
Member

Build is working on Linux and Mac. The failure is because of the incompatibility with GMT master.


mkdir -p "$INSTALLDIR"
mkdir -p "$COASTLINEDIR"
git clone --depth=1 --branch=master https://github.com/GenericMappingTools/gmt.git tmp
Copy link
Member Author

Choose a reason for hiding this comment

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

Why not clone it to gmt-master directly? Any side-effects?

Copy link
Member

Choose a reason for hiding this comment

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

It's because of the cache. I'm having Travis cache gmt-master/build so we don't recompile things that didn't change. But git won't checkout into an existing directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it.

@leouieda leouieda changed the title Test gmt-python with GMT master branch Build GMT master instead of using conda-forge Jan 21, 2019
@leouieda
Copy link
Member

@seisman I'm wondering if we should stop relying on the conda-forge development builds and tell people to build GMT from source. This is package is still under development and doesn't have to be trivial to use at the moment. When GMT 6 is release, we can start relying on the conda-forge package to make installing easier. What do you think?

We can add some instructions to the Install page to make this easier.

@seisman
Copy link
Member Author

seisman commented Jan 21, 2019

@seisman I'm wondering if we should stop relying on the conda-forge development builds and tell people to build GMT from source. This is package is still under development and doesn't have to be trivial to use at the moment. When GMT 6 is release, we can start relying on the conda-forge package to make installing easier. What do you think?

We can add some instructions to the Install page to make this easier.

Agree with you, we should drop conda-forge before GMT 6 is released.

@leouieda
Copy link
Member

The only thing is that we are gonna get a lot more issues because of this :( But I think it's a necessary evil.

@leouieda
Copy link
Member

OK, I'll update the install instructions and then we should be good to go. I think I'll try to maintain the conda-forge packages still but only as a last resort or for the binder demo.

@leouieda
Copy link
Member

@GenericMappingTools/python-contributors please be advised that we'll need to build GMT from source from now on. Don't hesitate to get in touch if you need help getting that to work. We'll work on including some instructions for this. A good starting point is this script https://github.com/GenericMappingTools/gmt/blob/master/ci/build-gmt.sh

.travis.yml Outdated
- liblapack-dev
- ghostscript
- curl
- graphicsmagick
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to install graphicsmagick, python and python-pip here, since they're used by GMT to do tests and build documentations.

Copy link
Member

Choose a reason for hiding this comment

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

Right, good catch

@leouieda
Copy link
Member

Merging.

@leouieda leouieda merged commit 79c879e into master Jan 21, 2019
@leouieda leouieda deleted the test-gmt-master branch January 21, 2019 21:04
leouieda added a commit that referenced this pull request Jan 22, 2019
It's broken and after #261 we won't have the conda-forge GMT builds
anymore. That makes it tricky to offer the demo. It served its purpose
and we can revive it after GMT 6 is out.
leouieda added a commit that referenced this pull request Jan 22, 2019
It's broken and after #261 we won't have the conda-forge GMT builds
anymore. That makes it tricky to offer the demo. It served its purpose
and we can revive it after GMT 6 is out.
weiji14 added a commit to weiji14/deepbedmap that referenced this pull request Feb 8, 2019
Couple of breaking changes. Since GenericMappingTools/pygmt#261, the conda-forge binary package is broken (hence all the recent CI build failures...) and we have to build the GMT binary from source, sigh. The new Dockerfile code for handling this isn't nice but it'll do for now.

Also, the Python `gmt-python` package has been renamed to `pygmt` in GenericMappingTools/pygmt#265, so the Pipfile is updated accordingly here. Also pinning to a newer git version on the upstream branch instead of my personal fork, since GenericMappingTools/pygmt#245 has been merged in!
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.

None yet

2 participants