Repository navigation
platform.py: use -b option for file command in _syscmd_file() #53769
Description
| BPO | 9560 |
|---|---|
| Nosy | @malemburg, @pitrou, @vstinner, @merwok, @bitdancer |
| Files |
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 2010-08-13.16:30:35.525>
created_at = <Date 2010-08-10.17:21:31.939>
labels = []
title = 'platform.py: use -b option for file\tcommand\tin\t_syscmd_file()'
updated_at = <Date 2010-08-13.16:30:35.524>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2010-08-13.16:30:35.524>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2010-08-13.16:30:35.525>
closer = 'vstinner'
components = []
creation = <Date 2010-08-10.17:21:31.939>
creator = 'vstinner'
dependencies = []
files = ['18494']
hgrepos = []
issue_num = 9560
keywords = ['patch']
message_count = 13.0
messages = ['113549', '113553', '113727', '113728', '113729', '113741', '113755', '113769', '113770', '113775', '113776', '113780', '113793']
nosy_count = 5.0
nosy_names = ['lemburg', 'pitrou', 'vstinner', 'eric.araujo', 'r.david.murray']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue9560'
versions = ['Python 3.2']
Activity
vstinner commentedon Aug 10, 2010
Lib/platform.py was created 7 years ago by r32391. _syscmd_file() docstring was never changed whereas it is inconsistent with the implementation:
---
---
It doesn't use -L option but use Python to follow the link, and use an regex to remove the filename.
Attached patch enables -b option to avoid problem with non-ascii filenames but ascii locale encoding (see bpo-8611 and bpo-9425) and updates the docstring.
--
To fix the non-ascii problem, I tried a different approach by using subprocess API which gives a bytes version of stdout and so avoid the encoding issue. But I commited the patch on python trunk (2.7) which had a bootstrap issue. py3k had no bootstrap issue, but the new patch (use -b option) is simpler.
Commits: r80166 (trunk), r80167 (py3k). Reverted: r80171+r80189 (trunk) and r80190 (py3k).
More details in the following mail thread:
http://mail.python.org/pipermail/python-checkins/2010-April/092092.html
malemburg commentedon Aug 10, 2010
STINNER Victor wrote:
The patch looks good. Just one nit: could you please indent the doc-string
to match the original indentation ?
BTW: I had a look in my archive for platform.py, but couldn't find
where the symlink logic was changed to use Python's APIs instead.
The main reason why this doesn't work on trunk is that platform.py is
supposed to work for many different Python versions, since it's
original use case was to provide platform information to build systems
targeting multiple Python versions.
On Python3, the same is true, but only for the 3.x versions.
[-]platform.py: use -b option for file command in _syscmd_file()[/-][+]platform.py: use -b option for file command in _syscmd_file()[/+]vstinner commentedon Aug 13, 2010
Can it/should it be applied to 2.7 too?
Done. New patch attached. Is it ok like that?
vstinner commentedon Aug 13, 2010
Hey! I don't know why, but I posted a truncated patch. It doesn't remove the code removing the filename and so it breaks the code. New try: version 3 should be ok :-)
merwok commentedon Aug 13, 2010
Is it guaranteed that the -b option will be present in every version of file?
bitdancer commentedon Aug 13, 2010
Guaranteed? No. -b is not required by posix/SUS. I bet it exists everywhere we care about, though. (cf. http://en.wikipedia.org/wiki/File_(command))
malemburg commentedon Aug 13, 2010
Éric Araujo wrote:
Looking at the only use case of _syscmd_file(), it may not even be worth
the trouble of adding the -b option. Fixing the doc-string may be enough:
Note how architecture() already chops off the filename, so it
relies on the existing behavior, rather than the one described
in the doc-string of _syscmd_file().
Sorry, Victor, for not spotting this earlier on.
[-]platform.py: use -b option for file command in _syscmd_file()[/-][+]platform.py: use -b option for file command in _syscmd_file()[/+]vstinner commentedon Aug 13, 2010
Well, my problem is that _syscmd_file() fails with a non encodable filename on Linux because the file program writes the filename to stdout, but os.popen() is unable to parse it because the filename is not decodable from the filesystem encoding (without surrogateescape).
I would like to add -b option just to avoid this encoding problem.
The bug:
---
See also bpo-8611 and bpo-9425.
vstinner commentedon Aug 13, 2010
@lemburg: Your mail client likes to change the issues' title by adding some spaces :-)
malemburg commentedon Aug 13, 2010
STINNER Victor wrote:
Ah, right. Well, then we probably have to use -b after all.
A note on your example: executable is really only meant to be used for
pointing platform to a non-standard location of the Python interpreter
(e.g. in case you used a freeze tool or embedded Python in an application).
However, it is still possible to have that path contain weird
characters, so the example is still valid.
The problem is related to the way Mailman splits long subject lines
in the header. The roundup email interface should probably
remove any extra spaces or simply ignore the title altogether.
[-]platform.py: use -b option for file command in _syscmd_file()[/-][+]platform.py: use -b option for file command in _syscmd_file()[/+]vstinner commentedon Aug 13, 2010
@r.david.murray, @lemburg: So, can I commit my patch?
It's not only a question of "weird" characters. Use case of bpo-8611: install Python in a directory with a name not encodable to the filesystem encoding and try run Python. Install or use a Python installed in an external media like an USB key (encoding problem is not a rare problem with USB keys).
On Windows, it's quite easy to find a character not encodable to the filesystem encoding (mbcs). Eg. Ł for codepage 1252. On Linux, any non-ASCII can be used if you run Python with C locale (locale encoding: ascii).
bpo-4352 and bpo-8988 (Windows issues, so mbcs encoding) are directly related to bpo-8611.
malemburg commentedon Aug 13, 2010
STINNER Victor wrote:
Yes, go ahead with 3.2 patch. It should not go into 2.7, though,
since it doesn't solve any problems in 2.7.
If we get complaints about the -b option than we can add a work-around,
but I doubt that we will. It would be good if you could add a test
case for this, since without -b support, the function won't give
an error: it will simply use the defaults.
[-]platform.py: use -b option for file command in _syscmd_file()[/-][+]platform.py: use -b option for file command in _syscmd_file()[/+]vstinner commentedon Aug 13, 2010
Commited as r83981.