Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Fixed #8149 -- Use universal line terminators when iterating `File` obje... #323

Closed
wants to merge 4 commits into from

5 participants

Tai Lee Tim Graham Alex Gaynor Ramiro Morales Aymeric Augustin
Tai Lee

...cts line-by-line.

Tim Graham
Owner

The test doesn't pass on Python 3.

Tim Graham
Owner

Please open a new PR if you are able to update this, thanks!

Tim Graham timgraham closed this September 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 4 unique commits by 4 authors.

Mar 10, 2013
Alex Gaynor Merge pull request #892 from JonLoy/ticket_20018
Fixed #20018: Added backtick to fix reference
3f43f5f
Ramiro Morales Made (make|compile)messages check for availability of gettext commands.
Refs #19584.
7fca441
Aymeric Augustin Fixed #20019 -- Ensured HttpRequest.resolver_match always exists.
Obviously it isn't set until the URL is resolved.
ce76fbf
Mar 11, 2013
Fixed #8149 -- Use universal line terminators when iterating `File` o…
…bjects line-by-line.
e611cf6
This page is out of date. Refresh to see the latest.
4  django/core/files/base.py
@@ -94,9 +94,7 @@ def __iter__(self):
94 94
         # Iterate over this file-like object by newlines
95 95
         buffer_ = None
96 96
         for chunk in self.chunks():
97  
-            chunk_buffer = BytesIO(chunk)
98  
-
99  
-            for line in chunk_buffer:
  97
+            for line in chunk.splitlines(True):
100 98
                 if buffer_:
101 99
                     line = buffer_ + line
102 100
                     buffer_ = None
7  django/core/management/commands/compilemessages.py
@@ -5,7 +5,7 @@
5 5
 from optparse import make_option
6 6
 
7 7
 from django.core.management.base import BaseCommand, CommandError
8  
-from django.core.management.utils import popen_wrapper
  8
+from django.core.management.utils import find_command, popen_wrapper
9 9
 from django.utils._os import npath
10 10
 
11 11
 def has_bom(fn):
@@ -16,6 +16,10 @@ def has_bom(fn):
16 16
             sample.startswith(codecs.BOM_UTF16_BE)
17 17
 
18 18
 def compile_messages(stderr, locale=None):
  19
+    program = 'msgfmt'
  20
+    if find_command(program) is None:
  21
+        raise CommandError("Can't find %s. Make sure you have GNU gettext tools 0.15 or newer installed." % program)
  22
+
19 23
     basedirs = [os.path.join('conf', 'locale'), 'locale']
20 24
     if os.environ.get('DJANGO_SETTINGS_MODULE'):
21 25
         from django.conf import settings
@@ -42,7 +46,6 @@ def compile_messages(stderr, locale=None):
42 46
                     if has_bom(fn):
43 47
                         raise CommandError("The %s file has a BOM (Byte Order Mark). Django only supports .po files encoded in UTF-8 and without any BOM." % fn)
44 48
                     pf = os.path.splitext(fn)[0]
45  
-                    program = 'msgfmt'
46 49
                     args = [program, '--check-format', '-o', npath(pf + '.mo'), npath(pf + '.po')]
47 50
                     output, errors, status = popen_wrapper(args)
48 51
                     if status:
112  django/core/management/commands/makemessages.py
@@ -5,11 +5,11 @@
5 5
 import sys
6 6
 from itertools import dropwhile
7 7
 from optparse import make_option
8  
-from subprocess import PIPE, Popen
9 8
 
10 9
 import django
11 10
 from django.core.management.base import CommandError, NoArgsCommand
12  
-from django.core.management.utils import handle_extensions
  11
+from django.core.management.utils import (handle_extensions, find_command,
  12
+    popen_wrapper)
13 13
 from django.utils.functional import total_ordering
14 14
 from django.utils.text import get_text_list
15 15
 from django.utils.jslex import prepare_js_for_gettext
@@ -18,6 +18,13 @@
18 18
 STATUS_OK = 0
19 19
 
20 20
 
  21
+def check_programs(*programs):
  22
+    for program in programs:
  23
+        if find_command(program) is None:
  24
+            raise CommandError("Can't find %s. Make sure you have GNU "
  25
+                    "gettext tools 0.15 or newer installed." % program)
  26
+
  27
+
21 28
 @total_ordering
22 29
 class TranslatableFile(object):
23 30
     def __init__(self, dirpath, file_name):
@@ -58,12 +65,24 @@ def process(self, command, potfile, domain, keep_pot=False):
58 65
             work_file = os.path.join(self.dirpath, thefile)
59 66
             with open(work_file, "w") as fp:
60 67
                 fp.write(src_data)
61  
-            cmd = (
62  
-                'xgettext -d %s -L C %s %s --keyword=gettext_noop '
63  
-                '--keyword=gettext_lazy --keyword=ngettext_lazy:1,2 '
64  
-                '--keyword=pgettext:1c,2 --keyword=npgettext:1c,2,3 '
65  
-                '--from-code UTF-8 --add-comments=Translators -o - "%s"' %
66  
-                (domain, command.wrap, command.location, work_file))
  68
+            args = [
  69
+                'xgettext',
  70
+                '-d', domain,
  71
+                '--language=C',
  72
+                '--keyword=gettext_noop',
  73
+                '--keyword=gettext_lazy',
  74
+                '--keyword=ngettext_lazy:1,2',
  75
+                '--keyword=pgettext:1c,2',
  76
+                '--keyword=npgettext:1c,2,3',
  77
+                '--from-code=UTF-8',
  78
+                '--add-comments=Translators',
  79
+                '--output=-'
  80
+            ]
  81
+            if command.wrap:
  82
+                args.append(command.wrap)
  83
+            if command.location:
  84
+                args.append(command.location)
  85
+            args.append(work_file)
67 86
         elif domain == 'django' and (file_ext == '.py' or file_ext in command.extensions):
68 87
             thefile = self.file
69 88
             orig_file = os.path.join(self.dirpath, self.file)
@@ -76,18 +95,32 @@ def process(self, command, potfile, domain, keep_pot=False):
76 95
                 with open(os.path.join(self.dirpath, thefile), "w") as fp:
77 96
                     fp.write(content)
78 97
             work_file = os.path.join(self.dirpath, thefile)
79  
-            cmd = (
80  
-                'xgettext -d %s -L Python %s %s --keyword=gettext_noop '
81  
-                '--keyword=gettext_lazy --keyword=ngettext_lazy:1,2 '
82  
-                '--keyword=ugettext_noop --keyword=ugettext_lazy '
83  
-                '--keyword=ungettext_lazy:1,2 --keyword=pgettext:1c,2 '
84  
-                '--keyword=npgettext:1c,2,3 --keyword=pgettext_lazy:1c,2 '
85  
-                '--keyword=npgettext_lazy:1c,2,3 --from-code UTF-8 '
86  
-                '--add-comments=Translators -o - "%s"' %
87  
-                (domain, command.wrap, command.location, work_file))
  98
+            args = [
  99
+                'xgettext',
  100
+                '-d', domain,
  101
+                '--language=Python',
  102
+                '--keyword=gettext_noop',
  103
+                '--keyword=gettext_lazy',
  104
+                '--keyword=ngettext_lazy:1,2',
  105
+                '--keyword=ugettext_noop',
  106
+                '--keyword=ugettext_lazy',
  107
+                '--keyword=ungettext_lazy:1,2',
  108
+                '--keyword=pgettext:1c,2',
  109
+                '--keyword=npgettext:1c,2,3',
  110
+                '--keyword=pgettext_lazy:1c,2',
  111
+                '--keyword=npgettext_lazy:1c,2,3',
  112
+                '--from-code=UTF-8',
  113
+                '--add-comments=Translators',
  114
+                '--output=-'
  115
+            ]
  116
+            if command.wrap:
  117
+                args.append(command.wrap)
  118
+            if command.location:
  119
+                args.append(command.location)
  120
+            args.append(work_file)
88 121
         else:
89 122
             return
90  
-        msgs, errors, status = _popen(cmd)
  123
+        msgs, errors, status = popen_wrapper(args)
91 124
         if errors:
92 125
             if status != STATUS_OK:
93 126
                 if is_templatized:
@@ -109,15 +142,6 @@ def process(self, command, potfile, domain, keep_pot=False):
109 142
         if is_templatized:
110 143
             os.unlink(work_file)
111 144
 
112  
-
113  
-def _popen(cmd):
114  
-    """
115  
-    Friendly wrapper around Popen for Windows
116  
-    """
117  
-    p = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE, close_fds=os.name != 'nt', universal_newlines=True)
118  
-    output, errors = p.communicate()
119  
-    return output, errors, p.returncode
120  
-
121 145
 def write_pot_file(potfile, msgs):
122 146
     """
123 147
     Write the :param potfile: POT file with the :param msgs: contents,
@@ -225,8 +249,9 @@ def handle_noargs(self, *args, **options):
225 249
                     "is not created automatically, you have to create it by hand "
226 250
                     "if you want to enable i18n for your project or application.")
227 251
 
  252
+        check_programs('xgettext')
228 253
         # We require gettext version 0.15 or newer.
229  
-        output, errors, status = _popen('xgettext --version')
  254
+        output, errors, status = popen_wrapper(['xgettext', '--version'])
230 255
         if status != STATUS_OK:
231 256
             raise CommandError("Error running xgettext. Note that Django "
232 257
                         "internationalization requires GNU gettext 0.15 or newer.")
@@ -248,6 +273,9 @@ def handle_noargs(self, *args, **options):
248 273
             locale_dirs = filter(os.path.isdir, glob.glob('%s/*' % localedir))
249 274
             locales = [os.path.basename(l) for l in locale_dirs]
250 275
 
  276
+        if locales:
  277
+            check_programs('msguniq', 'msgmerge', 'msgattrib')
  278
+
251 279
         try:
252 280
             for locale in locales:
253 281
                 if self.verbosity > 0:
@@ -307,8 +335,13 @@ def write_po_file(self, potfile, locale):
307 335
 
308 336
         Uses mguniq, msgmerge, and msgattrib GNU gettext utilities.
309 337
         """
310  
-        msgs, errors, status = _popen('msguniq %s %s --to-code=utf-8 "%s"' %
311  
-                                        (self.wrap, self.location, potfile))
  338
+        args = ['msguniq', '--to-code=utf-8']
  339
+        if self.wrap:
  340
+            args.append(self.wrap)
  341
+        if self.location:
  342
+            args.append(self.location)
  343
+        args.append(potfile)
  344
+        msgs, errors, status = popen_wrapper(args)
312 345
         if errors:
313 346
             if status != STATUS_OK:
314 347
                 raise CommandError(
@@ -324,8 +357,13 @@ def write_po_file(self, potfile, locale):
324 357
         if os.path.exists(pofile):
325 358
             with open(potfile, 'w') as fp:
326 359
                 fp.write(msgs)
327  
-            msgs, errors, status = _popen('msgmerge %s %s -q "%s" "%s"' %
328  
-                                            (self.wrap, self.location, pofile, potfile))
  360
+            args = ['msgmerge', '-q']
  361
+            if self.wrap:
  362
+                args.append(self.wrap)
  363
+            if self.location:
  364
+                args.append(self.location)
  365
+            args.extend([pofile, potfile])
  366
+            msgs, errors, status = popen_wrapper(args)
329 367
             if errors:
330 368
                 if status != STATUS_OK:
331 369
                     raise CommandError(
@@ -340,9 +378,13 @@ def write_po_file(self, potfile, locale):
340 378
             fp.write(msgs)
341 379
 
342 380
         if self.no_obsolete:
343  
-            msgs, errors, status = _popen(
344  
-                'msgattrib %s %s -o "%s" --no-obsolete "%s"' %
345  
-                (self.wrap, self.location, pofile, pofile))
  381
+            args = ['msgattrib', '-o', pofile, '--no-obsolete']
  382
+            if self.wrap:
  383
+                args.append(self.wrap)
  384
+            if self.location:
  385
+                args.append(self.location)
  386
+            args.append(pofile)
  387
+            msgs, errors, status = popen_wrapper(args)
346 388
             if errors:
347 389
                 if status != STATUS_OK:
348 390
                     raise CommandError(
40  django/core/management/utils.py
... ...
@@ -1,17 +1,27 @@
  1
+from __future__ import absolute_import
  2
+
1 3
 import os
2 4
 from subprocess import PIPE, Popen
  5
+import sys
3 6
 
4 7
 from django.utils.encoding import force_text, DEFAULT_LOCALE_ENCODING
  8
+from django.utils import six
  9
+
  10
+from .base import CommandError
5 11
 
6 12
 
7  
-def popen_wrapper(args):
  13
+def popen_wrapper(args, os_err_exc_type=CommandError):
8 14
     """
9 15
     Friendly wrapper around Popen.
10 16
 
11 17
     Returns stdout output, stderr output and OS status code.
12 18
     """
13  
-    p = Popen(args, shell=False, stdout=PIPE, stderr=PIPE,
14  
-              close_fds=os.name != 'nt', universal_newlines=True)
  19
+    try:
  20
+        p = Popen(args, shell=False, stdout=PIPE, stderr=PIPE,
  21
+                close_fds=os.name != 'nt', universal_newlines=True)
  22
+    except OSError as e:
  23
+        six.reraise(os_err_exc_type, os_err_exc_type('Error executing %s: %s' %
  24
+                    (args[0], e.strerror)), sys.exc_info()[2])
15 25
     output, errors = p.communicate()
16 26
     return (
17 27
         output,
@@ -43,3 +53,27 @@ def handle_extensions(extensions=('html',), ignored=('py',)):
43 53
         if not ext.startswith('.'):
44 54
             ext_list[i] = '.%s' % ext_list[i]
45 55
     return set([x for x in ext_list if x.strip('.') not in ignored])
  56
+
  57
+def find_command(cmd, path=None, pathext=None):
  58
+    if path is None:
  59
+        path = os.environ.get('PATH', []).split(os.pathsep)
  60
+    if isinstance(path, six.string_types):
  61
+        path = [path]
  62
+    # check if there are funny path extensions for executables, e.g. Windows
  63
+    if pathext is None:
  64
+        pathext = os.environ.get('PATHEXT', '.COM;.EXE;.BAT;.CMD').split(os.pathsep)
  65
+    # don't use extensions if the command ends with one of them
  66
+    for ext in pathext:
  67
+        if cmd.endswith(ext):
  68
+            pathext = ['']
  69
+            break
  70
+    # check if we find the command on PATH
  71
+    for p in path:
  72
+        f = os.path.join(p, cmd)
  73
+        if os.path.isfile(f):
  74
+            return f
  75
+        for ext in pathext:
  76
+            fext = f + ext
  77
+            if os.path.isfile(fext):
  78
+                return fext
  79
+    return None
1  django/http/request.py
@@ -44,6 +44,7 @@ def __init__(self):
44 44
         self.path = ''
45 45
         self.path_info = ''
46 46
         self.method = None
  47
+        self.resolver_match = None
47 48
         self._post_parse_error = False
48 49
 
49 50
     def __repr__(self):
9  docs/topics/http/file-uploads.txt
@@ -258,10 +258,11 @@ In addition to those inherited from :class:`~django.core.files.File`, all
258 258
         for line in uploadedfile:
259 259
             do_something_with(line)
260 260
 
261  
-    However, *unlike* standard Python files, :class:`UploadedFile` only
262  
-    understands ``\n`` (also known as "Unix-style") line endings. If you know
263  
-    that you need to handle uploaded files with different line endings, you'll
264  
-    need to do so in your view.
  261
+    .. versionchanged:: 1.6
  262
+
  263
+    In previous versions, :class:`UploadedFile` only understood ``\n`` (also
  264
+    known as "Unix-style") line endings. Now, you can read files line-by-line
  265
+    when they use ``\r`` line endings as well.
265 266
 
266 267
 Upload Handlers
267 268
 ===============
7  tests/files/tests.py
... ...
@@ -1,6 +1,7 @@
1 1
 from __future__ import absolute_import
2 2
 
3 3
 import gzip
  4
+import io
4 5
 import shutil
5 6
 import tempfile
6 7
 
@@ -146,3 +147,9 @@ def test_file_mode(self):
146 147
         file = SimpleUploadedFile("mode_test.txt", b"content")
147 148
         self.assertFalse(hasattr(file, 'mode'))
148 149
         g = gzip.GzipFile(fileobj=file)
  150
+
  151
+    def test_universal_newlines(self):
  152
+        # See #8149 for more information.
  153
+        self.assertEqual(
  154
+            list(File(io.BytesIO(b'1\r2\r3'))),
  155
+            ['1\r', '2\r', '3'])
9  tests/i18n/commands/extraction.py
@@ -131,8 +131,13 @@ def test_extraction_warning(self):
131 131
         os.chdir(self.test_dir)
132 132
         shutil.copyfile('./code.sample', './code_sample.py')
133 133
         stdout = StringIO()
134  
-        management.call_command('makemessages', locale=LOCALE, stdout=stdout)
135  
-        os.remove('./code_sample.py')
  134
+        try:
  135
+            management.call_command('makemessages', locale=LOCALE, stdout=stdout)
  136
+        finally:
  137
+            try:
  138
+                os.remove('./code_sample.py')
  139
+            except OSError:
  140
+                pass
136 141
         self.assertIn("code_sample.py:4", force_text(stdout.getvalue()))
137 142
 
138 143
     def test_template_message_context_extractor(self):
26  tests/i18n/commands/tests.py
@@ -2,35 +2,11 @@
2 2
 import re
3 3
 from subprocess import Popen, PIPE
4 4
 
5  
-from django.utils import six
  5
+from django.core.management.utils import find_command
6 6
 
7 7
 can_run_extraction_tests = False
8 8
 can_run_compilation_tests = False
9 9
 
10  
-def find_command(cmd, path=None, pathext=None):
11  
-    if path is None:
12  
-        path = os.environ.get('PATH', []).split(os.pathsep)
13  
-    if isinstance(path, six.string_types):
14  
-        path = [path]
15  
-    # check if there are funny path extensions for executables, e.g. Windows
16  
-    if pathext is None:
17  
-        pathext = os.environ.get('PATHEXT', '.COM;.EXE;.BAT;.CMD').split(os.pathsep)
18  
-    # don't use extensions if the command ends with one of them
19  
-    for ext in pathext:
20  
-        if cmd.endswith(ext):
21  
-            pathext = ['']
22  
-            break
23  
-    # check if we find the command on PATH
24  
-    for p in path:
25  
-        f = os.path.join(p, cmd)
26  
-        if os.path.isfile(f):
27  
-            return f
28  
-        for ext in pathext:
29  
-            fext = f + ext
30  
-            if os.path.isfile(fext):
31  
-                return fext
32  
-    return None
33  
-
34 10
 # checks if it can find xgettext on the PATH and
35 11
 # imports the extraction tests if yes
36 12
 xgettext_cmd = find_command('xgettext')
6  tests/urlpatterns_reverse/tests.py
@@ -9,7 +9,7 @@
9 9
 from django.core.urlresolvers import (reverse, resolve, get_callable,
10 10
     get_resolver, NoReverseMatch, Resolver404, ResolverMatch, RegexURLResolver,
11 11
     RegexURLPattern)
12  
-from django.http import HttpResponseRedirect, HttpResponsePermanentRedirect
  12
+from django.http import HttpRequest, HttpResponseRedirect, HttpResponsePermanentRedirect
13 13
 from django.shortcuts import redirect
14 14
 from django.test import TestCase
15 15
 from django.utils import unittest, six
@@ -529,6 +529,10 @@ def test_resolver_match_on_request(self):
529 529
         resolver_match = response.resolver_match
530 530
         self.assertEqual(resolver_match.url_name, 'test-resolver-match')
531 531
 
  532
+    def test_resolver_match_on_request_before_resolution(self):
  533
+        request = HttpRequest()
  534
+        self.assertIsNone(request.resolver_match)
  535
+
532 536
 class ErroneousViewTests(TestCase):
533 537
     urls = 'urlpatterns_reverse.erroneous_urls'
534 538
 
13  tests/user_commands/tests.py
... ...
@@ -1,13 +1,14 @@
1 1
 import sys
2 2
 
3 3
 from django.core import management
4  
-from django.core.management.base import CommandError
5  
-from django.test import TestCase
  4
+from django.core.management import CommandError
  5
+from django.core.management.utils import popen_wrapper
  6
+from django.test import SimpleTestCase
6 7
 from django.utils import translation
7 8
 from django.utils.six import StringIO
8 9
 
9 10
 
10  
-class CommandTests(TestCase):
  11
+class CommandTests(SimpleTestCase):
11 12
     def test_command(self):
12 13
         out = StringIO()
13 14
         management.call_command('dance', stdout=out)
@@ -58,3 +59,9 @@ def test_configured_locale_preserved(self):
58 59
         with translation.override('pl'):
59 60
             management.call_command('leave_locale_alone_true', stdout=out)
60 61
             self.assertEqual(out.getvalue(), "pl\n")
  62
+
  63
+
  64
+class UtilsTests(SimpleTestCase):
  65
+
  66
+    def test_no_existent_external_program(self):
  67
+        self.assertRaises(CommandError, popen_wrapper, ['a_42_command_that_doesnt_exist_42'])
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.