Skip to content

Use "shared" msvc hermetic toolchain - #694

Merged
f0rmiga merged 1 commit into
bazel-contrib:mainfrom
jheaff1:use_shared_msvc_toolchain
May 23, 2022
Merged

Use "shared" msvc hermetic toolchain#694
f0rmiga merged 1 commit into
bazel-contrib:mainfrom
jheaff1:use_shared_msvc_toolchain

Conversation

@jheaff1

@jheaff1 jheaff1 commented Apr 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Using python packages with C modules, e.g. numpy, would fail when using
the "static" toolchain. See
Issue Number: astral-sh/python-build-standalone#124

What is the new behavior?

When using the "shared" toolchain, importing numpy is successful

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@jheaff1
jheaff1 force-pushed the use_shared_msvc_toolchain branch from b52b4dd to e6052a4 Compare April 30, 2022 20:34
@jheaff1
jheaff1 marked this pull request as ready for review April 30, 2022 21:14
@jheaff1
jheaff1 requested review from brandjon and lberki as code owners April 30, 2022 21:14
@jheaff1

jheaff1 commented Apr 30, 2022

Copy link
Copy Markdown
Contributor Author

The build failure is due to the //:examples/wheel:filename_escaping target. If I run bazel build -s //:examples/wheel:filename_escaping to print out the command that is actually run, and then run said command, it seems to run successfully on this new branch but not on main, which is odd

@f0rmiga

f0rmiga commented May 3, 2022

Copy link
Copy Markdown
Member

The build failure is due to the //:examples/wheel:filename_escaping target. If I run bazel build -s //:examples/wheel:filename_escaping to print out the command that is actually run, and then run said command, it seems to run successfully on this new branch but not on main, which is odd

Are you referring to the build failure on CI?

@jheaff1

jheaff1 commented May 3, 2022

Copy link
Copy Markdown
Contributor Author

The build failure is due to the //:examples/wheel:filename_escaping target. If I run bazel build -s //:examples/wheel:filename_escaping to print out the command that is actually run, and then run said command, it seems to run successfully on this new branch but not on main, which is odd

Are you referring to the build failure on CI?

Yes

@UebelAndre

UebelAndre commented May 13, 2022

Copy link
Copy Markdown
Contributor

@jheaff1 @f0rmiga Howdy folks! Any updates here?

@jheaff1

jheaff1 commented May 13, 2022

Copy link
Copy Markdown
Contributor Author

@jheaff1 @f0rmiga Howdy folks! Any updates here?

Hey there! I’m on holiday at the moment so don’t have the time to look into this sadly. It might be a few more weeks before I can take a look

@phlax phlax mentioned this pull request May 18, 2022
Merged
@phlax

phlax commented May 19, 2022

Copy link
Copy Markdown
Contributor

we are hitting similar problems to the ones resolved here trying to make use of the hermetic toolchain on windows.

the error we are receiving when trying to make use of an egg that requires compilation is:

  error: Microsoft Visual C++ 14.0 or greater is required. Get it with "Microsoft C++ Build Tools": https://visualstudio.microsoft.com/visual-cpp-build-tools/

but afaict we do have c++ build tools in the env (ref envoyproxy/envoy#20432 (comment)) so im guessing this is an issue related to using the static standalone binary

i tested using this PR - sadly it didnt work at all (afaict some kind of file not found issue - perhaps the expected shared libs - struggling to get useful info out of our win CI)

@jheaff1
jheaff1 force-pushed the use_shared_msvc_toolchain branch from e6052a4 to 500bd88 Compare May 20, 2022 11:29
@jheaff1

jheaff1 commented May 20, 2022

Copy link
Copy Markdown
Contributor Author

@jheaff1 @f0rmiga Howdy folks! Any updates here?

CI now passes

@jheaff1
Use "shared" msvc hermetic toolchain
7ea10b7 
Using python packages with C modules, e.g. numpy, would fail when using
the "static" toolchain. See astral-sh/python-build-standalone#124

When using the "shared" toolchain, importing numpy is successful
@jheaff1
jheaff1 force-pushed the use_shared_msvc_toolchain branch from 500bd88 to 7ea10b7 Compare May 20, 2022 12:19
@jheaff1

jheaff1 commented May 20, 2022

Copy link
Copy Markdown
Contributor Author

we are hitting similar problems to the ones resolved here trying to make use of the hermetic toolchain on windows.

the error we are receiving when trying to make use of an egg that requires compilation is:

  error: Microsoft Visual C++ 14.0 or greater is required. Get it with "Microsoft C++ Build Tools": https://visualstudio.microsoft.com/visual-cpp-build-tools/

but afaict we do have c++ build tools in the env (ref envoyproxy/envoy#20432 (comment)) so im guessing this is an issue related to using the static standalone binary

i tested using this PR - sadly it didnt work at all (afaict some kind of file not found issue - perhaps the expected shared libs - struggling to get useful info out of our win CI)

To fix the CI build I had to modify the BUILD file for the toolchain repo to include the DLLs provided in the standalone python tarball. Does this updated PR resolve your issue?

@phlax

phlax commented May 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Does this updated PR resolve your issue?

not exactly, but it works now - no missing file issue

we kinda ~fixed the problem already by upgrading our deps - i guess the "fix" was mostly just not needing to compile

i have tested this branch with/out the "fix" - without it i get the same original error about missing Visual C++ - im wondering if there is something amiss with our win env.

@f0rmiga

f0rmiga commented May 23, 2022

Copy link
Copy Markdown
Member

Until I get to compiling wheels from source inside actions using the resolved toolchains, it's preferable to consume binary wheels.

@f0rmiga

f0rmiga commented May 23, 2022

Copy link
Copy Markdown
Member

Thanks for fixing this!

@f0rmiga
f0rmiga merged commit 4f4f792 into bazel-contrib:main May 23, 2022
@jheaff1
jheaff1 deleted the use_shared_msvc_toolchain branch May 23, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@f0rmiga f0rmiga f0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jheaff1 @f0rmiga @UebelAndre @phlax