Created attachment 250238 [details] 0001-emulators-hatari-Update-to-2.5.0.patch Changes: - Update 2.3.1 to 2.5.0 Changelog http://www.hatari.tuxfamily.org/news.html - Remove sld1 option because not supported by hatari - Add submitter as maintainer QA: - portlint: OK - poudriere: OK
Is -DENABLE_SDL2:BOOL=ON necessary since it is a requirement? Also, --- 2016-01-03 Thomas Huth <snip> * CMakeLists.txt: Use SDL2_FOUND instead of ENABLE_SDL2 where appropriate since ENABLE_SDL2 only says whether SDL2 should be used, not whether it is really available now. --- It seems that it is safe to remove this line. Thanks
Created attachment 250257 [details] 0001-emulators-hatari-Update-to-2.5.0.patch remove ENABLE_SDL2 Good catch, I've removed -DENABLE_SDL2:BOOL=ON and tested. I also forgot to add to the list of changes: - Add test target Here is the new patch, thanks for the feedback!
All good. If possible do a cleanup on depends to pet Q/A check without breaking funtionality: =>> Checking shared library dependencies 0x0000000000000001 NEEDED Shared library: [libICE.so.6] 0x0000000000000001 NEEDED Shared library: [libSDL2-2.0.so.0] 0x0000000000000001 NEEDED Shared library: [libSM.so.6] 0x0000000000000001 NEEDED Shared library: [libX11.so.6] 0x0000000000000001 NEEDED Shared library: [libXext.so.6] 0x0000000000000001 NEEDED Shared library: [libc.so.7] 0x0000000000000001 NEEDED Shared library: [libm.so.5] 0x0000000000000001 NEEDED Shared library: [libpng16.so.16] 0x0000000000000001 NEEDED Shared library: [libudev.so.0] 0x0000000000000001 NEEDED Shared library: [libz.so.6] ====> Running Q/A tests (stage-qa) Warning: you might not need LIB_DEPENDS on libportaudio.so Warning: you might not need LIB_DEPENDS on libpng.so **Could be fixed with -libpng.so:graphics/png +libpng16.so:graphics/png Warning: you might not need LIB_DEPENDS on libatk-1.0.so Warning: you might not need LIB_DEPENDS on libglib-2.0.so Warning: you might not need LIB_DEPENDS on libintl.so Warning: you might not need LIB_DEPENDS on libgtk-3.so ** It seems that it doesn't depends on gtk30 Warning: you might not need LIB_DEPENDS on libpango-1.0.so Warning: you might not need LIB_DEPENDS on libreadline.so.8 Warning: you might not need LIB_DEPENDS on libSDL2.so ** nothing to do here, Mk/Uses related and: Warning: Possible REINPLACE_CMD issues: - - REINPLACE_CMD ran, but did not modify file contents: python-ui/hatariui.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/atari-convert-dir.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/atari-hd-image.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/hatari-prg-args.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/zip2st.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/debugger/gst2ascii.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/debugger/hatari_profile.1 - - REINPLACE_CMD ran, but did not modify file contents: tools/hmsa/hmsa.1
I'll produce another patch because I found that I was missing a dependency on pygobject3, and I'll also clean up the dependencies as much as I can. In the meantime, I have a question about your suggestion. > Warning: you might not need LIB_DEPENDS on libpng.so > **Could be fixed with > -libpng.so:graphics/png > +libpng16.so:graphics/png Is it recommended to do this? I don't particularly like this change because it creates a version dependency that doesn't exist otherwise. The code depends on libpng.so, and during build it happened to find libpng16.so, but it doesn't depend on that version specifically. Is it good practice to be so specific? I looked in the porter's handbook for the answer but I didn't find it. Also for the REINPLACE_CMD warnings, that's due to the loop that's been there since 2012 in b955182027cb5. I could fix that but it seems tedious and harmless to leave it as is. Are you OK with that?
(In reply to Laurent from comment #4) Well, that's a way to pet Q/A check since program links with: 0x0000000000000001 NEEDED Shared library: [libpng16.so.16] and lib_depends should match it. But not a problem to leave it as it is since program found it anyway.
Created attachment 250291 [details] 0001-emulators-hatari-Update-to-2.5.0.patch v3 Restored hatariui with addition of pygobject3 dependency and cleaned up obsolete dependency. Won't fix remaining warnings for now. Ready for commit
Thank you!
(In reply to Laurent from comment #6) Last point: Builds are OK but I've noticed that DEBUG option builds with -DNDEBUG and logs show CMAKE_BUILD_TYPE="release", what means that DEBUG_CFLAGS_OFF= -DNDEBUG isn't working. As I didn't found an option to enable debug we could use: CMAKE_BUILD_TYPE=Debug what causes a change in plist: ===> Parsing plist ===> Checking for items in STAGEDIR missing from pkg-plist Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/hatariui/release-notes.txt Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/hconsole/release-notes.txt Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/release-checklist.txt Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/release-notes.txt ===> Checking for items in pkg-plist which are not in STAGEDIR Error: Missing: %%DOCSDIR%%/%%CMAKE_BUILD_TYPE%%-checklist.txt Error: Missing: %%DOCSDIR%%/%%CMAKE_BUILD_TYPE%%-notes.txt Error: Missing: %%DOCSDIR%%/hatariui/%%CMAKE_BUILD_TYPE%%-notes.txt Error: Missing: %%DOCSDIR%%/hconsole/%%CMAKE_BUILD_TYPE%%-notes.txt ===> Error: Plist issues found. Could you check that?
(In reply to Nuno Teixeira from comment #8) (...) %%PORTDOCS%%%%DOCSDIR%%/hatariui/%%CMAKE_BUILD_TYPE%%-notes.txt is wrong as it should read: %%PORTDOCS%%%%DOCSDIR%%/hatariui/release-notes.txt and logs shows that: CMAKE_BUILD_TYPE="debug" DNDEBUG doesn't shows up
(In reply to Nuno Teixeira from comment #8) Yes, I'll have a look at it, thanks for bringing my attention to it.
(In reply to Nuno Teixeira from comment #9) Right, there are a few changes in the PORTDOCS when using combinations of DOCS and DEBUG. I'm still investigating because it seems that -DCMAKE_BUILD_TYPE does not make a difference in the binary produced. They're stripped binaries of exactly the same size in both cases.
(In reply to Laurent from comment #11) As I seen in plg-plist, %%CMAKE_BUILD_TYPE%% is a wrong assumption of framework. I should be replaced/fixed by the real file name "release"...
(In reply to Nuno Teixeira from comment #12) That's pretty funny! It's because there is "release" in the filename. I'll fix that too. For the type of build, I'll have to prevent the Mk system from stripping the debug info for the DEBUG target. It works properly when building directly. I'm also changing the options to reflect what's currently in CMakeLists.txt. WinUAE is no longer there, but there are other options. Once all this is done, I'll submit a new patch for your review.
(In reply to Laurent from comment #13) Just a side note, also check WITH_DEBUG if it suits better this port.
(In reply to Nuno Teixeira from comment #14) I was going to use: DEBUG_CMAKE_ON= -DCMAKE_BUILD_TYPE="Debug" DEBUG_CMAKE_OFF= -DCMAKE_BUILD_TYPE="Release" DEBUG_INSTALL_TARGET= install It seems to work. Is there a better way with WITH_DEBUG? PS: I know that -DCMAKE_BUILD_TYPE="Release" is not strictly required since it's the default, but it doesn't hurt to be explicit.
(In reply to Laurent from comment #15) > DEBUG_CMAKE_ON= -DCMAKE_BUILD_TYPE="Debug" It's correct this way. > DEBUG_CMAKE_OFF= -DCMAKE_BUILD_TYPE="Release" If default is "Release" we shouldn't use this when _OFF and if we wanted to set it explicit, then the right place should be CMAKE_ARGS. I don't recommend set it as it is the default. > DEBUG_INSTALL_TARGET= install Can't understand this one. -- Related to WITH_DEBUG I will need to research to find differences with DEBUG option but from memory, WITH_DEBUG is for cases where a port have that funcionality but without using option control.
(In reply to Nuno Teixeira from comment #16) (...) in Mk/Uses/cmake.mk: #Variables for ports which use cmake for configure # CMAKE_BUILD_TYPE <snip> # Default: Release, if neither WITH_DEBUG nor # WITH_DEBUGINFO is set, # RelWithDebInfo, if WITH_DEBUGINFO is set, # Debug, if WITH_DEBUG is set. . if defined(WITH_DEBUG) CMAKE_BUILD_TYPE?= Debug . elif defined(WITH_DEBUGINFO) CMAKE_BUILD_TYPE?= RelWithDebInfo . else CMAKE_BUILD_TYPE?= Release . endif #defined(WITH_DEBUG) So we could simplify port with WITH_DEBUG as an alternative to DEBUG option
(In reply to Laurent from comment #15) >> DEBUG_INSTALL_TARGET= install > Can't understand this one. The default for INSTALL_TARGET is install-strip, so I override it to only do an install without stripping, when the DEBUG option is activated. >> DEBUG_CMAKE_OFF= -DCMAKE_BUILD_TYPE="Release" > If default is "Release" we shouldn't use this when _OFF I'm not sure what you mean. We *do* want to use Release when DEBUG is off. > and if we wanted to set it explicit, then the right place should be CMAKE_ARGS. According to https://docs.freebsd.org/en/books/porters-handbook/book/#options-cmake-helpers, that's going right to CMAKE_ARGS >I don't recommend set it as it is the default. My experience is that relying on default behavior is dangerous and can lead to things breaking in the future, because defaults can change. There is really nothing wrong in being explicit here.
(In reply to Nuno Teixeira from comment #14) >Just a side note, also check WITH_DEBUG if it suits better this port. My understanding is that WITH_DEBUG is meant to be used when calling make, as in "make -DWITH_DEBUG". That's how it's done in the porter's handbook (https://docs.freebsd.org/en/books/porters-handbook/book/#testing-debugging-ports). It's meant to be an override for the whole port, not used as an option.
Created attachment 250319 [details] 0001-emulators-hatari-Update-to-2.5.0.patch v4 emulators/hatari: Update to 2.5.0 Changes: - Update 2.3.1 to 2.5.0 Changelog http://www.hatari.tuxfamily.org/news.html - Update options - Bypass broken readline detection by cmake - Add test target - Add submitter as maintainer QA: - portlint: OK - poudriere: OK (no new warnings)
Created attachment 250320 [details] 0001-emulators-hatari-Update-to-2.5.0.patch v4.1 Some tabs were accidentally replaced by spaces in patch 250319
(In reply to Laurent from comment #20) Ok, lets stay with this patch. Something is overriding CMAKE_BUILD_TYPE: --- The following configuration options are available for hatari-2.5.0: DEBUG=on: Build with debug information .. PORTDOCS="" CMAKE_BUILD_TYPE="release" GTK2_VERSION="2.10.0" GTK3_VERSION="3.0.0" ... .. -fstack-protector-strong -fno-strict-aliasing -DNDEBUG .. Install configuration: "Release" --- The only way that I found to get debug is using global CMAKE_BUILD_TYPE="Debug" in Makefile outside CMAKE_ARGS. With this I will strongly recommend a condition: .if ${PORT_OPTIONS:MDEBUG} CMAKE_BUILD_TYPE=Debug INSTALL_TARGET=install .else CMAKE_BUILD_TYPE=Release .endif instead of: DEBUG_CMAKE_ON= -DCMAKE_BUILD_TYPE="Debug" DEBUG_CMAKE_OFF= -DCMAKE_BUILD_TYPE="Release" DEBUG_INSTALL_TARGET= install until we know what is overriding it. I will send a patch with this change so I can commit.
Created attachment 250323 [details] if condition to force CMAKE_BUILD_TYPE
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=8580a08f3e0100cddca19ab29fb2e823f03b9687 commit 8580a08f3e0100cddca19ab29fb2e823f03b9687 Author: Laurent <laurent.chardon@gmail.com> AuthorDate: 2024-05-02 08:36:13 +0000 Commit: Nuno Teixeira <eduardo@FreeBSD.org> CommitDate: 2024-05-02 08:36:13 +0000 emulators/hatari: Update to 2.5.0 - Submitter becomes maintainer - Update options - Bypass broken readline detection by cmake - Add test target ChangeLog: https://www.hatari.tuxfamily.org/news.html PR: 278590 emulators/hatari/Makefile | 59 ++++++++++------- emulators/hatari/distinfo | 6 +- emulators/hatari/files/patch-CMakeLists.txt (gone) | 16 ----- .../files/patch-cmake_FindReadline.cmake (new) | 17 +++++ emulators/hatari/files/patch-share_CMakeLists.txt | 16 ++--- .../hatari/files/patch-tools_atari-hd-image.sh | 14 ++-- emulators/hatari/pkg-plist | 74 +++++++++++++--------- 7 files changed, 113 insertions(+), 89 deletions(-)
Committed with CMAKE_BUILD_TYPE workaround from #Comment 22. Thank you!