Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libretro-pcsx-rearmed: fix build for OdroidGoAdvance #9717

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

luzpaz
Copy link
Contributor

@luzpaz luzpaz commented Jan 24, 2025

Copy link
Contributor

@heitbaum heitbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use the following commit message

  • libretro-pcsx-rearmed: fix build for OdroidGoAdvance

note - I think this is dead code. a) We don’t build addons for devices. b) we build ARCH=aarch64 for armv8 targets now. @garbear?

@garbear
Copy link
Contributor

garbear commented Jan 24, 2025

@HiassofT
Copy link
Member

That OdroidGoAdvance DEVICE seems to have sneaked in with e3b9f5e

We never build anything for this DEVICE (and it's the first time I read about it) so I think that check should just be dropped and the PKG_MAKE_OPTS_TARGET+=" platform=unix" statement from the else clause should be used for ARCH=arm instead

BTW: git grep OdroidGoAdvance shows some more dead code that sneaked in which should be verifyed/dropped as well:

hias@lenny:~/private/libreelec/libreelec-git$ git grep OdroidGoAdvance
packages/emulation/libretro-fbneo/package.mk:  if [ "${PROJECT}" = "Rockchip" -a "${DEVICE}" = "OdroidGoAdvance" ]; then
packages/emulation/libretro-pcsx-rearmed/package.mk:  if [ "${DEVICE}" = "OdroidGoAdvance" ]; then
packages/emulation/libretro-scummvm/package.mk:  if [ "${DEVICE}" = "OdroidGoAdvance" ]; then
packages/emulation/libretro-snes9x/package.mk:if [ "${DEVICE}" = "OdroidGoAdvance" ]; then
packages/emulation/libretro-snes9x2010/package.mk:if [ "${PROJECT}" = "Rockchip" -a "${DEVICE}" = "OdroidGoAdvance" ]; then

@garbear
Copy link
Contributor

garbear commented Jan 24, 2025

Let's remove all the dead code. It's all left over from when I originally ported the cores from Lakka to LE.

@luzpaz
Copy link
Contributor Author

luzpaz commented Jan 25, 2025

Since it's deadcode... forgo merging this PR ?

@garbear
Copy link
Contributor

garbear commented Jan 28, 2025

The change here will be removed and we don't build for that ${DEVICE}, so probably close this and address the dead code removal.

Before doing that, note I first synced with Lakka in 2023: #7705

There's been full syncs of package files since then, but I'm not sure when the last one was. I'd recommend syncing all LE cores with fixes (if any), then dropping all dead code.

@luzpaz

This comment was marked as resolved.

@luzpaz
Copy link
Contributor Author

luzpaz commented Feb 3, 2025

Renamed commit and rebased

@luzpaz luzpaz changed the title emulation: fix source typo libretro-pcsx-rearmed: fix build for OdroidGoAdvance Feb 3, 2025
Copy link
Contributor

@garbear garbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also upstream the change: https://github.com/libretro/Lakka-LibreELEC/blob/Lakka-v6.x/packages/lakka/libretro_cores/pcsx_rearmed/package.mk#L34

I'll take on the responsibility of syncing the cores with upstream changes again someday. If this gets upstream then it'll be one less thing to sync.

Either way, good find here! Thanks for the improvement.

@antonlacon antonlacon merged commit 5d50e8d into LibreELEC:master Feb 6, 2025
@antonlacon
Copy link
Contributor

Lakka has the change.

@luzpaz luzpaz deleted the typo-source branch February 6, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants