Skip to content

Conversation

@egorzhdan
Copy link
Contributor

Currently SwiftPM compiles the package manifest with an -rpath linker argument which is not supported on Windows.
On Windows it should instead execute the compiled manifest with an additional PATH component representing the PackageDescription.dll location.

This implementation relies on swiftlang/swift-tools-support-core#92 to pass the PATH environment variable to the manifest executable, but these PRs can be merged in any order.

After this change the compiled manifests can be executed manually by running id-manifest.exe -fileno 1
– it prints the package manifest JSON to stdout.
(Execution by SwiftPM itself still fails because of a different issue: the file descriptor of the JSON output is not shared properly with the child process)

This argument is not supported on Windows and results in a linker error
This allows the executable to find the `PackageDescription.dll` successfully
@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

I don't have the knowledge to review the Windows side of the branches, but the non-Windows cases look to preserve the existing behavior as far as I can see. Is there someone else who should review the Windows side? Since AFAIK the Windows support is still being brought up, I'm inclined to merge even if it turns out that the Window branches end up needing further adjustments.

@MaxDesiatov
Copy link
Contributor

CC @compnerd

@egorzhdan
Copy link
Contributor Author

Hmm, the test failure seems to be unrelated to these changes...
Perhaps the CI picks fresh tests from master for this slightly outdated branch (created before 933c5d7)?
Can't say for sure since I don't have access to the swift-continuous-integration repo.

@MaxDesiatov
Copy link
Contributor

@swift-ci please smoke test

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Jul 23, 2020

I think that the commit the branch was created off shouldn't matter as it should run CI as if the branch is merged into master. At least that's how most CI services do it, and I hope that Swift CI does it too. Retriggering the build should help if that's the case, if not, I apologize if triggering the CI wasted any resources.

@egorzhdan
Copy link
Contributor Author

Rerunning the build helped, thanks!
Swift Test Linux Platform (self hosted) failed during the previous run but passed successfully now.

@egorzhdan
Copy link
Contributor Author

Could this PR be merged if there are no issues with the changes?

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MaxDesiatov MaxDesiatov merged commit 46cff00 into swiftlang:master Aug 12, 2020
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.

3 participants