Adding environment variables support to Builder.class

[danyShapiro]

Hi,
I am developing Che Builder that should be able to execute build requests while the execution relies on environment variables.
i.e. the commands that are going to be executed by the builder read some information from the environment variables.
Hence the environment variables should be updated before the build execution dynamically for every build.

Following current implementation the information could be passed to the builder using the BuildRequest, but as I see it, update of the environment variables before the build execution is missing.
So I'm proposing to add the following method to the Builder.class

public Map<String, String> getEnvironmentVariables (){
return new HashMap<String, String>();
}

Any implementing builder class can override this method , by default it will be empty .
so that in Line 440 in Builder.class ,the code could be changed to the following :

ProcessBuilder processBuilder = new ProcessBuilder().command(commandLine.toShellCommand()).directory(
configuration.getWorkDir()).redirectErrorStream(true);

HashMap<String, String> env = processBuilder.environment();
env. putAll(getEnvironmentVariables());

Process process = processBuilder.start();

This way the command executed by the builder will be able to use the environment variables passed in to the builder using the BuildRequest.

Thanks
Dany.

[skabashnyuk]

Can we do that without exposing additional method ?
public Map<String, String> updateEnvironmentVariables(Map<String, String> env)

[danyShapiro]

Hi,
Additional alternative solution can be to "piggy back" the BuilderConfiguration object .
i.e the createCommandLine will update the configuration internally (in additional field ) and when the
configuration is passed to execute(...) and then to createTaskFor (...) the processBuilder will be updated from configuration using updateEnvironmentVariables(..) that i will change to private.

Do you prefer this solution ?
If have other ideas , do tell ...

BR.
Dany.

[skabashnyuk]

@danyShapiro can you make this pr up to date?

[skabashnyuk]

Closed as outdated.