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

Remote: Don't check declared outputs for failed action #15151

Closed
wants to merge 2 commits into from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Mar 31, 2022

Fix a bug that Bazel print message

Invalid action cache entry ...: expected output ... does not exist.

instead of the underlying error message when remote action failed.

@coeuvre coeuvre requested a review from a team as a code owner March 31, 2022 11:32
keith added a commit to keith/envoy that referenced this pull request Mar 31, 2022
This avoids this issue envoyproxy#20599
and I do not believe we were hit by the original issue that introduced
this regression.

There is a fix in bazelbuild/bazel#15151 that
should get us past this in bazel 5.2+. It seems this warning was just a
red herring and actually meant your actions failed.

This reverts commit fd92486.

Signed-off-by: Keith Smiley <[email protected]>
keith added a commit to envoyproxy/envoy that referenced this pull request Mar 31, 2022
This avoids this issue #20599
and I do not believe we were hit by the original issue that introduced
this regression.

There is a fix in bazelbuild/bazel#15151 that
should get us past this in bazel 5.2+. It seems this warning was just a
red herring and actually meant your actions failed.

This reverts commit fd92486.

Signed-off-by: Keith Smiley <[email protected]>
@bazel-io bazel-io closed this in 2b92a31 Apr 1, 2022
@brentleyjones
Copy link
Contributor

@bazel-io fork 5.2

@sgowroji sgowroji added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Apr 5, 2022
@Wyverald
Copy link
Member

Wyverald commented Apr 5, 2022

@bazel-io fork 5.1.1

Wyverald pushed a commit to Wyverald/bazel that referenced this pull request Apr 5, 2022
Fix a bug that Bazel print message

```
Invalid action cache entry ...: expected output ... does not exist.
```

instead of the underlying error message when remote action failed.

Closes bazelbuild#15151.

PiperOrigin-RevId: 438815494
Wyverald added a commit that referenced this pull request Apr 5, 2022
Fix a bug that Bazel print message

```
Invalid action cache entry ...: expected output ... does not exist.
```

instead of the underlying error message when remote action failed.

Closes #15151.

PiperOrigin-RevId: 438815494

Co-authored-by: Chi Wang <[email protected]>
oliviernotteghem pushed a commit to oliviernotteghem/bazel that referenced this pull request Apr 27, 2022
Fix a bug that Bazel print message

```
Invalid action cache entry ...: expected output ... does not exist.
```

instead of the underlying error message when remote action failed.

Closes bazelbuild#15151.

PiperOrigin-RevId: 438815494
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
This avoids this issue envoyproxy#20599
and I do not believe we were hit by the original issue that introduced
this regression.

There is a fix in bazelbuild/bazel#15151 that
should get us past this in bazel 5.2+. It seems this warning was just a
red herring and actually meant your actions failed.

This reverts commit fd92486.

Signed-off-by: Keith Smiley <[email protected]>
@kjlubick
Copy link
Contributor

kjlubick commented Jun 21, 2022

I'm still seeing this behavior in 5.2.0, 5.1.1, and 6.0.0-pre.20220608.2. If I go back to 5.0.0, the error messages work fine.

I'm compiling C++ code, FWIW

@kjlubick
Copy link
Contributor

I think I figured out my problem. I had an unintentional exit 0 after my compilation command, so the exit code was not correct, thus everything was being treated as a success, even when the files were not created.

After fixing that, Bazel 5.2.0 works as expected.

@coeuvre
Copy link
Member Author

coeuvre commented Jun 22, 2022

I see. This check was implemented to intentionally avoid that issue, see #14543.

@coeuvre
Copy link
Member Author

coeuvre commented Jun 22, 2022

Created #15724, with that, the error message will still be printed in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants