New issue
Advanced search Search tips

Issue 822042 link

Starred by 5 users

Issue metadata

Status: Available
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----

Blocking:
issue 822103



Sign in to add a comment

postprocess_for_goma.upload_log failing on multiple builders

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Mar 14 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of crouleau@chromium.org

postprocess_for_goma.upload_log failing on multiple builders

Builders failed on: 
- Linux CFI: 
  https://build.chromium.org/p/chromium.memory/builders/Linux%20CFI

Traceback (most recent call last):
  File "/b/rr/tmpXFBZoP/rw/checkout/scripts/slave/upload_goma_logs.py", line 13, in <module>
    from google.cloud import bigquery
ImportError: No module named google.cloud


 
Cc: iannucci@chromium.org yyanagisawa@chromium.org shinyak@chromium.org
Components: Infra
I'm thinking this is the culprit: https://chromium-review.googlesource.com/c/chromium/tools/build/+/956647
Owner: yyanagisawa@chromium.org
Status: Assigned (was: Available)
Something is pretty weird though; this error only seems to happen sporadically. 
i.e. on the next build it's green: https://ci.chromium.org/buildbot/chromium.memory/Linux%20CFI/6464
The PYTHONPATH for both failed and working builds are identical
Cc: -crouleau@chromium.org
Labels: -Sheriff-Chromium
Since I can't revert the culprit (button doesn't exist) (and maybe I don't know enough about infra to tell if that is a safe decision anyway), I will remove sheriff label from this and let trooper or Robbie or yyanagisawa@ handle this.
Blocking: 822103
 Issue 822103  has been merged into this issue.
@yyanagisawa: Is your CL revertible? Could you please take care of that? Otherwise I'd try to blindly revert it.
Let me do yet another attempts on this.  As far as I understand Robbie guessed that vpython in vpython brought flakiness, and I have submitted the cl to remove such vpython in vpython:
https://chromium-review.googlesource.com/c/chromium/tools/build/+/963953


I saw you added me as reviewer on the CL, but it doesn't open on my laptop. Too large.

Generally I'd advice against such whole-fleet-changer CLs. Unless you are around long enough in case everything breaks.

Please also make sure to guide the non-trivial recipe-roller CLs after that CL.
Now it's all getting purple, e.g.:
https://ci.chromium.org/buildbot/chromium.linux/Fuchsia%20ARM64%20Cast%20Audio/6774

Can you please revert?
Labels: -Pri-0 Pri-1
The CL from comment 11 was reverted.

@Robbie, could you come up with another theory? ;)

I don't know if it helps, but this morning (CET) the problem suddenly spiked on a lot of bots, see comment 8, but then got better again.
This is just guessing what is happening and I am not confident but...
I guess that when we use vpython, vpython might occasionally warm up the cache, and after the warm up, vpython won't fail.

I come up with several questions.
Is vpython verifies all wheels listed in .vpython are downloaded?  If yes, what happens if one of necessary wheels is not available?
google.cloud.bigquery is first one listed in .vpython file, and I also would like to know the case all wheels are missing.

Also, how we can debug if python says some package is missing?  In other words, how I can know what package is installed in vpython's cache?  Is there any ways to show list of actually installed packages on error?


 Issue 821709  has been merged into this issue.
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/fd888d31f47f99cd6b933c9c2ce026f91f800ea2

commit fd888d31f47f99cd6b933c9c2ce026f91f800ea2
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Fri Mar 16 07:16:26 2018

Workaround: load BQ-related modules only if they are needed.

BigQuery (aka BQ) modules sometimes not available when executing
upload_goma_logs.py and upload_goma_logs.py sometimes fail.
Since we need BQ module only if uploading GomaStats to BQ,
let me avoid to load BQ module when it is not used
to minimize the possibility of the execution failure.

Bug:  822103 
Bug: 822042
Change-Id: I213ce4520005a52900eb600d765a217ec21748c3
Reviewed-on: https://chromium-review.googlesource.com/965862
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Takuto Ikuta <tikuta@google.com>

[modify] https://crrev.com/fd888d31f47f99cd6b933c9c2ce026f91f800ea2/scripts/slave/upload_goma_logs.py

As far as I investigated, webrtc and v8 seems not set --build-id when executing upload_goma_logs.py.  Since upload of stats won't occur if --build-id is not set, I provided a workaround for not importing libraries if --build-id is not given.
I hope it make your life a bit better.
And, let me look into vpython
https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/master/vpython/

a. add more logs for ease of seeing what happens on import failure.
b. see vpython corner case code especially failure of downloading wheels.  I guess corner case handling is not good.

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/96150ef7d01611d18a0f4353cfd13c4f32ae559c

commit 96150ef7d01611d18a0f4353cfd13c4f32ae559c
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Tue Mar 20 02:45:59 2018

Print paths before importing google.cloud.bigquery module.

To investigate crbug.com/822042, we first want to know what is
happening.  As a first step, let me check the file actually exists or
not, and why not exist.

For that purpose, let me print library paths every time, and show
directory traverse on import failure time.

Bug: 822042
Change-Id: I26a5544801d293752e30f37a92a35551b5948942
Reviewed-on: https://chromium-review.googlesource.com/970042
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/96150ef7d01611d18a0f4353cfd13c4f32ae559c/scripts/slave/upload_goma_logs.py

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/bf2efddbc1f84363b8f9fdceb732d5b84cdb63be

commit bf2efddbc1f84363b8f9fdceb732d5b84cdb63be
Author: Robert Iannucci <iannucci@chromium.org>
Date: Thu Mar 22 22:16:25 2018

[common/env.py] Remove GetSysPythonPath and GetEnvPythonPath.

GetSysPythonPath is interfering with using vpython for sub-invocations
(and I believe it was actually never necessary and was added due to a
misunderstanding of how PYTHONPATH works).

PYTHONPATH is prefixed to the internal calculation of sys.path of
whatever python interpreter starts up. The interpreter takes PYTHONPATH
from the env, then adds in any libraries which are installed with that
interpreter.

When you're using system python for both runit and also the subprocess,
exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
runit would export:

  PYTHONPATH=/build.git:/sys_python

And then when the subpython starts up, it would calculate sys.path to be:

  sys.path=/build.git:/sys_python:/sys_python

However, when you're using vpython to invoke runit and the subprocess
with two different virtualenvs, you get:

  PYTHONPATH=/build.git:/vpython/deadbeef
  sys.path=/build.git:/vpython/deadbeef:/vpython/feedface

Where 'feedface' is the environment that your script actually wants!
By removing GetSysPythonPath from the equation, we'll now do:

  PYTHONPATH=/build.git
  sys.path=/build.git:/vpython/feedface

Which should actually work correctly. I believe that GetSysPythonPath
was added under the assumption that python ONLY consults PYTHONPATH for
libraries when PYTHONPATH is set in the environment, and so we needed
to preserve the current sys.path entries for correct behavior.

GetEnvPythonPath seems to be entirely unused.

R=dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org

Bug: 776430,814607, 822103 ,822042
Change-Id: I8f6a724a6612b8837e0cd1b6fa8ad7a20e793efe
Reviewed-on: https://chromium-review.googlesource.com/974759
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: smut <smut@google.com>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/bf2efddbc1f84363b8f9fdceb732d5b84cdb63be/scripts/common/env.py
[modify] https://crrev.com/bf2efddbc1f84363b8f9fdceb732d5b84cdb63be/scripts/common/unittests/env_test.py

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/a4b3c396e413ff37b94203fa18467650d9708471

commit a4b3c396e413ff37b94203fa18467650d9708471
Author: Robbie Iannucci <iannucci@chromium.org>
Date: Thu Mar 22 22:44:56 2018

Revert "[common/env.py] Remove GetSysPythonPath and GetEnvPythonPath."

This reverts commit bf2efddbc1f84363b8f9fdceb732d5b84cdb63be.

Reason for revert: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium%2FLinux_x64%2F60972%2F%2B%2Frecipes%2Fsteps%2Fsizes%2F0%2Fstdout

Original change's description:
> [common/env.py] Remove GetSysPythonPath and GetEnvPythonPath.
> 
> GetSysPythonPath is interfering with using vpython for sub-invocations
> (and I believe it was actually never necessary and was added due to a
> misunderstanding of how PYTHONPATH works).
> 
> PYTHONPATH is prefixed to the internal calculation of sys.path of
> whatever python interpreter starts up. The interpreter takes PYTHONPATH
> from the env, then adds in any libraries which are installed with that
> interpreter.
> 
> When you're using system python for both runit and also the subprocess,
> exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
> runit would export:
> 
>   PYTHONPATH=/build.git:/sys_python
> 
> And then when the subpython starts up, it would calculate sys.path to be:
> 
>   sys.path=/build.git:/sys_python:/sys_python
> 
> However, when you're using vpython to invoke runit and the subprocess
> with two different virtualenvs, you get:
> 
>   PYTHONPATH=/build.git:/vpython/deadbeef
>   sys.path=/build.git:/vpython/deadbeef:/vpython/feedface
> 
> Where 'feedface' is the environment that your script actually wants!
> By removing GetSysPythonPath from the equation, we'll now do:
> 
>   PYTHONPATH=/build.git
>   sys.path=/build.git:/vpython/feedface
> 
> Which should actually work correctly. I believe that GetSysPythonPath
> was added under the assumption that python ONLY consults PYTHONPATH for
> libraries when PYTHONPATH is set in the environment, and so we needed
> to preserve the current sys.path entries for correct behavior.
> 
> GetEnvPythonPath seems to be entirely unused.
> 
> R=​dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org
> 
> Bug: 776430,814607, 822103 ,822042
> Change-Id: I8f6a724a6612b8837e0cd1b6fa8ad7a20e793efe
> Reviewed-on: https://chromium-review.googlesource.com/974759
> Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
> Reviewed-by: smut <smut@google.com>
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

TBR=maruel@chromium.org,iannucci@chromium.org,vadimsh@chromium.org,smut@google.com,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org

Change-Id: I75d7f647a8f2b67cc68b7c055053008b96dabd38
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 776430, 814607,  822103 , 822042
Reviewed-on: https://chromium-review.googlesource.com/976921
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/a4b3c396e413ff37b94203fa18467650d9708471/scripts/common/env.py
[modify] https://crrev.com/a4b3c396e413ff37b94203fa18467650d9708471/scripts/common/unittests/env_test.py

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/e935b61348012b96ebf48f2189d51fe5f6779604

commit e935b61348012b96ebf48f2189d51fe5f6779604
Author: Robert Iannucci <iannucci@chromium.org>
Date: Fri Mar 23 02:22:46 2018

Stop passing flakiness_dash, deprecate generate_json_file.

This is the sole reason that runtest.py imports config, a bug filed in
2014. It appears that this feature is long since abandoned anyway.

R=dpranke@chromium.org, jbudorick@chromium.org, nednguyen@chromium.org, seanmccullough@chromium.org

Bug:  403564 ,776430,814607, 822103 ,822042
Change-Id: Ie3113fe08a111dcd36de54fcf78e274fb201d5a4
Reviewed-on: https://chromium-review.googlesource.com/976876
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium_trybot.expected/invalid_results.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/dynamic_gtest_memory_mac64.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium_trybot.expected/compile_because_of_analyze_with_filtered_compile_targets_exclude_all.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium_tests/tests/steps/generate_gtest.expected/set_up and tear down.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/msan.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/one_failure_keeps_going_dynamic_tests.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium/tests/runtest.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium_trybot.expected/compile_failure_without_patch_deapply_fn.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium_trybot.expected/dont_deapply_patch.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/buildnumber_zero.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/dynamic_gtest_win.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium_tests/tests/steps/generate_gtest.expected/basic.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/dynamic_gtest.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/master/factory/commands.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium_tests/tests/api/main_waterfall_steps.expected/tester.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/dynamic_gtest_memory_asan_no_lsan.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/findit/chromium/flake.expected/flakiness_non-swarming_tests.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium_tests/tests/api/trybot_steps.expected/basic.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/findit/chromium/test.expected/none_swarming_tests.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/webrtc/steps.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium/api.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipes/chromium.expected/tsan.json
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/README.recipes.md
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/runtest.py
[modify] https://crrev.com/e935b61348012b96ebf48f2189d51fe5f6779604/scripts/slave/recipe_modules/chromium/tests/runtest.expected/annotate.json

Comment 24 by efoo@chromium.org, Mar 28 2018

Components: -Infra Infra>Goma
Components: Infra>Platform
Using vpython from runit.py caused this, and I do not think the issue only specific to goma.

As far as I understand, the issue could be fixed or mitigated by https://chromium-review.googlesource.com/c/chromium/tools/build/+/976878 land?
Owner: iannucci@chromium.org
To tell the truth, all I am doing now is waiting for https://chromium-review.googlesource.com/c/chromium/tools/build/+/976878.
Let me change owner until the cl submitted.  If the cl submitted and the issue continues, please feel free to move the issue back to me.
Project Member

Comment 28 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/c55d8676ce4193462972ae3f0d17826ea7b0978f

commit c55d8676ce4193462972ae3f0d17826ea7b0978f
Author: Robert Iannucci <iannucci@chromium.org>
Date: Tue May 01 08:06:38 2018

[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath.

GetSysPythonPath is interfering with using vpython for sub-invocations
(and I believe it was actually never necessary and was added due to a
misunderstanding of how PYTHONPATH works).

PYTHONPATH is prefixed to the internal calculation of sys.path of
whatever python interpreter starts up. The interpreter takes PYTHONPATH
from the env, then adds in any libraries which are installed with that
interpreter.

When you're using system python for both runit and also the subprocess,
exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
runit would export:

  PYTHONPATH=/build.git:/sys_python

And then when the subpython starts up, it would calculate sys.path to be:

  sys.path=/build.git:/sys_python:/sys_python

However, when you're using vpython to invoke runit and the subprocess
with two different virtualenvs, you get:

  PYTHONPATH=/build.git:/vpython/deadbeef
  sys.path=/build.git:/vpython/deadbeef:/vpython/feedface

Where 'feedface' is the environment that your script actually wants!
By removing GetSysPythonPath from the equation, we'll now do:

  PYTHONPATH=/build.git
  sys.path=/build.git:/vpython/feedface

Which should actually work correctly. I believe that GetSysPythonPath
was added under the assumption that python ONLY consults PYTHONPATH for
libraries when PYTHONPATH is set in the environment, and so we needed
to preserve the current sys.path entries for correct behavior.

GetEnvPythonPath seems to be entirely unused.

Original CL: https://chromium-review.googlesource.com/974759
Fix for original revert: https://chromium-review.googlesource.com/976876

R=dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org

Bug: 776430,814607, 822103 ,822042, 825290 
Change-Id: I5eb0d32fd94ae4282f6877c894e6d00f01e810ec
Reviewed-on: https://chromium-review.googlesource.com/976878
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/c55d8676ce4193462972ae3f0d17826ea7b0978f/scripts/common/env.py
[modify] https://crrev.com/c55d8676ce4193462972ae3f0d17826ea7b0978f/scripts/common/unittests/env_test.py

Project Member

Comment 29 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/da82356fbf21e140a6b1bc749f85b52adee3aeec

commit da82356fbf21e140a6b1bc749f85b52adee3aeec
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Tue May 01 08:57:19 2018

Revert "[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath."

This reverts commit c55d8676ce4193462972ae3f0d17826ea7b0978f.

Reason for revert: 
This cl might have broken following builder.
https://ci.chromium.org/buildbot/chromium/Mac/41039

Original change's description:
> [common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath.
> 
> GetSysPythonPath is interfering with using vpython for sub-invocations
> (and I believe it was actually never necessary and was added due to a
> misunderstanding of how PYTHONPATH works).
> 
> PYTHONPATH is prefixed to the internal calculation of sys.path of
> whatever python interpreter starts up. The interpreter takes PYTHONPATH
> from the env, then adds in any libraries which are installed with that
> interpreter.
> 
> When you're using system python for both runit and also the subprocess,
> exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
> runit would export:
> 
>   PYTHONPATH=/build.git:/sys_python
> 
> And then when the subpython starts up, it would calculate sys.path to be:
> 
>   sys.path=/build.git:/sys_python:/sys_python
> 
> However, when you're using vpython to invoke runit and the subprocess
> with two different virtualenvs, you get:
> 
>   PYTHONPATH=/build.git:/vpython/deadbeef
>   sys.path=/build.git:/vpython/deadbeef:/vpython/feedface
> 
> Where 'feedface' is the environment that your script actually wants!
> By removing GetSysPythonPath from the equation, we'll now do:
> 
>   PYTHONPATH=/build.git
>   sys.path=/build.git:/vpython/feedface
> 
> Which should actually work correctly. I believe that GetSysPythonPath
> was added under the assumption that python ONLY consults PYTHONPATH for
> libraries when PYTHONPATH is set in the environment, and so we needed
> to preserve the current sys.path entries for correct behavior.
> 
> GetEnvPythonPath seems to be entirely unused.
> 
> Original CL: https://chromium-review.googlesource.com/974759
> Fix for original revert: https://chromium-review.googlesource.com/976876
> 
> R=​dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org
> 
> Bug: 776430,814607, 822103 ,822042, 825290 
> Change-Id: I5eb0d32fd94ae4282f6877c894e6d00f01e810ec
> Reviewed-on: https://chromium-review.googlesource.com/976878
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> Reviewed-by: Nodir Turakulov <nodir@chromium.org>

TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org

Change-Id: I8e786ec50a5f4871391fa2bb658b2331816255ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 776430, 814607,  822103 , 822042,  825290 
Reviewed-on: https://chromium-review.googlesource.com/1037103
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>

[modify] https://crrev.com/da82356fbf21e140a6b1bc749f85b52adee3aeec/scripts/common/env.py
[modify] https://crrev.com/da82356fbf21e140a6b1bc749f85b52adee3aeec/scripts/common/unittests/env_test.py

Project Member

Comment 30 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/677b29788aedb625fdbfd5b75cd09cbbc406fb72

commit 677b29788aedb625fdbfd5b75cd09cbbc406fb72
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Mon May 14 02:33:23 2018

[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath."

This reverts commit da82356fbf21e140a6b1bc749f85b52adee3aeec.

Reason for reland:
The fix has been landed:
https://chromium-review.googlesource.com/c/chromium/src/+/1053674

Original change's description:
> Revert "[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath."
> 
> This reverts commit c55d8676ce4193462972ae3f0d17826ea7b0978f.
> 
> Reason for revert: 
> This cl might have broken following builder.
> https://ci.chromium.org/buildbot/chromium/Mac/41039
> 
> Original change's description:
> > [common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath.
> > 
> > GetSysPythonPath is interfering with using vpython for sub-invocations
> > (and I believe it was actually never necessary and was added due to a
> > misunderstanding of how PYTHONPATH works).
> > 
> > PYTHONPATH is prefixed to the internal calculation of sys.path of
> > whatever python interpreter starts up. The interpreter takes PYTHONPATH
> > from the env, then adds in any libraries which are installed with that
> > interpreter.
> > 
> > When you're using system python for both runit and also the subprocess,
> > exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
> > runit would export:
> > 
> >   PYTHONPATH=/build.git:/sys_python
> > 
> > And then when the subpython starts up, it would calculate sys.path to be:
> > 
> >   sys.path=/build.git:/sys_python:/sys_python
> > 
> > However, when you're using vpython to invoke runit and the subprocess
> > with two different virtualenvs, you get:
> > 
> >   PYTHONPATH=/build.git:/vpython/deadbeef
> >   sys.path=/build.git:/vpython/deadbeef:/vpython/feedface
> > 
> > Where 'feedface' is the environment that your script actually wants!
> > By removing GetSysPythonPath from the equation, we'll now do:
> > 
> >   PYTHONPATH=/build.git
> >   sys.path=/build.git:/vpython/feedface
> > 
> > Which should actually work correctly. I believe that GetSysPythonPath
> > was added under the assumption that python ONLY consults PYTHONPATH for
> > libraries when PYTHONPATH is set in the environment, and so we needed
> > to preserve the current sys.path entries for correct behavior.
> > 
> > GetEnvPythonPath seems to be entirely unused.
> > 
> > Original CL: https://chromium-review.googlesource.com/974759
> > Fix for original revert: https://chromium-review.googlesource.com/976876
> > 
> > R=​dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org
> > 
> > Bug: 776430,814607, 822103 ,822042, 825290 
> > Change-Id: I5eb0d32fd94ae4282f6877c894e6d00f01e810ec
> > Reviewed-on: https://chromium-review.googlesource.com/976878
> > Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> > Reviewed-by: Nodir Turakulov <nodir@chromium.org>
> 
> TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org
> 
> Change-Id: I8e786ec50a5f4871391fa2bb658b2331816255ca
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 776430, 814607,  822103 , 822042,  825290 
> Reviewed-on: https://chromium-review.googlesource.com/1037103
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>

TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org

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

Bug: 776430, 814607,  822103 , 822042,  825290 
Change-Id: I2035a01125452fbb63fbb93f8fe01a9415ac52e0
Reviewed-on: https://chromium-review.googlesource.com/1054909
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>

[modify] https://crrev.com/677b29788aedb625fdbfd5b75cd09cbbc406fb72/scripts/common/env.py
[modify] https://crrev.com/677b29788aedb625fdbfd5b75cd09cbbc406fb72/scripts/common/unittests/env_test.py

Project Member

Comment 31 by bugdroid1@chromium.org, May 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/c802f8211a5018cfd2a6913e1023c51c76c870dd

commit c802f8211a5018cfd2a6913e1023c51c76c870dd
Author: Michael Moss <mmoss@chromium.org>
Date: Tue May 15 22:50:08 2018

Revert "[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath.""

This reverts commit 677b29788aedb625fdbfd5b75cd09cbbc406fb72.

Reason for revert: Breaking official release builds. See discussion in  crbug.com/843012.

Original change's description:
> [common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath."
> 
> This reverts commit da82356fbf21e140a6b1bc749f85b52adee3aeec.
> 
> Reason for reland:
> The fix has been landed:
> https://chromium-review.googlesource.com/c/chromium/src/+/1053674
> 
> Original change's description:
> > Revert "[common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath."
> > 
> > This reverts commit c55d8676ce4193462972ae3f0d17826ea7b0978f.
> > 
> > Reason for revert: 
> > This cl might have broken following builder.
> > https://ci.chromium.org/buildbot/chromium/Mac/41039
> > 
> > Original change's description:
> > > [common/env.py] Reland: Remove GetSysPythonPath and GetEnvPythonPath.
> > > 
> > > GetSysPythonPath is interfering with using vpython for sub-invocations
> > > (and I believe it was actually never necessary and was added due to a
> > > misunderstanding of how PYTHONPATH works).
> > > 
> > > PYTHONPATH is prefixed to the internal calculation of sys.path of
> > > whatever python interpreter starts up. The interpreter takes PYTHONPATH
> > > from the env, then adds in any libraries which are installed with that
> > > interpreter.
> > > 
> > > When you're using system python for both runit and also the subprocess,
> > > exporting sys.path to PYTHONPATH is redundant, but not harmful. e.g.
> > > runit would export:
> > > 
> > >   PYTHONPATH=/build.git:/sys_python
> > > 
> > > And then when the subpython starts up, it would calculate sys.path to be:
> > > 
> > >   sys.path=/build.git:/sys_python:/sys_python
> > > 
> > > However, when you're using vpython to invoke runit and the subprocess
> > > with two different virtualenvs, you get:
> > > 
> > >   PYTHONPATH=/build.git:/vpython/deadbeef
> > >   sys.path=/build.git:/vpython/deadbeef:/vpython/feedface
> > > 
> > > Where 'feedface' is the environment that your script actually wants!
> > > By removing GetSysPythonPath from the equation, we'll now do:
> > > 
> > >   PYTHONPATH=/build.git
> > >   sys.path=/build.git:/vpython/feedface
> > > 
> > > Which should actually work correctly. I believe that GetSysPythonPath
> > > was added under the assumption that python ONLY consults PYTHONPATH for
> > > libraries when PYTHONPATH is set in the environment, and so we needed
> > > to preserve the current sys.path entries for correct behavior.
> > > 
> > > GetEnvPythonPath seems to be entirely unused.
> > > 
> > > Original CL: https://chromium-review.googlesource.com/974759
> > > Fix for original revert: https://chromium-review.googlesource.com/976876
> > > 
> > > R=​dnj@chromium.org, nodir@chromium.org, vadimsh@chromium.org, yyanagisawa@chromium.org
> > > 
> > > Bug: 776430,814607, 822103 ,822042, 825290 
> > > Change-Id: I5eb0d32fd94ae4282f6877c894e6d00f01e810ec
> > > Reviewed-on: https://chromium-review.googlesource.com/976878
> > > Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> > > Reviewed-by: Nodir Turakulov <nodir@chromium.org>
> > 
> > TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org
> > 
> > Change-Id: I8e786ec50a5f4871391fa2bb658b2331816255ca
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 776430, 814607,  822103 , 822042,  825290 
> > Reviewed-on: https://chromium-review.googlesource.com/1037103
> > Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> > Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
> 
> TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 776430, 814607,  822103 , 822042,  825290 
> Change-Id: I2035a01125452fbb63fbb93f8fe01a9415ac52e0
> Reviewed-on: https://chromium-review.googlesource.com/1054909
> Reviewed-by: Nodir Turakulov <nodir@chromium.org>
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>

TBR=iannucci@chromium.org,vadimsh@chromium.org,yyanagisawa@chromium.org,dnj@chromium.org,nodir@chromium.org

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

Bug: 776430, 814607,  822103 , 822042,  825290 , 843012
Change-Id: I4ccffdc3f1fe9fa878ef78a0212c8b39d91c0189
Reviewed-on: https://chromium-review.googlesource.com/1060394
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>

[modify] https://crrev.com/c802f8211a5018cfd2a6913e1023c51c76c870dd/scripts/common/env.py
[modify] https://crrev.com/c802f8211a5018cfd2a6913e1023c51c76c870dd/scripts/common/unittests/env_test.py

Project Member

Comment 32 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/ab63c10029dc010d0178057775b4817b7dbf01b3

commit ab63c10029dc010d0178057775b4817b7dbf01b3
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Tue May 22 01:07:26 2018

[common/env.py] Remove GetSysPythonPath.

This reverts commit c802f8211a5018cfd2a6913e1023c51c76c870dd.
Since PYTHONPATH need to be loaded for official builder,
let me remove GetSysPythonPath by default but allow to add
PYTHONPATH if with_third_party_lib is set.

Bug: 776430, 814607,  822103 , 822042,  825290 , 843012, 843012
Change-Id: Idc7306ce33e083c235f6e3c7f3a7d2e1d5d618b3
Reviewed-on: https://chromium-review.googlesource.com/1067224
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>

[modify] https://crrev.com/ab63c10029dc010d0178057775b4817b7dbf01b3/scripts/common/env.py
[modify] https://crrev.com/ab63c10029dc010d0178057775b4817b7dbf01b3/scripts/common/unittests/env_test.py

Status: Fixed (was: Assigned)
Since we do not see any additional flakiness after landing https://chromium-review.googlesource.com/c/chromium/tools/build/+/1067224, let me think the issue fixed.
Status: Available (was: Fixed)
It seems this failure happens again on https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Fuchsia%20x64%20Cast%20Audio/40208. 

Reopen this bug for investigation.
google.cloud seems not be included in python search path???
Owner: iannu...@google.com
Cc: iannu...@google.com
Cc: -iannucci@chromium.org

Sign in to add a comment