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

Use CPPFLAGS, CFLAGS, and LDFLAGS #46

Merged
merged 1 commit into from
Aug 17, 2023
Merged

Conversation

mr-c
Copy link
Contributor

@mr-c mr-c commented Aug 17, 2023

Hello from Debian :-)

@sfiligoi
Copy link
Collaborator

@mr-c Why do you need the CCFLAGS and LDFLAGS?
a)
The whole idea of disabling them in test was to make sure things actually worked without any special flags being used.
(i.e. that the shared library was not dependent on the compiler version or compilation flags used)
b)
For the combined Makefile, it is a very thin wrapper that only invokes other shared libraries.
No strong opinions there.

test/Makefile Outdated
Comment on lines 3 to 5
CFLAGS :=
LDFLAGS :=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a) The whole idea of disabling them in test was to make sure things actually worked without any special flags being used. (i.e. that the shared library was not dependent on the compiler version or compilation flags used)
b)

In Debian we can't distinguish regular compilations from test compilations, so it is easier for us to patch in the flag propagation everywhere. I'll remove this part from this PR

@mr-c
Copy link
Contributor Author

mr-c commented Aug 17, 2023

@mr-c Why do you need the CCFLAGS and LDFLAGS?

Debian injects additional flags to harden builds. We check for the presence of those additional flags in the build logs.

https://wiki.debian.org/Hardening

@sfiligoi sfiligoi merged commit 904e802 into biocore:main Aug 17, 2023
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