Fix override of existing values in prepend & append

[nicolasbender]

Summary

Currently, when using NewFormattedMapFromEnvironment(environment), entries like "$KEY:value" are overwritten by the last loop execution when using both append and prepend on the same $KEY. This fix appends and prepends values to entries which are already existing.

Use Cases

We stumbled upon this in the following PR: paketo-buildpacks/npm-install#688 (comment). Integration tests in this project are not working because they check for specific log entries written with NewFormattedMapFromEnvironment(environment), which yield the wrong result, because they overwrite "$KEY:value" when append and prepend are used on the same $KEY.

Checklist

• I have viewed, signed, and submitted the Contributor License Agreement.
• I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
• I have added an integration test, if necessary.
• I have reviewed the styleguide for guidance on my code quality.
• I'm happy with the commit history on this PR (I have rebased/squashed as needed).

[c0d1ngm0nk3y]

Some suggestions. Mostly a matter of taste, but existingValue.(string) might cause a panic.

[robdimsdale]

Hello and welcome, @nicolasbender !

This broadly looks good - thanks for adding test coverage - and I'd like to see this PR incorporate the changes suggested by @c0d1ngm0nk3y . I don't want to just merge them in myself in case you have opinions on them.

[ForestEckhardt]

@nicolasbender To be clear. This will only affect the way these environment variables are ultimately logged no functionality will change as the setting of environment variables from the files generated is done by the lifecycle. I think that this is still a great fix I just wanted to lay that out cause it is unclear to me in the write up that is the intended outcome.

[nicolasbender]

@ForestEckhardt

This will only affect the way these environment variables are ultimately logged

Yes, we are aware of that. The issue we had with this was only in some integration tests from the npm-install buildpack which tested the outcome of the logged result and not the result itself (paketo-buildpacks/npm-install#688 (comment)). And since the logging did not work properly, the integration tests failed.

[ForestEckhardt]

Perfect!

[nicolasbender]

Hi @ForestEckhardt @robdimsdale, thanks for merging! Is it possible to release this change in a patch version? Although this is a very small change, this PR is blocked until the next release of packit including this change.

[c0d1ngm0nk3y]

@paketo-buildpacks/tooling-maintainers Are there any plans on releasing packit?

[mhdawson]

@sophiewigmore not sure if you are involved in this repo, but getting a release would be great to unlock the Node.js pr.