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

Fix cgo flags #23

Merged
merged 1 commit into from
Aug 10, 2021
Merged

Fix cgo flags #23

merged 1 commit into from
Aug 10, 2021

Conversation

dubo-dubon-duponey
Copy link
Contributor

Suggestion to fix #22

This is replacing hard-coded compiler/linker flags in the source code of the project with pkg-config sourced info - works fine on mac and debian derivatives by default for me, since they all provide pc files for libfdk-aac with the appropriate include and lib locations.

If both the static and dynamic versions exist in the system, this will pick-up the dynamic one by default (unless the whole build is -static). If ONLY the static version exist, the static version will be picked-up in all cases without any modification of the go source code.

If one wants to force static linking of JUST fdk-aac, when both dyn and static versions are available on the system, one can manually edit the lib pc file.

As for what should happen in the dockerfile / future binary distribution of this project, I do believe it should be built fully statically (possibly including glibc - but more on that later with an updated dockerfile).

@AlbanSeurat let me know if you think this is an acceptable trade-off.
I do believe the upsides are:

  • build will now work by default on all supported platforms, including anything with a non-standard location, as long as PKG_CONFIG_PATH is pointed in the right direction
  • picking your flavor (static vs. dynamic) is no longer a matter of changing go code and is properly delegated to the system

Signed-off-by: dubo-dubon-duponey <[email protected]>
@AlbanSeurat AlbanSeurat merged commit dcbcdf3 into openairplay:main Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Issues with CFLAGS and LDFLAGS
2 participants