From f9e9d10bcb60b807466ddb646a9c0a0a447f7a20 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 15 Dec 2021 07:42:13 +0200 Subject: Return stable imported target name from import_direct() --- libbuild2/file.cxx | 58 ++++++++++++++++++++++++++++++++++-------------- libbuild2/file.hxx | 22 +++++++++++++----- libbuild2/file.ixx | 18 +++++++++------ libbuild2/target-key.hxx | 2 ++ libbuild2/target.hxx | 5 +++++ 5 files changed, 75 insertions(+), 30 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 682f89d..4cf6d82 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -2679,11 +2679,10 @@ namespace build2 // if (metadata) { - pair r ( + import_result r ( import_direct (base, move (tgt), ph2, opt, metadata, loc)); - return make_pair (r.first != nullptr ? r.first->as_name () : names {}, - r.second); + return make_pair (move (r.name), r.kind); } pair> r ( @@ -2725,7 +2724,13 @@ namespace build2 nullopt /* metadata */, false /* existing */, loc)) + { + // Note that here r.first was still project-qualified and we + // have no choice but to call as_name(). This shouldn't cause + // any problems since the import() call assigns the extension. + // ns = t->as_name (); + } else ns.clear (); // NULL } @@ -2765,6 +2770,10 @@ namespace build2 // Target type-specific search. // + // Note that if this function returns a target, it should have the + // extension assigned (like the find/insert_target() functions) so that + // as_name() returns a stable name. + // const target_key& tk (pk.tk); const target_type& tt (*tk.type); @@ -2862,7 +2871,7 @@ namespace build2 dr << endf; } - pair + import_result import_direct (bool& new_value, scope& base, name tgt, @@ -2872,7 +2881,7 @@ namespace build2 const location& loc, const char* what) { - // This is like normal import() except we return the target rather than + // This is like normal import() except we return the target in addition to // its name. // tracer trace ("import_direct"); @@ -2888,7 +2897,7 @@ namespace build2 // auto meta (metadata ? optional (tgt.value) : nullopt); - names ns; + names ns, rns; import_kind k; const target* pt (nullptr); @@ -2911,7 +2920,7 @@ namespace build2 if (r.first.empty ()) { assert (opt); - return make_pair (pt, k); // NULL + return import_result {nullptr, {}, k}; // NULL } else if (r.first.qualified ()) { @@ -2931,22 +2940,34 @@ namespace build2 } if (pt == nullptr) - return make_pair (pt, k); // NULL + return import_result {nullptr, {}, k}; // NULL + + // Note that here r.first was still project-qualified and we have no + // choice but to call as_name() (below). This shouldn't cause any + // problems since the import() call assigns the extension. - // Otherwise fall through. + // Fall through. } else + { + // It's a bit fuzzy in which cases we end up here. So for now we keep + // the original if it's absolute and call as_name() otherwise. + // + if (r.first.absolute ()) + rns.push_back (r.first); + ns.push_back (move (r.first)); // And fall through. + } } else { k = r.first.absolute () ? import_kind::adhoc : import_kind::normal; - ns = import_load (base.ctx, move (r), metadata, loc).first; + rns = ns = import_load (base.ctx, move (r), metadata, loc).first; } if (pt == nullptr) { - // Similar logic to perform's search(). + // Similar logic to perform's search(). Note: modifies ns. // target_key tk (base.find_target_key (ns, loc)); pt = ctx.targets.find (tk, trace); @@ -2954,6 +2975,9 @@ namespace build2 fail (loc) << "unknown imported target " << tk; } + if (rns.empty ()) + rns = pt->as_name (); + target& t (pt->rw ()); // Load phase. // Note that if metadata is requested via any of the import*() functions, @@ -3025,18 +3049,18 @@ namespace build2 fail (loc) << "no metadata for imported target " << t; } - return make_pair (pt, k); + return import_result {pt, move (rns), k}; } ostream& - operator<< (ostream& o, const pair& p) + operator<< (ostream& o, const import_result& r) { - assert (p.first != nullptr); + assert (r.target != nullptr); - if (p.second == import_kind::normal) - o << *p.first; + if (r.kind == import_kind::normal) + o << *r.target; else - o << p.first->process_path (); + o << r.target->process_path (); return o; } diff --git a/libbuild2/file.hxx b/libbuild2/file.hxx index 1c8e57e..b47d8dc 100644 --- a/libbuild2/file.hxx +++ b/libbuild2/file.hxx @@ -360,9 +360,19 @@ namespace build2 // The what argument specifies what triggered the import (for example, // "module load") and is used in diagnostics. // - // This function also returns the kind of import that was performed. + // This function also returns the stable exported target name (see + // target::as_name() for details) as well as the kind of import that was + // performed. // - pair + template + struct import_result + { + const T* target; + names name; + import_kind kind; + }; + + import_result import_direct (scope& base, name, bool phase2, @@ -376,7 +386,7 @@ namespace build2 // details. Note that a phase 2 fallback/default logic is not considered new // (though this can be easily adjusted based on import kind). // - LIBBUILD2_SYMEXPORT pair + LIBBUILD2_SYMEXPORT import_result import_direct (bool& new_value, scope& base, name, @@ -388,13 +398,13 @@ namespace build2 template - pair + import_result import_direct (scope&, name, bool, bool, bool, const location&, const char* = "import"); template - pair + import_result import_direct (bool&, scope&, name, @@ -406,7 +416,7 @@ namespace build2 // build system modules to print the configuration report. // LIBBUILD2_SYMEXPORT ostream& - operator<< (ostream&, const pair&); + operator<< (ostream&, const import_result&); // As import phase 2 but only imports as an already existing target. But // unlike it, this function can be called during the load and execute diff --git a/libbuild2/file.ixx b/libbuild2/file.ixx index bd138a0..dbd892d 100644 --- a/libbuild2/file.ixx +++ b/libbuild2/file.ixx @@ -42,7 +42,7 @@ namespace build2 return *import (ctx, pk, false, nullopt, false, location ()); } - inline pair + inline import_result import_direct (scope& base, name tgt, bool ph2, bool opt, bool md, @@ -53,19 +53,21 @@ namespace build2 } template - inline pair + inline import_result import_direct (scope& base, name tgt, bool ph2, bool opt, bool md, const location& loc, const char* w) { auto r (import_direct (base, move (tgt), ph2, opt, md, loc, w)); - return make_pair (r.first != nullptr ? &r.first->as () : nullptr, - r.second); + return import_result { + r.target != nullptr ? &r.target->as () : nullptr, + move (r.name), + r.kind}; } template - inline pair + inline import_result import_direct (bool& nv, scope& base, name tgt, @@ -73,8 +75,10 @@ namespace build2 const location& loc, const char* w) { auto r (import_direct (nv, base, move (tgt), ph2, opt, md, loc, w)); - return make_pair (r.first != nullptr ? &r.first->as () : nullptr, - r.second); + return import_result { + r.target != nullptr ? &r.target->as () : nullptr, + move (r.name), + r.kind}; } inline const target* diff --git a/libbuild2/target-key.hxx b/libbuild2/target-key.hxx index f5351b8..cd1ba83 100644 --- a/libbuild2/target-key.hxx +++ b/libbuild2/target-key.hxx @@ -33,6 +33,8 @@ namespace build2 // Append/return the target name or a pair of names if out-qualified. // + // See also target::as_name() for the returned name stability guarantees. + // void as_name (names&) const; diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 1c10621..c2df3e7 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -315,6 +315,11 @@ namespace build2 target_key key_locked () const; + // Note that the returned name is guaranteed to be "stable" (e.g., for + // hashing) only if the target has the extension assigned. This happens, + // for example, when a path is derived for a path-based target (which + // normally happens when such a target is matched for update). + // names as_name () const; -- cgit v1.1