From the Copilot review on PR #724, which flagged that
vfs_implementation_cdrom.h was removed while still included under
HAVE_CDROM.
The finding is right that the include dangles, but wrong on causation:
deleting the header (616edf0) did not break HAVE_CDROM builds. They were
already impossible, because that header includes <cdrom/cdrom.h> and
libretro-common/include/cdrom/ was NEVER vendored here -- this core
carries a subset of libretro-common.
Actually supporting HAVE_CDROM would mean vendoring the whole cdrom
subsystem to supply the six retro_vfs_file_*_cdrom symbols the 12 blocks
in this file need -- code this core has no use for, since its Jaguar CD
support is its own implementation in src/cd/. That is not a release-time
change, and restoring the header alone would just reinstate a file that
cannot compile.
So the right-sized fix is to make the unsupported path say so, rather
than leave a missing-header error for whoever tries it. Nothing in this
repo defines HAVE_CDROM, so this #error never fires in any of our builds
-- confirmed by the full suite and by CI across 16 platforms.
Verified: make TEST_EXPORTS=1 test exit 0, zero FAILs.
Co-Authored-By: Claude Opus 5 <[email protected]>
libretro-common/include/vfs/vfs_implementation_cdrom.h includes
<cdrom/cdrom.h>, and libretro-common/include/cdrom/ was never vendored --
this repo carries a subset. So the header cannot compile.
It goes unnoticed because its only consumer is guarded:
/* libretro-common/vfs/vfs_implementation.c:210 */
#ifdef HAVE_CDROM
#include <vfs/vfs_implementation_cdrom.h>
#endif
This core never defines HAVE_CDROM and never will -- it has its own CD stack
under src/cd/. The header is therefore simultaneously dead (nothing includes
it) and broken (it could not compile if anything did).
Any build that parses headers rather than only compiling translation units
trips over it. Clang module maps do exactly that, which is how the SwiftPM
build finds it:
libretro-common/include/vfs/vfs_implementation_cdrom.h:27:10:
fatal error: 'cdrom/cdrom.h' file not found
src/core/log.h:6:10: fatal error: could not build module 'libretro_common'
Deleting rather than vendoring the missing dependency (which would add
unused code for a path we never take) or wrapping the body in
#ifdef HAVE_CDROM (which makes it parse but leaves a header that still
cannot work, turning today's clear missing-include error into link errors).
After this, setting HAVE_CDROM fails at vfs_implementation.c:211 with "no
such file", which correctly says "vendor the cdrom bits first".
A wholesale libretro-common re-sync will bring it back; that is true of
every file the vendored subset omits.
Deliberately NOT paired with deleting src/core/boolean.h, which the
downstream fork branch does alongside this. Measured, that deletion is
unnecessary -- with only this header removed, swift build completes clean
with src/core/boolean.h in place, and the cyclic module import it was blamed
for does not occur. It would also not be behaviour-neutral: src/core's shim
applies the `#define bool unsigned char` hack for ALL _MSC_VER, while
libretro-common's applies it only below 1800, and src/core precedes
libretro-common on the include path. Removing it would flip msvc2015/msvc2017
to real <stdbool.h> _Bool, changing assignment semantics (_Bool b = 4 stores
1; unsigned char b = 4 stores 4) on a path CI cannot fully verify.
Fixes#621
Co-Authored-By: Claude Opus 5 <[email protected]>
Update vendored libretro-common from ~Oct 2021 to current upstream.
Brings 4+ years of bug fixes, new API support, platform compatibility
improvements, and security fixes.
Refs #105
Co-Authored-By: Claude Opus 4.6 <[email protected]>
