classification
Title: PEP 277: Unicode file name support
Type: Stage:
Components: Windows Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mhammond Nosy List: gvanrossum, loewis, mhammond, nnorwitz, tim.peters
Priority: normal Keywords: patch

Created on 2002-08-12 11:33 by loewis, last changed 2002-10-05 09:47 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
pep277.txt loewis, 2002-08-12 11:33
pep277.txt loewis, 2002-09-04 12:06 Version 2
unicode_comments.txt mhammond, 2002-09-30 06:33 Comments on the patch
pep277-2.txt mhammond, 2002-09-30 12:54 Better version of that patch that doesn't crash anything!
doc.txt loewis, 2002-10-01 17:41
Messages (25)
msg40910 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-08-12 11:33
This patch is in an updated version of the patch [1]
mentioned in the PEP. In addition to merging it with
the CVS, it fixes a few formatting problems.
msg40911 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-08-12 13:38
Logged In: YES 
user_id=6380

I'd make unicode_filenames() a macro that expands to 0 on
platforms without this wart. I'd also test for wfunc!=NULL
before calling unicode_filenames().

There's a lot of hairy code here. Are you sure that there
are test cases in the test suite that exercise all of it?

Aren't there some #ifdefs missing? posix_[12]str have code
that's only relevant for Windows but isn't #ifdef'ed out
like it is elsewhere.

There should probably be a separate #define in pyport.h to
test for this that's equivalent to defined(MS_WINDOWS) &&
!defined(Py_UNICODE_WIDE), so this can be uniformly tested
to see whether this code is necessary.
msg40912 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-09-04 12:06
Logged In: YES 
user_id=21627

This is a new version:
- wrapped all Windows wide API code with #ifdef 
Py_WIN_WIDE_FILENAMES
- added test case

Mark, can you please take a look at this?
msg40913 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-09-22 15:31
Logged In: YES 
user_id=21627

Tim, can you review this?
msg40914 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2002-09-25 19:43
Logged In: YES 
user_id=31435

Sorry, Martin, bouncing back to Mark -- I really don't know 
anything about the Windows filename APIs.
msg40915 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-09-26 06:24
Logged In: YES 
user_id=21627

Mark, can you please indicate whether you will review this
patch?
msg40916 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 01:03
Logged In: YES 
user_id=14198

Will review this instant - sorry for the delay.
msg40917 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 06:33
Logged In: YES 
user_id=14198

Uploading text file with my comments on the patch.  Also
uploading a new patch with these corrections and even better
(?) error handling.
msg40918 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 06:34
Logged In: YES 
user_id=14198

Back to Martin for comments on my comments ;)
msg40919 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 06:35
Logged In: YES 
user_id=14198

Back to martin for comments on my comments ;)
msg40920 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-09-30 10:43
Logged In: YES 
user_id=21627

Thanks for your comments, just shouting at me to correct all
these problems would have been fine :-)

In what case do you still get an exception? 
msg40921 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 12:20
Logged In: YES 
user_id=14198

I get an exception running test_unicode_file.py - we end up
with a MBCS encoded string passed to
PyUnicode_FromEncodedObject(), that attempts to decode using
the system default encoding rather than the file-system
default.  The error is a NULL pointer deref so should be
obvious - unfortunately the fix requires a local
reimplementation of PyUnicode_FromEncodedObject(), passing
an explicit encoding for string objects.
msg40922 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 12:21
Logged In: YES 
user_id=14198

I missed the fact this was re-assigned to me - should I
check my new patch in?
msg40923 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 12:52
Logged In: YES 
user_id=14198

Sorry - not sure what I was thinking - obviously can't check
in a patch known to crash the test suite!

Attaching a new patch.  This fixes the crash in
test_unicode_file I mentioned.  It also fixes another error
I introduced where PyErr_SetWithFilename functions can have
NULL filenames.  Also added an assert for symmetry with one
I added before.

Martin - back to me if you want me to check it in.
msg40924 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 12:54
Logged In: YES 
user_id=14198

Sorry - not sure what I was thinking - obviously can't check
in a patch known to crash the test suite!

Attaching a new patch - fixes the problem I mentioned with
test_unicode_files.py, fixes an error I introduced where the
"WithFilename" error functions could be passed a NULL
filename, and adding an assert to match the one I added
previously.
msg40925 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-09-30 23:48
Logged In: YES 
user_id=14198

Although SF implies otherwise, a bad filename in a
submission appears to only prevent the file being uploaded -
the rest of the comments etc come through.  Hence my
double-comments and strange status changes you can see.

So, back to Martin as I don't think he is waiting on me for
anything.  Happy to check in if you like, but would like to
see someone run a debug build of this through Linux first to
see if any asserts fire - some asserts did indeed fire in
the first version of this patch, and Linux may trigger
others, or even the 2 that I added.
msg40926 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-10-01 09:59
Logged In: YES 
user_id=14198

The block:

+ 				if (!wpath1 || !wpath2)
+ 					return NULL;

Should include Py_XDECREF calls for wpath1 and wpath2.

Can't be bothered with a new patch just for this :)
msg40927 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-10-01 17:41
Logged In: YES 
user_id=21627

The patch passes the test suite on Solaris. I've also
attached a documentation change, so I think the patch is
ready now. Back to Mark for checkin.
msg40928 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-10-03 05:15
Logged In: YES 
user_id=14198

/cvsroot/python/python/dist/src/Include/pyerrors.h,v  <-- 
pyerrors.h
new revision: 2.61; previous revision: 2.60
/cvsroot/python/python/dist/src/Lib/test/test_unicode_file.py,v
 <--  test_unicode_file.py
new revision: 1.6; previous revision: 1.5
/cvsroot/python/python/dist/src/Modules/posixmodule.c,v  <--
 posixmodule.c
new revision: 2.261; previous revision: 2.260
/cvsroot/python/python/dist/src/Objects/fileobject.c,v  <--
 fileobject.c
new revision: 2.169; previous revision: 2.168
/cvsroot/python/python/dist/src/PC/pyconfig.h,v  <--  pyconfig.h
new revision: 1.15; previous revision: 1.14
/cvsroot/python/python/dist/src/Python/errors.c,v  <--  errors.c
new revision: 2.72; previous revision: 2.71
msg40929 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-10-03 07:00
Logged In: YES 
user_id=14198

Oops - I missed checking in Lib/test/test_pep277.py.

With this file in place, the test suite fails for me:
test_pep277
test test_pep277 produced unexpected output:
...

However, I seem to recall that new tests do not need output
files.  Should I use the old technique of "regrtest -g ..."
and check both the test and output file in, or is there
something else I should be doing?
msg40930 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-10-03 08:08
Logged In: YES 
user_id=21627

Generating the output file with -g and committing it is the
right thing to do. I noticed that a .sort call is missing
parenthesis, which would need to be added.
msg40931 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-10-03 23:15
Logged In: YES 
user_id=14198

/cvsroot/python/python/dist/src/Lib/test/test_pep277.py,v 
<--  test_pep277.py
initial revision: 1.1
RCS file:
/cvsroot/python/python/dist/src/Lib/test/output/test_pep277,v
/cvsroot/python/python/dist/src/Lib/test/output/test_pep277,v
 <--  test_pep277
initial revision: 1.1
msg40932 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2002-10-03 23:43
Logged In: YES 
user_id=33168

Mark, I had a bunch of code review notes I sent to
python-checkins.  I'm not sure you saw them, since my mail
to you @ users.sf.net apparently doesn't want to go out.
msg40933 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2002-10-03 23:53
Logged In: YES 
user_id=14198

I will try and fix my sf.net mail - in tne meantime Neal, if
you could forward them to my skippinet address I will
address them.
msg40934 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-10-05 09:47
Logged In: YES 
user_id=21627

/cvsroot/python/python/dist/src/Doc/lib/libos.tex,v  <-- 
libos.tex
new revision: 1.97; previous revision: 1.96
done
Checking in Misc/NEWS;
/cvsroot/python/python/dist/src/Misc/NEWS,v  <--  NEWS
new revision: 1.493; previous revision: 1.492
History
Date User Action Args
2002-08-12 11:33:37loewiscreate