Bug 252168 - astro/opencpn: update to 5.2.4 [maintainer update]
Summary: astro/opencpn: update to 5.2.4 [maintainer update]
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Rainer Hurling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-26 18:15 UTC by ml
Modified: 2021-01-05 20:08 UTC (History)
1 user (show)

See Also:
ml: maintainer-feedback+


Attachments
SVN diff (12.67 KB, patch)
2020-12-26 18:15 UTC, ml
no flags Details | Diff
patch with additions and corrections (172.62 KB, patch)
2020-12-27 19:01 UTC, Rainer Hurling
no flags Details | Diff
patch with additions and corrections, v2 (175.81 KB, patch)
2020-12-27 19:55 UTC, Rainer Hurling
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ml 2020-12-26 18:15:05 UTC
Created attachment 220956 [details]
SVN diff
Comment 1 Rainer Hurling freebsd_committer 2020-12-27 19:01:34 UTC
Created attachment 221023 [details]
patch with additions and corrections

Hi Andrea,

Thanks for your patch!

I did some tests on Poudriere and with 'portlint -AC' and found some issues, for which I prepared an updated patch. These are the changes against your patch:

- USES=libarchive 
- Two dependencies added to LIB_DEPENDS: graphics/libexif and audio/libsndfile
- Documents should be stored under DOCSDIR. This needs a patched CMakeLists.txt
- No need to patch data/doc/help_en_US.html anymore
- files/patch-CMakeLists.txt should not replace wx-config, which is already
  substituted by REINPLACE_CMD in Makefile
- Complete rework of pkg-plist to get rid of %%PORTDOCS%%%%DATASDIR%%, 
  which 'portlint -AC' complains about. Instead there is now a real usage of DOCSDIR

Please feel free and ask, if you have questions ;)

If you like and agree the additions and corrections, I will prepare a review on Phabricator as my next step. It would be nice, if you could test a bit and give some feedback :)
Comment 2 Rainer Hurling freebsd_committer 2020-12-27 19:55:22 UTC
Created attachment 221025 [details]
patch with additions and corrections, v2

Sorry, I attached an incomplete patch before ...
Comment 3 ml 2020-12-28 09:34:19 UTC
Fine for me.
Comment 4 Rainer Hurling freebsd_committer 2020-12-28 14:38:13 UTC
After creating a review [1] on Phabricator with the latest patch, I got feedback from one of my mentors, see the inlince comment after line 40.

Is there any reason to work like this with LDFLAGS (lines 42+43)?

Could you also test please his suggestion about setting CMAKE_{C,CXX}_FLAGS instead of your CFLAGS and CXXFLAGS (lines 40+41)?

[1] https://reviews.freebsd.org/D27798
Comment 5 ml 2020-12-28 18:38:11 UTC
Sorry, I think this is beyond my skills.

I tried removing LDFLAGS, CFLAGS and CXXFLAGS: the port compiles and I can fire it up.
It's possible those were needed in the past and are not required anymore.

Unfortunately I don't have the chance to test the program properly (no harware at the time :-().
I was in touch with an user who tested the original patch and he says it works properly; maybe he will try this too.

BTW, how would I set CMAKE_C[XX]_FLAGS?
The previous Makefile has "C[XX]_FLAGS+=-fpic": notice the plus.
Would I put the plus in CMAKE_... too?
Comment 6 Rainer Hurling freebsd_committer 2020-12-28 20:56:59 UTC
(In reply to ml from comment #5)
> Sorry, I think this is beyond my skills.
Me too. Perhaps my mentors could give some more advice ;)

> I tried removing LDFLAGS, CFLAGS and CXXFLAGS:
> the port compiles and I can fire it up.
> It's possible those were needed in the past and are not required anymore.
I can confirm that it builds, installs and runs fine so far without LDFLAGS and CFLAGS|CXXFLAGS. But no chance to test with the special hardware ...

> Unfortunately I don't have the chance to test the program properly
> (no harware at the time :-().
> I was in touch with an user who tested the original patch and he
> says it works properly; maybe he will try this too.
That would be fine.

> BTW, how would I set CMAKE_C[XX]_FLAGS?
> The previous Makefile has "C[XX]_FLAGS+=-fpic": notice the plus.
> Would I put the plus in CMAKE_... too?
I am not even sure, if CMAKE_CFLAGS and CMAKE_CXXFLAGS exist at all. But instead of '+=' a'=' seems sufficient here: CFLAGS=fPIC ...
Comment 7 Rainer Hurling freebsd_committer 2021-01-02 10:07:19 UTC
> I was in touch with an user who tested the original patch
> and he says it works properly; maybe he will try this too.

In the meantime, did you have contact with the user who was able to test the original patch with his hardware?

How should we proceed?

I would suggest to commit the patch without the LDFLAGS, but with CFLAGS and CXXFLAGS.
Comment 8 ml 2021-01-02 15:21:21 UTC
(In reply to Rainer Hurling from comment #7)

Unfortunately I didn't hear back from him.

It's fine for me to do what you suggest (i.e. remove LDFLAGS, but leave CFLAGS and CXXFLAGS).
Do you want me to submit a new patch for this?
Comment 9 Rainer Hurling freebsd_committer 2021-01-02 15:30:45 UTC
(In reply to ml from comment #8)

Thanks for the fast response.

It is not necessary to prepare an updated patch here.

Instead I will update my review on Phabricator (see URL in comment #4). If my mentors accept the review, I will commit it.
Comment 10 commit-hook freebsd_committer 2021-01-05 20:06:02 UTC
A commit references this bug:

Author: rhurlin
Date: Tue Jan  5 20:05:03 UTC 2021
New revision: 560424
URL: https://svnweb.freebsd.org/changeset/ports/560424

Log:
  astro/opencpn: Update to 5.2.4

  - New Plugin Management System
  - Support for signalK input data streams
  - New Head-Up Navigation Mode
  - Docking the Dashboard with dual canvases
  - Realtime prediction of AIS target location

  Changelog: https://opencpn.org/OpenCPN/about/ver520.html

  PR:		252168
  Submitted by:	Andrea Venturoli <ml@netfence.it> (maintainer)
  Approved by:	arrowd (mentor)
  Differential Revision:	https://reviews.freebsd.org/D27798

Changes:
  head/astro/opencpn/Makefile
  head/astro/opencpn/distinfo
  head/astro/opencpn/files/patch-CMakeLists.txt
  head/astro/opencpn/files/patch-cmake_TargetSetup.cmake
  head/astro/opencpn/files/patch-libs_wxcurl_include_wx_curl_thread.h
  head/astro/opencpn/files/patch-src_PluginPaths.cpp
  head/astro/opencpn/files/patch-src_chart1.cpp
  head/astro/opencpn/files/patch-src_glChartCanvas.cpp
  head/astro/opencpn/pkg-plist
Comment 11 Rainer Hurling freebsd_committer 2021-01-05 20:08:27 UTC
Committed, thanks!

I removed CFLAGS, CXXFLAGS, and LDFLAGS for now. It seems to work fine without them.