This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Replace optparse with argparse in setup.py
Type: enhancement Stage: resolved
Components: Build Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: 30152 Superseder:
Assigned To: Nosy List: brett.cannon, serhiy.storchaka, skrah, vstinner, xdegaye, yan12125
Priority: normal Keywords: patch

Created on 2017-02-04 07:26 by yan12125, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 139 merged yan12125, 2017-02-16 16:10
Messages (21)
msg286922 - (view) Author: (yan12125) * Date: 2017-02-04 07:26
The change is clear and self-explained. See the patch.

Motivations:

1. The hack "To prevent optparse from raising an exception..." works for single letter flags (-I, -L, etc.) only. I plan to add --sysroot related codes for Android builds and I don't want more hacks. Apparently argparse does not need this hack as it can parse all known flags before throwing an error
2. optparse is deprecated

Add the developer that introduced this hack. Also Serhiy, who sseems interested in removing optparse from the code base. (issue18973, issue18971)
msg286977 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-02-04 16:55
I didn't look at this one, but some "hacks" are necessary in setup.py (example: usage of os.system() in some places).
msg287691 - (view) Author: (yan12125) * Date: 2017-02-13 13:33
I've found a simpler patch set for supporting Android builds, so I don't need this patch anymore. If you don't think it's necessary, just close it.

> usage of os.system() in some places

I'm indeed sick with them :D
msg287696 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-02-13 14:27
It's not a matter of *necessary*, you simply cannot use argparse in setup.py.  Try to build with your patch and you'll see.
msg287700 - (view) Author: (yan12125) * Date: 2017-02-13 15:04
I have used my old patch several days on Android, and it seems quite fine. Anyway that's not important anymore.
msg287702 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-02-13 15:09
On Mon, Feb 13, 2017 at 03:04:09PM +0000, Chi Hsuan Yen wrote:
> 
> Chi Hsuan Yen added the comment:
> 
> I have used my old patch several days on Android, and it seems quite fine. Anyway that's not important anymore.

I find that very surprising:

./python -E -S -m sysconfig --generate-posix-vars ;\
        if test $? -ne 0 ; then \
                echo "generate-posix-vars failed" ; \
                rm -f ./pybuilddir.txt ; \
                exit 1 ; \
        fi
Traceback (most recent call last):
  File "./setup.py", line 4, in <module>
    import sys, os, importlib.machinery, re, argparse
  File "/home/stefan/althome/pydev/commit-master-LATEST/Lib/argparse.py", line 93, in <module>
    from gettext import gettext as _, ngettext
  File "/home/stefan/althome/pydev/commit-master-LATEST/Lib/gettext.py", line 49, in <module>
    import locale, copy, io, os, re, struct, sys
  File "/home/stefan/althome/pydev/commit-master-LATEST/Lib/struct.py", line 13, in <module>
    from _struct import *
ModuleNotFoundError: No module named '_struct'

Yes, it is important, because you called the original a "dirty hack", which
some core devs do not appreciate.
msg287705 - (view) Author: (yan12125) * Date: 2017-02-13 15:46
You're right. argparse has indirect dependency on dynamic modules so it can't be used in setup.py. Cross builds use prebuilt Python binaries so there's no problem. Sorry for those noises.

I still think it's a dirty hack as it limits scalability and brings lots of problems when adding new features, but I can't find a good way to remove it :(
msg287855 - (view) Author: (yan12125) * Date: 2017-02-15 13:59
Aha the fix is simple => issue29567
msg287956 - (view) Author: (yan12125) * Date: 2017-02-16 16:13
Now I have a working patch at PR 139, and dropping optparse is still the way to go, so I reopen it.

I'm not familiar with CPython conventions. Sorry if reopening an issue is impolite.
msg287964 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-02-16 17:48
It's okay to reopen if some conditions have changed (which is the case here).
msg288125 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2017-02-19 10:42
> I'm not familiar with CPython conventions. Sorry if reopening an issue is impolite.

IMO asking if it's ok to reopen this issue by using the word 'impolite' is a clear reference to the fact that Stefan had told you previously that you are being insulting by saying "it's a dirty hack ". Not only did not you take the opportunity given by Stefan at that time to express your apologies as expected, but instead you repeated it, and here your are making fun of him by using the word 'impolite' when you could have asked if it is correct or allowed to reopen an issue.

Few months ago you uttered the same kind of insults then directed at me in msg264605, a post addressed to Guido van Rossum in issue 26858.

Please do read the CPython Code of Conduct and try to remember that the people that take the time to answer your posts do it on their own free time and that you should be nice to them.

I am nosying Brett Cannon, the CPython maintainer and core dev in charge of the respect of the CoC.
msg288127 - (view) Author: (yan12125) * Date: 2017-02-19 11:14
I see your points. Indeed in many times I didn't think carefully before leaving a comment, and not even noticing that my comments can be offensive afterwards. I apologize for all those cases and I'll be more careful when interacting with others in the future.

@Xavier: I'm gratitude for your patience on taking so much time explaining what I've done wrong. It's an invaluable course.

Let me change the title of this issue first. That will make it more moderate and also clearer.
msg288132 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-19 13:18
> (example: usage of os.system() in some places)

See similar issue7438.
msg288159 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-02-19 19:29
I just wanted to acknowledge that I read this thread. I accept Chi's acknowledgement of his actions and I hope there aren't future missteps, but please also realize that there has been a warning about how you communicate here, Chi.
msg292091 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-04-22 00:09
Did someone test the patch? Does Python still bootstrap after a distclean or not? If not, I suggest to close the issue, except if Chi Hsuan Yen finf an elegant way to fix them :-)
msg292117 - (view) Author: (yan12125) * Date: 2017-04-22 12:31
I guess you were asking whether the newly compiled Python binary is able to build extension modules or not? I rebased PR 139 against the latest git master and seems it's fine. Running `make distclean`, `./configure` and then `make` builds all modules as expected. The result is the same on Travis CI and Appveyor.

About "elegant": I guess the only issue regarding elegancy in my patch is the local import in  GNUTranslations._parse(). For me moving setup.py away from deprecated modules is a good reason for introducing local imports. Does it sound OK?
msg292201 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-24 07:38
See also issue30152. It includes Chi's change for argparse (actually it was inspired by Chi's change) and much more.
msg292210 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-04-24 08:59
I consider that the issue #30152 is now a dependency of this change.
msg303052 - (view) Author: (yan12125) * Date: 2017-09-26 16:46
Thanks to Serhiy Storchaka's work in issue 30152, this patch can be simplified. I've rebased my branch and updated the pull request.
msg321416 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2018-07-11 08:48
New changeset 09b2bece78f62c3d363236dce593ecfe130afd2f by INADA Naoki (Chih-Hsuan Yen) in branch 'master':
bpo-29442: Replace optparse with argparse in setup.py (GH-139)
https://github.com/python/cpython/commit/09b2bece78f62c3d363236dce593ecfe130afd2f
msg321431 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-11 10:18
> bpo-29442: Replace optparse with argparse in setup.py (GH-139)

I'm surprised that it works, but I tested manually and I confirm that it works :-)

$ git clean -fdx
$ ./configure --with-pydebug
$ make
$ ./python -c "import select"

No compilation error ;-)
History
Date User Action Args
2022-04-11 14:58:42adminsetgithub: 73628
2018-07-11 10:18:53vstinnersetmessages: + msg321431
2018-07-11 08:49:12methanesetstatus: open -> closed
versions: + Python 3.8, - Python 3.7
nosy: - methane

resolution: fixed
stage: patch review -> resolved
2018-07-11 08:48:54methanesetnosy: + methane
messages: + msg321416
2017-09-26 16:46:07yan12125setmessages: + msg303052
2017-04-24 08:59:10vstinnersetdependencies: + Reduce the number of imports for argparse
messages: + msg292210
2017-04-24 07:38:31serhiy.storchakasetmessages: + msg292201
stage: resolved -> patch review
2017-04-22 12:31:26yan12125setmessages: + msg292117
2017-04-22 00:09:51vstinnersetmessages: + msg292091
2017-02-19 19:29:21brett.cannonsetmessages: + msg288159
2017-02-19 13:18:19serhiy.storchakasetmessages: + msg288132
2017-02-19 11:14:50yan12125setmessages: + msg288127
title: Use argparse and drop dirty optparse hacks in setup.py -> Replace optparse with argparse in setup.py
2017-02-19 10:42:26xdegayesetnosy: + brett.cannon, xdegaye, vstinner
messages: + msg288125
2017-02-16 17:48:17skrahsetmessages: + msg287964
2017-02-16 16:13:36yan12125setfiles: - setup-argparse.patch
2017-02-16 16:13:17yan12125setstatus: closed -> open
resolution: not a bug -> (no value)
messages: + msg287956
2017-02-16 16:10:05yan12125setpull_requests: + pull_request99
2017-02-15 13:59:17yan12125setmessages: + msg287855
2017-02-13 15:46:04yan12125setmessages: + msg287705
2017-02-13 15:09:41skrahsetmessages: + msg287702
2017-02-13 15:04:09yan12125setmessages: + msg287700
2017-02-13 14:51:43skrahsetstatus: open -> closed
resolution: not a bug
stage: resolved
2017-02-13 14:27:04skrahsetmessages: + msg287696
2017-02-13 13:33:32yan12125setmessages: + msg287691
2017-02-05 22:15:26brett.cannonsetnosy: - brett.cannon
2017-02-04 16:55:21skrahsetnosy: + skrah
messages: + msg286977
2017-02-04 07:26:28yan12125create