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

Flatbuffers is failing with Bazel@HEAD #7937

Closed
sgowroji opened this issue May 5, 2023 · 11 comments · Fixed by #7952
Closed

Flatbuffers is failing with Bazel@HEAD #7937

sgowroji opened this issue May 5, 2023 · 11 comments · Fixed by #7952

Comments

@sgowroji
Copy link
Contributor

sgowroji commented May 5, 2023

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2997#0187e485-ec05-438c-bccf-6055e8225ebf

Platform :Linux (18.04, 20.04)

Logs:

FAILED: //tests/ts:bazel_repository_test

Steps :

  1. git clone -v https://github.com/google/flatbuffers.git
  2. git reset 67084b9 --hard
  3. export USE_BAZEL_VERSION=8087520b153832656695fef6f3654ed432642c98
  4. bazel. build //...

CC @meteorcloudy

@dbaileychess
Copy link
Collaborator

Its green on our CI: https://buildkite.com/bazel/flatbuffers/builds?branch=master

I'll take a look if the CI is missing something.

@dbaileychess
Copy link
Collaborator

Oh 67084b9 is not head, why are we looking at an old commit?

@meteorcloudy
Copy link
Contributor

meteorcloudy commented May 8, 2023

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3007#0187f987-8d55-49ae-bfbb-8b59c0093ef8
It's also failing at 197ae6c with Bazel@HEAD

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //tests/ts:bazel_repository_test
-----------------------------------------------------------------------------
2023/05/08 07:13:52 could not link local Bazel: could not copy file from /tmp/tmpsegu_b6r/bazel to /var/lib/buildkite-agent/.cache/bazelisk/local/-tmp-tmpsegu-b6r-bazel/bin/bazel: open /tmp/tmpsegu_b6r/bazel: no such file or directory

Probably related to c1e7aee

@meteorcloudy
Copy link
Contributor

/tmp/tmpsegu_b6r/bazel is the Bazel binary built from HEAD, looks like bazel_repository_test is doing something weird.

@dbaileychess
Copy link
Collaborator

@bjornharrtell @philsc Can you take a look, its in the ts test code

@bjornharrtell
Copy link
Collaborator

Not sure how to find any meaningful output to look at. But I guess #7933 is a suspect here... but why is this build not at CI?

@philsc
Copy link
Contributor

philsc commented May 10, 2023

I can take a look tonight. Sorry about the breakage.

@philsc
Copy link
Contributor

philsc commented May 11, 2023

@meteorcloudy , @sgowroji, can either of you clarify something for me?
I see this in the test invocation:

$ bazel test <snip> --sandbox_tmpfs_path=/tmp <snip> --test_env=USE_BAZEL_VERSION <snip>

The environment variable setup has this:

USE_BAZEL_VERSION=(/tmp/tmpldhsynjt/bazel)

So it looks like the HEAD CI is both pointing tests at bazel in /tmp, but at the same time mounting a fresh /tmp in the test. So the USE_BAZEL_VERSION variable that is explicitly passed in is not useful at all.
My question: What's the intent of this combination of things in your bazel HEAD CI?

A few solutions I see:

  1. Don't pass in --test_env=USE_BAZEL_VERSION when --sandbox_tmpfs_path=/tmp is specified.
  2. Put bazel HEAD somewhere other than /tmp since the CI run forces a fresh /tmp via --sandbox_tmpfs_path=/tmp.
  3. Change the test in flatbuffers to clear USE_BAZEL_VERSION because it can't be trusted in the bazel HEAD CI.

Is it possible that I'm misunderstanding something?

@philsc
Copy link
Contributor

philsc commented May 11, 2023

Actually, USE_BAZEL_VERSION appears to only be relevant when bazelisk is involved. So it's more likely that the test I wrote is reaching out to bazelisk somehow 🤔

EDIT: Looks like the test is indeed finding bazelisk somehow. I can reproduce with something like this:

diff --git a/tests/ts/bazel_repository_test.sh b/tests/ts/bazel_repository_test.sh
index 50308093..17a04590 100755
--- a/tests/ts/bazel_repository_test.sh
+++ b/tests/ts/bazel_repository_test.sh
@@ -26,4 +26,4 @@ export PATH="$(dirname "${BAZEL_BIN}"):${PATH}"

 cd tests/ts/bazel_repository_test_dir/

-bazel test //...
+bazelisk test //...

and
$ bazel --nohome_rc test --sandbox_tmpfs_path=/tmp //tests/ts:bazel_repository_test --nocache_test_results --test_env=USE_BAZEL_VERSION=/tmp/foo/bar --test_output=streamed --test_env=HOME=$HOME --sandbox_writable_path=$HOME/.cache/bazelisk

@philsc
Copy link
Contributor

philsc commented May 11, 2023

I think #7952 should fix the issue. Really sorry about breaking this.

Still curious about the motivation behind the combination of options I mentioned here: #7937 (comment)

@meteorcloudy
Copy link
Contributor

@philsc Thanks for debugging this!

Still curious about the motivation behind the combination of options I mentioned here: #7937 (comment)

I think this is indeed a problem on Bazel CI. bazelbuild/continuous-integration#846 was introduced to address a problem on Windows which doesn't have sandbox support.

Put bazel HEAD somewhere other than /tmp since the CI run forces a fresh /tmp via --sandbox_tmpfs_path=/tmp.

Sounds like this could be a solution!

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

Successfully merging a pull request may close this issue.

5 participants