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

Windows support for Cabal python wrapper #1133

Merged
merged 8 commits into from Oct 23, 2019
Merged

Windows support for Cabal python wrapper #1133

merged 8 commits into from Oct 23, 2019

Conversation

aherrmann
Copy link
Member

@aherrmann aherrmann commented Oct 23, 2019

  • Support Windows paths in Cabal python wrapper (e.g. C:\foo\bar)
  • Look for executables using bazel-runfiles in Cabal python wrapper. Necessary on Windows when no symlinks are created in the sandbox and executables need to be looked up in the runtime manifest instead.
  • Use Bazel 1.1.0 on Windows - required for cc_wrapper on Windows, see sh_binary wrapper fails on upper case letters in file extension on Windows bazelbuild/bazel#9390.
  • Test network and language-c on Windows - these use ./configure scripts and are more problematic on Windows.

@aherrmann aherrmann marked this pull request as ready for review October 23, 2019 14:24

def _prepare_cabal_inputs(hs, cc, unix, dep_info, cc_info, component, package_id, tool_inputs, tool_input_manifests, cabal, setup, srcs, flags, cabal_wrapper, package_database):
"""Compute Cabal wrapper, arguments, inputs."""
with_profiling = is_profiling_enabled(hs)

(ghci_extra_libs, env) = get_ghci_extra_libs(hs, cc_info)
env.update(**hs.env)
env["PATH"] = _make_path(hs, tool_inputs) + ":" + ":".join(unix.paths)
path_list_sep = ";" if hs.toolchain.is_windows else ":"
Copy link
Member

Choose a reason for hiding this comment

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

nit: You can move this to the toplevel to avoid having to repeat it

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, turns out this is a good opportunity to also refactor the path separator in LD_LIBRARY_PATH, so I'll do that in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in #1134

@aherrmann aherrmann added the merge-queue merge on green CI label Oct 23, 2019
The execroot path can contain special characters, e.g. `\` on Windows.
Same as other paths, e.g. `LD_LIBRARY_PATH`. The reason is that Windows
paths can contain ':', e.g. `C:\foo\bar`.
Use bazel_runfiles on Windows to find executables. By default Bazel does
not generated symlinks to binaries in the execroot, so relative paths to
executables will not work. Instead, they must be looked up in the
runfiles manifest using bazel_runfiles.
To exit early if any of the `configure`, `build`, `install` steps fails.
Bazel 1.1 requires these now.
This version fixes the issue of `sh_binary`s called with upper-case
`.EXE` extension not finding the actual script file. This issue was
triggered by Cabal packages with `./configure` scripts, which called
`cc` in 8.3 file name format.
Requires Bazel 1.1.0 on Windows
@mergify mergify bot merged commit ae8d9c2 into master Oct 23, 2019
@mergify mergify bot removed the merge-queue merge on green CI label Oct 23, 2019
@aherrmann aherrmann deleted the cabal-windows branch October 29, 2019 14:38
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.

None yet

3 participants