Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2548163002:
Unbreak test-webkitpy. (Closed)
DescriptionUnbreak test-webkitpy.
The wpt_github code imports libhttp2, which is not part of the
standard Python libraries (at least on Mac). Luckily, we have
a hermetic instance of it in depot_tools, so let's use that instead.
R=jeffcarp
BUG=
Committed: https://crrev.com/2cda52ae01a80ac18bf1345a5d1d6a9e97514052
Cr-Commit-Position: refs/heads/master@{#436183}
Patch Set 1 #Patch Set 2 : Added TODO and link to bug. #Messages
Total messages: 20 (10 generated)
The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL.
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
Interesting. I had also noticed this breakage a week or two ago but hadn't gotten around to figuring out what was going on. The code should probably not be depending on things from depot_tools at all, and we should either not use httplib2 or import it into Tools/Scripts/webkitpy/thirdparty. Separately, it's probably a bug that the PYTHONPATH of the recipe environment is getting passed to scripts executed in the recipes. @dglazkov - mind filing a bug or two to track these issues, and then referencing at least the first problem w/ a TODO? lgtm otherwise.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/03 at 23:13:35, dpranke wrote: > @dglazkov - mind filing a bug or two to track these issues, and then referencing at least the first problem w/ a TODO? Sure thing! > The code should probably not be depending on things from depot_tools at all, and we should either not use httplib2 or import it into Tools/Scripts/webkitpy/thirdparty. SGTM! > Separately, it's probably a bug that the PYTHONPATH of the recipe environment is getting passed to scripts executed in the recipes. Can you expand on this a bit? I want to make sure I capture the bug correctly. What does the "recipe environment" mean and what are recipes? > lgtm otherwise. \o/
Added TODO and link to bug.
The CQ bit was checked by dglazkov@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/2548163002/#ps20001 (title: "Added TODO and link to bug.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Unbreak test-webkitpy. The wpt_github code imports libhttp2, which is not part of the standard Python libraries (at least on Mac). Luckily, we have a hermetic instance of it depot_tools, so let's use that instead. R=jeffcarp BUG= ========== to ========== Unbreak test-webkitpy. The wpt_github code imports libhttp2, which is not part of the standard Python libraries (at least on Mac). Luckily, we have a hermetic instance of it in depot_tools, so let's use that instead. R=jeffcarp BUG= ==========
On 2016/12/04 00:27:49, dglazkov wrote: > > Separately, it's probably a bug that the PYTHONPATH of the recipe environment > is getting passed to scripts executed in the recipes. > > Can you expand on this a bit? I want to make sure I capture the bug correctly. > What does the "recipe environment" mean and what are recipes? The recipe is the Python script that controls what is run on the builder. When infra sets up the environment to run that script, they are adding a bunch of paths into the PYTHONPATH env var, and that var is propagating into the called test-webkitpy process, so stuff is available for import that probably shouldn't be. Hence the reason why this was passing on the bots but failing locally. The right thing to do is probably a bit less obvious, because there are going to be some infra-side scripts that do need the env var to be set. But this isn't a complicated problem to solve.
On 2016/12/04 at 00:55:44, dpranke wrote: > On 2016/12/04 00:27:49, dglazkov wrote: > > > Separately, it's probably a bug that the PYTHONPATH of the recipe environment > > is getting passed to scripts executed in the recipes. > > > > Can you expand on this a bit? I want to make sure I capture the bug correctly. > > What does the "recipe environment" mean and what are recipes? > > The recipe is the Python script that controls what is run on the builder. When infra > sets up the environment to run that script, they are adding a bunch of paths into > the PYTHONPATH env var, and that var is propagating into the called test-webkitpy > process, so stuff is available for import that probably shouldn't be. Hence the > reason why this was passing on the bots but failing locally. > > The right thing to do is probably a bit less obvious, because there are going to > be some infra-side scripts that do need the env var to be set. But this isn't a > complicated problem to solve. Got it. Filed http://crbug.com/670990 and http://crbug.com/670987.
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480811893264590,
"parent_rev": "ad29a15d79b125b2f3dff9a8dd347ba0356ae0ba", "commit_rev":
"976901a0eafdf1f76ff3d1ec718a4bfd019d5ad5"}
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Unbreak test-webkitpy. The wpt_github code imports libhttp2, which is not part of the standard Python libraries (at least on Mac). Luckily, we have a hermetic instance of it in depot_tools, so let's use that instead. R=jeffcarp BUG= ========== to ========== Unbreak test-webkitpy. The wpt_github code imports libhttp2, which is not part of the standard Python libraries (at least on Mac). Luckily, we have a hermetic instance of it in depot_tools, so let's use that instead. R=jeffcarp BUG= Committed: https://crrev.com/2cda52ae01a80ac18bf1345a5d1d6a9e97514052 Cr-Commit-Position: refs/heads/master@{#436183} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/2cda52ae01a80ac18bf1345a5d1d6a9e97514052 Cr-Commit-Position: refs/heads/master@{#436183}
Message was sent while issue was closed.
On 2016/12/04 at 01:42:21, commit-bot wrote: > Patchset 2 (id:??) landed as https://crrev.com/2cda52ae01a80ac18bf1345a5d1d6a9e97514052 > Cr-Commit-Position: refs/heads/master@{#436183} Belated lgtm - thanks for making this, I'll work on https://crbug.com/670987 this week. |
