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

Ported build system to cmake #19

Merged
merged 3 commits into from
Feb 24, 2017
Merged

Ported build system to cmake #19

merged 3 commits into from
Feb 24, 2017

Conversation

Algebro7
Copy link
Contributor

@Algebro7 Algebro7 commented Feb 24, 2017

Sorry to hit you with another PR so soon, but I finished porting the project to optionally use cmake as the build system. Heres how to do it:

  1. Clone the repo
  2. cd Pokopom && mkdir build
  3. cmake ../

The cmake command will automatically generate a Visual Studio Solution file or GCC Makefile for you in the build directory, depending on your platform. You can open the solution with Visual Studio and build the library. I am doing all of my Windows 7 testing in a VM and I cannot test how well the plugin works or if I made any mistakes, but I have confirmed that PCSX2 sees the plugin and can configure it normally.

CMakeLists.txt Outdated
project(Pokopomclion)

set(CMAKE_CXX_STANDARD 11)
set(CMAKE_BUILD_TYPE Debug)
Copy link
Owner

Choose a reason for hiding this comment

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

Would setting this to Release make it the default build type on Visual Studio? That would be preferable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. I'll test it and update the commit if it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per this article https://stackoverflow.com/questions/19024259/how-to-change-the-build-type-to-release-mode-in-cmake, it looks like for Visual Studio you have to specify it when you actually run cmake, like this:
cmake --build ../ --target ALL_BUILD --config Release
I can provide these instructions somewhere if you'd like, or include a batch file. Also, building with the original VS solution file rather than cmake still works if users prefer that.

I really appreciate your help reviewing these changes!

CMakeLists.txt Outdated
ADD_DEFINITIONS(-DUNICODE)
ADD_DEFINITIONS(-D_UNICODE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -m32 -D_UNICODE -DUNICODE")
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
Copy link
Owner

Choose a reason for hiding this comment

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

Does this really mean it would export all sysmbols? Only those in Exports.def should be exported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, let me look at how to do this with cmake

Build-type is now release, but this only affects makefiles. Visual Studio projects will still have to be modified to release from inside VS. Also specified the symbols file as Exports.def, so only those symbols are exported now.
@Algebro7
Copy link
Contributor Author

Thanks again for the review, I've fixed the issues you mentioned in the most recent commit.

@KrossX
Copy link
Owner

KrossX commented Feb 24, 2017

Alright!

@KrossX KrossX merged commit 2232063 into KrossX:master Feb 24, 2017
@halian
Copy link

halian commented Nov 21, 2020

When building Pokopom this way under Arch Linux, I get an unusable plugin, with PCSX2 throwing [wx] /usr/lib/pcsx2/libpadPokopom.so: wrong ELF class: ELFCLASS32 on load.

@KrossX
Copy link
Owner

KrossX commented Nov 21, 2020

This build makes a 32bit plugin, so unless you use it with a 32bit program, it will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants