-
Notifications
You must be signed in to change notification settings - Fork 376
Rust: remove shipped feature flag #2960
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
Changes from 11 commits
3478646
662cec8
a58e7d8
3e4d856
6802597
8d19b24
068f150
67812dd
68da2c5
bfa52a8
aa456a5
a9c4652
486a50d
df1ceac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,6 @@ import { | |
getRequiredInput, | ||
getTemporaryDirectory, | ||
persistInputs, | ||
isDefaultSetup, | ||
} from "./actions-util"; | ||
import { getGitHubVersion } from "./api-client"; | ||
import { | ||
|
@@ -32,7 +31,7 @@ import { | |
makeDiagnostic, | ||
} from "./diagnostics"; | ||
import { EnvVar } from "./environment"; | ||
import { Feature, featureConfig, Features } from "./feature-flags"; | ||
import { Feature, Features } from "./feature-flags"; | ||
import { | ||
checkInstallPython311, | ||
checkPacksForOverlayCompatibility, | ||
|
@@ -360,37 +359,29 @@ async function run() { | |
} | ||
core.endGroup(); | ||
|
||
// Set CODEQL_ENABLE_EXPERIMENTAL_FEATURES for Rust. We need to set this environment | ||
// variable before initializing the config, otherwise Rust analysis will not be | ||
// enabled. | ||
// Set CODEQL_ENABLE_EXPERIMENTAL_FEATURES for Rust if between 2.19.3 (included) and 2.22.1 (excluded) | ||
// We need to set this environment variable before initializing the config, otherwise Rust | ||
// analysis will not be enabled (experimental language packs are only active with that environment | ||
// variable set to `true`). | ||
if ( | ||
// Only enable Rust analysis if the user has explicitly requested it - don't | ||
// enable it via language autodetection. | ||
// Only enable the experimental features env variable for Rust analysis if the user has explicitly | ||
// requested rust - don't enable it via language autodetection. | ||
configUtils | ||
.getRawLanguagesNoAutodetect(getOptionalInput("languages")) | ||
.includes(KnownLanguage.rust) | ||
) { | ||
const feat = Feature.RustAnalysis; | ||
const minVer = featureConfig[feat].minimumVersion as string; | ||
const envVar = "CODEQL_ENABLE_EXPERIMENTAL_FEATURES"; | ||
// if in default setup, it means the feature flag was on when rust was enabled | ||
// if the feature flag gets turned off, let's not have rust analysis throwing a configuration error | ||
// in that case rust analysis will be disabled only when default setup is refreshed | ||
if (isDefaultSetup() || (await features.getValue(feat, codeql))) { | ||
core.exportVariable(envVar, "true"); | ||
} | ||
if (process.env[envVar] !== "true") { | ||
throw new ConfigurationError( | ||
`Experimental and not officially supported Rust analysis requires setting ${envVar}=true in the environment`, | ||
); | ||
} | ||
const experimental = "2.19.3"; | ||
const publicPreview = "2.22.1"; | ||
const actualVer = (await codeql.getVersion()).version; | ||
if (semver.lt(actualVer, minVer)) { | ||
if (semver.lt(actualVer, experimental)) { | ||
throw new ConfigurationError( | ||
`Experimental rust analysis is supported by CodeQL CLI version ${minVer} or higher, but found version ${actualVer}`, | ||
`Rust analysis is supported by CodeQL CLI version ${experimental} or higher, but found version ${actualVer}`, | ||
); | ||
} | ||
logger.info("Experimental rust analysis enabled"); | ||
if (semver.lt(actualVer, publicPreview)) { | ||
core.exportVariable(EnvVar.EXPERIMENTAL_FEATURES, "true"); | ||
logger.info("Experimental rust analysis enabled"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Should "rust" be capitalised here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense |
||
} | ||
} | ||
|
||
config = await initConfig({ | ||
|
@@ -665,34 +656,6 @@ async function run() { | |
core.exportVariable(bmnVar, value); | ||
} | ||
|
||
// For rust: set CODEQL_ENABLE_EXPERIMENTAL_FEATURES, unless codeql already supports rust without it | ||
if ( | ||
config.languages.includes(KnownLanguage.rust) && | ||
!(await codeql.resolveLanguages()).rust | ||
) { | ||
const feat = Feature.RustAnalysis; | ||
const minVer = featureConfig[feat].minimumVersion as string; | ||
const envVar = "CODEQL_ENABLE_EXPERIMENTAL_FEATURES"; | ||
// if in default setup, it means the feature flag was on when rust was enabled | ||
// if the feature flag gets turned off, let's not have rust analysis throwing a configuration error | ||
// in that case rust analysis will be disabled only when default setup is refreshed | ||
if (isDefaultSetup() || (await features.getValue(feat, codeql))) { | ||
core.exportVariable(envVar, "true"); | ||
} | ||
if (process.env[envVar] !== "true") { | ||
throw new ConfigurationError( | ||
`Experimental and not officially supported Rust analysis requires setting ${envVar}=true in the environment`, | ||
); | ||
} | ||
const actualVer = (await codeql.getVersion()).version; | ||
if (semver.lt(actualVer, minVer)) { | ||
throw new ConfigurationError( | ||
`Experimental rust analysis is supported by CodeQL CLI version ${minVer} or higher, but found version ${actualVer}`, | ||
); | ||
} | ||
logger.info("Experimental rust analysis enabled"); | ||
} | ||
|
||
// Restore dependency cache(s), if they exist. | ||
if (shouldRestoreCache(config.dependencyCachingEnabled)) { | ||
await downloadDependencyCaches(config.languages, logger); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha, well, I should have double checked what copilot was writing here. Good thing someone suggested to put in more testing 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this was causing a failure in the 2.19.3 case, which is now fixed)