Created attachment 220956 [details] SVN diff
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 :)
Created attachment 221025 [details] patch with additions and corrections, v2 Sorry, I attached an incomplete patch before ...
Fine for me.
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
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?
(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 ...
> 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.
(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?
(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.
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
Committed, thanks! I removed CFLAGS, CXXFLAGS, and LDFLAGS for now. It seems to work fine without them.