Skip to content

Commit be3d36e

Browse files
committed
fix(build): 多段 glob 源文件路径在 Windows 上保持原生分隔符 (#390)
manifest glob(如 `generated/modules/**/*.cppm`)用 `/` 拼路径; MSVC 的 std::filesystem::path 保留输入分隔符原样,于是 `root / prefix` 与目录迭代子路径都是混合形式 (`...\9bca0b44ae3aa660\generated/modules\ccc.when.cppm`), 一路流入 CompileUnit::source → compile_commands.json 的 file/-c 字段, CLion 拒绝解析。ninja 侧一律 generic_string()(全正斜杠)所以构建正常, CDB 是第一个 .string() 消费者,缺陷只在那里显现。 新增 mcpp::modgraph::native_path_from_generic(glob.cppm),在 glob_literal_prefix / expand_dir_glob / scan_one_into / 绝对 include dir 分支 / directives::abs_against 摄入点归一化;顺带修复 build.mcpp 指令 路径与 TOML 绝对 include dir(C:/SDL2/include)同类混合问题。 单测:GlobLiteralPrefixUsesNativeSeparators / NativePathFromGeneric / ExpandGlobMultiSegmentUsesNativeSeparators / ExpandDirGlobMultiSegmentUsesNativeSeparators。 e2e:76 增加多段 glob 源 + Windows 原生分隔符断言;47 增加无残留引号断言。
1 parent 8625c45 commit be3d36e

7 files changed

Lines changed: 166 additions & 10 deletions

File tree

src/build/directives.cppm

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,10 @@ const Def* find_by_tag(std::string_view tag) {
366366
}
367367

368368
std::string abs_against(const fs::path& base, std::string_view p) {
369-
fs::path pp(p);
369+
// Native spelling (see mcpp::modgraph::native_path_from_generic): a
370+
// directive path like `generated/modules/x` would otherwise stay mixed
371+
// on MSVC and leak into include flags / the CDB.
372+
fs::path pp = mcpp::modgraph::native_path_from_generic(p);
370373
if (pp.is_relative()) pp = base / pp;
371374
return pp.lexically_normal().string();
372375
}

src/build/prepare.cppm

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import mcpp.platform.axis;
1616
import mcpp.libs.json;
1717
import mcpp.log;
1818
import mcpp.manifest;
19+
import mcpp.modgraph.glob;
1920
import mcpp.modgraph.graph;
2021
import mcpp.modgraph.scanner;
2122
import mcpp.modgraph.validate;
@@ -2996,7 +2997,11 @@ prepare_build(bool print_fingerprint,
29962997
std::vector<std::filesystem::path> dirs;
29972998
for (auto const& inc : manifest.buildConfig.includeDirs) {
29982999
if (inc.is_absolute()) {
2999-
appendUniquePath(dirs, inc);
3000+
// Native spelling (see native_path_from_generic): a TOML
3001+
// `C:/SDL2/include` stays mixed on MSVC and leaks into the
3002+
// CDB's -I otherwise.
3003+
appendUniquePath(dirs,
3004+
mcpp::modgraph::native_path_from_generic(inc.generic_string()));
30003005
continue;
30013006
}
30023007
for (auto& dir : mcpp::modgraph::expand_dir_glob(
@@ -3016,7 +3021,8 @@ prepare_build(bool print_fingerprint,
30163021
std::vector<std::filesystem::path> dirs;
30173022
for (auto const& inc : manifest.buildConfig.includeDirsAfter) {
30183023
if (inc.is_absolute()) {
3019-
appendUniquePath(dirs, inc);
3024+
appendUniquePath(dirs,
3025+
mcpp::modgraph::native_path_from_generic(inc.generic_string()));
30203026
continue;
30213027
}
30223028
for (auto& dir : mcpp::modgraph::expand_dir_glob(

src/modgraph/glob.cppm

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,30 @@ import std;
1212

1313
export namespace mcpp::modgraph {
1414

15+
// Convert a manifest-style path or glob prefix (always spelled with the
16+
// generic `/` separator) to the platform's native spelling.
17+
//
18+
// MSVC's std::filesystem::path preserves the separators of the string it
19+
// was constructed from instead of normalizing them, so wrapping a raw
20+
// `generated/modules` in a path and joining it with `root / p` yields the
21+
// MIXED `C:\...\generated/modules` — and the directory-walk children built
22+
// on top of that stay mixed. `.string()` then carries the mixed form into
23+
// `compile_commands.json` (its `file` / `-c` fields), which CLion refuses
24+
// to parse. Ninja never notices because it renders everything via
25+
// generic_string(); the CDB is the first `.string()` consumer.
26+
//
27+
// POSIX is untouched (its native separator already is `/`). Replacing only
28+
// `/` is also safe for already-native Windows input: it never contains `/`.
29+
std::filesystem::path native_path_from_generic(std::string_view s) {
30+
constexpr char kSep = std::filesystem::path::preferred_separator;
31+
if (kSep == '/') return std::filesystem::path(s);
32+
std::string p(s);
33+
for (auto& c : p) {
34+
if (c == '/') c = kSep;
35+
}
36+
return std::filesystem::path(std::move(p));
37+
}
38+
1539
// Does `candidate` match `glob`, interpreted relative to `root`?
1640
//
1741
// Supports "**" (any number of directory levels) and "*" (within one segment).

src/modgraph/scanner.cppm

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,12 @@ std::filesystem::path glob_literal_prefix(std::string_view glob) {
268268
? glob : glob.substr(0, wildcard);
269269
auto slash = literal.find_last_of('/');
270270
if (slash == std::string_view::npos) return {};
271-
return std::filesystem::path(literal.substr(0, slash));
271+
// Native separators, not the raw generic form: MSVC keeps the input's
272+
// `/` verbatim, and `root / p` plus the directory walk then propagate a
273+
// MIXED `root\generated/modules` into every downstream path — which is
274+
// what `compile_commands.json`'s `file` field showed on Windows for
275+
// multi-segment globs. See mcpp::modgraph::native_path_from_generic.
276+
return native_path_from_generic(literal.substr(0, slash));
272277
}
273278

274279
// mcpp#228: `{a,b}` alternation, recursively. Finds the first top-level `{`,
@@ -442,7 +447,9 @@ std::vector<std::filesystem::path> expand_dir_glob(const std::filesystem::path&
442447
// expand_glob) — include_dirs entries are meant to name one literal
443448
// directory each; a caller wanting alternatives lists multiple entries.
444449
if (glob.find('*') == std::string_view::npos) {
445-
auto p = root / std::filesystem::path(glob);
450+
// Native spelling (see native_path_from_generic — a raw `a/b` would
451+
// come back mixed from .string() on MSVC).
452+
auto p = root / native_path_from_generic(glob);
446453
if (std::filesystem::is_directory(p, ec)) out.push_back(p);
447454
return out;
448455
}
@@ -682,7 +689,10 @@ local_include_dirs_for(const std::filesystem::path& root,
682689
std::vector<std::filesystem::path> dirs;
683690
for (auto const& inc : manifest.buildConfig.includeDirs) {
684691
if (inc.is_absolute()) {
685-
dirs.push_back(inc);
692+
// A TOML value like `C:/SDL2/include` keeps its `/` on MSVC —
693+
// normalize so the CDB's -I comes out native (mixed separators
694+
// break CLion). See mcpp::modgraph::native_path_from_generic.
695+
dirs.push_back(native_path_from_generic(inc.generic_string()));
686696
continue;
687697
}
688698
for (auto& d : expand_dir_glob(root, inc.generic_string())) {
@@ -701,7 +711,7 @@ local_include_dirs_after_for(const std::filesystem::path& root,
701711
std::vector<std::filesystem::path> dirs;
702712
for (auto const& inc : manifest.buildConfig.includeDirsAfter) {
703713
if (inc.is_absolute()) {
704-
dirs.push_back(inc);
714+
dirs.push_back(native_path_from_generic(inc.generic_string()));
705715
continue;
706716
}
707717
for (auto& d : expand_dir_glob(root, inc.generic_string())) {
@@ -738,7 +748,10 @@ void scan_one_into(ScanResult& result,
738748
// Literal absolute entry — e.g. a dependency build.mcpp's OUT_DIR
739749
// generated source, which lives OUTSIDE the (possibly read-only)
740750
// package root. No glob expansion; taken as-is when it exists.
741-
if (std::filesystem::path gp(g); gp.is_absolute()) {
751+
// Native spelling: a raw `C:/abs/x.cppm` would stay mixed on MSVC
752+
// (see native_path_from_generic) and leak into the CDB.
753+
auto gp = native_path_from_generic(g);
754+
if (gp.is_absolute()) {
742755
std::error_code aec;
743756
if (std::filesystem::is_regular_file(gp, aec)) all_files.insert(gp);
744757
continue;

tests/e2e/47_cdb_prebuilt_module_path_abs.sh

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ cd app
2222
cdb=compile_commands.json
2323
[[ -f "$cdb" ]] || { echo "FAIL: no $cdb generated"; exit 1; }
2424

25+
# jq-independent early guard for the stray-quote bug: before the CDB
26+
# splitter understood shell quoting, flags.cppm's ninja-side quoting leaked
27+
# into the raw JSON as `\"-fprebuilt-module-path=...` (Windows) / `'-...`
28+
# (POSIX). The GCC flow emits no such flag at all, so no-match is the
29+
# expected pass there.
30+
if grep -q '\\"-fprebuilt-module-path' "$cdb" \
31+
|| grep -q "'-fprebuilt-module-path" "$cdb"; then
32+
echo "FAIL: -fprebuilt-module-path retains shell quoting in raw CDB"
33+
exit 1
34+
fi
35+
2536
command -v jq >/dev/null 2>&1 || {
2637
echo "SKIP: jq not on PATH (preinstalled on GitHub-hosted runners)"
2738
exit 0
@@ -63,6 +74,16 @@ while IFS= read -r v; do
6374
fail=1
6475
fi
6576

77+
# Nor shell quoting: the flags string is assembled for the NINJA command
78+
# line, where shell_quote_arg wraps every token containing a Windows `\`
79+
# in double quotes — and those quotes used to land VERBATIM in the CDB
80+
# (`"-fprebuilt-module-path=C:\...\pcm.cache"`), which clangd execs
81+
# literally and cannot resolve. The CDB splitter must have undone them.
82+
if [[ "$v" == '"'* || "$v" == "'"* || "$v" == *'"' || "$v" == *"'" ]]; then
83+
echo "FAIL: value retains shell quoting: '$v'"
84+
fail=1
85+
fi
86+
6687
# Absolute: POSIX (starts with '/') or Windows drive (e.g. 'C:').
6788
if [[ "$v" =~ ^/ || "$v" =~ ^[A-Za-z]: ]]; then
6889
:

tests/e2e/76_compile_commands_generated.sh

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,21 @@ trap "rm -rf $TMP" EXIT
1616
cd "$TMP"
1717
"$MCPP" new app > /dev/null
1818
cd app
19+
20+
# A second source reached through a MULTI-SEGMENT glob (literal prefix
21+
# "generated/modules") — the shape that used to leak MIXED separators into
22+
# the CDB's `file`/`-c` on Windows (`root\generated/modules\extra.cpp`),
23+
# because MSVC's std::filesystem::path keeps the `/` from the manifest glob.
24+
mkdir -p generated/modules
25+
cat > generated/modules/extra.cpp <<'EOF'
26+
int mcpp_extra_anchor() { return 1; }
27+
EOF
28+
cat >> mcpp.toml <<'EOF'
29+
30+
[build]
31+
sources = ["src/**/*.cpp", "generated/modules/**/*.cpp"]
32+
EOF
33+
1934
"$MCPP" build > /dev/null
2035

2136
cdb=compile_commands.json
@@ -45,12 +60,19 @@ grep -q 'main\.cpp' "$cdb" || { echo "FAIL: $cdb has no entry for src/main.cpp";
4560
# above as the portable baseline.
4661
if command -v python3 >/dev/null 2>&1; then
4762
python3 - "$cdb" <<'PY' || exit 1
48-
import json, sys
63+
import json, sys, os
4964
d = json.load(open(sys.argv[1], encoding="utf-8"))
5065
assert isinstance(d, list) and d, "CDB must be a non-empty JSON array"
5166
for e in d:
5267
assert "file" in e and "directory" in e, "entry missing file/directory: %r" % e
5368
assert ("command" in e) or ("arguments" in e), "entry missing command/arguments: %r" % e
69+
# Native separators on Windows: a multi-segment manifest glob used to
70+
# yield MIXED `root\generated/modules\x.cppm` file paths (MSVC's path
71+
# keeps the `/` from the glob prefix), which CLion refuses to parse.
72+
# Ninja hides the problem (it renders generic_string()); the CDB is
73+
# the .string() consumer.
74+
if os.name == "nt" and "/" in e["file"]:
75+
raise AssertionError("file must use native separators on Windows: %r" % e["file"])
5476
print(" json validation OK (%d entries)" % len(d))
5577
PY
5678
fi

tests/unit/test_modgraph.cpp

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include <gtest/gtest.h>
22

33
import std;
4+
import mcpp.modgraph.glob;
45
import mcpp.modgraph.graph;
56
import mcpp.modgraph.scanner;
67
import mcpp.modgraph.validate;
@@ -151,7 +152,7 @@ TEST(Scanner, GlobLiteralPrefixDerivation) {
151152
// Wildcard already in the first segment: no literal directory to bound to.
152153
EXPECT_EQ(glob_literal_prefix("**/*.c"), "");
153154
// No wildcard at all: the full parent directory path is the prefix.
154-
EXPECT_EQ(glob_literal_prefix("a/b/c.cpp"), "a/b");
155+
EXPECT_EQ(glob_literal_prefix("a/b/c.cpp").generic_string(), "a/b");
155156
// Truncate back to the last COMPLETE '/' before the first wildcard char —
156157
// "x*.cpp" is a partial segment, not a real directory named "x".
157158
EXPECT_EQ(glob_literal_prefix("src/x*.cpp"), "src");
@@ -161,6 +162,72 @@ TEST(Scanner, GlobLiteralPrefixDerivation) {
161162
EXPECT_EQ(glob_literal_prefix("a/{x,y}/z"), "a");
162163
}
163164

165+
// MSVC's std::filesystem::path preserves the separators of the string it was
166+
// constructed from, so a raw `a/b` prefix stays generic and `root / p` turns
167+
// into a MIXED `root\a/b` — which used to leak into compile_commands.json
168+
// (`file` / `-c` for every source under a multi-segment glob) and break CLion.
169+
// glob_literal_prefix must return NATIVE separators so the walk and everything
170+
// downstream is native too.
171+
TEST(Scanner, GlobLiteralPrefixUsesNativeSeparators) {
172+
EXPECT_EQ(glob_literal_prefix("a/b/c.cpp").generic_string(), "a/b");
173+
if constexpr (std::filesystem::path::preferred_separator == '\\') {
174+
EXPECT_EQ(glob_literal_prefix("a/b/c.cpp").string(), "a\\b");
175+
EXPECT_EQ(glob_literal_prefix("a/b/c.cpp").string().find('/'),
176+
std::string::npos);
177+
}
178+
}
179+
180+
// The exported converter itself — both spelling directions.
181+
TEST(Glob, NativePathFromGeneric) {
182+
auto p = mcpp::modgraph::native_path_from_generic("a/b/c");
183+
EXPECT_EQ(p.generic_string(), "a/b/c");
184+
if constexpr (std::filesystem::path::preferred_separator == '\\') {
185+
EXPECT_EQ(p.string(), "a\\b\\c");
186+
// Already-native input is untouched.
187+
EXPECT_EQ(mcpp::modgraph::native_path_from_generic("C:\\x\\y").string(),
188+
"C:\\x\\y");
189+
}
190+
}
191+
192+
// The end-to-end shape of the reported bug: a source under a multi-segment
193+
// glob (`generated/modules/**/*.cppm`) must come out of expand_glob with
194+
// NATIVE separators on Windows — the mixed `root\generated/modules\a.cppm`
195+
// was what compile_commands.json's `file` field showed before the fix.
196+
TEST(Scanner, ExpandGlobMultiSegmentPrefixUsesNativeSeparators) {
197+
auto dir = make_tempdir("mcpp-scanner-multi");
198+
write(dir / "generated" / "modules" / "a.cppm", "export module a;\n");
199+
200+
auto files = expand_glob(dir, "generated/modules/**/*.cppm");
201+
202+
ASSERT_EQ(files.size(), 1u);
203+
if constexpr (std::filesystem::path::preferred_separator == '\\') {
204+
EXPECT_EQ(files[0].string().find('/'), std::string::npos) << files[0];
205+
}
206+
EXPECT_EQ(files[0].generic_string(),
207+
(dir / "generated" / "modules" / "a.cppm").generic_string());
208+
209+
std::filesystem::remove_all(dir);
210+
}
211+
212+
// Same contract for the INCLUDE-DIR channel (expand_dir_glob): a multi-segment
213+
// `third_party/inc` entry must yield a native path or the CDB's -I carries the
214+
// mixed form.
215+
TEST(Scanner, ExpandDirGlobMultiSegmentUsesNativeSeparators) {
216+
auto dir = make_tempdir("mcpp-scanner-dirglob");
217+
std::filesystem::create_directories(dir / "third_party" / "inc");
218+
219+
auto dirs = expand_dir_glob(dir, "third_party/inc");
220+
221+
ASSERT_EQ(dirs.size(), 1u);
222+
if constexpr (std::filesystem::path::preferred_separator == '\\') {
223+
EXPECT_EQ(dirs[0].string().find('/'), std::string::npos) << dirs[0];
224+
}
225+
EXPECT_EQ(dirs[0].generic_string(),
226+
(dir / "third_party" / "inc").generic_string());
227+
228+
std::filesystem::remove_all(dir);
229+
}
230+
164231
// mcpp#225: expand_glob must bound its walk to the glob's literal directory
165232
// prefix ("src" for "src/**/*.cppm") instead of always walking the whole
166233
// root and lexically filtering afterward. This is the FUNCTIONAL half of the

0 commit comments

Comments
 (0)