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

Windows,Skylark: write rule migration guide #3889

Closed
laszlocsomor opened this issue Oct 11, 2017 · 20 comments
Closed

Windows,Skylark: write rule migration guide #3889

laszlocsomor opened this issue Oct 11, 2017 · 20 comments
Labels
area-Windows Windows-specific issues and feature requests P2 We'll consider working on this in future. (Assignee optional) platform: windows team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: documentation (cleanup)

Comments

@laszlocsomor
Copy link
Contributor

Write a guide on how to write portable Skylark rules.

@laszlocsomor
Copy link
Contributor Author

Bumping to P1 -- we committed to doing this work in Q4'2017.

@laszlocsomor laszlocsomor added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Dec 14, 2017
@laszlocsomor
Copy link
Contributor Author

I plan to work on this next week.

@j3parker
Copy link
Contributor

Some things I've hit:

  • Windows command line length limits. Many(?) tools use the "response file" idiom which (usually?) maps to Bazel's param files (via ctx.Args) directly (example PR: https://github.com/bazelbuild/rules_dotnet/pull/47/files )
  • ctx.actions.run should be used when possible (vs. the deprecated ctx.action) to avoid bash
  • Bash/etc. on windows should be avoided if possible (general perf, difficulty with consistent environment, weird edge cases like Extremely slow genrule on Windows due to LDAP #4013)
  • Path length limits: a lot of tools (e.g. the C# compiler IIRC) are still very limited. I don't think this has come up yet for me in rule-writing specifically but maybe it could be a factor (we do set --output_user_root to C:\tmp to generally mitigate this.)

Something that would be interesting is any advice for making the best of unsandboxed execution. For example, we're able to vendor the C# compiler via an external package, but we rely on the system-wide .NET framework installation. My assumption (we haven't got to the point of cache sharing yet) is that these unsandboxed/unvendored resources won't contribute to the digests used in caching. This could lead (naturally) to non-repeatable builds in a way that could be dangerous with cache-sharing.

My guess at what I'll have to do is find some proxy to include as an input from outside the sandbox (e.g. there are files/registry keys that indicate the specific .NET framework release number) and "hope for the best". Any kind of insight here would be really helpful. If there's specific things we should document as rule authors on this topic that'd be good to know.

@alexeagle
Copy link
Contributor

alexeagle commented Dec 14, 2017 via email

@laszlocsomor
Copy link
Contributor Author

Thank you @j3parker and @alexeagle , these are insightful and valuable ideas and questions. I'll incorporate them / their answers in the doc.

@j3parker :

Something that would be interesting is any advice for making the best of unsandboxed execution. For example, we're able to vendor the C# compiler via an external package, but we rely on the system-wide .NET framework installation. My assumption (we haven't got to the point of cache sharing yet) is that these unsandboxed/unvendored resources won't contribute to the digests used in caching. This could lead (naturally) to non-repeatable builds in a way that could be dangerous with cache-sharing.

Frankly I have no good answer at the moment. (For the record, we don't know about the feasibility of sandboxing on Windows.)

My guess at what I'll have to do is find some proxy to include as an input from outside the sandbox (e.g. there are files/registry keys that indicate the specific .NET framework release number) and "hope for the best".

That'd be one option. Another is to create a local repository for the .NET framework and write the .NET rules such that their actions depend on that local repository.

@alexeagle :

Thanks for mentioning runfiles, I'll write a section on that. There's also #4279.

@laszlocsomor
Copy link
Contributor Author

Update: sorry for having dropped the ball on this issue. I never submitted any docs. @j3parker and @alexeagle , do you think it's still worthwhile to publish best practices for writing platform-aware Skylark rules?

@alexeagle
Copy link
Contributor

alexeagle commented May 17, 2018 via email

@laszlocsomor
Copy link
Contributor Author

Thanks for the feedback. I'll try to get something done before the end of this quarter.

@laszlocsomor laszlocsomor added P2 We'll consider working on this in future. (Assignee optional) area-Windows Windows-specific issues and feature requests and removed P1 I'll work on this now. (Assignee required) labels Oct 17, 2018
@laszlocsomor
Copy link
Contributor Author

@filipesilva suggested (#6764 (comment)) (in response to my question: "Where would you look for [this guide]?"):

I would look for it in https://bazel.build/ -> Documentation -> Rules -> Developing your own rules (a new section.

I'd expect to find a small tutorial on how to make my own language rule and how to ensure windows compatibility, and some resources for further reading.

@mboes
Copy link
Contributor

mboes commented Jan 12, 2019

Is there any preliminary version of this guide somewhere? Would be very useful for porting rules_haskell to Windows.

@laszlocsomor
Copy link
Contributor Author

laszlocsomor commented Jan 16, 2019

Sorry, there isn't yet.

Here are some tips that will go into that doc:

Actions

  • Use ctx.actions.run_shell when you need to run Bash command(s).
  • Use ctx.actions.run if you have to run a native binary or a xx_binary target. Don't use ctx.actimons.run_shell for that, and also don't run Bash commands in ctx.actions.run.
  • You can use ctx.resolve_command (and soon ctx.resolve_tools, which is better because it doesn't require Bash) if you need to run xx_binary targets in your action. Example that works with Bazel 0.21 (though still needs Bash): https://github.com/bazelbuild/bazel-skylib/blob/8d4f7612b264849b4b0d7961ab5532f6879d9239/rules/maprule_private.bzl#L397
  • don't use ctx.action -- it's deprecated and Bazel cannot correctly execute it on Windows when the command is e.g. cat <path>

Windows specific

  • Keep in mind that Windows commands/programs often don't understand paths with / separator, so when creating an action that will run on Windows, you have to replace these with \.
  • To run simple commands on Windows, you can use the Command Prompt in actions: ctx.actions.run(..., executable=["cmd.exe"], arguments=["/C", cmd], ...) -- cmd should be a cmd.exe command.
  • Mind the command line length limit when creating actions for cmd.exe -- it's only 8K characters so it's easy to hit the limit when you e.g. enumerate source files. A workaround is to set source file paths as envvars, and use the paths from the envvar. This keeps the command short. Example:
    def _windows_action(ctx, files):
    cmd = "(FOR %F IN (%SRCS%) DO ((SET X=%F)&ECHO ===== !X:\\=/! =====&TYPE %F&ECHO.&ECHO.)) > %OUT%"
    ctx.actions.run(
    inputs = depset(direct = files),
    outputs = [ctx.outputs.out],
    executable = "cmd.exe",
    arguments = ["/V:ON", "/E:ON", "/Q", "/C", cmd],
    env = {
    "OUT": ctx.outputs.out.path.replace("/", "\\"),
    "SRCS": " ".join([f.path.replace("/", "\\") for f in files]),
    },
    )
  • Remember that cmd.exe doesn't understand Bash quoting rules, so echo "hello" will retain the quotes.

Misc

  • For genrules that do 1:1 (or 1:N) transformations of source files to output files, you can use maprule from Skylib.
  • Runfiles: to use runfiles (a.k.a data dependencies), Bazel provides built-in libraries for C++, Java, Python, and Bash.

@laszlocsomor
Copy link
Contributor Author

laszlocsomor commented Jan 16, 2019

Some more:

cmd.exe

  • I often go to Steve Jansen's Batch Scripting Tutorial when I need to write batch scripts. You may like it too.
  • cmd.exe semantics are different from Bash, and often surprising. AFAIK there's no equivalent of Bash's &&. The equivalent of Bash's || operator in cmd.exe is &: echo hello & echo world.
  • One possible pattern to AND together commands (e.g. in Bash: c1 && c2 && ... && cN) that short-circuit upon failure is to OR them together and error-check after each one: c1 & if "%errorlevel%" neq "0" (exit 1) & c2 & if "%errorlevel%" neq "0" (exit 1) & ... & cN

@njlr
Copy link

njlr commented Mar 21, 2019

Question: do genrules on Windows use Bash? Does Bazel ship with a Bash layer for Windows?

@rongjiecomputer
Copy link
Contributor

Yes, genrule uses Bash even on Windows. Bazel does not ship bash in the binary, it must be installed from Git (Git bash) or Msys2.

@njlr
Copy link

njlr commented Mar 22, 2019

So if I run Bazel in PowerShell, genrules will not work?

Is it recommended to run Bazel in Git Bash / Msys2?

@rongjiecomputer
Copy link
Contributor

Bazel works in any Windows shell (cmd, powershell and bash). It is just that Bazel will internally call bash for genrule, sh_test, sh_binary etc.

@njlr
Copy link

njlr commented Mar 22, 2019

OK, that sounds like what I was hoping for! 🙂

Just to clarify, if I run Bazel in PowerShell, and then build a genrule, Bazel will automatically find my Git Bash (or similar) installation and run that to build the target?

@rongjiecomputer
Copy link
Contributor

It is better to set BAZEL_SH env var to absolute path of bash.exe that you want to use (regardless of what shell you use).

@laszlocsomor
Copy link
Contributor Author

@njlr :

Just to clarify, if I run Bazel in PowerShell, and then build a genrule, Bazel will automatically find my Git Bash (or similar) installation and run that to build the target?

Correct. As @rongjiecomputer says, better to set BAZEL_SH.

I advise against using Git Bash as BAZEL_SH. See #5751

laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 21, 2019
See bazelbuild#3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 21, 2019
See bazelbuild#3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2
laszlocsomor added a commit to laszlocsomor/bazel that referenced this issue May 21, 2019
See bazelbuild#3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2
bazel-io pushed a commit that referenced this issue May 22, 2019
See #3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2

Closes #8420.

Change-Id: I59c7952582decee585bbb23f7e8dc6d88e21aa73
PiperOrigin-RevId: 249431252
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
See bazelbuild#3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2

Closes bazelbuild#8420.

Change-Id: I59c7952582decee585bbb23f7e8dc6d88e21aa73
PiperOrigin-RevId: 249431252
@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@meteorcloudy
Copy link
Member

I'm closing this one because @laszlocsomor has done an excellent job in https://docs.bazel.build/versions/master/skylark/windows_tips.html

Please file new issue or PR if you think anything else should be added in that guide.

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P2 We'll consider working on this in future. (Assignee optional) platform: windows team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

10 participants