Replace optparse with argparse in setup.py

[yan12125]

BPO	29442
Nosy	@brettcannon, @vstinner, @skrah, @xdegaye, @serhiy-storchaka, @yan12125
PRs	bpo-29442: Replace optparse with argparse in setup.py #139
Dependencies	bpo-30152: Reduce the number of imports for argparse

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-07-11.08:49:12.934>
created_at = <Date 2017-02-04.07:26:28.157>
labels = ['type-feature', '3.8', 'build']
title = 'Replace optparse with argparse in setup.py'
updated_at = <Date 2018-07-11.10:18:53.398>
user = 'https://github.com/yan12125'

bugs.python.org fields:

activity = <Date 2018-07-11.10:18:53.398>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-11.08:49:12.934>
closer = 'methane'
components = ['Build']
creation = <Date 2017-02-04.07:26:28.157>
creator = 'yan12125'
dependencies = ['30152']
files = []
hgrepos = []
issue_num = 29442
keywords = ['patch']
message_count = 21.0
messages = ['286922', '286977', '287691', '287696', '287700', '287702', '287705', '287855', '287956', '287964', '288125', '288127', '288132', '288159', '292091', '292117', '292201', '292210', '303052', '321416', '321431']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'vstinner', 'skrah', 'xdegaye', 'serhiy.storchaka', 'yan12125']
pr_nums = ['139']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29442'
versions = ['Python 3.8']

[yan12125]

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. (bpo-18973, bpo-18971)

[skrah]

I didn't look at this one, but some "hacks" are necessary in setup.py (example: usage of os.system() in some places).

[yan12125]

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

[skrah]

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.

[yan12125]

I have used my old patch several days on Android, and it seems quite fine. Anyway that's not important anymore.

[skrah]

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.

[yan12125]

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 :(

[yan12125]

Aha the fix is simple => bpo-29567

[yan12125]

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.

[skrah]

It's okay to reopen if some conditions have changed (which is the case here).

[xdegaye]

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

[yan12125]

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.

[serhiy-storchaka]

(example: usage of os.system() in some places)

See similar bpo-7438.

[brettcannon]

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.

[vstinner]

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 :-)

[yan12125]

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?

[serhiy-storchaka]

See also bpo-30152. It includes Chi's change for argparse (actually it was inspired by Chi's change) and much more.

[vstinner]

I consider that the issue bpo-30152 is now a dependency of this change.

[yan12125]

Thanks to Serhiy Storchaka's work in bpo-30152, this patch can be simplified. I've rebased my branch and updated the pull request.

[methane]

New changeset 09b2bec by INADA Naoki (Chih-Hsuan Yen) in branch 'master':
bpo-29442: Replace optparse with argparse in setup.py (GH-139)
09b2bec

[vstinner]

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 ;-)