zardus / preeny

Some helpful preload libraries for pwning stuff.
BSD 2-Clause "Simplified" License
1.56k stars 171 forks source link

Switch to CMake #46

Closed ZetaTwo closed 5 years ago

ZetaTwo commented 5 years ago

I looked into supporting building multiple architectures at the same time, for example to be able to have both 32bit and 64bit versions of preeny at the same time. The problem with the old Makefile if that it always used the host's arch for naming the output directory. As I started digging more into the Makefile I couldn't find a sane way to do it and after a while I experimented with using CMake instead.

There are of course pros & cons with this change: Pros:

Cons:

I also added a test runner script for the test binaries which is copied into the build directory to make it super simple to run the tests.

Unfortuntaley, libini-config on Ubuntu can't co-exist with both 32 and 64 bit versions anyway so this still isn't a complete solution to building 32 and 64 bit at the same time.

What do you think?

zardus commented 5 years ago

I'm hesitant to take this change wholesale. I'm not necessarily super proficient with cmake, and accepting analogous changes in other projects (an unfamiliar testing framework in ctf-tools) led to that component simply atrophying. I'm not comfortable replacing the Makefiles, imperfect as they are, with cmake.

That being said, I'm totally happy with cmake being a build system alongside make. If you can amend this PR to support that (probably just undeleting the Makefiles), I'm all for it!

ZetaTwo commented 5 years ago

Totally respect that stance. I really wish I was better at make then I would just improve the Makefile. I will update the PR to keep the Makefiles and let this live in parallel. Expect updated PR this weekend.

zardus commented 5 years ago

Looks like the cross-compilation Makefile stuff was fixed in #50! If you rebase this to remove the Makefile changes, I think we can go ahead and merge it!

zardus commented 5 years ago

Ping :-)

ZetaTwo commented 5 years ago

Sorry for the delay. Now I think this PR is less intrusive and should be mergable.

zardus commented 5 years ago

Sweet, thanks!