libretro / bsnes-mercury

Fork of bsnes with various performance improvements.
GNU General Public License v3.0
47 stars 39 forks source link

Make clean is cleaning the wrong directories. #60

Closed orbea closed 6 years ago

orbea commented 6 years ago

The libraries are made in out/ and not obj/. I am not sure about .a so I left that alone. @Alcaro, can you make sure this is right?

Alcaro commented 6 years ago

Good point, but that raises the question of whether .res .manifest really are in the repo root. They look Windows specific; I'd expect .res in obj/ and .manifest in out/. Actually, I suspect they're leftover from standalone bsnes and don't exist at all for us.

.a is used for static linking, but from what I can see, no such platform is configured. Should probably look in out/ anyways.

tldr: Looks good to me

orbea commented 6 years ago

There are no references to '.res' in any of the bsnes repos outside of the make clean and mercury is the only one with .manifest files, but they are never moved anywhere that make clean needs to remove them. I can remove those from the Makefile if you want, but I chose to make as few changes as needed this time.

Alcaro commented 6 years ago

mercury is the only one with .manifest files

yeah, remember why?

Most if not all of those look MSVC specific. But judging by the lack of .obj, there are no supported MSVC configurations, so they don't matter. And if they do, it's easy to put back.

orbea commented 6 years ago

I made another PR for that repo to mirror this one more. :) https://github.com/libretro/bsnes-libretro-cplusplus98/pull/20/commits/df6cbd2a5deb701bd752ec02afe5fc409516a8d2

orbea commented 6 years ago

Also in case you misunderstood, I didn't mean mercury was the only one with reference to manifest files. Mercury is the only one with actual manifest files.

Example. https://github.com/libretro/bsnes-mercury/blob/master/data/higan.Manifest

I suppose others had them too at some point and were removed before I looked.

Alcaro commented 6 years ago

Oh right, .manifest is source code. Whoever originally added it to make clean should be slapped with the largest trout available.

Deleting it means a fair chance build breaks on the standalone UI. But nobody uses that, so who cares. I don't even know if it still exists.

orbea commented 6 years ago

Alright I added one more commit to remove .Manifest and .res files from make clean. I don't really care if the actual .Manifest files remain or not and they do not interfere with any clean up efforts in the buildbot script.

orbea commented 6 years ago

Also links to the other PRs for bsnes repos.

https://github.com/libretro/bsnes-libretro/pull/51 https://github.com/libretro/bsnes-libretro-cplusplus98/pull/20

(And yes, github broke my edit button...)

orbea commented 6 years ago

@Alcaro Unless you see something else that could be improved, can you merge this? It would help to have some sort of test commit for the buildbot.