Issue 23657: Don't do isinstance checks in zipapp

As it stand, zipapp's code checks for str and then otherwise assumes an object is a file-like object. It might work out better to do some duck typing and simply check if an object has 'read' and 'readline' attributes then it's a file-like object and otherwise that it's a path.

Doing this would then potentially make it easier to use pathlib.Path through the module rather than the os module.

That sounds reasonable. I'll have a look at this. The code was originally based on a similar pattern in the zipfile module, so maybe zipfile should be changed in the same way?

Here is a proposed patch that does what I was thinking. What do you think?

As for updating zipfile, it's possible but slightly risky because of backwards-compatibility. Since this is all-new code there is no worry about breaking someone's pre-existing code because they were relying on str being treated in a special way instead of the file-like object case.

Looks good. Would it be worth adding tests for providing pathlib.Path objects, or is that overkill?

Depends on whether you want to support pathlib.Path objects explicitly. =) If so then yes, we should add tests.

Which reminds me, it might be time for you to request commit privileges so you can commit patches like this yourself.

Cool, I'll look at sorting it out.

Updated version of the patch with tests, plus doc update noting that path objects are explicitly supported.

Updated patch. Added the requested test and a set of tests for the command line API. This also highlighted a bug in the command line --info option, which is also fixed.

Coverage now:

Name          Stmts   Miss  Cover   Missing
-------------------------------------------
test_zipapp     254     12    95%   252-257, 263-268
zipapp          102      2    98%   30, 192
-------------------------------------------
TOTAL           356     14    96%

Remaining misses are Unix-only tests (this was run on Windows) and the __main__ block.

Updated patch with fixes for review comments. I did remove the tests for the exact error messages, as testing for a non-zero exit code was actually what I was trying to do, and I found a better way of doing that.

New changeset 0b2993742650 by Paul Moore in branch 'default':
#23657 Don't explicitly do an isinstance check for str in zipapp
https://hg.python.org/cpython/rev/0b2993742650

messages: + msg238818

messages: + msg238685

messages: + msg238243

stage: needs patch -> patch review