bazel-embedded icon indicating copy to clipboard operation
bazel-embedded copied to clipboard

Windows updates

Open willtoth opened this issue 5 years ago • 12 comments

Adding a few updates for better support on Windows, currently WIP.

Changes:

  • Enables param files, this is critical for Windows due to the command length limitation which is hit very easily when using STM32CubeMX drivers even for a basic project.
  • Improve runfile support for openocd tools. Windows does not do runfiles well in Bazel currently due to issues with symlinks. Details here.

TODO:

  • [x] Decide if batch script with MANIFEST file is actually robust enough. I am definitely no expert with batch scripts, nor do I particularly enjoy working with them. Other options may be to use a library like this one and wrap openocd with that.
  • [x] Need a better way to enable/disable param files support based on platform, as this is only required on Windows.

willtoth avatar Mar 22 '21 15:03 willtoth

Apologies for the delay here. I mostly work on a linux based system so the effort to boot into a windows machine is the main blocker for me here. The goto's in that batch script make me a little uncomfortable. I'm not sure if I would be able to maintain the batch script if I merge this in. I've been looking at how the rules_docker stuff works, and they seem to have found a relatively elegant way of supporting multiple different platforms without the host path seperator hacks that I've previously used.

Firstly they use this to get the full host path on both windows and linux. This is paired up with an environment variable set in their templated script.

Then they use a bunch of selects from a macro to configure based on the platform. Which I had no idea was possible, but I'm glad I do now.

What are your thoughts on a setup like this?

I should have some time over the next couple weeks to make some of those changes towards a 'rules_docker' approach. You are welcome to have a crack at it in the mean time.

Of your changes I'd like to use;

  • Adds windows implementation of print_all_features
  • Fixes command line length too long for Windows

But I'm not quite sold on the others.

nathaniel-brough avatar Apr 08 '21 08:04 nathaniel-brough

I agree that is much nicer than the batch script and host path separator. I will try to put some time into this, but my own Windows usage my soon be very limited as well.

willtoth avatar Apr 08 '21 15:04 willtoth

Trying to take a shot at this.

It looks like the rules_docker skylib is a bit different thatn the skylib repo, specifically:

https://github.com/bazelbuild/rules_docker/blob/8c3a8110a0c519929a7e79c39ac345a0f8c74d04/skylib/path.bzl#L88

I'm planning to just copy over some of the code here, unless you think we should just depend on rules_docker.

willtoth avatar Apr 21 '21 16:04 willtoth

@silvergasp How is RUNFILES_DIR defined https://github.com/silvergasp/bazel-embedded/blob/master/tools/openocd/defs.bzl#L168

willtoth avatar Apr 21 '21 18:04 willtoth

Trying to take a shot at this.

It looks like the rules_docker skylib is a bit different thatn the skylib repo, specifically:

https://github.com/bazelbuild/rules_docker/blob/8c3a8110a0c519929a7e79c39ac345a0f8c74d04/skylib/path.bzl#L88

I'm planning to just copy over some of the code here, unless you think we should just depend on rules_docker.

I'm happy with something similar over here. If you are directly copying the files over please add a comment in the top of the file that matches their licence. It doesn't have to be the full license terms but at least a reference of where the file came from.

nathaniel-brough avatar Apr 22 '21 06:04 nathaniel-brough

I've got a bit more time to play with this. Just wondering where you got to with all this? It's looking like you've removed the Batch script in the latest commit. Is there anything that I can do to make this easier to push through, especially towards to approach that rules_docker uses?

nathaniel-brough avatar Apr 26 '21 02:04 nathaniel-brough

Also fixes #75 and #74

willtoth avatar Apr 25 '24 04:04 willtoth

Looks like this repo may not have as much traction as it had when I first opened the PR, but 3 years later, its ready if you want to take a look @silvergasp

May be slightly more important now, since this is technically broken in newer bazel versions

willtoth avatar Apr 25 '24 15:04 willtoth

I'm super busy at the moment moving back to Australia. So I'll hopefully take another pass at it in the next 2-3 weeks.

I've switched over to rust for most of my new embedded projects, so I'm rarely using Bazel anymore and can't justify the time to actively work on this project. But I'm happy to review PRs when I get a spare minute.

nathaniel-brough avatar Apr 25 '24 18:04 nathaniel-brough

@willtoth The description of this PR mentions that it fixes the same issue as #68, but it looks like the change was pulled out of this branch. Is it expected that this PR will fail because the command line invocation for ar is too long with STM32CubeMX drivers? If this is expected, I could make a separate branch that merges the two together to unblock me on building for now.

LandryNorris avatar Apr 26 '24 14:04 LandryNorris

@LandryNorris this is included in the PR already, though I noticed only for gcc, any chance you're actually building with clang?

willtoth avatar Apr 26 '24 16:04 willtoth

It's gcc. Here's the ar path: external\bazel_embedded\toolchains\gcc_arm_none_eabi\gcc_wrappers\windows\ar.bat. The project compiles successfully on Bazel 4, but fails with the ar command being too long (~8500 characters) when using Bazel 7.

LandryNorris avatar Apr 26 '24 16:04 LandryNorris