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

rework cmake to allow for integrations in other projects #306

Merged
merged 11 commits into from
Sep 5, 2024

Conversation

kylon
Copy link
Contributor

@kylon kylon commented Jul 21, 2024

minor includes clean up
fix warning due to missing include(GNUInstallDirs) in ryzenadj app cmake (on make install)
give libryzenadj its own cmake file
add install command to libryzenadj (also useful to make lib pkg for linux)
fix WinRing linking when building libryzenadj in a project
fix export errors when building libryzenadj in a project
renamed ryzenadj project to ryzenadjcli to avoid project name issues in Visual Studio and keep library project name as ryzenadj so that we have auto-naming with cmake (libryzenadj)

cmd/shell/IDE build tested on linux and windows 11, project integration as submodule tested on linux and windows 11

on linux, install will create

  • include/ryzenadj/ryzenadj.h
  • bin/ryzenadj
  • lib/libryzenadj.so

on Windows

  • bin/ryzenadj.exe
  • bin/libryzenadj.dll
  • include/ryzenadj/ryzenadj.h

@FlyGoat
Copy link
Owner

FlyGoat commented Jul 22, 2024

Hi, thanks for your work!

However, please don’t rename executable name, that will break existing package users.

Thanks

@kylon
Copy link
Contributor Author

kylon commented Jul 22, 2024

done

@FlyGoat
Copy link
Owner

FlyGoat commented Jul 22, 2024

That's much better, thanks. Will merge after some tests.

@FlyGoat FlyGoat merged commit 96b2cd6 into FlyGoat:master Sep 5, 2024
2 checks passed
@FlyGoat
Copy link
Owner

FlyGoat commented Sep 5, 2024

Sorry but I had to revert this change as it broke existing behaviour, ryzenadj(cli) must be linked statically against libryzenadj.

@kylon
Copy link
Contributor Author

kylon commented Sep 6, 2024

didn't know that, is it the only issue? i mean, open new pr or just push here and re-merge?

@kylon kylon mentioned this pull request Sep 7, 2024
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.

2 participants