Task / Configurable runner environment

[dmhts]

Motivation:

The motivation for this PR is to make Runner more flexible by optionally allowing the provision of a custom environment for the underlying UserToolchain.

Context:

In our company, we use ScipioKit in a quite advanced way. Specifically, we use it to precompile packages with multiple toolchains within a single run. For this, we set certain environment variables such as PATH and DEVELOPER_DIR before each Runner's run to delicately control the compiling process.

It all works well, except for the case when the environment is not provided to UserToolchain; it defaults to Environment.current. This is not a problem by itself; the problem is that such an environment is statically cached when called for the very first time, and there's no way to redefine it in the subsequent runs (as all the Environment state modifiers are internal).

Solution:

This problem can be easily fixed by extending the Runner interface so it takes an optional Environment parameter in its build options, which is then handed over to the underlying UserToolchain.

Important:

The PR ensures that the newly introduced environment property (in BuildOptions) does not affect the existing SwiftPMCacheKey, meaning the cache will remain environment-agnostic.

@giginet I'd love to hear your opinion on this.

[dmhts]

It seems the Environment API is not available in last year's SwiftPM. I'm going to make the respective changes.

Update: @giginet done.

[giginet]

Sorry to keep you waiting!

I understood the requirements and wish to merge this
I commented some points.

[giginet]

I'm sorry for asking so late.

I want to clarify whether changes to the Environment affect the cache.
In your current example, you are passing DEVELOPER_DIR and PATH. As such, these changes will not affect the final artifacts.

I'd like to know whether there are any other configuration values that could affect the final product due to changes in the Environment.

And if it is not necessary to incorporate Environment changes into CacheKey, it seems better to pass Environment directly to Runner rather than having it in BuildOptions.

BuildOptions contains configuration values that are directly included in CacheKey, so it may cause confusion if some values are not included depending on the property
How about a design where the values are passed to Runner.Options instead of BuildOptions and then propagated?

[dmhts]

@giginet What you say sounds reasonable, it would lead to a better architecture indeed. Additionally, environment configuration should be done once per run so it's applied for all build targets consistently, hence it fits more when done on the runner level.

• As pros, reasoning about build options is easier, requires no exceptions, and less code overall. Oh, and Environment doesn't need to be Hashable anymore!
• As a con, I had to extend both FrameworkProducer and PIFCompiler initializers as we don't pass the runner options directly there. I believe it's a very minor change though, and it still remains an implementation detail.

I just applied the changes as suggested here. Please have a look.

P.S: I just realized that the PR is called "Configurable runner environment" 🤦

[dmhts]

I didn't add any tests since there are no existing test cases for PIFCompiler or ToolchainGenerator. I believe integration tests are more appropriate for covering this change, as it primarily concerns configuration passed down to SwiftPM.

[giginet]

Almost looks great!

Thank you for the quick fixes.
I pointed trivial points.

[giginet]

@dmhts Thank you for the contribution! I'll release the new version soon.

By the way, I'm very excited that your company uses this product heavily. I'm happy to hear the case study someday.

[dmhts]

@giginet That's not it, there's one more PR coming soon :)

We're currently working on a tool that uses SwiftPM and Scipio at its core. Once everything is polished, our goal is to open-source it. It's already running successfully in our CI pipelines. You'll be one of the first people I let know when it's ready!