Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Don't install deps by default for all users #2031

Merged
merged 14 commits into from Jan 5, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ Note that the only difference between `v2` and `v3` of the CodeQL Action is the

## [UNRELEASED]

- We are rolling out a feature in January 2024 that will disable Python dependency installation by default for all users. This improves the speed of analysis while having only a very minor impact on results. You can override this behavior by setting `CODEQL_ACTION_DISABLE_PYTHON_DEPENDENCY_INSTALLATION=false` in your workflow, however we plan to remove this ability in future versions of the CodeQL Action. [#2031](https://github.com/github/codeql-action/pull/2031)
Copy link
Contributor

Choose a reason for hiding this comment

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

[Non-blocking] Currently we disable Python dependency installation for CodeQL v2.16.0 and later, however the Action supports CodeQL versions all the way back to 2.11.6. Do we plan to keep supporting Python dependency installation for old CLIs in the Action until support for v2.15.5 is deprecated in about a year from now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had hoped to be able delete the python-setup folder and all the logic for dependency installation soon, but let's discuss this aspect some more 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 We can always start applying this to earlier CLI versions later on. Happy to discuss!

- The CodeQL Action now requires CodeQL version 2.11.6 or later. For more information, see [the corresponding changelog entry for CodeQL Action version 2.22.7](#2227---16-nov-2023). [#2009](https://github.com/github/codeql-action/pull/2009)

## 3.22.12 - 22 Dec 2023
Expand Down
3 changes: 2 additions & 1 deletion lib/analyze.js

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

2 changes: 1 addition & 1 deletion lib/analyze.js.map

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions lib/feature-flags.js

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

2 changes: 1 addition & 1 deletion lib/feature-flags.js.map

Large diffs are not rendered by default.

9 changes: 7 additions & 2 deletions lib/init-action.js

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

2 changes: 1 addition & 1 deletion lib/init-action.js.map

Large diffs are not rendered by default.

8 changes: 6 additions & 2 deletions src/analyze.ts
Expand Up @@ -105,10 +105,14 @@ async function setupPythonExtractor(
}

if (
await features.getValue(
(await features.getValue(
Feature.DisablePythonDependencyInstallationEnabled,
codeql,
)
)) ||
(await features.getValue(
Feature.PythonDefaultIsToSkipDependencyInstallationEnabled,
codeql,
))
henrymercer marked this conversation as resolved.
Show resolved Hide resolved
) {
logger.warning(
"We recommend that you remove the CODEQL_PYTHON environment variable from your workflow. This environment variable was originally used to specify a Python executable that included the dependencies of your Python code, however Python analysis no longer uses these dependencies." +
Expand Down
10 changes: 10 additions & 0 deletions src/feature-flags.ts
Expand Up @@ -49,6 +49,7 @@ export enum Feature {
CppDependencyInstallation = "cpp_dependency_installation_enabled",
DisableKotlinAnalysisEnabled = "disable_kotlin_analysis_enabled",
DisablePythonDependencyInstallationEnabled = "disable_python_dependency_installation_enabled",
PythonDefaultIsToSkipDependencyInstallationEnabled = "python_default_is_to_skip_dependency_installation_enabled",
EvaluatorFineGrainedParallelismEnabled = "evaluator_fine_grained_parallelism_enabled",
ExportDiagnosticsEnabled = "export_diagnostics_enabled",
QaTelemetryEnabled = "qa_telemetry_enabled",
Expand Down Expand Up @@ -103,6 +104,15 @@ export const featureConfig: Record<
minimumVersion: undefined,
defaultValue: false,
},
[Feature.PythonDefaultIsToSkipDependencyInstallationEnabled]: {
// we can reuse the same environment variable as above. If someone has set it to
// `true` in their workflow this means dependencies are not installed, setting it to
// `false` means dependencies _will_ be installed. The same semantics are applied
// here!
envVar: "CODEQL_ACTION_DISABLE_PYTHON_DEPENDENCY_INSTALLATION",
minimumVersion: "2.16.0",
defaultValue: false,
},
};

/**
Expand Down
21 changes: 17 additions & 4 deletions src/init-action.ts
Expand Up @@ -294,10 +294,14 @@ async function run() {
getRequiredInput("setup-python-dependencies") === "true"
) {
if (
await features.getValue(
(await features.getValue(
Feature.DisablePythonDependencyInstallationEnabled,
codeql,
)
)) ||
(await features.getValue(
Feature.PythonDefaultIsToSkipDependencyInstallationEnabled,
codeql,
))
) {
logger.info("Skipping python dependency installation");
} else {
Expand Down Expand Up @@ -447,15 +451,24 @@ async function run() {

// Disable Python dependency extraction if feature flag set
if (
await features.getValue(
(await features.getValue(
Feature.DisablePythonDependencyInstallationEnabled,
codeql,
)
)) ||
(await features.getValue(
Feature.PythonDefaultIsToSkipDependencyInstallationEnabled,
codeql,
))
) {
core.exportVariable(
"CODEQL_EXTRACTOR_PYTHON_DISABLE_LIBRARY_EXTRACTION",
"true",
);
} else {
core.exportVariable(
"CODEQL_EXTRACTOR_PYTHON_FORCE_ENABLE_LIBRARY_EXTRACTION_UNTIL_2_17_0",
"true",
);
henrymercer marked this conversation as resolved.
Show resolved Hide resolved
}

const sourceRoot = path.resolve(
Expand Down