Skip to content

Package plugins don't work on Windows if the PATH environment variable isn't accessed with the right case #8126

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

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

jakepetroules
Copy link
Contributor

Builds of packages using package plugins may fail on Windows with the following error:

error: plugin process ended by an uncaught signal: 309 <command: C:\foo\.build\plugins\cache\myplugin.exe>, <output:
>

309 translates to 0x135, which maps to 0xc0000135 aka STATUS_DLL_NOT_FOUND.

This is due to the following code in DefaultPluginScriptRunner.swift:

#if os(Windows)
        let pluginLibraryPath = self.toolchain.swiftPMLibrariesLocation.pluginLibraryPath.pathString
        var env = ProcessInfo.processInfo.environment
        if let Path = env["Path"] {
            env["Path"] = "\(pluginLibraryPath);\(Path)"
        } else {
            env["Path"] = pluginLibraryPath
        }
        process.environment = env
#endif

Environment variable names are case-insensitive on Windows. On a real Windows host, it tends to be spelled "Path". In a Windows Container (Docker), it tends to be spelled "PATH".

The code will end up clearing out the other paths from the PATH environment variable in the else case because Path != PATH.

We need to access the path here in a case-insensitive manner -- use the existing Environment abstraction for this, which handles Windows case sensitivity concerns.

Closes #8125

[One line description of your change]

Motivation:

[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]

Modifications:

[Describe the modifications you've done.]

Result:

[After your change, what will change.]

@jakepetroules
Copy link
Contributor Author

@swift-ci please smoke test

@jakepetroules jakepetroules enabled auto-merge (rebase) November 19, 2024 05:38
@MaxDesiatov MaxDesiatov added needs tests This change needs test coverage bug windows labels Nov 19, 2024
…e isn't accessed with the right case

Builds of packages using package plugins may fail on Windows with the following error:

```
error: plugin process ended by an uncaught signal: 309 <command: C:\foo\.build\plugins\cache\myplugin.exe>, <output:
>
```

309 translates to 0x135, which maps to 0xc0000135 aka STATUS_DLL_NOT_FOUND.

This is due to the following code in DefaultPluginScriptRunner.swift:

```swift
#if os(Windows)
        let pluginLibraryPath = self.toolchain.swiftPMLibrariesLocation.pluginLibraryPath.pathString
        var env = ProcessInfo.processInfo.environment
        if let Path = env["Path"] {
            env["Path"] = "\(pluginLibraryPath);\(Path)"
        } else {
            env["Path"] = pluginLibraryPath
        }
        process.environment = env
#endif
```

Environment variable names are case-insensitive on Windows. On a real Windows host, it tends to be spelled "Path". In a Windows Container (Docker), it tends to be spelled "PATH".

The code will end up clearing out the other paths from the PATH environment variable in the else case because Path != PATH.

We need to access the path here in a case-insensitive manner -- use the existing Environment abstraction for this, which handles Windows case sensitivity concerns.

Also fix another occurrence of this same pattern in the manifest loader.

Closes #8125
@jakepetroules
Copy link
Contributor Author

@swift-ci please smoke test

@jakepetroules
Copy link
Contributor Author

@swift-ci test

@jakepetroules jakepetroules merged commit cfde665 into main Dec 3, 2024
5 checks passed
@jakepetroules jakepetroules deleted the plugins-path branch December 3, 2024 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs tests This change needs test coverage windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package plugins don't work on Windows if the PATH environment variable isn't accessed with the right case
6 participants