Windows portability fixes #1386
Conversation
Remove dependency on host shell since bash is not available on Windows
In case the content is e.g. behind a mirror with authentication, this is necessary. Note: get_auth was introduced in 7.1.0 and would break compatibility with earlier versions
FaBrand
requested review from
Bencodes,
ahumesky,
cgruber,
nkoroste and
restingbull
as code owners
| outputs = { | ||
| "ksp_generated_java_srcjar": ksp_generated_java_srcjar, | ||
| "ksp_generated_classes_jar": ksp_generated_classes_jar, | ||
| "ksp_generated_java_srcjar": ksp_generated_java_srcjar, |
There was a problem hiding this comment.
What's happening here? Looks like a sort or something?
There was a problem hiding this comment.
I had buildifier set-up to automatically fix the unsorted-dict-items. I decided to keep it as a simple fix, but bundled in the first of the three commits.
There was a problem hiding this comment.
Filtering the "auto-format" commit makes it a bit easier to review:
https://github.com/bazelbuild/rules_kotlin/pull/1386/files/24ca76eb315256ad964d007cd365e997ae226fa8..365f45137b7fb28c353fc2beba946d82364f4f07
There was a problem hiding this comment.
Could you include an update to the buildifier rules, since we'll need to keep this in order?
There was a problem hiding this comment.
@restingbull To confirm since i'm not 100% sure which of the two you mean?
a) Include the unsorted-dict-items in the buildifier.{check,fix} targets so everything gets sorted
b) Revert the auto-sort commit and add the warning to the excluded ones
I pushed 2 more commits.
One that adds the option to the buildifier rule and one that fixes a deprecation error i found along the way
Added fixes with: ``` bazel run //:buildifier.fix ```
When running `//:buildifier.check` this deprecation warning is issued ``` buildifier: selecting diff program with the BUILDIFIER_DIFF, BUILDIFIER_MULTIDIFF, and DISPLAY environment variables is deprecated, use flags -diff_command and -multi_diff instead ```
|
@restingbull And some more commits with some inconsistencies regarding the buildifier checks :) |
Since the order of the opts changed, the order in which the flags are produced changes as well
bazelci uses 8.2.0
FaBrand
force-pushed
the
windows-portability-fixes
branch
from
4778c10 to
5e55cb5
Compare
2 participants
I would like to add some compatibility changes i had before as patches that should be (mostly) compatible
One that change breaks compatibility with bazel version < 7.1.0 (the
get_authmethod frombazel_toolsgot introduced with 7.1.0)Confined the buildifier formatting changes in one commit for reviewability