Skip to content

fix(bzlmod): Fixing Windows Python Interpreter symlink issues#1265

Merged
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
chrislovecnm:windows-symlink-pip
Jun 13, 2023
Merged

fix(bzlmod): Fixing Windows Python Interpreter symlink issues#1265
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
chrislovecnm:windows-symlink-pip

Conversation

@chrislovecnm
Copy link
Copy Markdown
Contributor

@chrislovecnm chrislovecnm commented Jun 13, 2023

When using Windows you cannot run the symlink directly.

Because of how symlinks work in Windows we need to path realpath resolve the link.
Unlike Linux and OSX we cannot execute the Python symlink directly. Windows throws an error "-1073741515", when we execute the symlink directory. This error means that the Python binary cannot find dlls. This is because the symlink that we create is not in the same folder as the dlls.

@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch from 76e1fe5 to 863dc7e Compare June 13, 2023 16:26
@chrislovecnm chrislovecnm changed the title feat(bzlmod): Fixing Windows Python Interpreter symlink issues fix(bzlmod): Fixing Windows Python Interpreter symlink issues Jun 13, 2023
@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch 2 times, most recently from 81fa3a1 to 0f08282 Compare June 13, 2023 16:44
When using Windows you cannot run the symlink directly.

Because of how symlinks work in Windows we need to use the path
realink.
Unlike Linux and OSX we cannot execute the Python symlink directly.
Windows throws an error "-1073741515", when we execute the symlink
directory.  This error means that the Python binary cannot find dlls.
This is because the symlink that we create is not in the same folder
as the dlls.

This commit introduces code that resolves the actual location of the
interpreter using the path realink when we are running bzlmod and
Windows.
@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch from 0f08282 to 98b51ab Compare June 13, 2023 16:51
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

Just doc and ux improvements, otherwise lgtm


if result.return_code:
fail("whl_library %s failed: %s (%s)" % (rctx.attr.name, result.stdout, result.stderr))
fail("whl_library %s failed: %s (%s) error code: '%s'" % (rctx.attr.name, result.stdout, result.stderr, result.return_code))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since we're here...

Suggested change
fail("whl_library %s failed: %s (%s) error code: '%s'" % (rctx.attr.name, result.stdout, result.stderr, result.return_code))
fail(("whl_library '{name}' failed: wheel_installer failed\n" +
"command: {args}\n" +
"error code: {error_code}\n" +
"===== stdout ====={stdout}\n" +
"===== stderr ====={stderr}\n=====").format(
name = rctx.attr.name,
command = args,
code=result.return_code,
stdout=result.stdout,
stderr=result.stderr
))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did you test this code?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no. i'm fine if you want to skip it for now.

Making English better.

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
@rickeylev rickeylev enabled auto-merge June 13, 2023 18:55
@rickeylev rickeylev added this pull request to the merge queue Jun 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2023
@chrislovecnm chrislovecnm added this pull request to the merge queue Jun 13, 2023
@chrislovecnm chrislovecnm added the type: bzlmod bzlmod work label Jun 13, 2023
Merged via the queue into bazel-contrib:main with commit 3903d1a Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bzlmod bzlmod work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants