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

emulation: fix source typo #9717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

luzpaz
Copy link

@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
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
Copy link
Author

luzpaz commented Feb 1, 2025

I think I will defer this to the devs. Is it possible for someone to cherrypick this into a more comprehensive PR ?

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.

4 participants