Skip to content

Commit 05d03cd

Browse files
authored
fix: make mcpp.lock dependency hashes host-independent (#676)
The lock hash for an index dependency was computed with std::hash<std::string>, whose output is implementation-defined: MSVC implements it as FNV-1a while libstdc++/libc++ use MurmurHash. The same dependency therefore landed in mcpp.lock with a different hash on Windows and Linux, so a clean checkout showed a lock diff after every build while the fnv1a: prefix was only true on MSVC. Compute the digest with mcpp::toolchain::hash_string (FNV-1a on every host), centralised as mcpp::pm::index_package_digest. The git-dependency lock hash and the git cache directory key use the same deterministic hash. Pin the cross-platform vectors in a unit test and add e2e assertions to the SemVer project-index test.
1 parent 361874d commit 05d03cd

5 files changed

Lines changed: 76 additions & 11 deletions

File tree

src/build/prepare.cppm

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7840,10 +7840,15 @@ prepare_build(bool print_fingerprint,
78407840
// ── 2. is it on disk ──
78417841
// Cache key: hash(url + refkind + declared ref + resolved commit).
78427842
// For fixed rev/tag deps the declared ref is also the resolved ref.
7843-
std::hash<std::string> H;
7844-
auto gitRoot = mcppHome / "git" / std::format("{:016x}",
7845-
H(spec.git + "|" + spec.gitRefKind + "|" + spec.gitRev
7846-
+ "|" + resolvedGitRev));
7843+
// Deterministic across hosts: `std::hash` is not (see the note on
7844+
// mcpp::pm::index_package_digest). This key names the git cache
7845+
// directory AND the lock hash below, so a host-dependent hash made
7846+
// both the cache directory and mcpp.lock differ by platform.
7847+
auto H = [](std::string_view s) -> std::string {
7848+
return mcpp::toolchain::hash_string(s);
7849+
};
7850+
auto gitRoot = mcppHome / "git" / H(spec.git + "|" + spec.gitRefKind
7851+
+ "|" + spec.gitRev + "|" + resolvedGitRev);
78477852
std::error_code ec;
78487853
std::filesystem::create_directories(gitRoot.parent_path(), ec);
78497854

@@ -7917,9 +7922,9 @@ prepare_build(bool print_fingerprint,
79177922
if (spec.gitRefKind == "branch") source += "@" + resolvedGitRev;
79187923
root_git_lock_identities[name] = GitLockIdentity{
79197924
.source = std::move(source),
7920-
.hash = std::format("fnv1a:{:016x}", H(spec.git + "|"
7925+
.hash = "fnv1a:" + H(spec.git + "|"
79217926
+ spec.gitRefKind + "|" + spec.gitRev + "|"
7922-
+ resolvedGitRev)),
7927+
+ resolvedGitRev),
79237928
};
79247929
}
79257930
sourceCommit = resolvedGitRev;
@@ -14430,8 +14435,7 @@ prepare_build(bool print_fingerprint,
1443014435
if (gitIt == root_git_lock_identities.end()) {
1443114436
lp.source = std::format("git+{}#{}={}",
1443214437
spec.git, spec.gitRefKind, spec.gitRev);
14433-
std::hash<std::string> hasher;
14434-
lp.hash = std::format("fnv1a:{:016x}", hasher(lp.source));
14438+
lp.hash = "fnv1a:" + mcpp::toolchain::hash_string(lp.source);
1443514439
} else {
1443614440
lp.source = gitIt->second.source;
1443714441
lp.hash = gitIt->second.hash;
@@ -14461,9 +14465,13 @@ prepare_build(bool print_fingerprint,
1446114465
// Use a deterministic hash based on namespace + name + version.
1446214466
// A future PR can replace this with a real content hash from the
1446314467
// xpkg.lua's declared sha256 or from the install plan.
14464-
std::hash<std::string> hasher;
14465-
auto hashInput = std::format("{}:{}@{}", sourceIndex, lp.name, lp.version);
14466-
lp.hash = std::format("fnv1a:{:016x}", hasher(hashInput));
14468+
//
14469+
// NOT `std::hash<std::string>`: its output is implementation-defined
14470+
// (MSVC FNV-1a, libstdc++/libc++ MurmurHash), so the same dependency
14471+
// used to hash differently on Windows and Linux while the `fnv1a:`
14472+
// prefix claimed otherwise. `index_package_digest` is FNV-1a on
14473+
// every host.
14474+
lp.hash = mcpp::pm::index_package_digest(sourceIndex, lp.name, lp.version);
1446714475
lock.packages.push_back(std::move(lp));
1446814476
}
1446914477
if (!lock.packages.empty() || !lock.indices.empty()) {

src/lockfile.cppm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,10 @@ write(const Lockfile& lock, const std::filesystem::path& path) {
3333
inline std::string serialize(const Lockfile& lock) { return mcpp::pm::serialize(lock); }
3434
inline std::string compute_hash(const Lockfile& lock) { return mcpp::pm::compute_hash(lock); }
3535

36+
inline std::string index_package_digest(std::string_view indexNamespace,
37+
std::string_view name,
38+
std::string_view version) {
39+
return mcpp::pm::index_package_digest(indexNamespace, name, version);
40+
}
41+
3642
} // namespace mcpp::lockfile

src/pm/lock_io.cppm

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export module mcpp.pm.lock_io;
1616

1717
import std;
1818
import mcpp.libs.toml;
19+
import mcpp.toolchain.fingerprint; // hash_string — the cross-platform FNV-1a
1920

2021
export namespace mcpp::pm {
2122

@@ -67,6 +68,19 @@ std::expected<void, LockError> write(const Lockfile& lock, const std::filesy
6768
std::string serialize(const Lockfile& lock);
6869
std::string compute_hash(const Lockfile& lock);
6970

71+
// The `hash` field recorded for an index-resolved dependency, as
72+
// `fnv1a:<16 hex>` over "<indexNamespace>:<name>@<version>".
73+
//
74+
// It MUST be computed with a hash whose output is the same on every host.
75+
// `std::hash<std::string>` is not: MSVC implements it as FNV-1a while
76+
// libstdc++/libc++ use MurmurHash, so the very same dependency landed in
77+
// `mcpp.lock` with a different hash on Windows and on Linux — a diff on every
78+
// checkout, and a `fnv1a:` label that was only true on MSVC. The deterministic
79+
// FNV-1a in `mcpp.toolchain.fingerprint` is the project's one hash.
80+
std::string index_package_digest(std::string_view indexNamespace,
81+
std::string_view name,
82+
std::string_view version);
83+
7084
} // namespace mcpp::pm
7185

7286
namespace mcpp::pm {
@@ -193,6 +207,13 @@ std::string compute_hash(const Lockfile& lock) {
193207
return std::format("{:016x}", h);
194208
}
195209

210+
std::string index_package_digest(std::string_view indexNamespace,
211+
std::string_view name,
212+
std::string_view version) {
213+
return "fnv1a:" + mcpp::toolchain::hash_string(
214+
std::format("{}:{}@{}", indexNamespace, name, version));
215+
}
216+
196217
std::optional<LockedGitSource> parse_git_source(std::string_view source) {
197218
constexpr std::string_view prefix = "git+";
198219
if (!source.starts_with(prefix)) return std::nullopt;

tests/e2e/169_semver_project_index.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,16 @@ grep -q 'version = "2.1.0"' mcpp.lock || {
139139
echo "FAIL: the lock must record the resolved version, not the constraint"
140140
exit 1
141141
}
142+
# The digest is FNV-1a on EVERY host. It used `std::hash<std::string>`, whose
143+
# output is implementation-defined (MSVC FNV-1a vs libstdc++/libc++ MurmurHash),
144+
# so the same dependency produced a different mcpp.lock on Windows and Linux.
145+
# `acme:acme.gadget@2.1.0` is pinned here; a host-dependent hash fails on one of
146+
# the two platforms rather than silently drifting.
147+
grep -q 'hash = "fnv1a:2adea846f70078bc"' mcpp.lock || {
148+
cat mcpp.lock
149+
echo "FAIL: acme.gadget 2.1.0 lock hash is not the host-independent FNV-1a"
150+
exit 1
151+
}
142152

143153
"$MCPP" run > run.log 2>&1 || { cat run.log; echo "FAIL: run failed"; exit 1; }
144154

@@ -155,5 +165,11 @@ rm -f mcpp.lock
155165
exit 1
156166
}
157167
grep -q '2\.0\.0' mcpp.lock || { cat mcpp.lock; echo "FAIL: expected 2.0.0 pin"; exit 1; }
168+
# Same host-independent digest contract for the exact-version form.
169+
grep -q 'hash = "fnv1a:34544b46fc9621c3"' mcpp.lock || {
170+
cat mcpp.lock
171+
echo "FAIL: acme.gadget 2.0.0 lock hash is not the host-independent FNV-1a"
172+
exit 1
173+
}
158174

159175
echo "OK"

tests/unit/test_pm_lock_io.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,17 @@ TEST(PmLockIo, ParseNonGitSourceReturnsNullopt) {
105105
EXPECT_FALSE(
106106
mcpp::pm::parse_git_source("git+https://host/repo#bad").has_value());
107107
}
108+
109+
// The lock digest MUST be the same on every host. It used `std::hash<std::string>`,
110+
// whose output is implementation-defined — MSVC implements it as FNV-1a, while
111+
// libstdc++/libc++ use MurmurHash. The same `compat.catch2` therefore landed in
112+
// mcpp.lock as 50719400df192025 on Windows and f492c0206481c69a on Linux, so a
113+
// clean checkout showed a lock diff after every command, and the `fnv1a:` prefix
114+
// was only true on MSVC. These vectors are the cross-platform contract: a
115+
// host-dependent hash turns them red on one of the two platforms.
116+
TEST(PmLockIo, IndexPackageDigestIsFnv1aOnEveryHost) {
117+
EXPECT_EQ(mcpp::pm::index_package_digest("compat", "compat.catch2", "2.13.10"),
118+
"fnv1a:50719400df192025");
119+
EXPECT_EQ(mcpp::pm::index_package_digest("acme", "acme.gadget", "2.1.0"),
120+
"fnv1a:2adea846f70078bc");
121+
}

0 commit comments

Comments
 (0)