Skip to content

Conversation

@gjenkins8
Copy link
Collaborator

@gjenkins8 gjenkins8 commented Aug 18, 2025

What this PR does / why we need it:
Remove 'SetupPluginEnv', as this only works for subprocess plugins.

The change ended up being slightly involved, as PrepareCommands uses the processes environmental variables to expand CLI arguments. Which had to be replaced by passing in the intended child processes environment variables instead.

Special notes for your reviewer:

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@gjenkins8 gjenkins8 force-pushed the gjenkins/plugin-system/rm_setup_plugin_env branch from 86cbff9 to 22f5faa Compare August 18, 2025 18:47
@scottrigby
Copy link
Owner

Oh thank god. I figured this would be a post release cleanup. But happy you did this now.
Should we add an equivalent of TestSetupEnvWithSpace() to our tests?

@scottrigby
Copy link
Owner

I'd be happy to get this into upstream before the feature freeze if we have time after the required things. But it can also be a follow-up (non-breaking) change after freeze too.

@gjenkins8 gjenkins8 force-pushed the gjenkins/plugin-system/rm_setup_plugin_env branch 4 times, most recently from b772006 to 5056552 Compare August 22, 2025 22:01
Signed-off-by: George Jenkins <[email protected]>
@gjenkins8 gjenkins8 force-pushed the gjenkins/plugin-system/rm_setup_plugin_env branch from 5056552 to d691452 Compare August 22, 2025 22:05
@gjenkins8 gjenkins8 merged commit 9f5792d into scottrigby:plugin-system Aug 22, 2025
1 check passed
@gjenkins8 gjenkins8 deleted the gjenkins/plugin-system/rm_setup_plugin_env branch August 22, 2025 22:06
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 this pull request may close these issues.

2 participants