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

build: separate shared and static import targets #351

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

rkitover
Copy link
Contributor

Export the FAudio::FAudio-shared target when building a shared library and the FAudio::FAudio-static target when building a static library to separate target files so that both can be installed at the same time.

Alias FAudio::FAudio to the shared library if it is installed and the static library otherwise in the Config.

Export the FAudio::FAudio-shared target when building a shared library
and the FAudio::FAudio-static target when building a static library to
separate target files so that both can be installed at the same time.

Alias FAudio::FAudio to the shared library if it is installed and the
static library otherwise in the Config.

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
Copy link
Member

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

This change looks okay but since we're tagging in about 12 hours I'll wait until 24.07 is out - expect a merge soon!

@rkitover
Copy link
Contributor Author

rkitover commented Sep 9, 2024

Can this be merged now, or is there some legal order preventing that from happening? And are you actually forced to comply with it by any means?

@flibitijibibo
Copy link
Member

No legal barriers - I and the other maintainers are just stuck on SDL_GPU at the moment.

That said, it's still a failure on my part for not coordinating this better, so if it's okay I'd like to defer this to someone who is using this for similar purposes. @cybik, sorry to bother you but does this adversely affect the projects making use of the current CMake configurations?

@cybik
Copy link

cybik commented Sep 9, 2024

Full disclosure: I don't touch such projects often these days.

That being said: the last active project I recall using this, is using a frozen version and rarely updates. I'll reach out to them to make them aware of this change should they update FAudio; beyond that, since the default is BUILD_SHARED_LIBS ON, I'm going to assume it'll be fine. If the default was changed to static, legalities COULD end up being rude.

@flibitijibibo
Copy link
Member

FNA would still prioritize shared as ON, so as long as the ABI didn't get affected this should be okay!

@rkitover
Copy link
Contributor Author

rkitover commented Sep 9, 2024

The main purpose of this is to allow a distribution to do two builds with both -DBUILD_SHARED_LIBS=ON and -DBUILD_SHARED_LIBS=OFF, like MSYS2 does, and allow both to be installed simultaneously without the second build overwriting the CMake target import configuration of the first.

And when both are installed, it makes an FAudio::FAudio-static target available for static builds,

Currently the way I statically link FAudio in our project is by listing the libraries and their dependents because there is no target.

I have followed the convention here used by some other projects such as SDL2, which makes an SDL2::SDL2-static target available for this purpose.

In all other respects, this changes nothing.

We should really support building both targets from a single build, but that's a separate issue.

@rkitover
Copy link
Contributor Author

rkitover commented Sep 9, 2024

I thought it would be reasonable to make the FAudio::FAudio target point to the static libs if ONLY the static libs are installed on a distribution, but I can remove this if you are concerned about there being any confusion about this.

@rkitover
Copy link
Contributor Author

rkitover commented Sep 9, 2024

Actually now that I think about it, without this change, if only the static configuration is installed, then the FAudio::FAudio target points to the static libs as well, so that is also the same.

@flibitijibibo flibitijibibo merged commit 0c7ee1c into FNA-XNA:master Sep 10, 2024
5 checks passed
@rkitover rkitover deleted the static-target branch September 10, 2024 03:25
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