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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2017-02-19 13:18 |
> (example: usage of os.system() in some places)
See similar issue7438.
|
msg288159 - (view) |
Author: Brett Cannon (brett.cannon) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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 ;-)
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:42 | admin | set | github: 73628 |
2018-07-11 10:18:53 | vstinner | set | messages:
+ msg321431 |
2018-07-11 08:49:12 | methane | set | status: open -> closed versions:
+ Python 3.8, - Python 3.7 nosy:
- methane
resolution: fixed stage: patch review -> resolved |
2018-07-11 08:48:54 | methane | set | nosy:
+ methane messages:
+ msg321416
|
2017-09-26 16:46:07 | yan12125 | set | messages:
+ msg303052 |
2017-04-24 08:59:10 | vstinner | set | dependencies:
+ Reduce the number of imports for argparse messages:
+ msg292210 |
2017-04-24 07:38:31 | serhiy.storchaka | set | messages:
+ msg292201 stage: resolved -> patch review |
2017-04-22 12:31:26 | yan12125 | set | messages:
+ msg292117 |
2017-04-22 00:09:51 | vstinner | set | messages:
+ msg292091 |
2017-02-19 19:29:21 | brett.cannon | set | messages:
+ msg288159 |
2017-02-19 13:18:19 | serhiy.storchaka | set | messages:
+ msg288132 |
2017-02-19 11:14:50 | yan12125 | set | messages:
+ msg288127 title: Use argparse and drop dirty optparse hacks in setup.py -> Replace optparse with argparse in setup.py |
2017-02-19 10:42:26 | xdegaye | set | nosy:
+ brett.cannon, xdegaye, vstinner messages:
+ msg288125
|
2017-02-16 17:48:17 | skrah | set | messages:
+ msg287964 |
2017-02-16 16:13:36 | yan12125 | set | files:
- setup-argparse.patch |
2017-02-16 16:13:17 | yan12125 | set | status: closed -> open resolution: not a bug -> (no value) messages:
+ msg287956
|
2017-02-16 16:10:05 | yan12125 | set | pull_requests:
+ pull_request99 |
2017-02-15 13:59:17 | yan12125 | set | messages:
+ msg287855 |
2017-02-13 15:46:04 | yan12125 | set | messages:
+ msg287705 |
2017-02-13 15:09:41 | skrah | set | messages:
+ msg287702 |
2017-02-13 15:04:09 | yan12125 | set | messages:
+ msg287700 |
2017-02-13 14:51:43 | skrah | set | status: open -> closed resolution: not a bug stage: resolved |
2017-02-13 14:27:04 | skrah | set | messages:
+ msg287696 |
2017-02-13 13:33:32 | yan12125 | set | messages:
+ msg287691 |
2017-02-05 22:15:26 | brett.cannon | set | nosy:
- brett.cannon
|
2017-02-04 16:55:21 | skrah | set | nosy:
+ skrah messages:
+ msg286977
|
2017-02-04 07:26:28 | yan12125 | create | |