Conversation
PatriceVignola
requested review from
meisterT,
pmbethe09 and
vladmos
as code owners
| for /f "tokens=2" %%i in ('findstr /r "\<buildifier\.exe\>" MANIFEST') do (set buildifier_abs_path=%%i) | ||
|
|
||
| powershell ^ | ||
| function Buildify($Root)^ |
There was a problem hiding this comment.
Buildifier has the -r flag to recursively find all relevant files within a directory, maybe it makes sense to reuse it? The built-in recursive function can ignore some paths (currently just .git, but may support .buildifierignore in the future, see #801), I think it's better to have such logic defined once.
There was a problem hiding this comment.
But the Linux version does it recursively by default, without needing the -r flag. I think the Windows and Linux versions should have the same behavior.
There was a problem hiding this comment.
Maybe it had been implemented this way before the -r flag was introduced. I can fix it later, but won't be able to fix the windows version easily because don't have a windows machine around. So I suggest using -r from the beginng.
There was a problem hiding this comment.
How do I use the -r flag with bazel run //:buildifier? I'm able to use it with the standalone executable but not as a bazel rule.
There was a problem hiding this comment.
I noted on #1136 that the -r flag also does not match the same set of files. The set used by the runner template is more complete. Ideally, those should probably be aligned, but I'd be in favor of merging this in for running on Windows first.
|
This would be very useful for me. Is there any progress on this? |
|
What is the status of this PR? Any news? |
|
I agree for merging it now and fixing the behavior of recursive flags later, sorry for the delayed review. |
|
Cool thanks. Any estimations for when is planned the next release @vladmos? |
|
I'm not sure if I'm the only one but for me it does not seem to work the autodetection of being on windows |
|
@PatriceVignola @vladmos I'm not sure how this was validated but to me it does not work unless I remove the declaration of the bash file in Windows |
|
With #1262 it works for me. |
* Add Windows Support * Add Windows Support * Revert line ending change
5 participants
This PR adds Windows support to call
bazel run //:buildifierwithout needing msys2 workarounds by adding a native.batfile. This should address issues that have been opened a while ago: #770 and #346