buildEnvironment does not pass correct params to build process

[timkaas]

Using the latest build 1.2.1, it seems that specifying buildEnvironment as a param to multiple RustFunction instances causes only the first passed env vars for the first hit RustFunction instance to be passed for all following creations of RustFunction.

Minimum example:

const foo_lambda = new RustFunction(this, 'foo_lambda', {
	directory: path.resolve(path.join(__dirname, '../lambda')),
	package: 'foo',
	buildEnvironment: {
		FOO_ENV: 'foo',
	},
})

const bar_lambda = new RustFunction(this, 'bar_lambda', {
	directory: path.resolve(path.join(__dirname, '../lambda')),
	package: 'bar',
	buildEnvironment: { 
		BAR_ENV: 'bar' 
	},
})

With the rust code doing e.g.:

const FOO_ENV: &str = env!("FOO_ENV");
const BAR_ENV: &str = env!("BAR_ENV");

I'm getting the error when compiling the second rust functionenvironment variable "BAR_ENV" not defined, however - the FOO_ENV is available for the second function. If reversing the order of the two RustFunctions, now BAR_ENV is available, and compilation will fail with FOO_ENV not defined.

This issue can be worked-around by building each package individually by doing:

import { Settings } from 'rust.aws-cdk-lambda'
Settings.BUILD_INDIVIDUALLY = true

[rnag]

Hi @timkaas, first off thanks for opening this issue. I had a look, and realized that you are totally right - this is likely because we only call cargo lambda once by default on all bins/workspaces in a project, so the first buildEnvironment that the RustFunction encounters is the one that will be used for a first-time (and project wide) compilation.

More info and context can be found in these lines: https://github.com/rnag/rust.aws-cdk-lambda/blob/main/lib/build.ts#L116-L138

As can be seen, shouldCompile is a boolean flag and is only true for once per project, or else is always true when BUILD_INDIVIDUALLY is enabled.

Additionally, it can also be seen, that we default to BUILD_ENVIRONMENT if buildEnvironment is not passed in to RustFunction the first time:

options.buildEnvironment ||
Settings.BUILD_ENVIRONMENT;

Per these notes above, I would say that it might be slightly more performant to only call cargo lambda once per project, rather than once per RustFunction (which can easily add up if you have a lot of them).

I would suggest using Settings.BUILD_ENVIRONMENT in this case, and having merged environment defined under there:

Settings.BUILD_ENVIRONMENT = {
	FOO_ENV: 'foo',
	BAR_ENV: 'bar',
};

// this appends 'lambda' folder to the directory where `cdk` was invoked
// note that this should be the same as passing `directory` to each RustFunction as above
Settings.WORKSPACE_DIR = 'lambda'

const foo_lambda = new RustFunction(this, 'foo_lambda', {
	package: 'foo',
        // don't use `buildEnvironment` as we want to default to merged environment above
})

const bar_lambda = new RustFunction(this, 'bar_lambda', {
	package: 'bar',
        // don't use `buildEnvironment` as it will be ignored here
})

Please do try that out, and let me know how it goes.

[rnag]

A few TODO items I see, and potential areas of improvement in terms of documentation and clarity purposes:

• Documentation should be updated to indicate that BUILD_INDIVIDUALLY should be enabled (true) when multiple instances of buildEnvironment are passed in, since only the first value of buildEnvironment will be used for compilation step (since cargo lambda only runs once per project).
• We should definitely log or print out a user-friendly warning when subsequent buildEnvironment is encountered, but shouldCompile is false, meaning compilation step was already run. I feel a friendly message would be to suggest merging those env values into Settings.BUILD_ENVIRONMENT instead, before declaring any RustFunctions; or else enabling BUILD_INDIVIDUALLY as that could be another option.
• If buildEnvironment is passed in to a RustFunction but Settings.BUILD_ENVIRONMENT is also defined at this point, we should similarly display a warning, because these two should be mutually exclusive (one or the other, but not both).