C++: Add feature flag for TRAP caching.

[sashabu]

Merge / deployment checklist

• Confirm this change is backwards compatible with existing workflows.

• Confirm the readme has been updated if necessary.

• Confirm the changelog has been updated if necessary.

• Closes https://github.com/github/codeql-c-team/issues/2146

[henrymercer]

Thanks, two further comments:

• TRAP caching enablement should probably depend on a few other things. We have an explicit input for disabling TRAP caching, which we should respect. This is used in getTrapCachingEnabled(). This function also disables TRAP caching if we're running on a self-hosted runner where the connection to the artifact store might be slow. The only complication with reusing this function is that we probably want to let the trap-caching input override the value of the feature flag.
• We may want to set minimumVersion on the feature flag to avoid printing a log message like "Enabling CodeQL C++ TRAP caching support" when running against an old CLI that doesn't support TRAP caching.

[sashabu]

we probably want to let the trap-caching input override the value of the feature flag.

I don't think we do: the feature flag is intended for a gradual rollout rather than long-lived, so I think we do want the feature flag to override it. (And if I understand correctly, CODEQL_CPP_TRAP_CACHING will set the feature flag, so customers would have some control if they need to.)

• We may want to set minimumVersion on the feature flag to avoid printing a log message like "Enabling CodeQL C++ TRAP caching support" when running against an old CLI that doesn't support TRAP caching.

We're not using the environment variable in production yet (or turning the feature flag on). What's the right way to go about minimumVersion in this case? Last release, next release, leave undefined for now and fix later, hold off merging this PR?

[henrymercer]

I don't think we do: the feature flag is intended for a gradual rollout rather than long-lived, so I think we do want the feature flag to override it. (And if I understand correctly, CODEQL_CPP_TRAP_CACHING will set the feature flag, so customers would have some control if they need to.)

That's fair — the expectation for trap-caching: true could be that TRAP caching is enabled for all languages for which it's GA. However I do think trap-caching: false should override the feature flag, as this is a signal that the customer does not want us to populate Actions cache (typically because they are using it for something else and don't want their other caches to be evicted).

We're not using the environment variable in production yet (or turning the feature flag on). What's the right way to go about minimumVersion in this case? Last release, next release, leave undefined for now and fix later, hold off merging this PR?

I think we should probably hold off on merging this PR until we know what the minimum CLI version is that we'd want to enable this for. The alternative is to add an entry to the features property of codeql version and check for that in the Action instead of using a version number.

[sashabu]

However I do think trap-caching: false should override the feature flag, as this is a signal that the customer does not want us to populate Actions cache (typically because they are using it for something else and don't want their other caches to be evicted).

Agreed. My last commit should do that.

I think we should probably hold off on merging this PR until we know what the minimum CLI version is that we'd want to enable this for.

👍