-
Notifications
You must be signed in to change notification settings - Fork 213
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
Conversation
.travis.yml
Outdated
@@ -13,6 +13,7 @@ branches: | |||
- master | |||
# Regex to build tagged commits with version numbers | |||
- /\d+\.\d+(\.\d+)?(\S*)?$/ | |||
- /test-*/ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
ci/travis-build-gmt.sh
Outdated
|
||
git clone --depth=1 https://github.com/GenericMappingTools/gmt "${HOME}/gmt-master" | ||
cd "${HOME}/gmt-master" | ||
bash ci/travis-setup.sh |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
@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. |
Perhaps we should outsource this to a |
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. |
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).
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).
We're gonna recommend building from source at first
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
@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. |
…python into test-gmt-master
Agree with you, we should drop conda-forge before GMT 6 is released. |
The only thing is that we are gonna get a lot more issues because of this :( But I think it's a necessary evil. |
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. |
@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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, good catch
Merging. |
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.
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.
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!
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.