-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-33473: Be slightly better about CFLAGS, LDFLAGS, and related #6771
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
bpo-33473: Be slightly better about CFLAGS, LDFLAGS, and related #6771
Conversation
d97b5e6
to
2465e3c
Compare
2465e3c
to
3162aed
Compare
configure.ac
Outdated
if test "$ac_cv_cxx_thread" = "yes"; then | ||
CXX="$CXX -pthread" | ||
CXX="$CXX" | ||
BASECXXFLAGS="-pthread" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this now stomp over whatever BASECFLAGS
was before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call; I believe this has been fixed.
be7840e
to
e975ab2
Compare
@@ -2983,9 +2985,11 @@ then | |||
posix_threads=yes | |||
elif test "$ac_cv_pthread" = "yes" | |||
then | |||
CC="$CC -pthread" | |||
CC="$CC" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line seems rather otiose now
if test "$ac_cv_cxx_thread" = "yes"; then | ||
CXX="$CXX -pthread" | ||
CXX="$CXX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -2045,7 +2046,8 @@ then | |||
ac_cv_cxx_thread=yes | |||
elif test "$ac_cv_pthread" = "yes" | |||
then | |||
CXX="$CXX -pthread" | |||
CXX="$CXX" | |||
CXXFLAGS="-pthread" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also stamps over whatever CXXFLAGS
was before
Also, it seems the cases above need this treatment?
CC="$CC -pthread" | ||
AC_RUN_IFELSE([AC_LANG_SOURCE([[ | ||
CC="$CC" | ||
CFLAGS="-pthread" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you're going to change it, you need to save and restore CFLAGS
as is done for CC
I think I have a much better way of handling this. Let me try; if it works, I'll close this. |
#6788 will drastically change how this is implemented, if accepted. It still requires work, but I'd rather see that PR committed first so we can work from a non-conflicting base. |
4d052ae
to
692fb84
Compare
bf5317d
to
c3e6cc6
Compare
c3e6cc6
to
079c775
Compare
2b2b23c
to
66b076d
Compare
- "-pthread" is a part of the C compiler and linker flags. It is not part of the compiler. In particular 'make CC=clang' ought to work - checking for pthreads support is a link time issue, not run time Not done in this patch: - convert from "gcc" to "cc" to more correctly infer the current compiler - regenerate generated code such as "configure" or "aclocal.m4"
66b076d
to
d6ae20c
Compare
This PR is stale because it has been open for 30 days with no activity. |
part of the compiler. In particular 'make CC=clang' ought to work
Not done in this patch:
compiler
https://bugs.python.org/issue33473