From fda8ec42a39392fe7fa7af79de84139d325419f0 Mon Sep 17 00:00:00 2001 From: Arseniy Pendryak Date: Mon, 19 Apr 2021 15:57:01 +0300 Subject: [PATCH] CARGO: drop `org.rust.cargo.fetch.out.dir` feature The feature was initial attempt to support `include` macro with `OUT_DIR` variable. It has two major issues: - invocation of Cargo with `--build-plan` option force Cargo to recompile everything from the scratch next time - the option is supposed to be [removed](https://github.com/rust-lang/cargo/issues/7614). Also, there is 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 --- .../cargo/toolchain/impl/CargoMetadata.kt | 34 +- .../org/rust/cargo/toolchain/tools/Cargo.kt | 34 +- .../org/rust/ide/experiments/RsExperiments.kt | 1 - .../META-INF/experiments.xml | 3 - .../resources-stable/META-INF/experiments.xml | 3 - .../resolve/CargoGeneratedItemsResolveTest.kt | 331 +++++++----------- 6 files changed, 126 insertions(+), 280 deletions(-) diff --git a/src/main/kotlin/org/rust/cargo/toolchain/impl/CargoMetadata.kt b/src/main/kotlin/org/rust/cargo/toolchain/impl/CargoMetadata.kt index 6a9fa2ac13b..25fb0e32104 100644 --- a/src/main/kotlin/org/rust/cargo/toolchain/impl/CargoMetadata.kt +++ b/src/main/kotlin/org/rust/cargo/toolchain/impl/CargoMetadata.kt @@ -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 -> @@ -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) { @@ -341,7 +339,6 @@ object CargoMetadata { private fun Package.clean( fs: LocalFileSystem, isWorkspaceMember: Boolean, - variables: PackageVariables, enabledFeatures: Set, buildMessages: List ): CargoWorkspaceData.Package { @@ -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 } @@ -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>) { - - 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>() - 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) -} diff --git a/src/main/kotlin/org/rust/cargo/toolchain/tools/Cargo.kt b/src/main/kotlin/org/rust/cargo/toolchain/tools/Cargo.kt index 2c18137f12e..e3399f04c1e 100644 --- a/src/main/kotlin/org/rust/cargo/toolchain/tools/Cargo.kt +++ b/src/main/kotlin/org/rust/cargo/toolchain/tools/Cargo.kt @@ -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) @@ -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. @@ -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 diff --git a/src/main/kotlin/org/rust/ide/experiments/RsExperiments.kt b/src/main/kotlin/org/rust/ide/experiments/RsExperiments.kt index 7ff25f7b019..4ac14ca91a0 100644 --- a/src/main/kotlin/org/rust/ide/experiments/RsExperiments.kt +++ b/src/main/kotlin/org/rust/ide/experiments/RsExperiments.kt @@ -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" diff --git a/src/main/resources-nightly/META-INF/experiments.xml b/src/main/resources-nightly/META-INF/experiments.xml index 71af3bb2ebc..13fb75b384f 100644 --- a/src/main/resources-nightly/META-INF/experiments.xml +++ b/src/main/resources-nightly/META-INF/experiments.xml @@ -6,9 +6,6 @@ Show test results in Test Tool Window - - Fetch `OUT_DIR` environment variable value. It's needed to make proper name resolution for code included via `include` macro - Run build scripts while project refresh. It allows collecting information about generated items like `cfg` options, environment variables, etc diff --git a/src/main/resources-stable/META-INF/experiments.xml b/src/main/resources-stable/META-INF/experiments.xml index ab31649c50a..c961622ac9d 100644 --- a/src/main/resources-stable/META-INF/experiments.xml +++ b/src/main/resources-stable/META-INF/experiments.xml @@ -6,9 +6,6 @@ Show test results in Test Tool Window - - Fetch `OUT_DIR` environment variable value. It's needed to make proper name resolution for code included via `include` macro - Run build scripts while project refresh. It allows collecting information about generated items like `cfg` options, environment variables, etc diff --git a/src/test/kotlin/org/rustSlowTests/lang/resolve/CargoGeneratedItemsResolveTest.kt b/src/test/kotlin/org/rustSlowTests/lang/resolve/CargoGeneratedItemsResolveTest.kt index ff4418be699..d3ae8dde7b1 100644 --- a/src/test/kotlin/org/rustSlowTests/lang/resolve/CargoGeneratedItemsResolveTest.kt +++ b/src/test/kotlin/org/rustSlowTests/lang/resolve/CargoGeneratedItemsResolveTest.kt @@ -8,9 +8,6 @@ package org.rustSlowTests.lang.resolve import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.registry.Registry import com.intellij.testFramework.fixtures.impl.TempDirTestFixtureImpl -import org.rust.MinRustcVersion -import org.rust.UseOldResolve -import org.rust.cargo.toolchain.tools.Cargo import org.rust.fileTree import org.rust.ide.experiments.RsExperiments import org.rust.lang.core.psi.RsPath @@ -35,9 +32,7 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { // because it leads to too long path and compilation of test rust project fails on Windows override fun shouldContainTempFiles(): Boolean = false - @UseOldResolve - @MinRustcVersion("1.48.0") - fun `test include in workspace project`() = withEnabledFetchOutDirFeature { + fun `test include in workspace project`() = withEnabledEvaluateBuildScriptsFeature { val testProject = buildProject { toml("Cargo.toml", """ [package] @@ -73,18 +68,11 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { } """) } - } - buildProject() - - runWithInvocationEventsDispatching("Failed to resolve the reference") { - testProject.findElementInFile("src/main.rs").reference?.resolve() != null - } + }.checkReferenceIsResolved("src/main.rs") } // https://github.com/intellij-rust/intellij-rust/issues/4579 - @UseOldResolve - @MinRustcVersion("1.48.0") - fun `test do not overflow stack 1`() = withEnabledFetchOutDirFeature { + fun `test do not overflow stack 1`() = withEnabledEvaluateBuildScriptsFeature { val testProject = buildProject { toml("Cargo.toml", """ [package] @@ -120,18 +108,11 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { } """) } - } - buildProject() - - runWithInvocationEventsDispatching("Failed to resolve the reference") { - testProject.findElementInFile("src/main.rs").reference?.resolve() != null - } + }.checkReferenceIsResolved("src/main.rs") } // https://github.com/intellij-rust/intellij-rust/issues/4579 - @UseOldResolve - @MinRustcVersion("1.48.0") - fun `test do not overflow stack 2`() = withEnabledFetchOutDirFeature { + fun `test do not overflow stack 2`() = withEnabledEvaluateBuildScriptsFeature { val testProject = buildProject { toml("Cargo.toml", """ [workspace] @@ -201,43 +182,10 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { """) } } - } - buildProject() - - runWithInvocationEventsDispatching("Failed to resolve the reference") { - testProject.findElementInFile("intellij-rust-test-1/src/lib.rs").reference?.resolve() != null - } - } - - @UseOldResolve - @MinRustcVersion("1.48.0") - fun `test include in dependency`() = withEnabledFetchOutDirFeature { - val testProject = buildProject { - toml("Cargo.toml", """ - [package] - name = "intellij-rust-test" - version = "0.1.0" - authors = [] - - [dependencies] - code-generation-example = "0.1.0" - """) - dir("src") { - rust("lib.rs", """ - fn main() { - println!("{}", code_generation_example::message()); - //^ - } - """) - } - } - buildProject() - runWithInvocationEventsDispatching("Failed to resolve the reference") { - testProject.findElementInFile("src/lib.rs").reference?.resolve() != null - } + }.checkReferenceIsResolved("intellij-rust-test-1/src/lib.rs") } - fun `test include with build script info`() = withEnabledEvaluateBuildScriptsFeature { + fun `test include in dependency`() = withEnabledEvaluateBuildScriptsFeature { buildProject { toml("Cargo.toml", """ [package] @@ -283,32 +231,6 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { }.checkReferenceIsResolved("src/lib.rs") } - fun `test do not run build-plan if build script info is enough`() = withEnabledFetchOutDirFeature { - withEnabledEvaluateBuildScriptsFeature { - Cargo.Testmarks.fetchBuildPlan.checkNotHit { - buildProject { - toml("Cargo.toml", """ - [package] - name = "intellij-rust-test" - version = "0.1.0" - authors = [] - - [dependencies] - code-generation-example = "0.1.0" - """) - dir("src") { - rust("lib.rs", """ - fn main() { - println!("{}", code_generation_example::message()); - //^ - } - """) - } - }.checkReferenceIsResolved("src/lib.rs") - } - } - } - fun `test generated cfg option`() = withEnabledEvaluateBuildScriptsFeature { val libraryDir = tempDirFixture.getFile(".")!! val library = fileTree { @@ -674,145 +596,139 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { }.checkReferenceIsResolved("src/main.rs", toFile = ".../foo/bar/hello.rs") } - fun `test generated environment variables 2`() = withEnabledFetchOutDirFeature { - withEnabledEvaluateBuildScriptsFeature { - buildProject { - toml("Cargo.toml", """ - [package] - name = "intellij-rust-test" - version = "0.1.0" - authors = [] - """) - - dir("src") { - rust("main.rs", """ - include!(concat!(env!("GENERATED_ENV_DIR"), "/hello.rs")); - fn main() { - println!("{}", message()); - //^ - } - """) - } - rust("build.rs", """ - use std::env; - use std::fs; - use std::fs::File; - use std::io::Write; - use std::path::Path; + fun `test generated environment variables 2`() = withEnabledEvaluateBuildScriptsFeature { + buildProject { + toml("Cargo.toml", """ + [package] + name = "intellij-rust-test" + version = "0.1.0" + authors = [] + """) + dir("src") { + rust("main.rs", """ + include!(concat!(env!("GENERATED_ENV_DIR"), "/hello.rs")); fn main() { - let out_dir = env::var("OUT_DIR").unwrap(); - let gen_dir = Path::new(&out_dir).join("gen"); - if !gen_dir.exists() { - fs::create_dir(&gen_dir).unwrap(); - } - let dest_path = gen_dir.join("hello.rs"); - generate_file(&dest_path, b" - pub fn message() -> &'static str { - \"Hello, World!\" - } - "); - println!("cargo:rustc-env=GENERATED_ENV_DIR={}", gen_dir.display()); - } - - fn generate_file>(path: P, text: &[u8]) { - let mut f = File::create(path).unwrap(); - f.write_all(text).unwrap() + println!("{}", message()); + //^ } """) - }.checkReferenceIsResolved("src/main.rs", toFile = ".../gen/hello.rs") - } - } - - fun `test include without file name in literal`() = withEnabledFetchOutDirFeature { - withEnabledEvaluateBuildScriptsFeature { - buildProject { - toml("Cargo.toml", """ - [package] - name = "intellij-rust-test" - version = "0.1.0" - authors = [] - """) + } + rust("build.rs", """ + use std::env; + use std::fs; + use std::fs::File; + use std::io::Write; + use std::path::Path; - dir("src") { - rust("main.rs", """ - include!(env!("GENERATED_ENV_FILE")); - fn main() { - println!("{}", message()); - //^ + fn main() { + let out_dir = env::var("OUT_DIR").unwrap(); + let gen_dir = Path::new(&out_dir).join("gen"); + if !gen_dir.exists() { + fs::create_dir(&gen_dir).unwrap(); + } + let dest_path = gen_dir.join("hello.rs"); + generate_file(&dest_path, b" + pub fn message() -> &'static str { + \"Hello, World!\" } - """) + "); + println!("cargo:rustc-env=GENERATED_ENV_DIR={}", gen_dir.display()); } - rust("build.rs", """ - use std::env; - use std::fs; - use std::fs::File; - use std::io::Write; - use std::path::Path; - fn main() { - let out_dir = env::var("OUT_DIR").unwrap(); - let gen_dir = Path::new(&out_dir).join("gen"); - if !gen_dir.exists() { - fs::create_dir(&gen_dir).unwrap(); - } - let dest_path = gen_dir.join("hello.rs"); - generate_file(&dest_path, b" - pub fn message() -> &'static str { - \"Hello, World!\" - } - "); - println!("cargo:rustc-env=GENERATED_ENV_FILE={}", dest_path.display()); - } - - fn generate_file>(path: P, text: &[u8]) { - let mut f = File::create(path).unwrap(); - f.write_all(text).unwrap() - } - """) - }.checkReferenceIsResolved("src/main.rs", toFile = ".../gen/hello.rs") - } + fn generate_file>(path: P, text: &[u8]) { + let mut f = File::create(path).unwrap(); + f.write_all(text).unwrap() + } + """) + }.checkReferenceIsResolved("src/main.rs", toFile = ".../gen/hello.rs") } - fun `test include without file name in literal 2`() = withEnabledFetchOutDirFeature { - withEnabledEvaluateBuildScriptsFeature { - buildProject { - toml("Cargo.toml", """ - [package] - name = "intellij-rust-test" - version = "0.1.0" - authors = [] + fun `test include without file name in literal`() = withEnabledEvaluateBuildScriptsFeature { + buildProject { + toml("Cargo.toml", """ + [package] + name = "intellij-rust-test" + version = "0.1.0" + authors = [] + """) + + dir("src") { + rust("main.rs", """ + include!(env!("GENERATED_ENV_FILE")); + fn main() { + println!("{}", message()); + //^ + } """) + } + rust("build.rs", """ + use std::env; + use std::fs; + use std::fs::File; + use std::io::Write; + use std::path::Path; - dir("src") { - rust("main.rs", """ - include!(concat!(env!("OUT_DIR"), "/hello", ".rs")); - fn main() { - println!("{}", message()); - //^ + fn main() { + let out_dir = env::var("OUT_DIR").unwrap(); + let gen_dir = Path::new(&out_dir).join("gen"); + if !gen_dir.exists() { + fs::create_dir(&gen_dir).unwrap(); + } + let dest_path = gen_dir.join("hello.rs"); + generate_file(&dest_path, b" + pub fn message() -> &'static str { + \"Hello, World!\" } - """) + "); + println!("cargo:rustc-env=GENERATED_ENV_FILE={}", dest_path.display()); } - rust("build.rs", """ - use std::env; - use std::fs::File; - use std::io::Write; - use std::path::Path; - fn main() { - let out_dir = env::var("OUT_DIR").unwrap(); - let dest_path = Path::new(&out_dir).join("hello.rs"); - let mut f = File::create(&dest_path).unwrap(); + fn generate_file>(path: P, text: &[u8]) { + let mut f = File::create(path).unwrap(); + f.write_all(text).unwrap() + } + """) + }.checkReferenceIsResolved("src/main.rs", toFile = ".../gen/hello.rs") + } - f.write_all(b" - pub fn message() -> &'static str { - \"Hello, World!\" - }", - ).unwrap(); + fun `test include without file name in literal 2`() = withEnabledEvaluateBuildScriptsFeature { + buildProject { + toml("Cargo.toml", """ + [package] + name = "intellij-rust-test" + version = "0.1.0" + authors = [] + """) + + dir("src") { + rust("main.rs", """ + include!(concat!(env!("OUT_DIR"), "/hello", ".rs")); + fn main() { + println!("{}", message()); + //^ } """) - }.checkReferenceIsResolved("src/main.rs", toFile = ".../hello.rs") - } + } + rust("build.rs", """ + use std::env; + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn main() { + let out_dir = env::var("OUT_DIR").unwrap(); + let dest_path = Path::new(&out_dir).join("hello.rs"); + let mut f = File::create(&dest_path).unwrap(); + + f.write_all(b" + pub fn message() -> &'static str { + \"Hello, World!\" + }", + ).unwrap(); + } + """) + }.checkReferenceIsResolved("src/main.rs", toFile = ".../hello.rs") } fun `test do not fail on compilation error`() = withEnabledEvaluateBuildScriptsFeature { @@ -839,9 +755,6 @@ class CargoGeneratedItemsResolveTest : RunConfigurationTestBase() { }.checkReferenceIsResolved("src/main.rs", toFile = ".../src/bar.rs") } - private fun withEnabledFetchOutDirFeature(action: () -> Unit) = - runWithEnabledFeatures(RsExperiments.FETCH_OUT_DIR, action = action) - private fun withEnabledEvaluateBuildScriptsFeature(action: () -> Unit) = runWithEnabledFeatures(RsExperiments.EVALUATE_BUILD_SCRIPTS, action = action) }