bpo-45221: Fix handling of LDFLAGS and CPPFLAGS in setup.py by akulakov · Pull Request #29031 · python/cpython

akulakov · 2021-10-18T14:32:38Z

This processing was removed in 09b2bece78f when replacing optparse with argparse, because argparse will already ignore unknown arguments here; but the line is still useful to avoid mistakenly parsing known args.

I couldn't find any tests for setup.py, I can work on a test if someone can point me to where they are located..

https://bugs.python.org/issue45221

ned-deily

Alas, there are no explicit tests of setup.py. Thanks for the PR, it looks fine to me and appears to solve the problem which I can readily reproduce. Updated: oh, sorry, I just noticed that the NEWS blurb should mention CPPFLAGS as well.

akulakov · 2021-10-18T16:21:15Z

Alas, there are no explicit tests of setup.py. Thanks for the PR, it looks fine to me and appears to solve the problem which I can readily reproduce.

Sounds good, thanks for testing it and reviewing!

ned-deily · 2021-10-18T16:21:22Z

Misc/NEWS.d/next/Build/2021-10-18-10-25-56.bpo-45221.rnulhf.rst

@@ -0,0 +1,3 @@
+Fixed regression in handling of ``LDFLAGS`` options where


One small nit: the change also applies to flags in CPPFLAGS as well.

@ned-deily updated to mention CPPFLAGS

bedevere-bot · 2021-10-18T16:21:44Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

ned-deily

Thanks for the PR and the update!

ned-deily · 2021-10-18T16:52:10Z

Misc/NEWS.d/next/Build/2021-10-18-10-25-56.bpo-45221.rnulhf.rst

@@ -0,0 +1,3 @@
+Fixed regression in handling of ``LDFLAGS`` and ``CPPFLAGS`` options
+where :meth:`argparse.parse_known_args` could interpret an option as 


Looks like there's a trailing space after as.

pushed the fix...

bedevere-bot · 2021-10-18T16:52:21Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

akulakov · 2021-10-18T17:33:13Z

Looking over it again, it might be nice to retain the old comment that goes with the regex, updating it for argparse, something along the lines of:

                # To prevent argparse from raising an exception about any
                # options in env_val that it mistakes for known option, we
                # strip out all double dashes and any dashes followed by a
                # character that is not for the option we are dealing with.
                #
                # Please note that order of the regex is important!  We must
                # strip out double-dashes first so that we don't end up with
                # substituting "--Long" to "-Long" and thus lead to "ong" being
                # used for a library directory.

Does that sound good?

ned-deily · 2021-10-18T17:51:36Z

That looks good to me, thanks for taking the time to improve the PR.

miss-islington · 2021-10-18T18:26:56Z

Thanks @akulakov for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2021-10-18T18:27:24Z

GH-29037 is a backport of this pull request to the 3.10 branch.

…ythonGH-29031) (cherry picked from commit 6a533a4) Co-authored-by: andrei kulakov <andrei.avk@gmail.com>

…up.py (GH-29031) (GH-29037) (cherry picked from commit 6a533a4) Co-authored-by: andrei kulakov <andrei.avk@gmail.com> Automerge-Triggered-By: GH:ned-deily

bedevere-bot · 2021-10-18T22:08:07Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Arch Linux Asan 3.x has failed when building commit 6a533a4.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/582/builds/647) and take a look at the build logs.
Check if the failure is related to this commit (6a533a4) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/582/builds/647

Summary of the results of the build (if available):

Click to see traceback logs

Traceback (most recent call last):
  File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64.asan/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_c6a37706'


Traceback (most recent call last):
  File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64.asan/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_75fe8583'


Traceback (most recent call last):
  File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64.asan/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_954a0ee7'

bedevere-bot · 2021-10-18T23:26:16Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Arch Linux Asan Debug 3.x has failed when building commit 6a533a4.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/585/builds/652) and take a look at the build logs.
Check if the failure is related to this commit (6a533a4) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/585/builds/652

Summary of the results of the build (if available):

Click to see traceback logs

Note: switching to '6a533a423869e28d9086cf4d79029f59e9eec916'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 6a533a4238 [bpo-45221](https://bugs.python.org/issue45221): Fix handling of LDFLAGS and CPPFLAGS options in setup.py (GH-29031)
Switched to and reset branch 'main'

renaming build/scripts-3.11/pydoc3 to build/scripts-3.11/pydoc3.11
renaming build/scripts-3.11/idle3 to build/scripts-3.11/idle3.11
renaming build/scripts-3.11/2to3 to build/scripts-3.11/2to3-3.11

renaming build/scripts-3.11/pydoc3 to build/scripts-3.11/pydoc3.11
renaming build/scripts-3.11/idle3 to build/scripts-3.11/idle3.11
renaming build/scripts-3.11/2to3 to build/scripts-3.11/2to3-3.11

renaming build/scripts-3.11/pydoc3 to build/scripts-3.11/pydoc3.11
renaming build/scripts-3.11/idle3 to build/scripts-3.11/idle3.11
renaming build/scripts-3.11/2to3 to build/scripts-3.11/2to3-3.11
make: *** [Makefile:1349: buildbottest] Terminated

Cannot open file '/buildbot/buildarea/3.x.pablogsal-arch-x86_64.asan_debug/build/test-results.xml' for upload

add processing of env_val

2bda3e1

bedevere-bot added the awaiting review label Oct 18, 2021

the-knights-who-say-ni added the CLA signed label Oct 18, 2021

ned-deily approved these changes Oct 18, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Oct 18, 2021

ned-deily requested changes Oct 18, 2021

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Oct 18, 2021

add note on CPPFLAGS

a9fac7b

akulakov changed the title ~~bpo-45221: Fix handling of LDFLAGS in setup.py~~ bpo-45221: Fix handling of LDFLAGS and CPPFLAGS in setup.py Oct 18, 2021

ned-deily approved these changes Oct 18, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Oct 18, 2021

ned-deily requested changes Oct 18, 2021

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Oct 18, 2021

fix trailing space

c03753e

add a comment

7bd5791

ned-deily approved these changes Oct 18, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Oct 18, 2021

ned-deily merged commit 6a533a4 into python:main Oct 18, 2021

bedevere-bot removed the awaiting merge label Oct 18, 2021

ned-deily added the awaiting merge label Oct 18, 2021

ned-deily added the needs backport to 3.10 only security fixes label Oct 18, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 18, 2021

		@@ -0,0 +1,3 @@
		Fixed regression in handling of ``LDFLAGS`` options where

		@@ -0,0 +1,3 @@
		Fixed regression in handling of ``LDFLAGS`` and ``CPPFLAGS`` options
		where :meth:`argparse.parse_known_args` could interpret an option as

Uh oh!

Conversation

akulakov commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ned-deily left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akulakov commented Oct 18, 2021

Uh oh!

ned-deily Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

akulakov Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 18, 2021

Uh oh!

ned-deily left a comment

Choose a reason for hiding this comment

Uh oh!

ned-deily Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

akulakov Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 18, 2021

Uh oh!

akulakov commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ned-deily commented Oct 18, 2021

Uh oh!

miss-islington commented Oct 18, 2021

Uh oh!

bedevere-bot commented Oct 18, 2021

Uh oh!

bedevere-bot commented Oct 18, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

bedevere-bot commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

akulakov commented Oct 18, 2021 •

edited

Loading

ned-deily left a comment •

edited

Loading

akulakov commented Oct 18, 2021 •

edited

Loading

bedevere-bot commented Oct 18, 2021 •

edited

Loading