New issue
Advanced search Search tips

Issue 891831 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 790570



Sign in to add a comment

LUCI wpt-exporter sometimes crashes when trying to commit a change locally

Project Member Reported by robertma@chromium.org, Oct 3

Issue description

Example: https://logs.chromium.org/logs/infra/buildbucket/cr-buildbucket.appspot.com/8933661196317474288/+/steps/Export_Chromium_commits_and_in-flight_CLs_to_WPT/0/stdout

  Traceback (most recent call last):
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/wpt_export.py", line 27, in <module>
      main()
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/wpt_export.py", line 19, in main
      success = exporter.main()
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 80, in main
      self.process_chromium_commits(exportable_commits)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 135, in process_chromium_commits
      self.process_chromium_commit(commit)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 155, in process_chromium_commit
      self.create_or_update_pr_from_landed_commit(commit, pull_request)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 228, in create_or_update_pr_from_landed_commit
      self.create_or_update_pr_from_commit(commit, provisional=False, pr_number=pull_request.number)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 342, in create_or_update_pr_from_commit
      self.local_wpt.create_branch_with_patch(pr_branch_name, message, patch, author, force_push=updating)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/local_wpt.py", line 107, in create_branch_with_patch
      self.run(['git', 'commit', '--author', author_str, '-am', message])
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/w3c/local_wpt.py", line 61, in run
      return self.host.executive.run_command(command, cwd=self.path, **kwargs)
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/common/system/executive.py", line 319, in run_command
      close_fds=self._should_close_fds())
    File "/b/swarming/w/ir/kitchen-workdir/src/third_party/blink/tools/blinkpy/common/system/executive.py", line 388, in popen
      return subprocess.Popen(string_args, **kwargs)
    File "/b/swarming/w/ir/cipd_bin_packages/lib/python2.7/subprocess.py", line 390, in __init__
      errread, errwrite)
    File "/b/swarming/w/ir/cipd_bin_packages/lib/python2.7/subprocess.py", line 1025, in _execute_child
      raise child_exception
  TypeError: execv() arg 2 must contain only strings


This happens quite often, but not always. Even weirder, I tried logging into the bot, pausing the export script (kill -STOP), and manually executing it. I couldn't reproduce the error. Then I resumed the paused process (kill -CONT); the error also went away.

Let me add --verbose to see if I can find anything interesting.
 
Blocking: 790570
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 3

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/97fb44134d59e7f0da5acd20dd295cf1196cc8ed

commit 97fb44134d59e7f0da5acd20dd295cf1196cc8ed
Author: Robert Ma <robertma@chromium.org>
Date: Wed Oct 03 23:35:26 2018

Temporarily turn on verbose logging for wpt-export

In order to debug a mysterious exception. This CL will be reverted once
the issue is resolved.

Bug:  891831 
Change-Id: I706964ef4a9222bc80dcfcc02729af371488a3fb
Reviewed-on: https://chromium-review.googlesource.com/c/1259742
Auto-Submit: Robert Ma <robertma@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#18042}
[modify] https://crrev.com/97fb44134d59e7f0da5acd20dd295cf1196cc8ed/recipes/recipes/wpt_export.expected/wpt-export_experimental.json
[modify] https://crrev.com/97fb44134d59e7f0da5acd20dd295cf1196cc8ed/recipes/recipes/wpt_export.py
[modify] https://crrev.com/97fb44134d59e7f0da5acd20dd295cf1196cc8ed/recipes/recipes/wpt_export.expected/wpt-export.json

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9f339db027a660b6352625d01b8d5865b80d0d2

commit e9f339db027a660b6352625d01b8d5865b80d0d2
Author: Robert Ma <robertma@chromium.org>
Date: Thu Oct 04 18:48:01 2018

[WPT export] More temporary logging to debug crash

The working theory is that the locale isn't correctly set in the
Python process. The interactive shell on the bot does have a UTF-8
locale, but the environment of the Python process could be different. To
verify this, we add some more logging to wpt-export.

This CL will be reverted once the issue is resolved.

Bug:  891831 
Change-Id: Ice6478a2b4f3805f2a68b4e2b782f0f23e9aeb99
Reviewed-on: https://chromium-review.googlesource.com/c/1261974
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596782}
[modify] https://crrev.com/e9f339db027a660b6352625d01b8d5865b80d0d2/third_party/blink/tools/blinkpy/w3c/local_wpt.py
[modify] https://crrev.com/e9f339db027a660b6352625d01b8d5865b80d0d2/third_party/blink/tools/blinkpy/w3c/test_exporter.py

Cc: mar...@chromium.org
Alright, after lots of tinkling, the issue is indeed caused by empty locale on Swarming.

With the CL in #3, the exporter prints out the LANG environment variable and the return value of Python's `sys.getfilesystemencoding()`:
https://logs.chromium.org/logs/infra/buildbucket/cr-buildbucket.appspot.com/8933570223910370528/+/steps/Export_Chromium_commits_and_in-flight_CLs_to_WPT/0/stdout#L21084_0

This is a strong evidence that the locale isn't set and hence falls back to C, which breaks `subprocess.call` if any argument contains non-ASCII characters.

It was hard to debug because when I SSH'ed into the bot, the interactive shell (the user is chrome-bot, the same user that executes the recipes) did have `en_US.UTF-8` locale, so manually executing the script worked fine.

maruel@ : are the empty locale in recipe steps and UTF-8 locale in the user shell expected on Swarming bots?


Meanwhile, I'll modify blinkpy to always encode subprocess args.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2daf18b565838a9029c5b3910a1a2c7bc881edda

commit 2daf18b565838a9029c5b3910a1a2c7bc881edda
Author: Robert Ma <robertma@chromium.org>
Date: Fri Oct 05 00:10:01 2018

[blinkpy] Encode subprocess args in more cases

Previously, we only encode the arguments on Windows Python 2.X. This
turns out not sufficient. On POSIX systems, if the locale does not use
a Unicode encoding (e.g. the default "C" locale when locale isn't set),
subprocess.Popen will raise a TypeError if arguments contain non-ASCII
characters. This is the case on Swarming bots.

This change makes the Executive class also encode the arguments if the
system encoding is not UTF-8.

Bug:  891831 
Change-Id: Idc6fb9201997b5c53e7f29b2d499fce486033b3b
Reviewed-on: https://chromium-review.googlesource.com/c/1263045
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596935}
[modify] https://crrev.com/2daf18b565838a9029c5b3910a1a2c7bc881edda/third_party/blink/tools/blinkpy/common/system/executive.py

Confirmed the fix: https://chromium-review.googlesource.com/c/chromium/src/+/1264520

Reverting debugging CLs.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/674b54ac5c91b77e69ba3226d68b91eedef836a1

commit 674b54ac5c91b77e69ba3226d68b91eedef836a1
Author: Robert Ma <robertma@chromium.org>
Date: Fri Oct 05 16:03:17 2018

Revert "[WPT export] More temporary logging to debug crash"

This reverts commit e9f339db027a660b6352625d01b8d5865b80d0d2.

Reason for revert: issue has been solved.

Original change's description:
> [WPT export] More temporary logging to debug crash
> 
> The working theory is that the locale isn't correctly set in the
> Python process. The interactive shell on the bot does have a UTF-8
> locale, but the environment of the Python process could be different. To
> verify this, we add some more logging to wpt-export.
> 
> This CL will be reverted once the issue is resolved.
> 
> Bug:  891831 
> Change-Id: Ice6478a2b4f3805f2a68b4e2b782f0f23e9aeb99
> Reviewed-on: https://chromium-review.googlesource.com/c/1261974
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Commit-Queue: Robert Ma <robertma@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596782}

TBR=qyearsley@chromium.org,robertma@chromium.org

Change-Id: Icf05a7a7163a948fba10ff3991df2ccc34b4865a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  891831 
Reviewed-on: https://chromium-review.googlesource.com/c/1265017
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597152}
[modify] https://crrev.com/674b54ac5c91b77e69ba3226d68b91eedef836a1/third_party/blink/tools/blinkpy/w3c/local_wpt.py
[modify] https://crrev.com/674b54ac5c91b77e69ba3226d68b91eedef836a1/third_party/blink/tools/blinkpy/w3c/test_exporter.py

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/5d4e8a2515a82f3255d83d83dec42f5cdbadc0d8

commit 5d4e8a2515a82f3255d83d83dec42f5cdbadc0d8
Author: Robert Ma <robertma@chromium.org>
Date: Fri Oct 05 18:10:23 2018

Revert "Temporarily turn on verbose logging for wpt-export"

This reverts commit 97fb44134d59e7f0da5acd20dd295cf1196cc8ed.

Reason for revert: issue has been resolved.

Original change's description:
> Temporarily turn on verbose logging for wpt-export
> 
> In order to debug a mysterious exception. This CL will be reverted once
> the issue is resolved.
> 
> Bug:  891831 
> Change-Id: I706964ef4a9222bc80dcfcc02729af371488a3fb
> Reviewed-on: https://chromium-review.googlesource.com/c/1259742
> Auto-Submit: Robert Ma <robertma@chromium.org>
> Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#18042}

TBR=qyearsley@chromium.org,robertma@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  891831 
Change-Id: Ie7de3037390f5fc7cead2a644b62803265370c2a
Reviewed-on: https://chromium-review.googlesource.com/c/1265295
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#18106}
[modify] https://crrev.com/5d4e8a2515a82f3255d83d83dec42f5cdbadc0d8/recipes/recipes/wpt_export.expected/wpt-export_experimental.json
[modify] https://crrev.com/5d4e8a2515a82f3255d83d83dec42f5cdbadc0d8/recipes/recipes/wpt_export.py
[modify] https://crrev.com/5d4e8a2515a82f3255d83d83dec42f5cdbadc0d8/recipes/recipes/wpt_export.expected/wpt-export.json

Status: Fixed (was: Started)
I just recalled now; when starting under upstart, LANG is not setup. So it's not Swarming's fault, it's due to the initial environment.

I think(?) when we switch to systemd (i.e. Ubuntu 16.04+ or Debian 9.x) we won't have this problem anymore.

Sign in to add a comment