Bug 282692 - devel/libchdr: New port: Standalone library for reading MAME CHDv1-v5 formats
Summary: devel/libchdr: New port: Standalone library for reading MAME CHDv1-v5 formats
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: Robert Clausecker
URL: https://github.com/rtissera/libchdr
Keywords:
Depends on:
Blocks: 282691
  Show dependency treegraph
 
Reported: 2024-11-11 12:02 UTC by Stefan Schlosser
Modified: 2024-11-14 08:56 UTC (History)
2 users (show)

See Also:


Attachments
new port devel/libchdr (4.00 KB, patch)
2024-11-11 12:02 UTC, Stefan Schlosser
bsdcode: maintainer-approval+
Details | Diff
new port devel/libchdr (4.09 KB, patch)
2024-11-11 18:29 UTC, Stefan Schlosser
bsdcode: maintainer-approval+
Details | Diff
new port devel/libchdr v2 (4.71 KB, patch)
2024-11-12 19:51 UTC, Stefan Schlosser
bsdcode: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Schlosser 2024-11-11 12:02:27 UTC
Created attachment 255097 [details]
new port devel/libchdr

libchdr is a standalone library for reading MAME's CHDv1-v5 formats.

The code is based off of MAME's old C codebase which read up to CHDv4 with
OS-dependent features removed, and CHDv5 support backported from MAME's current
C++ codebase.

https://github.com/rtissera/libchdr


Getting this library into the ports tree allows future work to unbundle the dependency from at least these ports:

emulators/ares
emulators/flycast (port submitted)
emulators/duckstation
emulators/pcsx2
emulators/ppsspp
Comment 1 Robert Clausecker freebsd_committer freebsd_triage 2024-11-11 13:36:58 UTC
Why is the static library not called libchdr.a?  Is there a specific reason for it having a different library name?

Port looks fine otherwise.  CC'ed the emulators/mame maintainer.
Comment 2 Stefan Schlosser 2024-11-11 14:11:57 UTC
(In reply to Robert Clausecker from comment #1)
> Why is the static library not called libchdr.a?  Is there a specific reason for it
> having a different library name?
Upstream calls it libchdr-static.a. And projects using libchdr (by vendoring it and building it themselfs) often link against libchdr-static.a explicitly.

So I'd say it's the official name for the static library.
Comment 3 Robert Clausecker freebsd_committer freebsd_triage 2024-11-11 14:12:46 UTC
It might make sense to add a link from libchdr-static.a to libchdr.a so static linking works the usual way, too.  Your call.
Comment 4 Stefan Schlosser 2024-11-11 14:26:48 UTC
(In reply to Robert Clausecker from comment #3)
> It might make sense to add a link from libchdr-static.a to libchdr.a so static
> linking works the usual way, too.  Your call.
I think this is a good idea. I'll provide an updated patch soon.
Comment 5 Stefan Schlosser 2024-11-11 18:29:54 UTC
Created attachment 255104 [details]
new port devel/libchdr

The new patch creates a symbolic link from libchdr-static.a to libchdr.a.
Comment 6 Robert Clausecker freebsd_committer freebsd_triage 2024-11-11 20:22:15 UTC
Looks good, though I'll replace ${LN} -s with ${RLN} as per policy on commit.

Consider updating your other bug report such that it depends on this one.  You can add this PR as a dependency on the other.
Comment 7 Stefan Schlosser 2024-11-11 20:58:24 UTC
(In reply to Robert Clausecker from comment #6)
> Looks good, though I'll replace ${LN} -s with ${RLN} as per policy on commit.
Thanks, that's good to know for future PRs. I use ${LN} -s in the do-install target of devel/librashader's Makefile too. Because you are also reviewing and committing my recent PR for the librashader port, do you mind replacing it there too if it's appropriate?

> Consider updating your other bug report such that it depends on this one.  You can
> add this PR as a dependency on the other.
Oh I haven't started working on unbundling libchdr from the ports I maintain yet. For emulators/ares I will wait until upstream has completed its transition from GNU Makefile to CMake build system, which might still take a month or two. emulators/flycast might be a better candidate for now, if/when the port is accepted.
Comment 8 Alastair Hogge 2024-11-12 09:30:23 UTC
(In reply to Robert Clausecker from comment #1)

Hello,

I am not sure what input is required from me, other than, wow! awesome!

I had a quick look at the Mame monolith, and it does indeed look like the library is deeply embedded in Mame, so I see no conflict, based upon that brief investigation.
Comment 9 Robert Clausecker freebsd_committer freebsd_triage 2024-11-12 10:23:13 UTC
(In reply to Alastair Hogge from comment #8)

There's nothing you need to do.  I just wanted to have you look at it as it may be relevant to your MAME porting effort.
Comment 10 Robert Clausecker freebsd_committer freebsd_triage 2024-11-12 15:15:25 UTC
The build fails on i386 due to a configure issue.  This is a known problem with the zstd cmake files:

CMake Error at CMakeLists.txt:54 (find_package):
  Could not find a configuration file for package "zstd" that is compatible
  with requested version "".

  The following configuration files were considered but not accepted:

    /usr/local/lib/cmake/zstd/zstdConfig.cmake, version: 1.5.6 (64bit)



-- Configuring incomplete, errors occurred!

I do not consider this to be a blocker to get this port accepted, but I would like to give you a chance to look into this if you wish to take action.
Comment 11 Stefan Schlosser 2024-11-12 18:15:41 UTC
(In reply to Robert Clausecker from comment #10)

Oof... In lieu of fixing the cmake modules of the zstd port, I would either

 - set WITH_SYSTEM_ZSTD=OFF and use the vendored static build of zstd for i386 builds, or
 - patch CMakeLists.txt to use pkg_search_modules for i386 builds
   (that was version 1 of https://github.com/rtissera/libchdr/pull/131)

I'll have a look at both options, but I'd prefer the second option.

Either way, thanks for doing the testbuild for i386.
Comment 12 Stefan Schlosser 2024-11-12 18:19:37 UTC
not pkg_search_modules, but find_path and find_library
Comment 13 Robert Clausecker freebsd_committer freebsd_triage 2024-11-12 18:23:33 UTC
As you like, though I strongly prefer not to bundle zstd if possible.
Comment 14 Stefan Schlosser 2024-11-12 19:51:11 UTC
Created attachment 255122 [details]
new port devel/libchdr v2

Updated patch: In order to keep the port simple, I decided to use an architecture-independend patch for CMakeLists.txt which should work on all platforms. Instead of finding the cmake module with find_package, the patch uses pkg_check_modules to find the pkgconfig file for zstd. This required adding pkgconf to USES.

I also replaced the ${LN} -s with ${RLN} in the post-install target as requested.
Comment 15 Alastair Hogge 2024-11-13 10:58:31 UTC
(In reply to Robert Clausecker from comment #9)

Oh that is very lovely, thanking you very muchly.
Comment 16 commit-hook freebsd_committer freebsd_triage 2024-11-14 08:45:15 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c66fa584263efe3b13cf9d2dee1a5c97448c4a26

commit c66fa584263efe3b13cf9d2dee1a5c97448c4a26
Author:     Stefan Schlosser <bsdcode@disroot.org>
AuthorDate: 2024-11-09 20:51:33 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-11-14 08:43:26 +0000

    devel/libchdr: New port: Standalone library for reading MAME CHDv1-v5 formats

    libchdr is a standalone library for reading MAME's CHDv1-v5 formats.

    The code is based off of MAME's old C codebase which read up to CHDv4 with
    OS-dependent features removed, and CHDv5 support backported from MAME's current
    C++ codebase.

    WWW: https://github.com/rtissera/libchdr

    PR:             282692

 devel/Makefile                                 |  1 +
 devel/libchdr/Makefile (new)                   | 26 ++++++++++++++++++++++++++
 devel/libchdr/distinfo (new)                   |  3 +++
 devel/libchdr/files/patch-CMakeLists.txt (new) | 26 ++++++++++++++++++++++++++
 devel/libchdr/pkg-descr (new)                  |  5 +++++
 devel/libchdr/pkg-plist (new)                  | 13 +++++++++++++
 6 files changed, 74 insertions(+)
Comment 17 Robert Clausecker freebsd_committer freebsd_triage 2024-11-14 08:56:12 UTC
Thank you for your contribution.