libretro / fbalpha2012_neogeo

Final Burn Alpha 2012. Port of Final Burn Alpha to Libretro (0.2.97.24). Standalone core for Neo Geo.
12 stars 29 forks source link

Cleanup Makefile #12

Closed dannbuckley closed 7 years ago

dannbuckley commented 7 years ago

I optimized the build for the unix systems and removed "system_platform" since it wasn't being used by anything.

sergiobenrocha2 commented 7 years ago

We should get rid of # Generic ARM (armv)

sergiobenrocha2 commented 7 years ago

Is HAVE_NEON used somewhere? We have a lot of garbage in these makefiles from Ctrl+C & Ctrl+V from another makefiles

dannbuckley commented 7 years ago

I removed the Generic ARM build stuff. As for HAVE_NEON, I do not know because I am not familiar with the source code for this project.

sergiobenrocha2 commented 7 years ago

grep -r "HAVE_NEON"

I can find it only in the Makefile, it's not being used.

sergiobenrocha2 commented 7 years ago

You removed EXE_EXT=.exe, makefile needs it in some places. Put it after line 8

sergiobenrocha2 commented 7 years ago

Are you sure "make platform=rpi3" will work? You need a:

ifneq (,$(findstring unix,$(platform)))

in line 37. Take a look again in the changes I did in the other makefiles.

dannbuckley commented 7 years ago

Ok, the latest commit should fix things. (Sorry, I'm not that experienced with Makefile)

sergiobenrocha2 commented 7 years ago

Why did you removed this?

else ifneq (,$(findstring armv,$(platform)))
   override platform += unix

Several packages/scripts use "make platform=armv" yet, you'll broke them without that.

sergiobenrocha2 commented 7 years ago

Test all the "make platform=xxx" you changed, make sure they are working.

dannbuckley commented 7 years ago

I have confirmed that all of the areas that I altered build without error.

dannbuckley commented 7 years ago

@twinaphex Can you merge this with the master branch?

inactive123 commented 7 years ago

Which master branch? You mean merging this PR or something else?

dannbuckley commented 7 years ago

I mean merging this PR.

inactive123 commented 7 years ago

@sergiobenrocha2 Tell me if this is ready to be merged and I'll do it.

sergiobenrocha2 commented 7 years ago

Yeah I think so