Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V8 jobs are always failing. #2256

Closed
mmarchini opened this issue Mar 31, 2020 · 22 comments
Closed

V8 jobs are always failing. #2256

mmarchini opened this issue Mar 31, 2020 · 22 comments
Labels

Comments

@mmarchini
Copy link
Contributor

02:36:16 v8/third_party/zlib (ERROR)
02:36:16 ----------------------------------------
02:36:16 [0:00:00] Started.
02:36:16 [0:00:00] Finished running: git config remote.origin.url
02:36:16 [0:00:00] Finished running: git rev-list -n 1 HEAD
02:36:16 [0:00:00] Finished running: git rev-parse --abbrev-ref=strict HEAD
02:36:16 ----------------------------------------
02:36:16 Error: 19> 
02:36:16 19> ____ v8/third_party/zlib at b9b9a5af7cca2e683e5f2aead8418e5bf9d5a7d5
02:36:16 19> 	You have unstaged changes.
02:36:16 19> 	Please commit, stash, or reset.
02:36:16 Checking out depot_tools.
02:36:16 Using depot tools in /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/centos7-ppcle/v8test/v8test/deps/v8/_depot_tools
02:36:16 Initializing temporary git repository in v8.
02:36:16 Fetching dependencies.
02:36:16 Uninitializing temporary git repository
02:36:16 >> Cleaning up /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/centos7-ppcle/v8test/v8test/deps/v8/.git
02:36:16 Traceback (most recent call last):
02:36:16   File "tools/node/fetch_deps.py", line 101, in <module>
02:36:16     FetchDeps(sys.argv[1])
02:36:16   File "tools/node/fetch_deps.py", line 85, in FetchDeps
02:36:16     env=env)
02:36:16   File "/usr/lib64/python2.7/subprocess.py", line 542, in check_call
02:36:16     raise CalledProcessError(retcode, cmd)
02:36:16 subprocess.CalledProcessError: Command '['/usr/bin/python', '/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/centos7-ppcle/v8test/v8test/deps/v8/_depot_tools/gclient.py', 'sync', '--spec', "solutions = [{'url': 'https://chromium.googlesource.com/v8/v8.git', 'managed': False, 'name': 'v8', 'deps_file': 'DEPS', 'custom_deps': {'v8/third_party/qemu-linux-x64': None, 'v8/third_party/catapult': None, 'v8/test/benchmarks/data': None, 'v8/test/mozilla/data': None, 'v8/third_party/colorama/src': None, 'v8/testing/gmock': None, 'v8/test/test262/data': None, 'v8/third_party/android_sdk': None, 'v8/test/test262/harness': None, 'v8/tools/swarming_client': None, 'v8/third_party/fuchsia-sdk': None, 'v8/third_party/googletest/src': None, 'v8/base/trace_event/common': None, 'v8/tools/luci-go': None, 'v8/third_party/instrumented_libraries': None, 'v8/third_party/android_ndk': None}}]"]' returned non-zero exit status 1
02:36:16 Makefile:295: recipe for target 'v8' failed

I think cleaning the workspace might be enough to fix it, so I'm trying it in one of the nodes. If it works I'll apply it in the other nodes as well.

@mmarchini
Copy link
Contributor Author

@mmarchini
Copy link
Contributor Author

Cleaned V8 test workspace on all nodes. Running a job to check now: https://ci.nodejs.org/job/node-test-commit-v8-linux/2981/

@mmarchini
Copy link
Contributor Author

Seems like when we run the CI again, it starts failing again for the same reason.

@mmarchini
Copy link
Contributor Author

Cleaned up again to run CI for a PR which should fix the test failures. Let's see if CI runs properly after that PR.

@mmarchini
Copy link
Contributor Author

Ok, it's happening again:

https://ci.nodejs.org/job/node-test-commit-v8-linux/2993/

But if I login into the machine, I don't see any unstaged changes:

img-2020-04-02-094731

@nodejs/build-infra any reasons we can't fully clean up the workspace and re-clone on every job run?

@rvagg
Copy link
Member

rvagg commented Apr 3, 2020

20:35:26  > git reset --hard # timeout=10
20:35:27  > git clean -fdx # timeout=10

-fdx is brutal in its reset, you're essentially getting a fresh clone. We could switch to a delete-and-clone but I don't know what the difference would be from an -fdx.

How is this a problem with our repo? It looks like depot_tools or the interaction with depot_tools is to blame here.

@mmarchini
Copy link
Contributor Author

Ok, got a workspace with unstaged changes on zlib:

img-2020-04-02-184504

FWIW, we still have two folders after running reset and clean, but I agree, this doesn't seem to be the underlying issue:

img-2020-04-02-184626

@mmarchini
Copy link
Contributor Author

mmarchini commented Apr 3, 2020

Was able to reproduce on my machine with:

$ make test-v8 -j18
... (cancel after test starts to run)
$ git reset --hard && git clean -xdf
...
$ git status
Not currently on any branch.
Untracked files:
  (use "git add <file>..." to include in what will be committed)

        deps/_bad_scm/

nothing added to commit but untracked files present (use "git add" to track)

$ make test-v8 -j18
tools/make-v8.sh x64.release 
+ BUILD_ARCH_TYPE=x64.release
+ V8_BUILD_OPTIONS=
+ cd deps/v8
+ tools/node/fetch_deps.py .
Using depot tools in /home/mmarchini/workspace/nodejs/node-v8-maybe/deps/v8/_depot_tools
Initializing temporary git repository in v8.
Fetching dependencies.
Syncing projects:  96% (28/29) v8/third_party/icu                                    

v8/third_party/zlib (ERROR)
----------------------------------------
[0:00:00] Started.
[0:00:00] Finished running: git config remote.origin.url
[0:00:00] Finished running: git rev-list -n 1 HEAD
[0:00:00] Finished running: git rev-parse --abbrev-ref=strict HEAD
----------------------------------------
Error: 19> 
19> ____ v8/third_party/zlib at 156be8c52f80cde343088b4a69a80579101b6e67
19>     You have unstaged changes.
19>     Please commit, stash, or reset.
Uninitializing temporary git repository
>> Cleaning up /home/mmarchini/workspace/nodejs/node-v8-maybe/deps/v8/.git
Traceback (most recent call last):
  File "tools/node/fetch_deps.py", line 101, in <module>
    FetchDeps(sys.argv[1])
  File "tools/node/fetch_deps.py", line 85, in FetchDeps
    env=env)
  File "/usr/lib/python2.7/subprocess.py", line 190, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/bin/python', '/home/mmarchini/workspace/nodejs/node-v8-maybe/deps/v8/_depot_tools/gclient.py', 'sync', '--spec', "solutions = [{'url': 'https://chromium.googlesource.com/v8/v8.git', 'managed': False, 'name': 'v8', 'deps_file': 'DEPS', 'custom_deps': {'v8/third_party/qemu-linux-x64': None, 'v8/third_party/catapult': None, 'v8/test/benchmarks/data': None, 'v8/test/mozilla/data': None, 'v8/third_party/colorama/src': None, 'v8/testing/gmock': None, 'v8/test/test262/data': None, 'v8/third_party/android_sdk': None, 'v8/test/test262/harness': None, 'v8/tools/swarming_client': None, 'v8/third_party/fuchsia-sdk': None, 'v8/third_party/googletest/src': None, 'v8/base/trace_event/common': None, 'v8/tools/luci-go': None, 'v8/third_party/instrumented_libraries': None, 'v8/third_party/android_ndk': None}}]"]' returned non-zero exit status 1
make: *** [Makefile:295: v8] Error 1

Edit: git clean -xdf is brutal, but it doesn't remove folders which are also repositories. I think something is not being cleaned properly because of that.

Edit 2: oh well

$ find . -name ".git"
./.git
./deps/v8/third_party/zlib/.git
./deps/v8/third_party/depot_tools/.git
./deps/v8/third_party/markupsafe/.git
./deps/v8/third_party/icu/.git
./deps/v8/third_party/protobuf/.git
./deps/v8/third_party/jinja2/.git
./deps/v8/third_party/perfetto/.git
./deps/v8/tools/clang/.git
./deps/v8/buildtools/.git
./deps/v8/buildtools/third_party/libc++/trunk/.git
./deps/v8/buildtools/third_party/libunwind/trunk/.git
./deps/v8/buildtools/third_party/libc++abi/trunk/.git
./deps/v8/buildtools/clang_format/script/.git
./deps/v8/_depot_tools/.git
./deps/v8/build/.git
./deps/_bad_scm/v8/third_party/zlibG1N69H/zlib/.git
./deps/_bad_scm/v8/third_party/jinja2vTZB0p/jinja2/.git
./deps/_bad_scm/v8/third_party/markupsafe0vG6W5/markupsafe/.git

Edit 3: the problem is reproducible with only git reset --hard between both make test-v8

@mmarchini
Copy link
Contributor Author

mmarchini commented Apr 3, 2020

gclient sync turns V8 third_party libraries into git repositories, and updates those when appropriate. When there's an update, it'll show as changes on the Node.js repository.

$ git status
Not currently on any branch.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   deps/v8/third_party/zlib/contrib/minizip/ChangeLogUnzip

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        deps/_bad_scm/

no changes added to commit (use "git add" and/or "git commit -a")

When we run git reset --hard, we get rid of the changes on zlib as perceived by the Node.js repository, but not as perceived by the zlib repository:

$ git reset --hard
HEAD is now at 87f5300474 tools: update v8 gypfiles

$ git status
Not currently on any branch.
Untracked files:
  (use "git add <file>..." to include in what will be committed)

        deps/_bad_scm/

nothing added to commit but untracked files present (use "git add" to track)

$ cd deps/v8/third_party/zlib
$ git status
HEAD detached at 156be8c
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   contrib/minizip/ChangeLogUnzip

no changes added to commit (use "git add" and/or "git commit -a")

Which causes the next run of gclient sync to fail. We need to either turn those third_party repositories back to regular folders, or we need to clear the space up every time. Maybe this is caused by a change upstream on depot_tools, but I'm not sure it's worth investigating to find it out.

@mmarchini
Copy link
Contributor Author

Maybe we should run find deps -name .git | xargs rm -rf before git reset and git clean?

cc @targos have we seen this error in the past? If so, how did we fix it?

@targos
Copy link
Member

targos commented Apr 10, 2020

I don't remember seeing this error before

@targos
Copy link
Member

targos commented Apr 10, 2020

It's possible that gclient changed recently how it deals with dependencies

@mmarchini
Copy link
Contributor Author

Maybe, not sure if it's worth investing much time trying to figure it out though. Since we already do git reset and git clean, I think it makes sense for us to also revert any folders converted to git repositories in deps/, otherwise both git operations will only partially work. WDYT?

@targos
Copy link
Member

targos commented Apr 10, 2020

SGTM

@mmarchini
Copy link
Contributor Author

I don't have access to edit the jobs configuration, someone else needs to do it.

Screenshot from 2020-04-10 12-59-12

find deps -name .git | xargs rm -rf or equivalent should be added before git reset

@richardlau
Copy link
Member

We could edit the job but wouldn't that still leave things broken for anyone running these tests manually (as per #2256 (comment))?

@mmarchini
Copy link
Contributor Author

I had problems in the past (like two years ago) running test-v8 multiple times, so I always assumed it needed to run on a disposable folder. I also don't think many folks run it on their own machines.

@mmarchini
Copy link
Contributor Author

mmarchini commented Apr 13, 2020

I'd be fine with someone making these changes temporarily on the job and then we can investigate later, if folks think this should really always work on dev machines. But we should unbreak V8 CI to restore our daily testing as well as testing on pull requests.

@mhdawson
Copy link
Member

@richardlau can you help edit the job? If not let me know and I'll take a look.

@richardlau
Copy link
Member

@richardlau can you help edit the job? If not let me know and I'll take a look.

I'll take a look in the morning.

@richardlau
Copy link
Member

I've PR'ed a band-aid fix: nodejs/node#32877

The underlying issue is that we have modified deps/v8/third_party/zlib from the upstream
Chromium commit (we deleted some files and it looks like there are some whitespace
differences in some of the files we have kept) so the glient sync is always going to see
unstaged changes.

mmarchini pushed a commit to nodejs/node that referenced this issue Apr 16, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
mmarchini pushed a commit to mmarchini/node that referenced this issue Apr 16, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: nodejs#32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit to nodejs/node that referenced this issue Apr 17, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: nodejs#32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
BridgeAR pushed a commit to nodejs/node that referenced this issue Apr 28, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit to nodejs/node that referenced this issue Apr 28, 2020
When running `make test-v8` V8's `gclient sync` converts folders
under `deps/v8/third_party` into git repositories. Unfortunately
the files that were checked in under `deps/v8/third_party/zlib`
have been modified from the upstream Chromium repository (some
files have been deleted and there are whitespace differences in
some of the files that were kept) so whenever the Node.js source
tree is hard reset/checked out `gclient sync` notices there are
unstaged changes as the files in the Node.js source tree do not
match those of the upstream Chromium third party zlib commit.

Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #32877
Refs: nodejs/build#2256
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@mmarchini
Copy link
Contributor Author

Seems like the jobs are working again (https://ci.nodejs.org/job/node-test-commit-v8-linux/3121), so I'll go ahead and close this. Feel free to reopen if the problem persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants