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

Allow a custom manifest loader to provide custom environment variables #3659

Merged

Conversation

abertelrud
Copy link
Contributor

@abertelrud abertelrud commented Aug 10, 2021

Extend ToolchainConfiguration to allow a customization of the environment used when compiling package manifests.

Motivation:

Clients of libSwiftPM can already provide a set of custom flags to pass to the Swift compiler when compiling and linking the manifest. This extends this support to allow custom environment entries as well. There's no immediate use for this in SwiftPM, but it could be used in the future, and there are clients that want to do so

Modifications:

  • add a new property to ToolchainConfiguration that contains a string-to-string mapping
  • default it to ProcessEnv.vars to maintain status quo — this happens to not be needed on Darwin but is needed on Linux
  • use it when compiling manifests and plugin scripts
  • add a unit tests that uses this functionality for a toy purpose (for testing)

A separate but related change (in a separate commit) is to make the properties of ToolchainConfiguration be mutable, so that clients can use ToolchainConfiguration.default and then mutate a single property without having to instantiate a new struct and pass all the properties from ToolchainConfigratuion.default.

Result:

Clients of libSwiftPM can customize the environment used when compiling package manifests.

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud marked this pull request as draft August 10, 2021 21:55
@abertelrud
Copy link
Contributor Author

abertelrud commented Aug 10, 2021

Marking as draft until I can rework it, since #3656 has changed a lot of what this is based on.

@tomerd
Copy link
Contributor

tomerd commented Aug 11, 2021

Marking as draft until I can rework it, since #3656 has changed a lot of what this is based on.

lgtm, the rebase on top of #3656 should be easy I hope - nothing changed there that should materially impact this

@abertelrud
Copy link
Contributor Author

Marking as draft until I can rework it, since #3656 has changed a lot of what this is based on.

lgtm, the rebase on top of #3656 should be easy I hope - nothing changed there that should materially impact this

Sound good — will rebase and mark as ready.

…r customization of a single property of an already existing toolchain. It is already a struct, so the whole struct can be either mutable or immutable based on use.
@abertelrud abertelrud force-pushed the eng/manifest-loader-improvements branch from cb592f5 to 54254d7 Compare August 18, 2021 00:51
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud marked this pull request as ready for review August 18, 2021 00:55
@abertelrud abertelrud added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Aug 18, 2021
@abertelrud abertelrud marked this pull request as draft August 18, 2021 00:59
@abertelrud abertelrud added ready Author believes the PR is ready to be merged & any feedback has been addressed and removed ready Author believes the PR is ready to be merged & any feedback has been addressed labels Aug 18, 2021
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud marked this pull request as ready for review August 18, 2021 01:14
… plugin compilation

It defaults to `ProcessEnv.vars` so that there is no change in semantics unless the client code customizes the environment.
@abertelrud abertelrud force-pushed the eng/manifest-loader-improvements branch from 5ef5879 to c2296e4 Compare August 18, 2021 04:33
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud
Copy link
Contributor Author

This is now ready to be merged once it has been reviewed.

@abertelrud abertelrud merged commit fb32460 into swiftlang:main Aug 18, 2021
@abertelrud abertelrud deleted the eng/manifest-loader-improvements branch August 18, 2021 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants