Skip to content

Windows: Use actions.run and explicitly format cmd.exe .bat invocation#2542

Closed
sunjayBhatia wants to merge 1 commit intobazel-contrib:masterfrom
greenhouse-org:windows-fix-remote-execute
Closed

Windows: Use actions.run and explicitly format cmd.exe .bat invocation#2542
sunjayBhatia wants to merge 1 commit intobazel-contrib:masterfrom
greenhouse-org:windows-fix-remote-execute

Conversation

@sunjayBhatia
Copy link

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

ERROR: C:/_eb/external/go_sdk/BUILD.bazel:43:1: GoToolchainBinaryCompile external/go_sdk/builder.exe.a failed (Exit 1): builder.exe.bat failed: error executing command 
  cd C:/_eb/execroot/envoy
  SET GOROOT=external/go_sdk
  bazel-out/host/bin/external/go_sdk/builder.exe.bat
Execution platform: @rbe_windows_msvc_cl//config:platform
'bazel-out' is not recognized as an internal or external command, operable program or batch file.

Which issues(s) does this PR fix?

N/A

Other Notes

If there is an alternate method of fixing this issue within Bazel, we would be open to it; there seems to be some discrepancy between how ctx.actions.run work on Windows when running locally (this change is not required for purely local builds) vs. via RBE (where this patch or reverting the relevant bits of #2488 is required).

This was noted as a new failure with commit 280a1ed on particular GCP
RBE environments given some combination of PATHEXT and other system
factors which didn't correctly kick off the given batch file and treated
that invocation as using forward slash notation, confusing the cmd.exe
command invocation.

Co-authored-by: William A Rowe Jr <wrowe@vmware.com>
Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia requested a review from jayconrod as a code owner June 17, 2020 21:01
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@sunjayBhatia
Copy link
Author

@googlebot I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@sunjayBhatia
Copy link
Author

Still working out corporate CLA, we were covered by Pivotal but need to get moved over to VMware

@jayconrod
Copy link
Collaborator

Sorry, I can't review this until the CLA is done.

In the mean time, could you open an issue describing the problem? From the PR description, it's not really clear what causes this problem.

@wrowe
Copy link

wrowe commented Jun 18, 2020

@googlebot I fixed it

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@sunjayBhatia
Copy link
Author

This actually looks like a general Bazel issue rather than something specific to rules_go, this is just the first place we've run into it, you can see https://github.com/greenhouse-org/bazel-issue-repro/tree/master/ctx_actions_run_rbe if youre interested @jayconrod

@wrowe
Copy link

wrowe commented Jun 20, 2020

@googlebot I fixed it

@jayconrod
Copy link
Collaborator

It looks like the CLA bot still is not happy. It looks like VMware is on the list of corporate signers. Some things to try:

  • Make sure all commit and author email addresses are on the approved CLA for your org. You may need to amend the comment and force-push the PR.
  • Make sure your GitHub accounts are associated with the same emails.

If that doesn't work, I can escalate to the folks that run the bot.

I'm still not really clear on what's going wrong here though. If this is a general Bazel issue (and from your repro it looks like this not Go-specific), it would be better to open an issue in bazelbuild/bazel and close this PR. If this is an issue with rules_go, please open an issue here with more information.

@sunjayBhatia
Copy link
Author

We've started some dialogue with RBE/Bazel folks and will continue digging into the exact cause of this issue (whether it is RBE or Bazel doing odd things with path conversions); we have a workaround for rules_go in Envoy so we can close this

@sunjayBhatia
Copy link
Author

bazelbuild/bazel#11636 is our Bazel issue if youre interested

@sunjayBhatia sunjayBhatia deleted the windows-fix-remote-execute branch November 19, 2020 15:17
yushan26 pushed a commit to yushan26/rules_go that referenced this pull request Jun 16, 2025
In a couple recent PRs, I put "VERSION_NEXT_{feature,patch}" as place
holders since I
wasn't sure what the next appropriate version would be for unreleased
code. e.g.
specifying `1.0.1` becomes invalid if a subsequent PR would make it
`1.1.0`.

To help remind us to populate these values before a release, have the
release workflow
check for the marker strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants