C++: Handle codeql_action_cpp_build_mode_none feature flag - #2568
calumgrant merged 3 commits into
Conversation
Sorry, something went wrong.
| logger.info("Enabling C++ build-mode: none"); | ||
| core.exportVariable(bmn_var, "true"); | ||
| } else { | ||
| logger.info("Disabling C++ build-mode: none"); | ||
| core.exportVariable(bmn_var, "false"); |
There was a problem hiding this comment.
I assume the reason why you are doing this is because the extractor requires this same env var to be set?
Sorry, something went wrong.
There was a problem hiding this comment.
It's not required, just setting it for completeness.
Sorry, something went wrong.
|
|
||
| // Set CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE | ||
| if (config.languages.includes(Language.cpp)) { | ||
| const bmn_var = "CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE"; |
There was a problem hiding this comment.
This is probably a linter error. Here's a more canonical name.
| const bmn_var = "CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE"; | |
| const bmnVar = "CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE"; |
Sorry, something went wrong.
There was a problem hiding this comment.
Is this simpler? features.getValue is guaranteed to return a boolean and if the env var is already set, it will use that as an override.
const bmnVar = await features.getValue(Feature.CppBuildModeNone, codeql);
core.exportVariable(bmnVar);
logger.info(`Setting C++ build-mode: none to ${bmnVar}`);
Sorry, something went wrong.
There was a problem hiding this comment.
That didn't quite compile, but I managed to simplify things. The code in init-action.ts is not written in this idiomatic way, and the docs to exportVariable() don't mention the behaviour when the variable is already set. Most of the code in init-action.ts defensively checks the environment.
I would suggest refactoring this file as most people will just adapt code that's already there.
Sorry, something went wrong.
| // Set CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE | ||
| if (config.languages.includes(Language.cpp)) { | ||
| const bmnVar = "CODEQL_EXTRACTOR_CPP_BUILD_MODE_NONE"; |
There was a problem hiding this comment.
If you think we might want users to interact with this option, we should make this an extractor option by changing the name to CODEQL_EXTRACTOR_CPP_OPTION_BUILD_MODE_NONE and including an entry in codeql-extractor.yml. That gives customers more options for configuring it. If this is only going to be used by the Action, then we can leave it as is.
Sorry, something went wrong.
There was a problem hiding this comment.
This is internal and short-lived, so I don't think we should give customers the option of configuring it.
Sorry, something went wrong.
Merge / deployment checklist
Continues #2565, tidying up the feature flag options and setting the environment variables.