Make windows hooks process env changes the same way as unix #1791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pda
approved these changes
Oct 18, 2022
moskyb
force-pushed
the
windows-hook-env-improvements
branch
2 times, most recently
from
2d2697c to
9a65b1b
Compare
|
The tests here are failing because apparently some windows env vars start with an The fix here could either be to silently ignore them, or to special case them in some way. I would slightly prefer to special case them - i don't think our previous implementation properly handled these, but it would probably be good to have the option available. |
moskyb
force-pushed
the
windows-hook-env-improvements
branch
from
9a65b1b to
f4529bf
Compare
DrJosh9000
reviewed
Nov 3, 2022
This uses the new buildkite-agent env command introduced in [#1781](#1781), and uses it for hooks on windows machines as well. This change was scoped out of the initial fix, as windows machines weren't affected by the bug, and the effort of testing them would add time between the issue being reported and a fix being issued. This PR ports the behaviour that we were previously using for hooks running on unix machines to windows machines as well, making the code a bunch nicer and allowing us to delete a whole bunch of useless env processing code.
DrJosh9000
force-pushed
the
windows-hook-env-improvements
branch
from
f126ac8 to
939fb24
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This uses the new buildkite-agent env command introduced in #1781, and uses it for hooks on windows machines as well. This change was scoped out of the initial fix, as windows machines weren't affected by the bug, and the effort of testing them would add time between the issue being reported and a fix being issued.
This PR ports the behaviour that we were previously using for hooks running on unix machines to windows machines as well, making the code a bunch nicer and allowing us to delete a whole bunch of useless env processing code.