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

a52dec: add -fPIC to CFLAGS #35895

Closed
wants to merge 1 commit into from
Closed

Conversation

sorokin
Copy link
Contributor

@sorokin sorokin commented Feb 27, 2018

Without -fPIC gst-plugins.ugly fails to compile. The problem
is that a52dec builds a static library. This library is then used
to create a gstreamer-plugin .so. The linking fails if a52dec is
compiled without -fPIC.

This problem manifests itself only when hardening is disabled.
As when hardening is enabled -fPIC is added automatically.

Things done

Tested on 17.09. I haven't built it on master.

Without -fPIC gst-plugins.ugly fails to compile. The problem
is that a52dec builds a static library. This library is then used
to create a gstreamer-plugin .so. The linking fails if a52dec is
compiled without -fPIC.

This problem manifests itself only when hardening is disabled.
As when hardening is enabled -fPIC is added automatically.
@globin
Copy link
Member

globin commented Feb 27, 2018

👎 this is handled by hardening, please override hardeningDisable if you are setting it elsewhere, this does not belong in upstream nixpkgs.

Closing if there is no further reason, other than having hardening disabled manually.

@globin globin closed this Feb 27, 2018
@7c6f434c
Copy link
Member

Also, overrideDerivation can set CFLAGS directly.

@sorokin
Copy link
Contributor Author

sorokin commented Feb 27, 2018

@globin But shouldn't packages be buildable even when hardening is disabled? I mean the reliance on the fact that hardening adds -fPIC seems to be a bit fragile. Isn't it better to specify this explicitly?

BTW, I would say that it is a bad practice to link a static library into a dynamic one. But this is a different issue.

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

Successfully merging this pull request may close these issues.

None yet

4 participants