Browse Source

Consistently check subprocess output

In some places we used universal_newlines and in others we didn't. In
some places we decoded from utf8 and in others we didn't. Switch to not
using universal_newlines at all because this implies getting back a str
instance on python 3 not a binary string which can't be decoded. Then
consistently decode using the users preferred local as that is what
universal_newlines will attempt to do.

We also update things to treat all exception output as binary initially,
convert to user's locale and then parse. This makes error handling
consistent with normal situation handling across the board for python2
and python3 as well.

Note that we don't appear to do any line splitting on newlines here
anyways so this shouldn't affect anything, also these all run on Linux
where you should get unix newlines anyways.

Change-Id: I8dc5ee0b14906324169def2cdc35fbcd4f776423
tags/2.3.0^0
Clark Boylan 2 years ago
parent
commit
428ae68863
2 changed files with 27 additions and 37 deletions
  1. 13
    13
      bindep/depends.py
  2. 14
    24
      bindep/tests/test_depends.py

+ 13
- 13
bindep/depends.py View File

@@ -15,6 +15,7 @@
15 15
 # See the License for the specific language governing permissions and
16 16
 # limitations under the License.
17 17
 
18
+from locale import getpreferredencoding
18 19
 import logging
19 20
 import os.path
20 21
 from parsley import makeGrammar
@@ -226,7 +227,7 @@ class Depends(object):
226 227
     def platform_profiles(self):
227 228
         output = subprocess.check_output(
228 229
             ["lsb_release", "-cirs"],
229
-            stderr=subprocess.STDOUT).decode('utf-8')
230
+            stderr=subprocess.STDOUT).decode(getpreferredencoding(False))
230 231
         lsbinfo = output.lower().split()
231 232
         # NOTE(toabctl): distro can be more than one string (i.e. "SUSE LINUX")
232 233
         codename = lsbinfo[len(lsbinfo) - 1:len(lsbinfo)][0]
@@ -279,12 +280,12 @@ class Dpkg(Platform):
279 280
             output = subprocess.check_output(
280 281
                 ["dpkg-query", "-W", "-f", "${Package} ${Status} ${Version}\n",
281 282
                  pkg_name],
282
-                stderr=subprocess.STDOUT,
283
-                universal_newlines=True).decode('utf-8')
283
+                stderr=subprocess.STDOUT).decode(getpreferredencoding(False))
284 284
         except subprocess.CalledProcessError as e:
285
+            eoutput = e.output.decode(getpreferredencoding(False))
285 286
             if (e.returncode == 1 and
286
-                (e.output.startswith('dpkg-query: no packages found') or
287
-                 e.output.startswith('No packages found matching'))):
287
+                (eoutput.startswith('dpkg-query: no packages found') or
288
+                 eoutput.startswith('No packages found matching'))):
288 289
                 return None
289 290
             raise
290 291
         # output looks like
@@ -309,11 +310,11 @@ class Rpm(Platform):
309 310
                 ["rpm", "--qf",
310 311
                  "%{NAME} %|EPOCH?{%{EPOCH}:}|%{VERSION}-%{RELEASE}\n", "-q",
311 312
                  pkg_name],
312
-                stderr=subprocess.STDOUT,
313
-                universal_newlines=True).decode('utf-8')
313
+                stderr=subprocess.STDOUT).decode(getpreferredencoding(False))
314 314
         except subprocess.CalledProcessError as e:
315
+            eoutput = e.output.decode(getpreferredencoding(False))
315 316
             if (e.returncode == 1 and
316
-                e.output.strip().endswith('is not installed')):
317
+                eoutput.strip().endswith('is not installed')):
317 318
                 return None
318 319
             raise
319 320
         # output looks like
@@ -335,8 +336,7 @@ class Emerge(Platform):
335 336
         try:
336 337
             output = subprocess.check_output(
337 338
                 ['equery', 'l', '--format=\'$version\'', pkg_name],
338
-                stderr=subprocess.STDOUT,
339
-                universal_newlines=True).decode('utf-8')
339
+                stderr=subprocess.STDOUT).decode(getpreferredencoding(False))
340 340
         except subprocess.CalledProcessError as e:
341 341
             if e.returncode == 3:
342 342
                 return None
@@ -358,10 +358,10 @@ class Pacman(Platform):
358 358
         try:
359 359
             output = subprocess.check_output(
360 360
                 ['pacman', '-Q', pkg_name],
361
-                stderr=subprocess.STDOUT,
362
-                universal_newlines=True)
361
+                stderr=subprocess.STDOUT).decode(getpreferredencoding(False))
363 362
         except subprocess.CalledProcessError as e:
364
-            if e.returncode == 1 and e.output.endswith('was not found'):
363
+            eoutput = e.output.decode(getpreferredencoding(False))
364
+            if e.returncode == 1 and eoutput.endswith('was not found'):
365 365
                 return None
366 366
             raise
367 367
         # output looks like

+ 14
- 24
bindep/tests/test_depends.py View File

@@ -361,8 +361,7 @@ class TestDpkg(TestCase):
361 361
         mock_checkoutput.assert_called_once_with(
362 362
             ["dpkg-query", "-W", "-f",
363 363
              "${Package} ${Status} ${Version}\n", "foo"],
364
-            stderr=subprocess.STDOUT,
365
-            universal_newlines=True)
364
+            stderr=subprocess.STDOUT)
366 365
 
367 366
     def test_unknown_package(self):
368 367
         platform = Dpkg()
@@ -371,15 +370,14 @@ class TestDpkg(TestCase):
371 370
 
372 371
         def _side_effect_raise(*args, **kwargs):
373 372
             raise subprocess.CalledProcessError(
374
-                1, [], "dpkg-query: no packages found matching foo\n")
373
+                1, [], b"dpkg-query: no packages found matching foo\n")
375 374
 
376 375
         mock_checkoutput.side_effect = _side_effect_raise
377 376
         self.assertEqual(None, platform.get_pkg_version("foo"))
378 377
         mock_checkoutput.assert_called_once_with(
379 378
             ["dpkg-query", "-W", "-f",
380 379
              "${Package} ${Status} ${Version}\n", "foo"],
381
-            stderr=subprocess.STDOUT,
382
-            universal_newlines=True)
380
+            stderr=subprocess.STDOUT)
383 381
 
384 382
     def test_installed_version(self):
385 383
         platform = Dpkg()
@@ -393,8 +391,7 @@ class TestDpkg(TestCase):
393 391
         mocked_checkoutput.assert_called_once_with(
394 392
             ["dpkg-query", "-W", "-f",
395 393
              "${Package} ${Status} ${Version}\n", "foo"],
396
-            stderr=subprocess.STDOUT,
397
-            universal_newlines=True)
394
+            stderr=subprocess.STDOUT)
398 395
 
399 396
 
400 397
 class TestEmerge(TestCase):
@@ -412,8 +409,7 @@ class TestEmerge(TestCase):
412 409
         self.assertEqual(None, platform.get_pkg_version("foo"))
413 410
         mocked_checkoutput.assert_called_once_with(
414 411
             ['equery', 'l', '--format=\'$version\'', 'foo'],
415
-            stderr=subprocess.STDOUT,
416
-            universal_newlines=True)
412
+            stderr=subprocess.STDOUT)
417 413
 
418 414
     def test_unknown_package(self):
419 415
         platform = Emerge()
@@ -428,8 +424,7 @@ class TestEmerge(TestCase):
428 424
         self.assertEqual(None, platform.get_pkg_version("foo"))
429 425
         mocked_checkoutput.assert_called_once_with(
430 426
             ['equery', 'l', '--format=\'$version\'', 'foo'],
431
-            stderr=subprocess.STDOUT,
432
-            universal_newlines=True)
427
+            stderr=subprocess.STDOUT)
433 428
 
434 429
     def test_installed_version(self):
435 430
         platform = Emerge()
@@ -439,8 +434,7 @@ class TestEmerge(TestCase):
439 434
         self.assertEqual("4.0.0", platform.get_pkg_version("foo"))
440 435
         mock_checkoutput.assert_called_once_with(
441 436
             ['equery', 'l', '--format=\'$version\'', 'foo'],
442
-            stderr=subprocess.STDOUT,
443
-            universal_newlines=True)
437
+            stderr=subprocess.STDOUT)
444 438
 
445 439
 
446 440
 class TestPacman(TestCase):
@@ -450,7 +444,7 @@ class TestPacman(TestCase):
450 444
 
451 445
         def _side_effect_raise(*args, **kwargs):
452 446
             raise subprocess.CalledProcessError(
453
-                1, [], "error: package 'foo' was not found")
447
+                1, [], b"error: package 'foo' was not found")
454 448
 
455 449
         mock_checkoutput = self.useFixture(
456 450
             fixtures.MockPatchObject(subprocess, "check_output")).mock
@@ -459,20 +453,18 @@ class TestPacman(TestCase):
459 453
         self.assertEqual(None, platform.get_pkg_version("foo"))
460 454
         mock_checkoutput.assert_called_once_with(
461 455
             ['pacman', '-Q', 'foo'],
462
-            stderr=subprocess.STDOUT,
463
-            universal_newlines=True)
456
+            stderr=subprocess.STDOUT)
464 457
         self.assertEqual(None, platform.get_pkg_version("foo"))
465 458
 
466 459
     def test_installed_version(self):
467 460
         platform = Pacman()
468 461
         mock_checkoutput = self.useFixture(
469 462
             fixtures.MockPatchObject(subprocess, "check_output")).mock
470
-        mock_checkoutput.return_value = 'foo 4.0.0-2'
463
+        mock_checkoutput.return_value = b'foo 4.0.0-2'
471 464
         self.assertEqual("4.0.0-2", platform.get_pkg_version("foo"))
472 465
         mock_checkoutput.assert_called_once_with(
473 466
             ['pacman', '-Q', 'foo'],
474
-            stderr=subprocess.STDOUT,
475
-            universal_newlines=True)
467
+            stderr=subprocess.STDOUT)
476 468
 
477 469
 
478 470
 class TestRpm(TestCase):
@@ -485,7 +477,7 @@ class TestRpm(TestCase):
485 477
 
486 478
         def _side_effect_raise(*args, **kwargs):
487 479
             raise subprocess.CalledProcessError(
488
-                1, [], "package foo is not installed\n")
480
+                1, [], b"package foo is not installed\n")
489 481
 
490 482
         mock_checkoutput = self.useFixture(
491 483
             fixtures.MockPatchObject(subprocess, 'check_output')).mock
@@ -495,8 +487,7 @@ class TestRpm(TestCase):
495 487
             ["rpm", "--qf",
496 488
              "%{NAME} %|EPOCH?{%{EPOCH}:}|%{VERSION}-%{RELEASE}\n", "-q",
497 489
              "foo"],
498
-            stderr=subprocess.STDOUT,
499
-            universal_newlines=True)
490
+            stderr=subprocess.STDOUT)
500 491
         self.assertEqual(None, platform.get_pkg_version("foo"))
501 492
 
502 493
     def test_installed_version(self):
@@ -509,8 +500,7 @@ class TestRpm(TestCase):
509 500
             ["rpm", "--qf",
510 501
              "%{NAME} %|EPOCH?{%{EPOCH}:}|%{VERSION}-%{RELEASE}\n", "-q",
511 502
              "foo"],
512
-            stderr=subprocess.STDOUT,
513
-            universal_newlines=True)
503
+            stderr=subprocess.STDOUT)
514 504
 
515 505
 
516 506
 class TestEval(TestCase):

Loading…
Cancel
Save