bpo-33473: Be slightly better about CFLAGS, LDFLAGS, and related by grimreaper · Pull Request #6771 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- "-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"
https://bugs.python.org/issue33473
[WIP] Be slightly better about CFLAGS, LDFLAGS, and related [WIP] issue33473: Be slightly better about CFLAGS, LDFLAGS, and related
[WIP] issue33473: Be slightly better about CFLAGS, LDFLAGS, and related bpo-33473: [WIP] Be slightly better about CFLAGS, LDFLAGS, and related
bpo-33473: [WIP] Be slightly better about CFLAGS, LDFLAGS, and related bpo-33473: Be slightly better about CFLAGS, LDFLAGS, and related
| if test "$ac_cv_cxx_thread" = "yes"; then |
|---|
| CXX="$CXX -pthread" |
| CXX="$CXX" |
| BASECXXFLAGS="-pthread" |
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?
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.
| @@ -2983,9 +2985,11 @@ then |
|---|
| posix_threads=yes |
| elif test "$ac_cv_pthread" = "yes" |
| then |
| CC="$CC -pthread" |
| CC="$CC" |
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" |
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" |
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" |
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.
- "-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"
This PR is stale because it has been open for 30 days with no activity.