From 16ea4dd2dbd1eab741305b848be9fe7e60afed4a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:29:27 +0800 Subject: [PATCH] fix: preserve CMake arguments and reuse configured builds --- Cargo.lock | 3 +- crates/ide-core/src/settings.rs | 26 +++++-- crates/ide-run/Cargo.toml | 1 - crates/ide-run/src/detect/cmake.rs | 27 ++++--- crates/ide-run/tests/detect.rs | 32 ++++++++- src/bindings.ts | 4 +- src/features/settings/__tests__/cmake.test.ts | 10 +++ .../settings/__tests__/cmakeEditor.test.tsx | 72 +++++++++++++++++++ .../settings/__tests__/dialog.test.tsx | 2 +- src/features/settings/pages/cmake.tsx | 54 ++++++++++---- src/platform/cmakeSettings.ts | 10 +-- src/platform/locales/text/zh.json | 1 - src/platform/locales/textCatalog.test.ts | 1 + 13 files changed, 202 insertions(+), 41 deletions(-) create mode 100644 src/features/settings/__tests__/cmakeEditor.test.tsx diff --git a/Cargo.lock b/Cargo.lock index ecc0a1061..6a87905c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4472,7 +4472,6 @@ dependencies = [ "quick-xml", "serde", "serde_json", - "shlex", "specta", "tempfile", "tokio", @@ -9069,7 +9068,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2", diff --git a/crates/ide-core/src/settings.rs b/crates/ide-core/src/settings.rs index 549d34c15..7c2c17a37 100644 --- a/crates/ide-core/src/settings.rs +++ b/crates/ide-core/src/settings.rs @@ -76,10 +76,10 @@ pub struct CMakeProfile { pub name: String, pub build_type: String, pub generator: String, - pub cmake_args: String, + pub cmake_args: Vec, /// Empty uses cmake-build-. pub build_dir: String, - pub build_args: String, + pub build_args: Vec, pub env: BTreeMap, } @@ -89,9 +89,9 @@ impl Default for CMakeProfile { name: "Debug".into(), build_type: "Debug".into(), generator: String::new(), - cmake_args: String::new(), + cmake_args: Vec::new(), build_dir: String::new(), - build_args: String::new(), + build_args: Vec::new(), env: BTreeMap::new(), } } @@ -803,6 +803,24 @@ impl Settings { mod tests { use super::*; + #[test] + fn cmake_profile_argument_arrays_round_trip() { + let mut settings = Settings::default(); + assert!(settings.cmake.profiles.is_none()); + assert!(CMakeProfile::default().cmake_args.is_empty()); + assert!(CMakeProfile::default().build_args.is_empty()); + let profile = CMakeProfile { + cmake_args: ["-DMESSAGE=two words", "-DQUOTE=\"hello\"", ""].map(String::from).into(), + build_args: ["-j", "8", "--", "LABEL=it's a build"].map(String::from).into(), + ..Default::default() + }; + settings.cmake.profiles = Some(vec![profile.clone()]); + let value = serde_json::to_value(&settings).unwrap(); + assert_eq!(value["cmake"]["profiles"][0]["cmakeArgs"], serde_json::json!(profile.cmake_args)); + assert_eq!(value["cmake"]["profiles"][0]["buildArgs"], serde_json::json!(profile.build_args)); + assert_eq!(serde_json::from_value::(value).unwrap(), settings); + } + #[test] fn ui_font_fallback_defaults_empty_and_round_trips() { let old: Settings = serde_json::from_str(r#"{"appearance":{"useCustomFont":true}}"#).unwrap(); diff --git a/crates/ide-run/Cargo.toml b/crates/ide-run/Cargo.toml index ce8d40adb..1e362ef3a 100644 --- a/crates/ide-run/Cargo.toml +++ b/crates/ide-run/Cargo.toml @@ -20,7 +20,6 @@ parking_lot.workspace = true tracing.workspace = true zip.workspace = true quick-xml = "0.42" -shlex = "2" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/crates/ide-run/src/detect/cmake.rs b/crates/ide-run/src/detect/cmake.rs index 80e5f3214..93abe0b98 100644 --- a/crates/ide-run/src/detect/cmake.rs +++ b/crates/ide-run/src/detect/cmake.rs @@ -90,10 +90,6 @@ pub(super) fn detect(root: &Path, profiles: Option<&[CMakeProfile]>, out: &mut V } } -fn arguments(value: &str) -> Vec { - shlex::split(value).unwrap_or_else(|| vec![value.to_owned()]) -} - fn detect_profile(root: &Path, profile: &CMakeProfile, index: usize, multiple: bool, out: &mut Vec) { let dir = if profile.build_dir.trim().is_empty() { format!("cmake-build-{}", profile.build_type.to_lowercase()) @@ -112,7 +108,7 @@ fn detect_profile(root: &Path, profile: &CMakeProfile, index: usize, multiple: b } configure.args.push(format!("-DCMAKE_BUILD_TYPE={}", profile.build_type)); configure.args.push("-DCMAKE_EXPORT_COMPILE_COMMANDS=ON".into()); - configure.args.extend(arguments(&profile.cmake_args)); + configure.args.extend_from_slice(&profile.cmake_args); configure.extras.params.insert("buildDir".into(), dir.clone()); out.push(configure); let mut all = config( @@ -122,8 +118,10 @@ fn detect_profile(root: &Path, profile: &CMakeProfile, index: usize, multiple: b "cmake", &["--build", &dir, "--config", &profile.build_type], ); - all.args.extend(arguments(&profile.build_args)); - before(&mut all, &configure_id); + all.args.extend_from_slice(&profile.build_args); + if !build.join("CMakeCache.txt").is_file() { + before(&mut all, &configure_id); + } out.push(all); let mut test = config( format!("{prefix}:test"), @@ -206,8 +204,10 @@ fn targets( if name.is_empty() { build_config.args.extend(["--config".into(), profile.build_type.clone()]); } - build_config.args.extend(arguments(&profile.build_args)); - before(&mut build_config, &format!("{prefix}:configure")); + build_config.args.extend_from_slice(&profile.build_args); + if !build.join("CMakeCache.txt").is_file() { + before(&mut build_config, &format!("{prefix}:configure")); + } } out.push(build_config); let binary = build.join(artifact); @@ -294,7 +294,7 @@ mod tests { name: "Production".into(), build_type: "Release".into(), build_dir: "output".into(), - build_args: "-j 8 -- verbose=1".into(), + build_args: ["-j", "8", "--", "verbose=1"].map(String::from).into(), env: [("FOO".into(), "bar".into())].into(), ..Default::default() }; @@ -311,6 +311,13 @@ mod tests { ); assert_eq!(build.env, profile.env); assert_eq!(build.extras.before_launch[0].target, "cmake:profile:0:configure"); + fs::write(root.path().join("output/CMakeCache.txt"), "CMAKE_GENERATOR:INTERNAL=Ninja\n").unwrap(); + let mut configured = Vec::new(); + detect(root.path(), Some(std::slice::from_ref(&profile)), &mut configured); + for id in ["cmake:profile:0:build", build.id.as_str()] { + let build = configured.iter().find(|c| c.id == id).unwrap(); + assert!(build.extras.before_launch.is_empty(), "{id} should use CMake's automatic regeneration"); + } } #[test] diff --git a/crates/ide-run/tests/detect.rs b/crates/ide-run/tests/detect.rs index d61bf70f0..578019334 100644 --- a/crates/ide-run/tests/detect.rs +++ b/crates/ide-run/tests/detect.rs @@ -662,9 +662,9 @@ fn cmake_profiles_control_commands_names_and_environment() { name: "Production".into(), build_type: "Release".into(), generator: generator.into(), - cmake_args: "-DFEATURE=ON '-DMESSAGE=two words'".into(), + cmake_args: ["-DFEATURE=ON", "-DMESSAGE=two words", "-DQUOTE=\"hello world\"", ""].map(String::from).into(), build_dir: "output dir".into(), - build_args: "-j 8 -- VERBOSE=1".into(), + build_args: ["-j", "8", "--", "VERBOSE=1"].map(String::from).into(), env: [("MY_ENV".into(), "yes".into())].into(), }; let configs = ide_run::detect_with_profiles(dir.path(), &[], Some(std::slice::from_ref(&profile))); @@ -678,6 +678,8 @@ fn cmake_profiles_control_commands_names_and_environment() { "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON", "-DFEATURE=ON", "-DMESSAGE=two words", + "-DQUOTE=\"hello world\"", + "", ]); assert_eq!(configure.args, expected); assert_eq!(configure.name, "CMake Configure (output dir)"); @@ -693,3 +695,29 @@ fn cmake_profiles_control_commands_names_and_environment() { assert_eq!(ide_run::detect_with_profiles(dir.path(), &[], None), detect(dir.path())); assert!(ide_run::detect_with_profiles(dir.path(), &[], Some(&[])).is_empty()); } + +#[test] +fn cmake_profile_build_configures_only_without_a_cache() { + use ide_core::settings::CMakeProfile; + let root = project(&[("CMakeLists.txt", "project(app)")]); + for dir in ["", "custom output"] { + let profile = CMakeProfile { build_dir: dir.into(), generator: "Ninja".into(), ..Default::default() }; + let build_dir = root.path().join(if dir.is_empty() { "cmake-build-debug" } else { dir }); + for cached in [false, true] { + if cached { + fs::create_dir_all(&build_dir).unwrap(); + // Generator changes are handled by an explicit Configure, not every Build. + fs::write(build_dir.join("CMakeCache.txt"), "CMAKE_GENERATOR:INTERNAL=Unix Makefiles\n").unwrap(); + } + let configs = ide_run::detect_with_profiles(root.path(), &[], Some(std::slice::from_ref(&profile))); + let build = find(&configs, "cmake:profile:0:build"); + if cached { + assert!(build.extras.before_launch.is_empty()); + } else { + assert_eq!(build.extras.before_launch.len(), 1); + assert_eq!(build.extras.before_launch[0].target, "cmake:profile:0:configure"); + } + assert_eq!(find(&configs, "cmake:profile:0:test").extras.before_launch[0].target, build.id); + } + } +} diff --git a/src/bindings.ts b/src/bindings.ts index a220a34d9..c5e1504f8 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -1643,10 +1643,10 @@ export type CMakeProfile = { name?: string, buildType?: string, generator?: string, - cmakeArgs?: string, + cmakeArgs?: string[], /** Empty uses cmake-build-. */ buildDir?: string, - buildArgs?: string, + buildArgs?: string[], env?: { [key in string]: string }, }; diff --git a/src/features/settings/__tests__/cmake.test.ts b/src/features/settings/__tests__/cmake.test.ts index d1c3ce3e6..bf5da3a79 100644 --- a/src/features/settings/__tests__/cmake.test.ts +++ b/src/features/settings/__tests__/cmake.test.ts @@ -7,6 +7,16 @@ describe("CMake settings", () => { expect(validateSettings({}).cmake).toEqual({}); expect(validateSettings({ cmake: { profiles: [] } }).cmake.profiles).toEqual([]); }); + it("preserves argument arrays and drops malformed argument values", () => { + const cmakeArgs = ["-DMESSAGE=two words", '-DQUOTE="hello"', ""]; + const buildArgs = ["-j", "8", "--", "LABEL=it's a build"]; + const profiles = validateSettings({ cmake: { profiles: [{ cmakeArgs, buildArgs }] } }).cmake.profiles; + expect(profiles?.[0]).toMatchObject({ cmakeArgs, buildArgs }); + expect( + validateSettings({ cmake: { profiles: [{ cmakeArgs: ["-DFOO=bar", 42, null] }] } }).cmake.profiles?.[0] + ?.cmakeArgs, + ).toEqual(["-DFOO=bar"]); + }); it("retains custom types and generators while normalizing malformed fields", () => { expect( validateSettings({ diff --git a/src/features/settings/__tests__/cmakeEditor.test.tsx b/src/features/settings/__tests__/cmakeEditor.test.tsx new file mode 100644 index 000000000..2d11a991b --- /dev/null +++ b/src/features/settings/__tests__/cmakeEditor.test.tsx @@ -0,0 +1,72 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, expect, it } from "vitest"; +import { DEFAULT_CMAKE_PROFILE } from "@/platform/cmakeSettings"; +import { parseArgs } from "@/platform/commandLine"; +import { PageContext } from "../components/context"; +import { validateSettings } from "../model/schema"; +import { useSettingsDialog } from "../model/staging"; +import { CMakeProfiles } from "../pages/cmake"; + +const savedDialog = useSettingsDialog.getState(); +afterEach(() => { + cleanup(); + useSettingsDialog.setState(savedDialog, true); +}); + +it.each([ + ["CMake arguments", "cmakeArgs"], + ["Build arguments", "buildArgs"], +] as const)("round-trips spaces, quotes and empty values in %s", async (label, key) => { + const args = ["two words", 'a "quote"', "it's", "", String.raw`C:\some path\file`]; + useSettingsDialog.setState({ + base: validateSettings({ cmake: { profiles: [{ ...DEFAULT_CMAKE_PROFILE, [key]: args }] } }), + staged: {}, + }); + const mount = () => + render( + + + , + ); + const view = mount(); + const input = screen.getByLabelText(label); + expect(parseArgs(input.value)).toEqual(args); + await userEvent.clear(input); + const text = String.raw`-DNAME="two words" "-DQUOTE=it's" ""`; + await userEvent.type(input, text); + expect(input).toHaveValue(text); + fireEvent.blur(input); + const expected = ["-DNAME=two words", "-DQUOTE=it's", ""]; + expect(parseArgs(input.value)).toEqual(expected); + expect(useSettingsDialog.getState().staged["project|cmake.profiles"]?.value).toEqual([ + { ...DEFAULT_CMAKE_PROFILE, [key]: expected }, + ]); + view.unmount(); + mount(); + expect(parseArgs(screen.getByLabelText(label).value)).toEqual(expected); +}); + +it("displays each profile's arguments when switching profiles", () => { + useSettingsDialog.setState({ + base: validateSettings({ + cmake: { + profiles: [ + { ...DEFAULT_CMAKE_PROFILE, name: "Debug" }, + { ...DEFAULT_CMAKE_PROFILE, name: "Release", cmakeArgs: ["-DMESSAGE=release build"] }, + ], + }, + }), + staged: {}, + }); + render( + + + , + ); + fireEvent.change(screen.getByLabelText("CMake arguments"), { target: { value: '"unfinished ' } }); + fireEvent.click(screen.getByRole("option", { name: "Release" })); + expect(parseArgs(screen.getByLabelText("CMake arguments").value)).toEqual([ + "-DMESSAGE=release build", + ]); +}); diff --git a/src/features/settings/__tests__/dialog.test.tsx b/src/features/settings/__tests__/dialog.test.tsx index 5bb275279..a72f47525 100644 --- a/src/features/settings/__tests__/dialog.test.tsx +++ b/src/features/settings/__tests__/dialog.test.tsx @@ -103,7 +103,7 @@ describe("Settings dialog", () => { expect(write[0]).toBe("project"); expect(JSON.parse(write[2])).toEqual([ expect.objectContaining({ name: "Debug", buildType: "Debug" }), - expect.objectContaining({ name: "Release", buildType: "Release", generator: "Ninja", buildArgs: "-j 8" }), + expect.objectContaining({ name: "Release", buildType: "Release", generator: "Ninja", buildArgs: ["-j", "8"] }), ]); }); diff --git a/src/features/settings/pages/cmake.tsx b/src/features/settings/pages/cmake.tsx index 8196b8628..6bdf008b0 100644 --- a/src/features/settings/pages/cmake.tsx +++ b/src/features/settings/pages/cmake.tsx @@ -1,5 +1,6 @@ import { useId, useState } from "react"; import { DEFAULT_CMAKE_PROFILE, type CMakeProfileValid } from "@/platform/cmakeSettings"; +import { formatArgs, parseArgs } from "@/platform/commandLine"; import { tx } from "@/platform/i18n"; import { IconButton, Input, T } from "@/ui"; import { Row, Section } from "../components/Form"; @@ -8,6 +9,22 @@ import { TableEditor } from "./workflowControls"; import styles from "./Workflow.module.css"; import own from "./CMake.module.css"; +function ArgumentsInput({ id, value, onChange }: { id: string; value: string[]; onChange: (args: string[]) => void }) { + const [text, setText] = useState(); + return ( + { + setText(e.target.value); + onChange(parseArgs(e.target.value)); + }} + onBlur={() => setText(undefined)} + /> + ); +} + export function CMakeProfiles() { const [saved, setProfiles] = useSetting("cmake.profiles"); const profiles = saved ?? [DEFAULT_CMAKE_PROFILE]; @@ -74,20 +91,29 @@ export function CMakeProfiles() {
{fields.map(([key, label]) => ( - update({ [key]: e.target.value })} - /> + {key === "cmakeArgs" || key === "buildArgs" ? ( + update({ [key]: args })} + /> + ) : ( + update({ [key]: e.target.value })} + /> + )} ))} diff --git a/src/platform/cmakeSettings.ts b/src/platform/cmakeSettings.ts index b6a1cd72d..6c4cd5edf 100644 --- a/src/platform/cmakeSettings.ts +++ b/src/platform/cmakeSettings.ts @@ -6,9 +6,9 @@ export const DEFAULT_CMAKE_PROFILE: CMakeProfileValid = { name: "Debug", buildType: "Debug", generator: "", - cmakeArgs: "", + cmakeArgs: [], buildDir: "", - buildArgs: "", + buildArgs: [], env: {}, }; @@ -16,6 +16,8 @@ export function validateCMake(raw: unknown): { profiles?: CMakeProfileValid[] } const profiles = (raw as { profiles?: unknown } | undefined)?.profiles; if (!Array.isArray(profiles)) return {}; const string = (value: unknown, fallback = "") => (typeof value === "string" ? value : fallback); + const args = (value: unknown): string[] => + Array.isArray(value) ? value.filter((arg): arg is string => typeof arg === "string") : []; return { profiles: profiles .filter((p) => p && typeof p === "object") @@ -23,9 +25,9 @@ export function validateCMake(raw: unknown): { profiles?: CMakeProfileValid[] } name: string(p.name, "Debug"), buildType: string(p.buildType, "Debug"), generator: string(p.generator), - cmakeArgs: string(p.cmakeArgs), + cmakeArgs: args(p.cmakeArgs), buildDir: string(p.buildDir), - buildArgs: string(p.buildArgs), + buildArgs: args(p.buildArgs), env: Object.fromEntries( Object.entries(p.env && typeof p.env === "object" ? p.env : {}).filter( (entry): entry is [string, string] => typeof entry[1] === "string", diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index 4b7dc185e..8c9fbafa7 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -5452,6 +5452,5 @@ "Build arguments": "构建参数", "CMake default": "CMake 默认", "Quote arguments containing spaces. Build arguments go to cmake --build; use -- before native build tool options.": "包含空格的参数请加引号。构建参数传给 cmake --build；原生构建工具参数前请加 --。", - "CMake": "CMake", "CMake profile {0}": "CMake 配置 {0}" } diff --git a/src/platform/locales/textCatalog.test.ts b/src/platform/locales/textCatalog.test.ts index 86841e022..59962b633 100644 --- a/src/platform/locales/textCatalog.test.ts +++ b/src/platform/locales/textCatalog.test.ts @@ -80,6 +80,7 @@ const BUILT_AT_RUNTIME = new Set([]); /** Text that stays as written in every locale: names, code, formats, shortcuts. */ const UNTRANSLATED = new Set([ // Product names + "CMake", "Prettier", // Go environment variables and placeholder values "CGO",