Hooks can introduce empty environment variables

[pda]

Background

The environment is captured before and after each hook runs; changes are detected and propagated to the rest of the build.

Problem

The current implementation does not detect newly introduced environment variables with an empty value.

This is due to the way Go's map type works; the key and value are statically typed, and accessing a key that does not exist returns the zero-value of the value type. That means for a map[string]string accessing a string key that doesn't exist returns ""; the zero-value of string is an empty string.

When a newly introduced FOO="" environment is detected, its empty-string value is compared to the pre-hook env["FOO"] which returns the empty string; this appears unchanged, so it's not included in the diff.

This is a problem if a hook attempts to declare (but not assign) a variable and a subsequent script running in set -u mode (crash on unbound variable access) tries to access it.

Solution

Go's map also returns a bool flag indicating whether the value was present. We were ignoring it, but now we're not.

Now, when a newly introduced empty environment variable is checked, its absence in the pre-hook environment causes it to be included in the diff.

Verification

env.Environment.Diff() unit test coverage has been added.

I've run a sample build where an agent environment hook sets an empty var; prior to this patch it did not propagate to the build step command, now it does.

[jayco]

LGTM

[yob]

I reverted the code change and confirmed the new test failed as expected.

Nice PR description to diff length ratio too 🎉