Two contributors naming one file: the closer one decides it, and one destination has one claimant

Review of the 0.12.0 branch before merge found the graph-contribution merge
inconsistent with the precedence its own documentation states.

`contributions` is ordered highest priority first: the application's own
libraries, then the graph's packages requesters first. The resources merge walks
it backwards, so a higher-priority contributor's resource overrides a lower
one's through `aapt2 -R`. The assets merge walked it forwards into
`collect_tree`, which copies with `overwrite_existing`, so the LAST write won
and the deepest dependency decided a file two packages name. Measured on the
fixture below: `assets/graph-asset.txt` held `from-the-deeper-library` where the
documented order gives `from-the-requester`. The walk is now backwards, for the
same reason the resources walk is.

`lib/<abi>/` is flat, so the application's own library, an archive's native
library of the same name, and a second archive's copy of it all address one
file. Each went through `place_library`, which emits a step whose id and output
are derived from the ABI and the leaf name: two claimants produced two steps
with one id writing one path. One destination now has one claimant, the first in
the priority order, and a later claim is reported by name rather than written.

`mcpp::plugins::json` decoded each `\u` escape on its own, so a code point above
U+FFFF -- which reaches JSON as a surrogate PAIR -- became two three-byte
sequences holding unpaired surrogates: not UTF-8, and silently wrong in whatever
`AndroidManifest.xml` or `Info.plist` the value was written into. Pairs are now
combined, and a surrogate that is not half of one is refused. mcpp's own writer
escapes only characters below 0x20 and passes UTF-8 through, so no producer
reaches this path today, which is why nothing reported it; the reader is shared
by `dist-apk` and `dist-apple` and should not corrupt what it cannot represent.

Criterion: `tests/apk-consumer-graph` gains `lib2/`, which `lib/` depends on, so
the two contribute one asset name at two distances. Leg (6) reads the packed
`assets/graph-asset.txt`. With the walk reverted it fails with
`from-the-deeper-library`; with it in place the five legs pass on 2026.9.16.1.

Author: Sunrisepeak

dist/apk.cppm

@@ -2090,7 +2090,25 @@ inline plan plan_for(options opt = {}) {
     // an archive's native libraries -- and to every library under an older
     // engine, which decides nothing.
     const bool engineDecided = engineStrip && *engineStrip;
-    const auto place_library = [&](const std::string& so, const std::string& abi, bool staged) {
+    // ONE DESTINATION HAS ONE CLAIMANT, AND THE FIRST CLAIM WINS. `lib/<abi>/`
+    // is flat, so the application's own library and an archive's native library
+    // of the same name address one file; so do two archives that carry it. Both
+    // loops below walk highest priority first -- the application's staged
+    // libraries, then `contributions`, which is ordered application first and
+    // each package above the packages it depends on -- so the first claim is the
+    // one the priority order names, and a later one is reported rather than
+    // written. Writing it would place two steps with one id and one output.
+    std::map<std::string, std::string> claimed;   // "<abi>/<leaf>" -> claimant
+    const auto place_library = [&](const std::string& so, const std::string& abi,
+                                   bool staged, std::string_view claimant) {
+        const std::string leafKey = abi + "/" + fs::path(so).filename().string();
+        if (auto [it, fresh] = claimed.try_emplace(leafKey, std::string(claimant)); !fresh) {
+            mcpp::warning(std::format(
+                "mcpp.dist.apk: {} and {} both carry lib/{}; {} is packed, because it comes "
+                "first in the priority order, and {} is left out.",
+                it->second, claimant, leafKey, it->second, claimant).c_str());
+            return;
+        }
         const fs::path dst = work / "lib" / abi / fs::path(so).filename();
         if (llvmStrip.empty() || (staged && engineDecided)) {
             collect_tree(so, dst, libInputs);
@@ -2144,7 +2162,7 @@ inline plan plan_for(options opt = {}) {
     };
     for (auto const& leg : legs)
         for (auto const& so : leg.libraries)
-            place_library(so, leg.abi, /*staged=*/true);
+            place_library(so, leg.abi, /*staged=*/true, "the application");
 
     // An AAR's native libraries, for every ABI this package carries.
     for (auto const& c : contributions) {
@@ -2158,16 +2176,20 @@ inline plan plan_for(options opt = {}) {
                 continue;
             }
             for (auto const& so : shared_objects_in(abiDir))
-                place_library(so, leg.abi, /*staged=*/false);
+                place_library(so, leg.abi, /*staged=*/false, c.label);
         }
     }
 
     const fs::path assetsDir = work / "assets";
     std::vector<std::string> assetInputs;
     // Library and archive assets first, so a file the build program deploys
-    // under the same name replaces theirs.
-    for (auto const& c : contributions)
-        if (!c.assets.empty()) collect_tree(c.assets, assetsDir, assetInputs);
+    // under the same name replaces theirs. `collect_tree` overwrites, so the
+    // last write wins; `contributions` is highest priority first, and is walked
+    // backwards here for the same reason the resources below are -- a package
+    // above the packages it depends on decides the file they share.
+    for (std::size_t k = contributions.size(); k-- > 0;)
+        if (!contributions[k].assets.empty())
+            collect_tree(contributions[k].assets, assetsDir, assetInputs);
     { // every deploy'd file, `<stage>/bin/<rel>` -> `assets/<rel>`: the engine
       // stages `mcpp::deploy`'s destinations under `bin/` on this row as on
       // every other.

src/plugins.cppm

@@ -269,7 +269,8 @@ inline void set_attr(node& n, const std::string& key, const std::string& value)
 // resolution report, and `mcpp::plugins::graph` below reads the engine's graph
 // document for `dist-apk` and `dist-apple`. A second parser for the second
 // document is how two readers come to disagree about one file, so there is one.
-// It reads objects, arrays, strings (with `\u` escapes below U+10000), numbers,
+// It reads objects, arrays, strings (with `\u` escapes, surrogate pairs
+// included, and a refusal for a surrogate that is not part of one), numbers,
 // `true`, `false` and `null`; a number keeps its spelling in `text`.
 export namespace mcpp::plugins::json {
 
@@ -293,6 +294,20 @@ struct reader {
     void skip_space() {
         while (i < s.size() && (s[i] == ' ' || s[i] == '\t' || s[i] == '\n' || s[i] == '\r')) ++i;
     }
+    // The four hexadecimal digits of one `\u` escape, `i` already past the `u`.
+    bool hex4(unsigned& cp) {
+        if (i + 4 > s.size()) return false;
+        cp = 0;
+        for (int k = 0; k < 4; ++k) {
+            const char h = s[i++];
+            cp <<= 4;
+            if (h >= '0' && h <= '9') cp |= static_cast<unsigned>(h - '0');
+            else if (h >= 'a' && h <= 'f') cp |= static_cast<unsigned>(h - 'a' + 10);
+            else if (h >= 'A' && h <= 'F') cp |= static_cast<unsigned>(h - 'A' + 10);
+            else return false;
+        }
+        return true;
+    }
     bool string(std::string& out) {
         if (i >= s.size() || s[i] != '"') return false;
         ++i;
@@ -307,19 +322,35 @@ struct reader {
                 case 'b': out += '\b'; break;
                 case 'f': out += '\f'; break;
                 case 'u': {
-                    if (i + 4 > s.size()) return false;
+                    // A code point above U+FFFF reaches JSON as a surrogate
+                    // PAIR, which is two `\u` escapes. Encoding each half on its
+                    // own produces two three-byte sequences holding unpaired
+                    // surrogates -- not UTF-8, and accepted by nothing that
+                    // reads the manifest this value is written into. The pair is
+                    // combined here, and a surrogate that is not part of one is
+                    // refused, so a malformed document is a refusal rather than
+                    // a silently corrupted string.
                     unsigned cp = 0;
-                    for (int k = 0; k < 4; ++k) {
-                        const char h = s[i++];
-                        cp <<= 4;
-                        if (h >= '0' && h <= '9') cp |= static_cast<unsigned>(h - '0');
-                        else if (h >= 'a' && h <= 'f') cp |= static_cast<unsigned>(h - 'a' + 10);
-                        else if (h >= 'A' && h <= 'F') cp |= static_cast<unsigned>(h - 'A' + 10);
-                        else return false;
+                    if (!hex4(cp)) return false;
+                    if (cp >= 0xd800 && cp <= 0xdbff) {
+                        if (i + 6 > s.size() || s[i] != '\\' || s[i + 1] != 'u') return false;
+                        i += 2;
+                        unsigned lo = 0;
+                        if (!hex4(lo)) return false;
+                        if (lo < 0xdc00 || lo > 0xdfff) return false;
+                        cp = 0x10000 + ((cp - 0xd800) << 10) + (lo - 0xdc00);
+                    } else if (cp >= 0xdc00 && cp <= 0xdfff) {
+                        return false;   // a low surrogate with no high half
                     }
                     if (cp < 0x80) out += static_cast<char>(cp);
                     else if (cp < 0x800) { out += static_cast<char>(0xc0 | (cp >> 6)); out += static_cast<char>(0x80 | (cp & 0x3f)); }
-                    else { out += static_cast<char>(0xe0 | (cp >> 12)); out += static_cast<char>(0x80 | ((cp >> 6) & 0x3f)); out += static_cast<char>(0x80 | (cp & 0x3f)); }
+                    else if (cp < 0x10000) { out += static_cast<char>(0xe0 | (cp >> 12)); out += static_cast<char>(0x80 | ((cp >> 6) & 0x3f)); out += static_cast<char>(0x80 | (cp & 0x3f)); }
+                    else {
+                        out += static_cast<char>(0xf0 | (cp >> 18));
+                        out += static_cast<char>(0x80 | ((cp >> 12) & 0x3f));
+                        out += static_cast<char>(0x80 | ((cp >> 6) & 0x3f));
+                        out += static_cast<char>(0x80 | (cp & 0x3f));
+                    }
                     break;
                 }
                 default: out += e;

tests/apk-consumer-graph/check-apk-graph.sh

@@ -8,7 +8,9 @@
 #     1. the library's string resource is in the APK with the library's value;
 #     2. `options::graph_libraries = false` packs no contribution;
 #     3. an application library defining the same resource wins it;
-#     4. a malformed contribution is refused, naming the package and the key.
+#     4. a malformed contribution is refused, naming the package and the key;
+#     6. of two contributors naming one asset, the one closer to the
+#        application decides it -- `lib/` over the `lib2/` it depends on.
 #   an older engine (no graph):
 #     5. the pack succeeds, and the APK carries no contribution.
 #
@@ -81,7 +83,7 @@ echo "== (4) a malformed contribution is refused, naming the package and the key
 COPY="../.apk-consumer-graph-malformed"
 rm -rf "$COPY"
 mkdir -p "$COPY"
-cp -r mcpp.toml build.mcpp src app-res lib "$COPY"/
+cp -r mcpp.toml build.mcpp src app-res lib lib2 "$COPY"/
 sed -i.bak 's/^resources = "res"$/resources = 3/' "$COPY/lib/mcpp.toml"
 grep -q '^resources = 3$' "$COPY/lib/mcpp.toml" || fail "the malformed copy was not written" "$COPY/lib/mcpp.toml"
 if ( cd "$COPY" && "$MCPP" pack --format apk --target "$TARGET" > refuse.log 2>&1 ); then
@@ -92,5 +94,19 @@ grep -q 'mcpp.apk-graph-lib@0.1.0 states `resources` as something other than a p
 rm -rf "$COPY"
 echo "ok: the refusal names mcpp.apk-graph-lib@0.1.0 and resources"
 
+echo "== (6) of two contributors naming one asset, the closer one decides it =="
+# `contributions` is highest priority first, and `collect_tree` overwrites, so a
+# forward walk would let the DEEPEST contributor win. lib/ requests lib2/, so
+# lib/ is closer to the application and its asset is the one that must survive.
+apk=$(pack graph-6.log)
+rm -rf graph-assets && mkdir -p graph-assets
+unzip -q -o "$apk" "assets/graph-asset.txt" -d graph-assets \
+    || fail "assets/graph-asset.txt is not in the APK" graph-6.log
+value=$(tr -d '\r\n' < graph-assets/assets/graph-asset.txt)
+[ "$value" = "from-the-requester" ] \
+    || fail "the deeper library won an asset the closer one names (got: '${value}')" graph-6.log
+rm -rf graph-assets
+echo "ok: assets/graph-asset.txt = from-the-requester"
+
 rm -rf target graph-*.log
 echo "PASS: dist-apk collects library contributions from the resolved graph ($engine)"

tests/apk-consumer-graph/lib/assets/graph-asset.txt

@@ -0,0 +1 @@
+from-the-requester

tests/apk-consumer-graph/lib/mcpp.toml

@@ -11,10 +11,14 @@ authors     = ["mcpp-community"]
 [package.metadata.dist-apk]
 package   = "org.mcpp.apkgraph.lib"
 resources = "res"
+assets    = "assets"
 
 [language]
 standard = "c++23"
 
+[dependencies]
+mcpp.apk-graph-lib2 = { path = "../lib2" }
+
 [targets.apk-graph-lib]
 kind = "lib"
 

tests/apk-consumer-graph/lib2/assets/graph-asset.txt

@@ -0,0 +1 @@
+from-the-deeper-library

tests/apk-consumer-graph/lib2/mcpp.toml

@@ -0,0 +1,24 @@
+# The deeper library of `tests/apk-consumer-graph`: `lib/` depends on it, so it
+# is further from the application than `lib/` is. Both contribute an asset of
+# the same name, which is how the precedence between two contributors is
+# measured: the one closer to the application decides the file they share.
+[package]
+name        = "apk-graph-lib2"
+namespace   = "mcpp"
+version     = "0.1.0"
+description = "Fixture: the deeper of two libraries contributing an asset of one name"
+license     = "Apache-2.0"
+authors     = ["mcpp-community"]
+
+[package.metadata.dist-apk]
+package = "org.mcpp.apkgraph.lib2"
+assets  = "assets"
+
+[language]
+standard = "c++23"
+
+[targets.apk-graph-lib2]
+kind = "lib"
+
+[build]
+sources = ["src/lib2.cpp"]

tests/apk-consumer-graph/lib2/src/lib2.cpp

@@ -0,0 +1 @@
+int apk_graph_lib2_answer() { return 43; }