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

[SR-588] Allow custom env variables in Package.swift #5433

Closed
swift-ci opened this issue Jan 20, 2016 · 12 comments
Closed

[SR-588] Allow custom env variables in Package.swift #5433

swift-ci opened this issue Jan 20, 2016 · 12 comments
Labels

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-588
Radar None
Original Reporter Memorion (JIRA User)
Type Bug
Status Resolved
Resolution Done
Environment

OS X 10.11

Additional Detail from JIRA
Votes 0
Component/s Package Manager
Labels Bug
Assignee None
Priority Medium

md5: 30222e0229f3a915aa19abe648c6ced4

Issue Description:

Currently if you try something like this in your Package.swift

if let _ = String.fromCString(getenv("TEST")) {
    let package = Package(name: "getenv_was_true")
} else {
    let package = Package(name: "getenv_was_false")
}

it uses the else path whether TEST is set or not which I assume is due to the explicit environment parameter here:

let toml = try popen(cmd, environment: ["SWIFT_DUMP_PACKAGE": "1"])

I wrote a sample project for the issue:
https://github.com/Memorion/getenvdemo

@ddunbar
Copy link
Member

ddunbar commented Jan 22, 2016

This is not something we want people to depend on.

If there is a need for Packages to have conditional behavior based on conditions, then we should surface that through real features in the package manager (a la Cargo's "features"), not through ad hoc mechanisms crafted by the individual Package.

@mxcl thoughts here? Should we close this out or dup it to an RFE for "features" (or reuse it for that purpose)?

@mxcl
Copy link
Contributor

mxcl commented Jan 22, 2016

I think it's cool, but only for the root package.

@swift-ci
Copy link
Contributor Author

Comment by Bastian Rinsche (JIRA)

I admit that this might take the "the manifest is valid Swift" a little too far, I was trying to use this as a workaround to conditionally exclude a Target that depends on XCTest being installed, but it may be better to wait for real testing support than to introduce this kind of "hack", not sure if there are more reasonable use cases.

@mxcl
Copy link
Contributor

mxcl commented Jan 22, 2016

I'm OK with the manifest is swift being taken too far, but only for the root package. Since packages you grab from the Internet should have a much more deterministic behavior, or they will easily cause random breakage and in an era where 100 packages is not uncommon, we must be able to predict what is possible and thus be able to manage behavior.

However, the end-user should be able to do whatever they want.

@ddunbar
Copy link
Member

ddunbar commented Jan 22, 2016

I'm cool with that. Ultimately I think it would be ideal to have proper features to cover the things users want to do, but I agree that for now unrestricting the root package is a convenient way to unblock people.

@ankitspd
Copy link
Member

I am trying to patch this but can't seem to find method to get all environment variables without Foundation's NSProcessInfo or dlsym. Is swiftpm trying to explicitly avoid Foundation or can be used?

@swift-ci
Copy link
Contributor Author

Comment by Bastian Rinsche (JIRA)

I think a dependency on Foundation should be avoided, I'd imagine there has to be some way to do that with libc, but I don't know much about interfacing between C and Swift.

@mxcl
Copy link
Contributor

mxcl commented Jan 26, 2016

Currently Package.swift is deliberately executed in a mostly empty environment.

@ddunbar
Copy link
Member

ddunbar commented Jan 26, 2016

@aciidb0mb3r Generally speaking, if you want to expose functionality into the manifest, the right way to do so is to add APIs to the PackageDescription module. swiftpm already has all the magic in place to make sure that that runtime library is available.

Introducing an additional dependency on Foundation in that module would be bad though, it should use our existing internal APIs.

@mxcl
Copy link
Contributor

mxcl commented Jan 30, 2016

We're going to hold on this ticket for the time being, pending something, something.

@abertelrud
Copy link
Contributor

I agree with Daniel here: enhancing the PackageDescription API to support new ways of declaring intent is great. This will also make it easier to one day provide good IDE integration; the more that is done via custom Swift code, the less visibility the IDE has into the behavior, capabilities, and requirements of a package.

@ankitspd
Copy link
Member

ankitspd commented Mar 8, 2017

Current environment is now passed to all subprocess by default.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants