Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions lib/entry-points.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 46 additions & 6 deletions src/config/file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ setupTests(test);
test("getConfigFileInput returns undefined by default", async (t) => {
const logger = new RecordingLogger();
const actionsEnv = getTestActionsEnv();
const result = getConfigFileInput(logger, actionsEnv, {});
const result = getConfigFileInput(logger, actionsEnv, {}, true);
t.is(result, undefined);
});

Expand All @@ -34,7 +34,12 @@ test("getConfigFileInput returns input value", async (t) => {

// Even though both an input and repository property are configured,
// we prefer the direct input to the Action.
const result = getConfigFileInput(logger, actionsEnv, repositoryProperties);
const result = getConfigFileInput(
logger,
actionsEnv,
repositoryProperties,
true,
);
t.is(result, testInput);

// Check for the expected log message.
Expand All @@ -46,7 +51,12 @@ test("getConfigFileInput returns repository property value", async (t) => {
const actionsEnv = getTestActionsEnv();

// Since there is no direct input, we should use the repository property.
const result = getConfigFileInput(logger, actionsEnv, repositoryProperties);
const result = getConfigFileInput(
logger,
actionsEnv,
repositoryProperties,
true,
);
t.is(result, repositoryProperties[RepositoryPropertyName.CONFIG_FILE]);

// Check for the expected log message.
Expand All @@ -62,8 +72,38 @@ test("getConfigFileInput ignores empty repository property value", async (t) =>
const actionsEnv = getTestActionsEnv();

// Since the repository property value is an empty/whitespace string, we should ignore it.
const result = getConfigFileInput(logger, actionsEnv, {
[RepositoryPropertyName.CONFIG_FILE]: " ",
});
const result = getConfigFileInput(
logger,
actionsEnv,
{
[RepositoryPropertyName.CONFIG_FILE]: " ",
},
true,
);
t.is(result, undefined);
});

test("getConfigFileInput ignores repository property value when FF is off", async (t) => {
const logger = new RecordingLogger();
const actionsEnv = getTestActionsEnv();

// Since the FF is off, we should ignore the repository property value.
const result = getConfigFileInput(
logger,
actionsEnv,
repositoryProperties,
false,
);
t.is(result, undefined);

t.false(
logger.hasMessage(
"Using configuration file input from repository property",
),
);
t.true(
logger.hasMessage(
"Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.",
),
);
});
16 changes: 12 additions & 4 deletions src/config/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export function getConfigFileInput(
logger: Logger,
actions: ActionsEnv,
repositoryProperties: Partial<RepositoryProperties>,
useRepositoryProperty: boolean,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: It would be clearer to use an object like { useRepositoryProperty: boolean } here so it's clearer when the function is called what the last argument means.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the point, but I expect to change this in line with the changes in #3973 (so that the features are available via the indexed ActionState).

): string | undefined {
const input = actions.getOptionalInput("config-file");

Expand All @@ -24,10 +25,17 @@ export function getConfigFileInput(
repositoryProperties[RepositoryPropertyName.CONFIG_FILE];

if (propertyValue !== undefined && propertyValue.trim().length > 0) {
logger.info(
`Using configuration file input from repository property: ${propertyValue}`,
);
return propertyValue;
// Only use the repository property value if the FF is enabled.
if (useRepositoryProperty) {
logger.info(
`Using configuration file input from repository property: ${propertyValue}`,
);
return propertyValue;
} else {
logger.info(
"Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.",
);
}
}

return undefined;
Expand Down
7 changes: 7 additions & 0 deletions src/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export enum Feature {
AllowMultipleAnalysisKinds = "allow_multiple_analysis_kinds",
AllowToolcacheInput = "allow_toolcache_input",
CleanupTrapCaches = "cleanup_trap_caches",
/** Whether to allow the `config-file` input to be specified via a repository property. */
ConfigFileRepositoryProperty = "config_file_repository_property",
CppDependencyInstallation = "cpp_dependency_installation_enabled",
CsharpCacheBuildModeNone = "csharp_cache_bmn",
CsharpNewCacheKey = "csharp_new_cache_key",
Expand Down Expand Up @@ -184,6 +186,11 @@ export const featureConfig = {
envVar: "CODEQL_ACTION_CLEANUP_TRAP_CACHES",
minimumVersion: undefined,
},
[Feature.ConfigFileRepositoryProperty]: {
defaultValue: false,
envVar: "CODEQL_ACTION_CONFIG_FILE_REPOSITORY_PROPERTY",
minimumVersion: undefined,
},
[Feature.CppDependencyInstallation]: {
defaultValue: false,
envVar: "CODEQL_EXTRACTOR_CPP_AUTOINSTALL_DEPENDENCIES",
Expand Down
10 changes: 9 additions & 1 deletion src/init-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,15 @@ async function run(startedAt: Date) {

core.exportVariable(EnvVar.INIT_ACTION_HAS_RUN, "true");

configFile = getConfigFileInput(logger, actionsEnv, repositoryProperties);
const useConfigFileProperty = await features.getValue(
Feature.ConfigFileRepositoryProperty,
);
configFile = getConfigFileInput(
logger,
actionsEnv,
repositoryProperties,
useConfigFileProperty,
);

// path.resolve() respects the intended semantics of source-root. If
// source-root is relative, it is relative to the GITHUB_WORKSPACE. If
Expand Down
Loading