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

_clone_or_update using bash instead of BAZEL_SH #3699

Closed
filipesilva opened this issue Sep 7, 2017 · 10 comments
Closed

_clone_or_update using bash instead of BAZEL_SH #3699

filipesilva opened this issue Sep 7, 2017 · 10 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required)
Milestone

Comments

@filipesilva
Copy link

Please provide the following information. The more we know about your system and use case, the more easily and likely we can help.

Description of the problem / feature request / question:

On Windows, the _clone_or_update function in git.bzl is using bash instead of BAZEL_SH:

st = ctx.execute(['bash', '-c', """

This is problematic because bash can be other shells, such as Linux Subsystem for Windows, instead of the required MSYS2 shell.

If possible, provide a minimal example to reproduce the problem:

Environment info

  • Operating System: Windows 10

  • Bazel version (output of bazel info release):

release 0.5.4
  • If bazel info release returns "development version" or "(@Non-Git)", please tell us what source tree you compiled Bazel from; git commit hash is appreciated (git rev-parse HEAD):

Have you found anything relevant by searching the web?

Anything else, information or logs or outputs that would be helpful?

/cc @dslomov @alexeagle

@dslomov dslomov self-assigned this Sep 7, 2017
@dslomov dslomov added this to the 0.6 milestone Sep 7, 2017
@dslomov dslomov added the P1 I'll work on this now. (Assignee required) label Sep 7, 2017
@dslomov
Copy link
Contributor

dslomov commented Sep 7, 2017

cc @laszlocsomor

@alexeagle
Copy link
Contributor

FYI I believe the workaround is just to use the native git_repository rule rather than loading the one from @bazel_tools.
In my case I didn't need the features from the native git client; the jgit used by the native rule works.
You can drop the priority/severity here.

@bimmlerd
Copy link

bimmlerd commented Sep 21, 2017

@alexeagle unfortunately the native rule is broken in interesting ways, thus not really a viable option:

Library issues - This implementation uses jGit, which we've discovered several issues with. It also lacks support for some authentication types you might use with your system git.

from https://docs.bazel.build/versions/master/be/workspace.html#git_repository

@alexeagle
Copy link
Contributor

Fair enough - internally we have an "sso" scheme which requires jgit, so I guess we just have to flip this back and forth depending on the environment.

@davido
Copy link
Contributor

davido commented Sep 21, 2017

I still think, that general purpose rules, and this what I would expect rules_nodejs to be, should just work on both git and jgit and all three major platforms. Gerrit Code Review is currently using custom JS tool chain but in the long run, its Polymer stack should be migrated to use standard JS/NodeJS rules.

@alexeagle
Copy link
Contributor

alexeagle commented Sep 21, 2017 via email

@bimmlerd
Copy link

Any update on this? 0.7 is out and still seems to have this bug.

@laszlocsomor
Copy link
Contributor

ping @dslomov

@laszlocsomor
Copy link
Contributor

I think this can be fixed simply by using actions.run_shell. Bazel supports that on Windows already.

@laszlocsomor
Copy link
Contributor

I'm removing the Windows label because this is a bug in the git rule implementation, not in Bazel's Windows-specific logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

6 participants