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

CI release builds fail when the release message is too long #1218

Closed
katre opened this issue Aug 30, 2021 · 6 comments · Fixed by #1219
Closed

CI release builds fail when the release message is too long #1218

katre opened this issue Aug 30, 2021 · 6 comments · Fixed by #1219

Comments

@katre
Copy link
Member

katre commented Aug 30, 2021

Bazel release 4.2.1 failed during release on Windows hosts only. The issue was eventually traced to being that the release message, when copied into the BUILDKITE_MESSAGE environment variable, was too long and was causing the crash.

Original failure: https://buildkite.com/bazel-trusted/bazel-release/builds/305#4397c7bb-e13f-4268-85b6-7d37fee966bb

We eventually worked around this by manually setting the BUILDKITE_MESSAGE flag: https://buildkite.com/bazel-trusted/bazel-release/builds/306

@katre
Copy link
Member Author

katre commented Aug 30, 2021

Sub-issues:

  • Fix CI to trim the message (and other variables) to keep them from being too long
  • Fix Bazel's release notes to not be so long (4.2.1 includes notes about 4.2.0, 4.1.0, and 4.0.0 due to the way notes are generated from master).

@meteorcloudy
Copy link
Member

meteorcloudy commented Aug 30, 2021

Also we should fix Bazel on Windows to not crash when receiving large environment variables. I'll file an issue to the Bazel repo.

@meteorcloudy
Copy link
Member

It turned out it's a Windows API limitation:
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setenvironmentvariable

So we probably couldn't do much on the Bazel side, let's make sure our CI doesn't pass large env var.

@philwo
Copy link
Member

philwo commented Sep 3, 2021

@meteorcloudy Mhm, if this were an unavoidable Windows API limitation, shouldn't our Python script already crash much earlier? Because the Buildkite Agent sets the environment variable and then calls Python to run our bazelci.py, it seems like Python itself and other programs we call during the script work fine, except Bazel (which crashes). (But maybe I'm missing something here.)

@philwo
Copy link
Member

philwo commented Sep 3, 2021

Since Windows Vista there is apparently no technical limitation anymore, just a "practical limit" because some APIs like SetEnvironmentVariable still have this limit (but apparently GetEnvironmentStrings doesn't).

The maximum size of a user-defined environment variable is 32,767 characters. There is no technical limitation on the size of the environment block. However, there are practical limits depending on the mechanism used to access the block. For example, a batch file cannot set a variable that is longer than the maximum command line length.

Windows Server 2003 and Windows XP: The maximum size of the environment block for the process is 32,767 characters. Starting with Windows Vista and Windows Server 2008, there is no technical limitation on the size of the environment block.

from https://docs.microsoft.com/en-us/windows/win32/procthread/environment-variables

@meteorcloudy
Copy link
Member

it seems like Python itself and other programs we call during the script work fine

@philwo That is a good point! I think it's an interesting Windows issue that our 20% contributor could look into ;)

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 a pull request may close this issue.

3 participants