Movatterモバイル変換


[0]ホーム

URL:


homepage

Issue24903

This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title:Do not verify destdir argument to compileall
Type:behaviorStage:resolved
Components:Library (Lib)Versions:Python 3.6, Python 3.4, Python 3.5
process
Status:closedResolution:fixed
Dependencies:Superseder:
Assigned To:Nosy List: jgarver, python-dev, r.david.murray
Priority:normalKeywords:patch

Created on2015-08-20 17:05 byjgarver, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.

Files
File nameUploadedDescriptionEdit
python34_compileall_ddir.diffjgarver,2015-08-20 17:05Proposed patch to Python 3.4
python34_compileall_ddir_2.diffjgarver,2015-09-11 17:59Updated patch, including test changesreview
Messages (10)
msg248900 -(view)Author: Jake Garver (jgarver)*Date: 2015-08-20 17:05
In compileall.py's main, we verify that the provided destdir (-d) exists at build time.  But destdir will commonly be used to override the build time path with a runtime path.  That runtime path will usually not exist at build time.Note that this logic was changed when compileall.py was migrated to argparse.  I think the old logic accidentally avoided the isdir() check at build time.https://github.com/python/cpython/commit/11e99b06bda2a23478fcec40df8c18edc8a06668With the attached patch, behavior is made consistent with python 2.7, intended or otherwise.
msg248906 -(view)Author: R. David Murray (r.david.murray)*(Python committer)Date: 2015-08-20 17:58
That line isn't checking the existence of destdir, but rather of the (single, because we have only one destdir to use) directory to compile.  What bug are you actually seeing?
msg248913 -(view)Author: Jake Garver (jgarver)*Date: 2015-08-20 19:36
Thanks for the response and sorry for the mis-read.In 2.7, I did something like:python -m compileall -d /runtime/path foo.pyBut in 3.4, I get:-d destdir requires exactly one directory argumentI'm doing this during a build. Then we package the pyc file into a runtime image.Let me know if I can further help (or confuse).
msg248914 -(view)Author: R. David Murray (r.david.murray)*(Python committer)Date: 2015-08-20 20:31
So, given that that does something useful, it appears that the error message (which is what is in 2.7 as well) is wrong to refer to 'directory argument', and we should just eliminate the isdir check (and fix the error message).The patch will need a test.
msg248916 -(view)Author: R. David Murray (r.david.murray)*(Python committer)Date: 2015-08-20 20:37
Actually, it looks like it is a bit more potentially complex than that.  The original code would issue the error only if there were more than one argument and the first one was not a directory.  So it would in fact compile more than one argument as long as the first one was a dir.It's not clear what purpose that check was supposed to serve.
msg248950 -(view)Author: Jake Garver (jgarver)*Date: 2015-08-21 12:58
I agree.  I couldn't find a use for the check, so I removed it entirely in the provided patch.  I'm running that way now with success, but of course I'm covering just one use case.Digging back a bit further, the isdir() check came in here:https://github.com/python/cpython/commit/dc6a4c158f8a6175c7d0d5d1a61c24c06767ef22And even further, near the beginning of time, we find the original check:https://github.com/python/cpython/commit/995d45ede2432306a2a0306ce29cea4013bc2850Speculation: In the context of the original code, "directory" in the error message makes more sense and that word should have been dropped when single file support was added.  However, given that we loop over the args, I don't see how -d was limited to a single arg.With my patch in place, I tested compileall.py with multiple args and the -d.  It works as expected and contains the -d path.python -m compileall -d /runtime/path foo.py bar.py foobar.pyI'm happy to craft a new patch.  How would you like it?
msg248960 -(view)Author: R. David Murray (r.david.murray)*(Python committer)Date: 2015-08-21 15:03
OK, so I think the goal was to prevent more than one *directory* from being specified, since as I said earlier that wouldn't make sense given we have only a single destdir path.However, for backward compatibility reasons we should probably not restrict it, but rather invoke the python "consenting adults" rule and let you shoot yourself in the foot that way if you want to.  It only affects error messages, after all.So yeah, if you can build a new patch including some tests that would be great.  Best way is to check out the repository, switch to the 3.4 branch, edit the files, and use 'hg diff' to generate the patch.  Doing it that way makes it easy to test your changes.  But whatever method of coming up with a context diff that you care to use is also fine.More detailed information about the normal workflow is in docs.python.org/devguide.  Note that in addition to the central repository at hg.python.org, there are also bitbucket and github mirrors, which should both be mentioned in thedevguide (if they aren't, that's a bug in thedevguide :)
msg250481 -(view)Author: Jake Garver (jgarver)*Date: 2015-09-11 17:59
Updated patch.  Same as before, but updates tests.Sorry I went MIA on this issue.
msg255918 -(view)Author: Roundup Robot (python-dev)(Python triager)Date: 2015-12-05 04:05
New changesetb63fd82a8528 by R David Murray in branch '3.4':#24903: Remove misleading error message to fix regression.https://hg.python.org/cpython/rev/b63fd82a8528New changesetc65e135df1dc by R David Murray in branch '3.5':Merge:#24903: Remove misleading error message to fix regression.https://hg.python.org/cpython/rev/c65e135df1dcNew changeset1e5aacddb67d by R David Murray in branch 'default':Merge:#24903: Remove misleading error message to fix regression.https://hg.python.org/cpython/rev/1e5aacddb67d
msg255919 -(view)Author: R. David Murray (r.david.murray)*(Python committer)Date: 2015-12-05 04:06
Thanks, Jake.
History
DateUserActionArgs
2022-04-11 14:58:20adminsetgithub: 69091
2015-12-05 04:06:27r.david.murraysetstatus: open -> closed
resolution: fixed
messages: +msg255919

stage: commit review -> resolved
2015-12-05 04:05:52python-devsetnosy: +python-dev
messages: +msg255918
2015-12-03 19:36:50r.david.murraysetstage: needs patch -> commit review
2015-09-11 17:59:56jgarversetfiles: +python34_compileall_ddir_2.diff

messages: +msg250481
2015-08-21 15:04:02r.david.murraysetversions: + Python 3.5, Python 3.6
2015-08-21 15:03:51r.david.murraysetmessages: +msg248960
2015-08-21 12:58:42jgarversetmessages: +msg248950
versions: - Python 3.5, Python 3.6
2015-08-20 20:37:12r.david.murraysetmessages: +msg248916
2015-08-20 20:31:20r.david.murraysetstage: needs patch
messages: +msg248914
versions: + Python 3.5, Python 3.6
2015-08-20 19:36:05jgarversetmessages: +msg248913
2015-08-20 17:58:05r.david.murraysetnosy: +r.david.murray
messages: +msg248906
2015-08-20 17:05:40jgarvercreate
Supported byThe Python Software Foundation,
Powered byRoundup
Copyright © 1990-2022,Python Software Foundation
Legal Statements

[8]ページ先頭

©2009-2026 Movatter.jp