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

gmt5: update gmt6 sub-port to version 6.0.0 #5675

Merged
merged 1 commit into from Nov 8, 2019
Merged

gmt5: update gmt6 sub-port to version 6.0.0 #5675

merged 1 commit into from Nov 8, 2019

Conversation

seisman
Copy link
Contributor

@seisman seisman commented Nov 1, 2019

Description

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.14.6 18G95
Xcode 10.3 10G8

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@tenomoto for port gmt5.

@macportsbot
Copy link

Travis Build #8982 Passed.

Lint results
--->  Verifying Portfile for gmt5
--->  0 errors and 0 warnings found.

Port gmt5 success on xcode10.3. Log
Port gmt6 success on xcode10.3. Log
Port gmt5 success on xcode9.4. Log
Port gmt6 success on xcode9.4. Log
Port gmt5 success on xcode8.3. Log
Port gmt6 success on xcode8.3. Log
Port gmt5 success on xcode7.3. Log

@seisman
Copy link
Contributor Author

seisman commented Nov 4, 2019

This PR is ready for review. Can anyone help review and merge it?

Copy link
Contributor

@remkos remkos left a comment

Choose a reason for hiding this comment

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

The new version 6 of GMT also looks for ghostscript, ffmpeg and graphicsmagick during the cmake configuration.

  • ghostscript is needed in any case during runtime, so it is better to add that as a dependency.
  • ffmpeg and graphicsmagick are only used during runtime to make movies or animations, so it is not essential to have those. Because there is a check at runtime whether either is available, there is no risk of having it been found during configuration and not at runtime.

Can you at least add -DGS_ROOT=${prefix} in case of gmt6 (gmt5 does not have this)?

Furthermore, the cmake configuration has a tendency to mix the library and include files for curl between the system and macports, respectively, which of course is unwanted. I came across this with Fink (fink/fink-distributions#493). This can be prevented using:
-DCURL_INCLUDE_DIR=${prefix}/include
-DCURL_LIBRARY=${prefix}/lib/libcurl.dylib
In addition, curl should be added as a dependency.

@remkos
Copy link
Contributor

remkos commented Nov 5, 2019

@seisman, if you can make the suggested updates, I can (probably) approve.

@seisman
Copy link
Contributor Author

seisman commented Nov 5, 2019

  • ghostscript is needed in any case during runtime, so it is better to add that as a dependency.

ghostscript is already in the list of depends_lib.

Can you at least add -DGS_ROOT=${prefix} in case of gmt6 (gmt5 does not have this)?

Added for gmt6.

Furthermore, the cmake configuration has a tendency to mix the library and include files for curl between the system and macports, respectively, which of course is unwanted. I came across this with Fink (fink/fink-distributions#493). This can be prevented using:
-DCURL_INCLUDE_DIR=${prefix}/include
-DCURL_LIBRARY=${prefix}/lib/libcurl.dylib
In addition, curl should be added as a dependency.

curl is added to depends_lib. I think -DCURL_ROOT is enough, right?

@macportsbot
Copy link

Travis Build #9069 Errored.

Lint results
--->  Verifying Portfile for gmt5
--->  0 errors and 0 warnings found.

Port gmt5 success on xcode10.3. Log
Port gmt5 success on xcode9.4. Log
Port gmt6 success on xcode9.4. Log
Port gmt5 success on xcode8.3. Log
Port gmt6 success on xcode8.3. Log
Port gmt5 success on xcode7.3. Log

The build timed out.

@macportsbot
Copy link

Travis Build #9077 Failed.

Lint results
--->  Verifying Portfile for gmt5
--->  0 errors and 0 warnings found.

Port gmt5 success on xcode10.3. Log
Port gmt6 success on xcode10.3. Log
Port gmt5 success on xcode9.4. Log
Port gmt6 fail on xcode9.4. Log
Port gmt5's dependencies fail on xcode8.3. Log
Port gmt6 success on xcode8.3. Log
Port gmt5 success on xcode7.3. Log
Port gmt6 success on xcode7.3. Log

Copy link
Contributor

@remkos remkos left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes.
It seems like it works fine with -DCURL_ROOT=${prefix}.
Approved.

science/gmt5/Portfile Show resolved Hide resolved
science/gmt5/Portfile Outdated Show resolved Hide resolved
@mf2k
Copy link
Contributor

mf2k commented Nov 6, 2019

The commit message should be:

gmt5: update gmt6 sub-port to version 6.0.0

@seisman
Copy link
Contributor Author

seisman commented Nov 6, 2019

@mf2k Thanks for your review. I've addressed your comments and fixed the commit message.

@seisman seisman changed the title gmt6: update to 6.0.0 gmt5: update gmt6 sub-port to version 6.0.0 Nov 6, 2019
@macportsbot
Copy link

Travis Build #9093 Failed.

Lint results
--->  Verifying Portfile for gmt5
--->  0 errors and 0 warnings found.

Port gmt5 success on xcode10.3. Log
Port gmt6 success on xcode10.3. Log
Port gmt5's dependencies fail on xcode9.4. Log
Port gmt6 success on xcode9.4. Log
Port gmt5 success on xcode8.3. Log
Port gmt6 success on xcode8.3. Log
Port gmt5 success on xcode7.3. Log
Port gmt6 success on xcode7.3. Log

@mf2k
Copy link
Contributor

mf2k commented Nov 6, 2019

While this LGTM, this port is not openmaintainer so I want to give @tenomoto more time to look at this.

@seisman
Copy link
Contributor Author

seisman commented Nov 8, 2019

@mf2k It's been two days. Could you merge it?

@cjones051073
Copy link
Member

Looks fine to me. Merging under maintainer timeout.

@cjones051073 cjones051073 merged commit 1babf77 into macports:master Nov 8, 2019
@seisman seisman deleted the gmt6.0.0 branch November 8, 2019 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants