Skip to content

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

Closed

Conversation

grimreaper
Copy link
Contributor

@grimreaper grimreaper commented May 12, 2018

  • "-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

@grimreaper grimreaper changed the title [WIP] Be slightly better about CFLAGS, LDFLAGS, and related [WIP] issue33473: Be slightly better about CFLAGS, LDFLAGS, and related May 12, 2018
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch from d97b5e6 to 2465e3c Compare May 12, 2018 17:06
@grimreaper grimreaper changed the title [WIP] issue33473: Be slightly better about CFLAGS, LDFLAGS, and related bpo-33473: [WIP] Be slightly better about CFLAGS, LDFLAGS, and related May 12, 2018
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch from 2465e3c to 3162aed Compare May 12, 2018 17:08
@grimreaper grimreaper changed the title bpo-33473: [WIP] Be slightly better about CFLAGS, LDFLAGS, and related bpo-33473: Be slightly better about CFLAGS, LDFLAGS, and related May 12, 2018
configure.ac Outdated
if test "$ac_cv_cxx_thread" = "yes"; then
CXX="$CXX -pthread"
CXX="$CXX"
BASECXXFLAGS="-pthread"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch 2 times, most recently from be7840e to e975ab2 Compare May 13, 2018 19:10
@@ -2983,9 +2985,11 @@ then
posix_threads=yes
elif test "$ac_cv_pthread" = "yes"
then
CC="$CC -pthread"
CC="$CC"
Copy link
Contributor

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"
Copy link
Contributor

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"
Copy link
Contributor

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"
Copy link
Contributor

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

@grimreaper
Copy link
Contributor Author

I think I have a much better way of handling this. Let me try; if it works, I'll close this.

@grimreaper
Copy link
Contributor Author

#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.

@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch 5 times, most recently from 4d052ae to 692fb84 Compare May 19, 2018 00:22
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch 2 times, most recently from bf5317d to c3e6cc6 Compare May 25, 2018 14:19
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch from c3e6cc6 to 079c775 Compare June 9, 2018 17:46
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch 2 times, most recently from 2b2b23c to 66b076d Compare June 29, 2018 14:32
- "-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"
@grimreaper grimreaper force-pushed the eax/pthread/pthread_pass_correct branch from 66b076d to d6ae20c Compare July 28, 2018 07:41
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 17, 2022
@grimreaper grimreaper closed this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants