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

pkg_zip failing on windows and embeddable python w/ Bazel 4.0.0 #475

Closed
perezd opened this issue Nov 10, 2021 · 22 comments
Closed

pkg_zip failing on windows and embeddable python w/ Bazel 4.0.0 #475

perezd opened this issue Nov 10, 2021 · 22 comments
Labels
invalid p4 An idea that we are not considering working on at this time.

Comments

@perezd
Copy link

perezd commented Nov 10, 2021

Error message:

PS C:\workspace> C:\bazel.exe build -s  //:protoc_release
INFO: Analyzed target //:protoc_release (21 packages loaded, 580 targets configured).
INFO: Found 1 target...
SUBCOMMAND: # //:protoc_release [action 'PackageZip protoc-3.19.1-x64_windows.zip', configuration: 9d96c5e0dbeafe11161fe69fc89d74bfea3cac17f680ef9ae77a57d635f7af83, execution platform: @local_config_pla
tform//:host]
cd C:/workspace/_build/out/execroot/com_google_protobuf
  SET PATH=C:\msys64\usr\bin;C:\msys64\bin;C:\Windows;C:\Windows\System32;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPo
werShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Program Files\dotnet\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps;C:\Users\ContainerAdministrator\.dotnet\tools;C:\Program Files\Nu
Get;C:\Program Files (x86)\Microsoft Visual Studio\2022\TestAgent\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Current\Bin;C:\
Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools;C:\Program Files (x86)\Microsoft SDKs\ClickOnce\SignTool;;C:\python;
    SET RUNFILES_MANIFEST_ONLY=1
  bazel-out/x64_windows-opt-exec-2B5CBBC6/bin/external/rules_pkg/private/build_zip.exe -o bazel-out/x64_windows-fastbuild/bin/protoc-3.19.1-x64_windows.zip -d / -t 315532800 -m 0555 --manifest bazel-out
/x64_windows-fastbuild/bin/protoc_release.manifest
ERROR: C:/workspace/BUILD:574:8: PackageZip protoc-3.19.1-x64_windows.zip failed: (Exit 1): build_zip.exe failed: error executing command bazel-out/x64_windows-opt-exec-2B5CBBC6/bin/external/rules_pkg/p
rivate/build_zip.exe -o bazel-out/x64_windows-fastbuild/bin/protoc-3.19.1-x64_windows.zip -d / -t 315532800 -m 0555 --manifest ... (remaining 1 argument(s) skipped)
Traceback (most recent call last):
  File "\\?\C:\Users\ContainerAdministrator\AppData\Local\Temp\Bazel.runfiles_1pl18lt_\runfiles\rules_pkg\private\build_zip.py", line 21, in <module>
    from private import build_info
ModuleNotFoundError: No module named 'private'
Target //:protoc_release failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 4.707s, Critical Path: 1.35s
INFO: 2 processes: 2 internal.
FAILED: Build did NOT complete successfully

Here's the BUILD target:
https://github.com/protocolbuffers/protobuf/blob/master/BUILD#L575

Any pointers? Any other context I can provide?

Python version: 3.9.8

@aiuto
Copy link
Collaborator

aiuto commented Nov 10, 2021

This is a protoc build, not rules_pkg. I'm going to transfer the issue there.

@aiuto aiuto transferred this issue from bazelbuild/rules_pkg Nov 10, 2021
@perezd
Copy link
Author

perezd commented Nov 10, 2021

No, this is a problem with rules_pkg. I am on the protobuf team.

@perezd
Copy link
Author

perezd commented Nov 10, 2021

We're attempting to use rules_pkg in protobuf's repo.

@aiuto aiuto transferred this issue from bazelbuild/rules_proto Nov 15, 2021
@aiuto
Copy link
Collaborator

aiuto commented Nov 15, 2021

OK. I took the issue back.
I need a lot more supporting info.

  • What release of rules_proto does this fail in, or what commit if from head? The WORKSPACE file matters as much as the BUILD.
  • Does it fail on linux too, or just windows

It's going to be hard to reproduce, since I don't have a working windows environment right now.
It also looks like it is a release before the last code restructuring, so you'll need to update what you pull, because we are not doing patch releases to the 0.x release line. If you update to rules_pkg at the head commit do you still see the problem.

@aiuto
Copy link
Collaborator

aiuto commented Nov 15, 2021

Also, are other python tools working for you with your particular combination of bazel and rules_python versions?

@aiuto
Copy link
Collaborator

aiuto commented Nov 15, 2021

More things to try.
If you make it pkg_tar instead of pkg_zip, does it fail in exactly the same way? It should.
Can you send an ls -lR (or windows equivalent) of the unfurled rules_pkg repository.

@perezd
Copy link
Author

perezd commented Nov 15, 2021

What release of rules_proto does this fail in, or what commit if from head? The WORKSPACE file matters as much as the BUILD.

This has nothing to do with the rules_proto repo to be clear, this is the http://github.com/protocolbuffers/protobuf repo for the Protocol Buffers project.

The deps/versions of things are also checked in to GitHub here: https://github.com/protocolbuffers/protobuf/blob/master/protobuf_deps.bzl
We're using the officially released rules_pkg 0.5.1, haven't tested with HEAD.

Does it fail on linux too, or just windows

Only Windows, linux is working as expected.

It's going to be hard to reproduce, since I don't have a working windows environment right now.

I'd suggest a simple GCP VM, this is how I've been doing diagnostics.

other python tools working for you with your particular combination of bazel and rules_python versions?

This is the only python tool we've introduced, thus far.

If you make it pkg_tar instead of pkg_zip, does it fail in exactly the same way? It should.

Correct, it does fail in the same way.

Can you send an ls -lR (or windows equivalent) of the unfurled rules_pkg repository.

https://gist.github.com/perezd/1014425bfa1d0674c1269fbdcb2452ba

@aiuto
Copy link
Collaborator

aiuto commented Nov 16, 2021

Thanks for that information.
If pkg_tar fails the same way, and the init.py is in private, as the ls shows, then this is a python path problem. But we are using the same rules_python versions, and our tests pass on Windows, but yours don't, so there is no smoking gun.

I'm going to be tied up with BazelCon for the rest of the week. I can potentially try some things out next week, but that is likely to be a busy/short week because of Thanksgiving.

@perezd
Copy link
Author

perezd commented Nov 16, 2021 via email

@aiuto
Copy link
Collaborator

aiuto commented Nov 16, 2021 via email

@perezd
Copy link
Author

perezd commented Nov 24, 2021

Any updates here?

@aiuto
Copy link
Collaborator

aiuto commented Nov 24, 2021

Not yet. We Bazelcon last week and Thanksgiving this week, I have not had a chance to even look at setting up a windows environment to test in.

@perezd
Copy link
Author

perezd commented Nov 24, 2021 via email

@nacl
Copy link
Collaborator

nacl commented Nov 27, 2021

I tried this out in a local Windows dev environment I have lying around, and the first thing I encountered was:

> bazelisk.exe build //:protoc_release
ERROR: C:/$BASE/dev/protobuf/BUILD:570:15: in package_naming rule //:protoc_pkg_naming:
Traceback (most recent call last):
        File "C:/$BASE/dev/protobuf/protobuf_release.bzl", line 35, column 9, in _package_naming_impl
                fail("Unrecognized platform")
Error in fail: Unrecognized platform
ERROR: Analysis of target '//:protoc_release' failed; build aborted: Analysis of target '//:protoc_pkg_naming' failed
INFO: Elapsed time: 2.754s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded, 48 targets configured)

Reading the file where the failure occurred, it looks like only mingw builds are supported for packaging (is this intentional?). This was easy enough to work around. After doing so, the build of the zip worked fine for me. I am using HEAD protobuf, rules_pkg 0.5.1.

Some differences between our environments:

  • I have VC 2022 tools installed locally, which requires use of bazel 5. (USE_BAZEL_VERSION=last_rc in my environment for bazelisk)
  • I have python 3.9 installed via the MSFT app store

Either of these may have made the difference. I currently don't have the time to investigate further, but I hope this information helps.

@aiuto
Copy link
Collaborator

aiuto commented Nov 29, 2021

That fail is in protobuf code, so I can't say if that is intentional or not, but it looks that way.
FWIW, rules_pkg is platform agnostic and will not take dependencies on any C++ compiler, so it won't matter if you use mingw or msvc. In any case, I don't have a C++ compiler on my machines, so I would be bit with the same problem.
For my testing, I would whack protobuf_release.bzl to default to win* if not macos or linux, but that might not be suitable for actually building protobuf.

This does indirectly answer the question you asked last week. "how can I help". You can find a reproduction of the problem that does not require a C++ complier. That would probably be something not trying to build protobuf.

@perezd
Copy link
Author

perezd commented Nov 29, 2021

Can you try checking out at this commit? protocolbuffers/protobuf@4eb8840

I think you're running into some rules_pkg stuff we're setting up that's environment specific.

As for the doesn't rqeuire a C++ compiler, you can simply mod the build file to not include :protoc and you'd have that repro.

@nacl
Copy link
Collaborator

nacl commented Nov 30, 2021

Can you try checking out at this commit? protocolbuffers/protobuf@4eb8840

I believe I already had it, but I synced to HEAD anyway today.

I think you're running into some rules_pkg stuff we're setting up that's environment specific.

I was, and was able to work around the problem OK. Apparently it was fixed in protocolbuffers/protobuf@667d5e9 nearly identically to how I fixed it locally.

As for the doesn't rqeuire a C++ compiler, you can simply mod the build file to not include :protoc and you'd have that repro.

I was able to build the target without MSVC by making the suggested edits. I am still not able to reproduce the problem at this time using either Bazel 4.0.0 or 4.2.1, even after syncing past the specified commit.

I am still using the copy of python (3.9.9) from the MSFT store, which may be the deciding difference. There may some repository state that needs to be cleaned up (bazel clean --expunge?).

@perezd
Copy link
Author

perezd commented Nov 30, 2021

One thing I mentioned earlier on in the thread is I am using the embeddable package of python for windows, found here:
https://www.python.org/downloads/release/python-399/

Perhaps its unique to this format? It's required for my use case (docker container)

@nacl
Copy link
Collaborator

nacl commented Dec 3, 2021

Are there any other differences in your environment that come to mind? Is there anything else special about the docker container? Do you have a global bazelrc file that could be adding adding additional flags to bazel that might be influencing how the python rules are run?

It would also be very helpful to have an automated way to reproduce this that properly emulates your environment. Would it be possible to provide one? Other notes about the environment would also be helpful; I might have time to look into this a little more over the next few days.

@perezd
Copy link
Author

perezd commented Dec 3, 2021

Nothing unusual w/ bazelrc, just changes to output_base and repository_cache.. Also running w/ --batch but nothing that I would suspect could be related to this, it looks like:

startup --output_base=C:\\workspace\\_build\\out --batch
build --repository_cache=C:\\workspace\\_build\\repo_cache
build:offline --nofetch

We start bazel w/ a custom entrypoint.ps1 powershell script that looks like:

Start-Process -FilePath "C:\bazel.exe" -WorkingDirectory "C:\workspace" -NoNewWindow -ArgumentList "$args" -Wait

Nothing special there either really.

Our docker container is based on: mcr.microsoft.com/dotnet/framework/sdk:4.8-windowsservercore-ltsc2019

We simply extract the embeddable python package to C:\python and add it to $env:PATH using:

RUN setx /M PATH $($env:PATH + ';C:\python')

@nacl
Copy link
Collaborator

nacl commented Dec 4, 2021

I was able to reproduce this locally using the embedded python package you mentioned earlier. My conclusion is that this is not a bug in rules_pkg, but either something in bazel, or (IMO, more likely) your setup.

You probably should not be using the embeddable python binary, since it ignores various aspects of the environment by design, according to https://bugs.python.org/issue28245 and the relevant documentation.

As it stands, the embeddable python binary is not compatible with Bazel: the bazel python stub launcher uses the PYTHONPATH environment variable to set the needed python module paths in the binary. Since the embeddable python binary ignores the environment by design, it is never configured with the bazel-specified module paths, and thus will always fail when you use modules outside of the standard library.

See also bazelbuild/bazel#7959, and the stub template itself for more details.

Local analysis:

PS C:\Users\...> $env:PYTHONPATH = "foo"
PS C:\Users\...> Get-Command python

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Application     python.exe                                         3.9.915... C:\python\python.exe

Actually running said binary in that enviornment:

Python 3.9.9 (tags/v3.9.9:ccb0e6a, Nov 15 2021, 18:08:50) [MSC v.1929 64 bit (AMD64)] on win32
>>> import sys; sys.path
['C:\\python\\python39.zip', 'C:\\python']
>>> import os; os.environ['PYTHONPATH']
'foo'

PTAL and close if you agree.

@nacl nacl changed the title pkg_zip failing on windows w/ Bazel 4.0.0 pkg_zip failing on windows and embeddable python w/ Bazel 4.0.0 Dec 4, 2021
@nacl nacl added p4 An idea that we are not considering working on at this time. invalid labels Dec 6, 2021
@perezd
Copy link
Author

perezd commented Dec 6, 2021

This is an interesting thing to at least add to rules_pkg README, potentially. I'm not sure how else to get Python running in a docker container, perhaps there are other strategies I can try. Thanks for looking into this!

@perezd perezd closed this as completed Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid p4 An idea that we are not considering working on at this time.
Projects
None yet
Development

No branches or pull requests

3 participants