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

Call bash wrapper on Windows #96

Merged
merged 8 commits into from
Dec 10, 2024
Merged

Call bash wrapper on Windows #96

merged 8 commits into from
Dec 10, 2024

Conversation

avdv
Copy link
Member

@avdv avdv commented Dec 5, 2024

  • remove powershell wrapper script (it was not equivalent to the bash script and maintaining both is cumbersome anyway)
  • fix the bash wrapper which would always return exit code 0 if there were no startup options (see Bazel does not fail for empty command bazelbuild/bazel#24577)
  • revert the stardoc dependency update to 0.6.2, as version 0.7.2 is not compatible with older Bazel versions (CI was green because no tests were run -- see above)
  • upgrade the Bazel Bash runfiles library
  • use rlocationpath instead of rootpath for Bazel 8 compatibility
  • disable POSIX-to-Windows path conversion in the batch wrapper (this was a problem for Bazel 5, as the triple slash we are using somehow ended up as a single slash)

avdv added 3 commits December 6, 2024 09:06
- remove powershell wrapper script (it was not equivalent to the bash script)
There might be no startup options and no command.
stardoc version 0.7.2 is not compatible with older Bazel versions.

Note, CI passed but that was because no tests were actually run.

This reverts commit 5fba4ed.
@avdv avdv force-pushed the cb/remove-ps1-wrapper branch from 36e7068 to 414028a Compare December 6, 2024 08:27
@avdv avdv force-pushed the cb/remove-ps1-wrapper branch from 414028a to 04b21e1 Compare December 6, 2024 08:32
@avdv avdv requested a review from aherrmann December 6, 2024 08:40
"rules_sh_tests/$(rootpath @rules_sh_import_test_create_shim_test_shim//:shimmed.exe)",
"rules_sh_tests/$(rootpath @rules_sh_import_test_create_shim_test_shim//:shimmed.shim)",
"rules_sh_tests/$(rootpath @rules_sh_import_test_create_shim_test_shim//prefix:another.exe)",
"rules_sh_tests/$(rootpath @rules_sh_import_test_create_shim_test_shim//prefix:another.shim)",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why rootpath was used here in the first place, rlocationpath seems to be a perfect fit. 🤔

avdv added 3 commits December 10, 2024 08:24
Bazel 8 has been released!
Bazel 8 uses the plus sign instead of tilde in repo names now (the
`--incompatible_use_plus_in_repo_names` flag is a no-op).

That causes the `rlocation` lookup for the arguments of the
`@rules_sh_tests//import:create_shim_test` to fail and just return empty
results.

```
comm: '': No such file or directory
Expected files to be equal, but they are different: '' and ''.
```
This disables automatic POSIX-to-Windows path conversion (/ -> c:\some\path, // -> /).

See https://github.com/git-for-windows/build-extra/blob/main/ReleaseNotes.md#known-issues
@avdv avdv force-pushed the cb/remove-ps1-wrapper branch from 04b21e1 to c703396 Compare December 10, 2024 07:24
It used the latest Bazel version by default and failed, see cgrindel/gha_join_jobs#67
@avdv avdv force-pushed the cb/remove-ps1-wrapper branch from 02206e9 to 1636949 Compare December 10, 2024 09:02
@avdv avdv requested a review from malt3 December 10, 2024 09:43
@avdv avdv added the merge-queue merge on green CI label Dec 10, 2024
@mergify mergify bot merged commit 0d39762 into master Dec 10, 2024
26 checks passed
@avdv avdv deleted the cb/remove-ps1-wrapper branch December 10, 2024 10:54
@mergify mergify bot removed the merge-queue merge on green CI label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants