[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Zachary Ware added the comment: Yep, Windows is happy with the latest patch. Since this is such an enormous patch, I'm assuming it's only going into 3.5 and have changed the version field accordingly. -- versions: +Python 3.5 -Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Yeah, I've been meaning to mark all the Derby patches as 3.5. We're not adding new Clinic conversions to 3.4. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Roundup Robot added the comment: New changeset 0c57aba6b1a3 by Larry Hastings in branch 'default': Issue #20170: Convert posixmodule to use Argument Clinic. http://hg.python.org/cpython/rev/0c57aba6b1a3 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Gonna keep an eye on the buildbots and make sure I haven't caused any new breakage. Otherwise... fingers crossed, I think it's done! Thanks for the help everybody (particularly Zach!). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Changes by Larry Hastings la...@hastings.org: -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Diff tweaked to undo the ill-concieved Py_RETURN_NONE change. Thanks, Zachary! Does it now compile and pass tests on Windows? -- Added file: http://bugs.python.org/file36240/larry.clinicize.posixmodule.7.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Zachary Ware added the comment: Another nit to pick: long lines in docstrings. There are several lines about 75-78 characters long in several different docstrings, which look absolutely terrible when you try import os;help(os) on an 80-character-wide terminal due to an 8 character indent. Blame can be spread pretty far and wide on this, but I wonder if Clinic should enforce a 72 character limit on docstring lines to try to mitigate this? Other than that (and the fix to utime mentioned earlier), I don't see anything obviously wrong with the patch, though I admit to not having read through the whole thing (it's huge!). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Zachary Ware added the comment: The patch applies and compiles cleanly, and I finally tracked down what was causing the errors I reported yesterday: os_utime_impl was changed to use Py_RETURN_NONE instead of just setting return_value = Py_None, so Windows skipped the exit routine and left an open handle on any call to os.utime. Commented on the bad line on Rietveld. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Here's a fresh diff. I did some cleanup this time (Clinic now generates the #ifndef versions of the METHODDEF structures) and I believe solved everything MSVC complains about. Zachary, can you try this one? -- Added file: http://bugs.python.org/file36164/larry.clinicize.posixmodule.5.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Zachary Ware added the comment: Close, but no cigar :). Posted Rietveld comments to address the last two compile issues (one of which also appears to be a major bug, but only a warning at compile time). Also, Victor has added os.get_blocking() and os.set_blocking(), which prevent your patch from applying cleanly (I tested against the parent of Victor's changeset). After fixing the two issues I pointed out on Rietveld, I still get major failure on test: == ERROR: test_1565150 (__main__.StatAttributeTests) -- Traceback (most recent call last): File P:\ath\to\cpython\lib\test\test_os.py, line 214, in tearDown os.rmdir(support.TESTFN) OSError: [WinError 145] The directory is not empty: '@test_13492_tmp' == ERROR: test_1686475, test_file_attributes, test_large_time, test_stat_attributes, test_stat_attributes_bytes, test_stat_result_pickle, test_utime, test_utime_dir, test_utime_invalid_arguments, test_utime_ns, test_utime_subsecond, test_exist_ok_existing_directory, test_exist_ok_existing_regular_file, test_exist_ok_s_isgid_directory, test_makedir (All the same error) -- Traceback (most recent call last): File P:\ath\to\cpython\lib\test\test_os.py, line ###, in setUp os.mkdir(support.TESTFN) FileExistsError: [WinError 183] Cannot create a file when that file already exists: '@test_13492_tmp' == ERROR: test_urandom_fd_reopened (__main__.URandomTests) -- Traceback (most recent call last): File P:\ath\to\cpython\lib\test\test_os.py, line 1138, in test_urandom_fd_reopened with open(support.TESTFN, 'wb') as f: PermissionError: [Errno 13] Permission denied: '@test_13492_tmp' == FAIL: test_chdir (__main__.Win32ErrorTests) -- Traceback (most recent call last): File P:\ath\to\cpython\lib\test\test_os.py, line 1274, in test_chdir self.assertRaises(OSError, os.chdir, support.TESTFN) AssertionError: OSError not raised by chdir -- Ran 164 tests in 5.122s The problem appears to be in unlink or rmdir, but I can't see anything amiss in either one. I'll keep looking, but you may have a better idea what's going wrong. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Two small fixes from Zach (thanks again Zach!) and I updated against current trunk so it should apply cleanly. How's it look now? -- Added file: http://bugs.python.org/file36172/larry.clinicize.posixmodule.6.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Here's an updated patch. I cleaned it up a little. I think it's about ready to go in. Zachary, iirc you're a Windows guy and have helped with ensuring patches apply cleanly to Windows in the past. Can you give this a try on Windows? -- Added file: http://bugs.python.org/file36136/larry.clinicize.posixmodule.4.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Zachary Ware added the comment: MSVC is not happy, here's some build output: P:\ath\to\cpython\PCbuild\pcbuild.sln (Build target) (1) - P:\ath\to\cpython\PCbuild\python.vcxproj (default target) (2) - P:\ath\to\cpython\PCbuild\pythoncore.vcxproj (default target) (3) - (ClCompile target) - ..\Modules\posixmodule.c(2886): warning C4047: 'initializing' : 'int' differs in levels of indirection from 'void *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(2896): warning C4047: 'return' : 'int' differs in levels of indirection from 'void *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(2900): warning C4047: 'return' : 'int' differs in levels of indirection from 'void *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4865): warning C4047: '=' : 'int' differs in levels of indirection from 'Py_UNICODE *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4866): warning C4047: '==' : 'int' differs in levels of indirection from 'void *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4877): warning C4047: 'function' : 'LPCWSTR' differs in levels of indirection from 'int' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4877): warning C4024: 'CreateFileW' : different types for formal and actual parameter 1 [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(5368): warning C4047: 'function' : 'path_t *' differs in levels of indirection from 'path_t **' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(5368): warning C4024: 'path_error2' : different types for formal and actual parameter 1 [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(5368): warning C4047: 'function' : 'path_t *' differs in levels of indirection from 'path_t **' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(5368): warning C4024: 'path_error2' : different types for formal and actual parameter 2 [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(7010): warning C4031: second formal parameter list longer than the first list [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(9894): warning C4013: 'waitpid' undefined; assuming extern returning int [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(10428): warning C4047: 'function' : 'path_t *' differs in levels of indirection from 'path_t **' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(10428): warning C4024: 'path_error2' : different types for formal and actual parameter 1 [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(10428): warning C4047: 'function' : 'path_t *' differs in levels of indirection from 'path_t **' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(10428): warning C4024: 'path_error2' : different types for formal and actual parameter 2 [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(16433): warning C4047: 'return' : 'int' differs in levels of indirection from 'void *' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] P:\ath\to\cpython\PCbuild\pcbuild.sln (Build target) (1) - P:\ath\to\cpython\PCbuild\python.vcxproj (default target) (2) - P:\ath\to\cpython\PCbuild\pythoncore.vcxproj (default target) (3) - (ClCompile target) - ..\Modules\posixmodule.c(3324): error C2231: '.wide' : left operand points to 'struct', use '-' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4863): error C2082: redefinition of formal parameter 'path' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4865): error C2065: 'path_wchar' : undeclared identifier [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4866): error C2065: 'path_wchar' : undeclared identifier [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(4877): error C2065: 'path_wchar' : undeclared identifier [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(7004): error C2370: 'os_spawnv__doc__' : redefinition; different storage class [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(7014): error C2084: function 'PyObject *os_spawnv(PyModuleDef *,PyObject *)' already has a body [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(7037): error C2084: function 'PyObject *os_spawnv_impl(PyModuleDef *,int,PyObject *,PyObject *,PyObject *)' already has a body [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(9019): error C2082: redefinition of formal parameter 'pid' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(10936): error C2231: '.wide' : left operand points to 'struct', use '-' [P:\ath\to\cpython\PCbuild\pythoncore.vcxproj] ..\Modules\posixmodule.c(13997): error C2085:
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: thanks! I'm flying from London to Brisbane (via Singapore), gonna take about a day. Now I have something to do on the flight ;-) (that and nullable ints) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Martin v. Löwis added the comment: This patch doesn't apply anymore (to c55300337932); please update it. -- nosy: +loewis ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Changes by Zachary Ware zachary.w...@gmail.com: -- nosy: +zach.ware ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Here's a complete patch, converts everything that I think should be converted for 3.4. With this patch applied, all unit tests pass on my 64-bit Linux box. I plan to also run tests with the buildbots before checking it in. The patch... well, it's 14,000 lines. 409k bytes. Do we have any takers? -- Added file: http://bugs.python.org/file33797/larry.clinicize.posixmodule.2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: By the way, my plan is to turn on the file preset just before checkin. The patch is *much* easier to read without turning that on first; with the file preset, now you have to keep two windows in sync to compare calls to PyArg_*(). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Sorry for the fresh update, but here's revision 3. Only changes: * Gave os.access a - bool return converter. * Fixed up a lot of whitespace. Now, major things are separated by two empty lines, but removed whitespace between #ifdef HAVE_SOMETHING and /*[clinic input] os.something (I used to randomly have blank lines there. No more!) -- Added file: http://bugs.python.org/file33798/larry.clinicize.posixmodule.3.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment:
Actually, forget about the file output preset. It wouldn't work for
posixmodule. 80% of the entry points are #ifdef conditional on platform
functionality. Which means the Clinic generated stuff needs to be #ifdef too.
It wouldn't be that hard to add the ability to #ifdef your generated code...
but then what? Should it generate an #endif too, right before the end of the
block?
If it closed the #ifdef, then it'd look dumb:
/*[clinic input]*
ifdef HAVE_WHATNOT
os.whatnot
[clinic start generated code]*/
#ifdef HAVE_WHATNOT
...
static PyObject *
os_whatnot(PyModuleType *)
#endif /* HAVE_WHATNOT */
/*[clinic end generated code: output=... ]*/
#ifdef HAVE_WHATNOT
{
...
}
#endif /* HAVE_WHATNOT */
If it didn't close the #ifdef, well, that looks dumb too:
/*[clinic input]*
ifdef HAVE_WHATNOT
os.whatnot
[clinic start generated code]*/
#ifdef HAVE_WHATNOT
...
static PyObject *
os_whatnot(PyModuleType *)
/*[clinic end generated code: output=... ]*/
{
...
}
#endif /* HAVE_WHATNOT */
though maybe that's less dumb.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20170
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Serhiy Storchaka added the comment: The curses module also has many conditionally implemented functions. I think Argument Clinic can detect preprocessor instructions (#if/#ifdef/#ifndef/#else/#endif) and generate needed #if/#endif in generated file. This would be more robust than explicitly repeat condition in clinic declaration, because external conditions can be changed. -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: That's a really good idea! I'm still thinking about how I'd do it, but I think I'm gonna give it a try. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Larry Hastings added the comment: Submitting this just so I beat the deadline. I'm *about* half done, but I'm still working on it, so I'm just going to keep going--should only be another couple of hours. (If somebody else pulls this stunt, I guess I'll accept their final patch too.) -- keywords: +patch stage: needs patch - patch review Added file: http://bugs.python.org/file33789/larry.clinicize.posixmodule.1.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
New submission from Larry Hastings: This issue is part of the Great Argument Clinic Conversion Derby, where we're trying to convert as much of Python 3.4 to use Argument Clinic as we can before Release Candidate 1 on January 19. This issue asks you to change the following bundle of files: Modules/posixmodule.c: 137 sites Talk to me (larry) if you only want to attack part of a bundle. For instructions on how to convert a function to work with Argument Clinic, read the howto: http://docs.python.org/dev/howto/clinic.html -- assignee: larry components: Library (Lib) keywords: easy messages: 207615 nosy: larry priority: normal severity: normal stage: needs patch status: open title: Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c type: behavior versions: Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Changes by Larry Hastings la...@hastings.org: -- components: +Extension Modules -Library (Lib) ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20170] Derby #1: Convert 137 sites to Argument Clinic in Modules/posixmodule.c
Changes by Larry Hastings la...@hastings.org: -- type: behavior - enhancement ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20170 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
