Windows updates

[willtoth]

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:

• 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.
• Need a better way to enable/disable param files support based on platform, as this is only required on Windows.

[nathaniel-brough]

This looks great! thanks for submitting. Just got a few comments and questions.

Need a better way to enable/disable param files support based on platform, as this is only required on Windows.

I'm not overly familiar with param files. Does it matter if this is enabled on linux or not? If not just go ahead and enable it.

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.

It looks like we'd have to vendor some of that code out or do something similar as the target itself is 'test_only' and 'private'. In terms of the MANIFEST files, my experience with them is that they are relatively stable, so I'm happy to depend on them. If the format changes in future versions of bazel then we can deal with that when/if it happens.

[nathaniel-brough]

I'm going to have to properly learn batch scripting before I can reasonably review this. I don't fully understand what is going on here.

[willtoth]

I'm added more comments to this snippet of code, and I'll move it out to a common function. I need the comments just to remember exactly how it works and I just put it together yesterday. Maybe someone more familiar with batch scripts can make sure I'm not totally off base :)

[nathaniel-brough]

No worries. I've got a bit more time over the next couple days, so I should have a bit more time for reviews.

[nathaniel-brough]

Just to consolidate my understanding here does this use symlinks and hence require developer mode. Or is this simulating symlinks (with inverted quotes).

[willtoth]

This does not use symlinks or developer mode.

The best that I can tell, when repository_ctx.symlink() is called, it simply copies the file to the new location. So this line creates a copy of the exe at the root of the openocd directory, but without the supporting files next to it

https://github.com/silvergasp/bazel-embedded/blob/34004ad4b06efa17450b64d21fb46d3a8a9f78ef/tools/openocd/openocd_repository.bzl#L48

This file is also the only entry in the MANIFEST file without making other changes, but its good enough to give a path to the actual files. So the batch script finds the entry that starts with com_openocd, then strips the fake symlink name and replaces is with /bin/openocd.exe

[nathaniel-brough]

I'll disable developer mode next time I'm booted on windows, and give this a proper look.

[nathaniel-brough]

Can we leave this as using the xpack/eclipse distribution urls. Happy to update to v0.11.0 but I'd rather get the openocd binaries all from one source for consistency. The ntfreak distrobution only packages up the windows binaries for release whereas the xpack distribution packages for most OS's. See here for urls and sha's.

[nathaniel-brough]

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.

[willtoth]

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]

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]

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

[nathaniel-brough]

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]

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?

[willtoth]

Also fixes #75 and #74

[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

[nathaniel-brough]

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.

[LandryNorris]

@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.

[willtoth]

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

[LandryNorris]

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.