Skip to content

Commit

Permalink
CARGO: drop org.rust.cargo.fetch.out.dir feature
Browse files Browse the repository at this point in the history
The feature was an initial attempt to support `include` macro with `OUT_DIR` variable.
It has two major issues:
- invocation of Cargo with `--build-plan` option forces Cargo to recompile everything from the scratch next time
- the option is supposed to be [removed](rust-lang/cargo#7614).

Also, there is a more powerful feature - `org.rust.cargo.evaluate.build.scripts` that allows plugin to know about all generated `cfg` options and environment variables (including `OUT_DIR`).

So I think we can drop `--build-plan` support since it has major cons and can be superseded by `org.rust.cargo.evaluate.build.scripts` feature
  • Loading branch information
Undin authored and 0x7CA committed Apr 19, 2021
1 parent 5314113 commit b4c24b7
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 280 deletions.
34 changes: 3 additions & 31 deletions src/main/kotlin/org/rust/cargo/toolchain/impl/CargoMetadata.kt
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,10 @@ object CargoMetadata {

fun clean(
project: Project,
buildMessages: BuildMessages? = null,
buildPlan: CargoBuildPlan? = null
buildMessages: BuildMessages? = null
): CargoWorkspaceData {
val fs = LocalFileSystem.getInstance()
val members = project.workspace_members
val variables = PackageVariables.from(buildPlan)
val packageIdToNode = project.resolve.nodes.associateBy { it.id }
return CargoWorkspaceData(
project.packages.map { pkg ->
Expand All @@ -319,7 +317,7 @@ object CargoMetadata {
}
val enabledFeatures = resolveNode?.features.orEmpty().toSet() // features enabled by Cargo
val pkgBuildMessages = buildMessages?.get(pkg.id).orEmpty()
pkg.clean(fs, pkg.id in members, variables, enabledFeatures, pkgBuildMessages)
pkg.clean(fs, pkg.id in members, enabledFeatures, pkgBuildMessages)
},
project.resolve.nodes.associate { node ->
val dependencySet = if (node.deps != null) {
Expand All @@ -341,7 +339,6 @@ object CargoMetadata {
private fun Package.clean(
fs: LocalFileSystem,
isWorkspaceMember: Boolean,
variables: PackageVariables,
enabledFeatures: Set<String>,
buildMessages: List<CompilerMessage>
): CargoWorkspaceData.Package {
Expand Down Expand Up @@ -389,8 +386,7 @@ object CargoMetadata {
"CARGO_CRATE_NAME" to name.replace('-', '_'),
)

val outDirPath = buildScriptMessage?.out_dir ?: variables.getOutDirPath(this)
val outDir = outDirPath
val outDir = buildScriptMessage?.out_dir
?.let { root.fileSystem.refreshAndFindFileByPath(it) }
?.let { if (isWorkspaceMember) it else it.canonicalFile }

Expand Down Expand Up @@ -534,27 +530,3 @@ object CargoMetadata {
private fun Target.replacePaths(replacer: (String) -> String): Target =
copy(src_path = replacer(src_path))
}

private class PackageVariables(private val variables: Map<PackageInfo, Map<String, String>>) {

fun getOutDirPath(pkg: CargoMetadata.Package): String? = getValue(pkg, "OUT_DIR")

fun getValue(pkg: CargoMetadata.Package, variableName: String): String? {
val key = PackageInfo(pkg.name, pkg.version)
return variables[key]?.get(variableName)
}

companion object {
fun from(buildPlan: CargoBuildPlan?): PackageVariables {
val result = mutableMapOf<PackageInfo, Map<String, String>>()
for ((packageName, packageVersion, compileMode, variables) in buildPlan?.invocations.orEmpty()) {
if (compileMode != "run-custom-build") continue
val targetInfo = PackageInfo(packageName, packageVersion)
result[targetInfo] = variables
}
return PackageVariables(result)
}
}

private data class PackageInfo(val name: String, val version: String)
}
34 changes: 1 addition & 33 deletions src/main/kotlin/org/rust/cargo/toolchain/tools/Cargo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,10 @@ open class Cargo(toolchain: RsToolchain, useWrapper: Boolean = false)
): CargoWorkspaceData {
val rawData = fetchMetadata(owner, projectDirectory, listener)
val buildScriptsInfo = fetchBuildScriptsInfo(owner, projectDirectory, listener)
val buildPlan = if (buildScriptsInfo?.containsOutDirInfo != true) {
fetchBuildPlan(owner, projectDirectory, listener)
} else {
null
}

val (rawDataAdjusted, buildScriptsInfoAdjusted) =
replacePathsSymlinkIfNeeded(rawData, buildScriptsInfo, projectDirectory)
return CargoMetadata.clean(rawDataAdjusted, buildScriptsInfoAdjusted, buildPlan)
return CargoMetadata.clean(rawDataAdjusted, buildScriptsInfoAdjusted)
}

@Throws(ExecutionException::class)
Expand Down Expand Up @@ -216,29 +211,6 @@ open class Cargo(toolchain: RsToolchain, useWrapper: Boolean = false)
return BuildMessages(messages)
}

private fun fetchBuildPlan(
owner: Project,
projectDirectory: Path,
listener: ProcessListener?
): CargoBuildPlan? {
if (!isFeatureEnabled(RsExperiments.FETCH_OUT_DIR)) return null
Testmarks.fetchBuildPlan.hit()
val additionalArgs = mutableListOf("-Z", "unstable-options", "--all-targets", "--build-plan")
// Hack to make cargo think that unstable options are available because we need unstable `--build-plan` option here
val envs = EnvironmentVariablesData.create(mapOf(
RUSTC_BOOTSTRAP to "1"
), true)
return try {
val json = CargoCommandLine("build", projectDirectory, additionalArgs, environmentVariables = envs)
.execute(owner, listener = listener)
.stdout
Gson().fromJson(json, CargoBuildPlan::class.java)
} catch (e: Exception) {
LOG.warn("Failed to fetch build-plan", e)
null
}
}

/**
* If project directory is 'source' which is a symlink to 'target' directory,
* then `cargo metadata` command inside 'source' directory produces output with 'target' paths.
Expand Down Expand Up @@ -587,10 +559,6 @@ open class Cargo(toolchain: RsToolchain, useWrapper: Boolean = false)
return needInstall
}
}

object Testmarks {
val fetchBuildPlan = Testmark("fetchBuildPlan")
}
}

// Note: the class is used as a cache key, so it must be immutable and must have a correct equals/hashCode
Expand Down
1 change: 0 additions & 1 deletion src/main/kotlin/org/rust/ide/experiments/RsExperiments.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package org.rust.ide.experiments
object RsExperiments {
const val BUILD_TOOL_WINDOW = "org.rust.cargo.build.tool.window"
const val TEST_TOOL_WINDOW = "org.rust.cargo.test.tool.window"
const val FETCH_OUT_DIR = "org.rust.cargo.fetch.out.dir"
const val EVALUATE_BUILD_SCRIPTS = "org.rust.cargo.evaluate.build.scripts"
const val CARGO_FEATURES_SETTINGS_GUTTER = "org.rust.cargo.features.settings.gutter"
const val MACROS_NEW_ENGINE = "org.rust.macros.new.engine"
Expand Down
3 changes: 0 additions & 3 deletions src/main/resources-nightly/META-INF/experiments.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
<experimentalFeature id="org.rust.cargo.test.tool.window" percentOfUsers="100">
<description>Show test results in Test Tool Window</description>
</experimentalFeature>
<experimentalFeature id="org.rust.cargo.fetch.out.dir" percentOfUsers="0">
<description>Fetch `OUT_DIR` environment variable value. It's needed to make proper name resolution for code included via `include` macro</description>
</experimentalFeature>
<experimentalFeature id="org.rust.cargo.evaluate.build.scripts" percentOfUsers="0">
<description>Run build scripts while project refresh. It allows collecting information about generated items like `cfg` options, environment variables, etc</description>
</experimentalFeature>
Expand Down
3 changes: 0 additions & 3 deletions src/main/resources-stable/META-INF/experiments.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
<experimentalFeature id="org.rust.cargo.test.tool.window" percentOfUsers="100">
<description>Show test results in Test Tool Window</description>
</experimentalFeature>
<experimentalFeature id="org.rust.cargo.fetch.out.dir" percentOfUsers="0">
<description>Fetch `OUT_DIR` environment variable value. It's needed to make proper name resolution for code included via `include` macro</description>
</experimentalFeature>
<experimentalFeature id="org.rust.cargo.evaluate.build.scripts" percentOfUsers="0">
<description>Run build scripts while project refresh. It allows collecting information about generated items like `cfg` options, environment variables, etc</description>
</experimentalFeature>
Expand Down
Loading

0 comments on commit b4c24b7

Please sign in to comment.