diff options
Diffstat (limited to 'libbuild2/version')
| -rw-r--r-- | libbuild2/version/buildfile | 8 | ||||
| -rw-r--r-- | libbuild2/version/init.cxx | 133 | ||||
| -rw-r--r-- | libbuild2/version/module.hxx | 5 | ||||
| -rw-r--r-- | libbuild2/version/rule.cxx | 71 | ||||
| -rw-r--r-- | libbuild2/version/rule.hxx | 11 | ||||
| -rw-r--r-- | libbuild2/version/snapshot-git.cxx | 69 | ||||
| -rw-r--r-- | libbuild2/version/snapshot.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/version/utility.cxx | 4 |
8 files changed, 203 insertions, 102 deletions
diff --git a/libbuild2/version/buildfile b/libbuild2/version/buildfile index b991f73..4808ded 100644 --- a/libbuild2/version/buildfile +++ b/libbuild2/version/buildfile @@ -4,14 +4,14 @@ # NOTE: shared imports should go into root.build. # include ../ -imp_libs = ../lib{build2} # Implied interface dependency. +impl_libs = ../lib{build2} # Implied interface dependency. include ../in/ -int_libs = ../in/lib{build2-in} +intf_libs = ../in/lib{build2-in} ./: lib{build2-version}: libul{build2-version}: \ {hxx ixx txx cxx}{** -**.test...} \ - $int_libs $imp_libs + $intf_libs $impl_libs # Unit tests. # @@ -40,7 +40,7 @@ objs{*}: cxx.poptions += -DLIBBUILD2_VERSION_SHARED_BUILD lib{build2-version}: { cxx.export.poptions = "-I$out_root" "-I$src_root" - cxx.export.libs = $int_libs + cxx.export.libs = $intf_libs } liba{build2-version}: cxx.export.poptions += -DLIBBUILD2_VERSION_STATIC diff --git a/libbuild2/version/init.cxx b/libbuild2/version/init.cxx index ced9c68..b3657bc 100644 --- a/libbuild2/version/init.cxx +++ b/libbuild2/version/init.cxx @@ -3,7 +3,9 @@ #include <libbuild2/version/init.hxx> -#include <libbutl/manifest-parser.mxx> +#include <cstring> // strchr() + +#include <libbutl/manifest-parser.hxx> #include <libbuild2/scope.hxx> #include <libbuild2/variable.hxx> @@ -143,61 +145,98 @@ namespace build2 } else if (nv.name == "depends") { - // According to the package manifest spec, the format of the - // 'depends' value is as follows: - // - // depends: [?][*] <alternatives> [; <comment>] - // - // <alternatives> := <dependency> [ '|' <dependency>]* - // <dependency> := <name> [<constraint>] - // <constraint> := <comparison> | <range> - // <comparison> := ('==' | '>' | '<' | '>=' | '<=') <version> - // <range> := ('(' | '[') <version> <version> (')' | ']') - // - // Note that we don't do exhaustive validation here leaving it - // to the package manager. - // string v (move (nv.value)); - size_t p; + // Parse the dependency and add it to the map (see + // bpkg::dependency_alternatives class for dependency syntax). + // + // Note that currently we only consider simple dependencies: + // singe package without alternatives, clauses, or newlines. + // In the future, if/when we add full support, we will likely + // keep this as a fast path. + // + // Also note that we don't do exhaustive validation here leaving + // it to the package manager. // Get rid of the comment. // + // Note that we can potentially mis-detect the comment + // separator, since ';' can be a part of some of the dependency + // alternative clauses. If that's the case, we will skip the + // dependency later. + // + size_t p; if ((p = v.find (';')) != string::npos) v.resize (p); - // Get rid of conditional/runtime markers. Note that enither of - // them is valid in the rest of the value. + // Skip the dependency if it is not a simple one. + // + // Note that we will check for the presence of the reflect + // clause later since `=` can also be in the constraint. + // + if (v.find_first_of ("{?|\n") != string::npos) + continue; + + // Find the beginning of the dependency package name, skipping + // the build-time marker, if present. // - if ((p = v.find_last_of ("?*")) != string::npos) - v.erase (0, p + 1); + bool buildtime (v[0] == '*'); + size_t b (buildtime ? v.find_first_not_of (" \t", 1) : 0); - // Parse as |-separated "words". + if (b == string::npos) + fail (l) << "invalid dependency " << v << ": no package name"; + + // Find the end of the dependency package name. + // + p = v.find_first_of (" \t=<>[(~^", b); + + // Dependency name (without leading/trailing white-spaces). // - for (size_t b (0), e (0); next_word (v, b, e, '|'); ) + string n (v, b, p == string::npos ? p : p - b); + + string vc; // Empty if no constraint is specified + + // Position to the first non-whitespace character after the + // dependency name, which, if present, can be a part of the + // version constraint or the reflect clause. + // + if (p != string::npos) + p = v.find_first_not_of (" \t", p); + + if (p != string::npos) + { + // Check if this is definitely not a version constraint and + // drop this dependency if that's the case. + // + if (strchr ("=<>[(~^", v[p]) == nullptr) + continue; + + // Ok, we have a constraint, check that there is no reflect + // clause after it (the only other valid `=` in a constraint + // is in the immediately following character as part of + // `==`, `<=`, or `>=`). + // + if (v.size () > p + 2 && v.find ('=', p + 2) != string::npos) + continue; + + vc.assign (v, p, string::npos); + trim (vc); + } + + // Finally, add the dependency to the map. + // + try + { + package_name pn (move (n)); + string v (pn.variable ()); + + ds.emplace (move (v), + dependency {move (pn), move (vc), buildtime}); + } + catch (const invalid_argument& e) { - string d (v, b, e - b); - trim (d); - - p = d.find_first_of (" \t=<>[(~^"); - string n (d, 0, p); - string c (p != string::npos ? string (d, p) : string ()); - - trim (n); - trim (c); - - try - { - package_name pn (move (n)); - string v (pn.variable ()); - - ds.emplace (move (v), dependency {move (pn), move (c)}); - } - catch (const invalid_argument& e) - { - fail (l) << "invalid package name for dependency " - << d << ": " << e; - } + fail (l) << "invalid dependency package name '" << n << "': " + << e; } } } @@ -246,7 +285,9 @@ namespace build2 { auto i (ds.find ("build2")); - if (i != ds.end () && !i->second.constraint.empty ()) + if (i != ds.end () && + i->second.buildtime && + !i->second.constraint.empty ()) try { check_build_version ( @@ -349,7 +390,7 @@ namespace build2 if (cast_false<bool> (rs["install.booted"])) { rs.insert_rule<manifest> ( - perform_install_id, "version.manifest", manifest_install_rule_); + perform_install_id, "version.install", manifest_install_rule_); } return true; diff --git a/libbuild2/version/module.hxx b/libbuild2/version/module.hxx index 26bd48a..8549e03 100644 --- a/libbuild2/version/module.hxx +++ b/libbuild2/version/module.hxx @@ -4,8 +4,6 @@ #ifndef LIBBUILD2_VERSION_MODULE_HXX #define LIBBUILD2_VERSION_MODULE_HXX -#include <map> - #include <libbuild2/types.hxx> #include <libbuild2/utility.hxx> @@ -24,9 +22,10 @@ namespace build2 { package_name name; string constraint; + bool buildtime; }; - using dependencies = std::map<string, dependency>; + using dependencies = map<string, dependency>; struct module: build2::module { diff --git a/libbuild2/version/rule.cxx b/libbuild2/version/rule.cxx index 919dfcf..65c1117 100644 --- a/libbuild2/version/rule.cxx +++ b/libbuild2/version/rule.cxx @@ -46,12 +46,31 @@ namespace build2 // in_rule // + + // Wrap the in::rule's perform_update recipe into a data-carrying recipe. + // + // To optimize this a bit further (i.e., to avoid the dynamic memory + // allocation) we are going to call in::rule::perform_update() directly + // (after all it's virtual and thus part of the in_rule's interface). + // + struct match_data + { + const module& mod; + const in_rule& rule; + + target_state + operator() (action a, const target& t) + { + return rule.perform_update (a, t); + } + }; + bool in_rule:: - match (action a, target& xt, const string&) const + match (action a, target& xt) const { tracer trace ("version::in_rule::match"); - file& t (static_cast<file&> (xt)); + file& t (xt.as<file> ()); const scope& rs (t.root_scope ()); bool fm (false); // Found manifest. @@ -74,14 +93,26 @@ namespace build2 if (!fi) l5 ([&]{trace << "no in file prerequisite for target " << t;}); - bool r (fm && fi); - - // If we match, lookup and cache the module for the update operation. + // If we match, derive the file name early as recommended by the in + // rule. // - if (r && a == perform_update_id) - t.data (rs.find_module<module> (module::name)); + if (fm && fi) + t.derive_path (); - return r; + return fm && fi; + } + + recipe in_rule:: + apply (action a, target& t) const + { + recipe r (rule::apply (a, t)); + + // Lookup and cache the module for the update operation. + // + return a == perform_update_id + ? match_data {*t.root_scope ().find_module<module> (module::name), + *this} + : move (r); } string in_rule:: @@ -89,12 +120,16 @@ namespace build2 action a, const target& t, const string& n, + optional<uint64_t> flags, + const substitution_map* smap, const optional<string>& null) const { + assert (!flags); + // Note that this code will be executed during up-to-date check for each // substitution so let's try not to do anything overly sub-optimal here. // - const module& m (*t.data<const module*> ()); + const module& m (t.data<match_data> (a).mod); // Split it into the package name and the variable/condition name. // @@ -110,7 +145,7 @@ namespace build2 a, t, p == string::npos ? n : string (n, p + 1), - null); + nullopt, smap, null); } string pn (n, 0, p); @@ -212,13 +247,13 @@ namespace build2 if (mav->snapshot ()) { - r += (p ? "(" : ""); + if (p) r += '('; r += cmp (vm, " < ", mav->version) + " || ("; r += cmp (vm, " == ", mav->version) + " && "; - r += cmp (sm, (mao ? " < " : " <= "), mav->snapshot_sn) + ")"; + r += cmp (sm, (mao ? " < " : " <= "), mav->snapshot_sn) + ')'; - r += (p ? ")" : ""); + if (p) r += ')'; } else r = cmp (vm, (mao ? " < " : " <= "), mav->version); @@ -232,13 +267,13 @@ namespace build2 if (miv->snapshot ()) { - r += (p ? "(" : ""); + if (p) r += '('; r += cmp (vm, " > ", miv->version) + " || ("; r += cmp (vm, " == ", miv->version) + " && "; - r += cmp (sm, (mio ? " > " : " >= "), miv->snapshot_sn) + ")"; + r += cmp (sm, (mio ? " > " : " >= "), miv->snapshot_sn) + ')'; - r += (p ? ")" : ""); + if (p) r += ')'; } else r = cmp (vm, (mio ? " > " : " >= "), miv->version); @@ -298,7 +333,7 @@ namespace build2 // manifest_install_rule // bool manifest_install_rule:: - match (action a, target& t, const string&) const + match (action a, target& t) const { // We only match project's manifest. // @@ -311,7 +346,7 @@ namespace build2 if (s.root_scope () != &s || s.src_path () != t.dir) return false; - return file_rule::match (a, t, ""); + return file_rule::match (a, t); } auto_rmfile manifest_install_rule:: diff --git a/libbuild2/version/rule.hxx b/libbuild2/version/rule.hxx index ddc5e11..0bdc090 100644 --- a/libbuild2/version/rule.hxx +++ b/libbuild2/version/rule.hxx @@ -20,16 +20,21 @@ namespace build2 class in_rule: public in::rule { public: - in_rule (): rule ("version.in 2", "version.in") {} + in_rule (): rule ("version.in 2", "version") {} virtual bool - match (action, target&, const string&) const override; + match (action, target&) const override; + + virtual recipe + apply (action, target&) const override; virtual string lookup (const location&, action, const target&, const string&, + optional<uint64_t>, + const substitution_map*, const optional<string>&) const override; }; @@ -41,7 +46,7 @@ namespace build2 manifest_install_rule () {} virtual bool - match (action, target&, const string&) const override; + match (action, target&) const override; virtual auto_rmfile install_pre (const file&, const install_dir&) const override; diff --git a/libbuild2/version/snapshot-git.cxx b/libbuild2/version/snapshot-git.cxx index 8eb7cc8..ab0224a 100644 --- a/libbuild2/version/snapshot-git.cxx +++ b/libbuild2/version/snapshot-git.cxx @@ -3,7 +3,7 @@ #include <ctime> // time_t -#include <libbutl/sha1.mxx> +#include <libbutl/sha1.hxx> #include <libbuild2/version/snapshot.hxx> @@ -14,11 +14,20 @@ namespace build2 { namespace version { + // We have to run git twice to extract the information we need and doing + // it repetitively is quite expensive, especially for larger repositories. + // So we cache it, which helps multi-package repositories. + // + static global_cache<snapshot, dir_path> cache; + snapshot - extract_snapshot_git (const dir_path& src_root) + extract_snapshot_git (context& ctx, dir_path rep_root) { + if (const snapshot* r = cache.find (rep_root)) + return *r; + snapshot r; - const char* d (src_root.string ().c_str ()); + const char* d (rep_root.string ().c_str ()); // On startup git prepends the PATH environment variable value with the // computed directory path where its sub-programs are supposedly located @@ -37,6 +46,12 @@ namespace build2 // prevent this we pass the git's exec directory via the --exec-path // option explicitly. // + // Note also that git has quite a few GIT_* environment variables and + // stray values for some of them could break our commands. So it may + // seem like a good idea to unset them. But on the other hand, they may + // be there for a reason: after all, we are operating on user's projects + // and user's environment may be setup to handle them. + // path p ("git"); process_path pp (run_search (p, true /* init */)); @@ -67,7 +82,11 @@ namespace build2 args[args_i + 1] = "--porcelain"; args[args_i + 2] = nullptr; + // @@ PERF: redo with custom stream reading code (then could also + // get rid of context). + // r.committed = run<string> ( + ctx, 3 /* verbosity */, pp, args, @@ -93,7 +112,8 @@ namespace build2 // (reluctantly) assume that the only reason git cat-file fails is if // there is no HEAD (that we equal with the "new repository" condition // which is, strictly speaking, might not be the case either). So we - // suppress any diagnostics, and handle non-zero exit code. + // suppress any diagnostics, and handle non-zero exit code (and so no + // diagnostics buffering is needed, plus we are in the load phase). // string data; @@ -102,12 +122,12 @@ namespace build2 args[args_i + 2] = "HEAD"; args[args_i + 3] = nullptr; - process pr (run_start (3 /* verbosity */, + process pr (run_start (3 /* verbosity */, pp, args, - 0 /* stdin */, - -1 /* stdout */, - false /* error */)); + 0 /* stdin */, + -1 /* stdout */, + 1 /* stderr (to stdout) */)); string l; try @@ -186,30 +206,31 @@ namespace build2 // that. } - if (!run_finish_code (args, pr, l)) + if (run_finish_code (args, pr, l, 2 /* verbosity */)) + { + if (r.sn == 0) + fail << "unable to extract git commit id/date for " << rep_root; + + if (r.committed) + { + sha1 cs; + cs.append ("commit " + to_string (data.size ())); // Includes '\0'. + cs.append (data.c_str (), data.size ()); + r.id.assign (cs.string (), 12); // 12-char abbreviated commit id. + } + else + r.sn++; // Add a second. + } + else { // Presumably new repository without HEAD. Return uncommitted snapshot // with UNIX epoch as timestamp. // r.sn = 19700101000000ULL; r.committed = false; - return r; } - if (r.sn == 0) - fail << "unable to extract git commit id/date for " << src_root; - - if (r.committed) - { - sha1 cs; - cs.append ("commit " + to_string (data.size ())); // Includes '\0'. - cs.append (data.c_str (), data.size ()); - r.id.assign (cs.string (), 12); // 12-characters abbreviated commit id. - } - else - r.sn++; // Add a second. - - return r; + return cache.insert (move (rep_root), move (r)); } } } diff --git a/libbuild2/version/snapshot.cxx b/libbuild2/version/snapshot.cxx index 3a885b5..000bcba 100644 --- a/libbuild2/version/snapshot.cxx +++ b/libbuild2/version/snapshot.cxx @@ -12,7 +12,7 @@ namespace build2 namespace version { snapshot - extract_snapshot_git (const dir_path&); + extract_snapshot_git (context&, dir_path); static const path git (".git"); @@ -46,7 +46,7 @@ namespace build2 if (butl::entry_exists (d / git, true /* follow_symlinks */, true /* ignore_errors */)) - return extract_snapshot_git (d); + return extract_snapshot_git (rs.ctx, move (d)); } return snapshot (); diff --git a/libbuild2/version/utility.cxx b/libbuild2/version/utility.cxx index 5e2375d..9fec7ec 100644 --- a/libbuild2/version/utility.cxx +++ b/libbuild2/version/utility.cxx @@ -3,8 +3,8 @@ #include <libbuild2/version/utility.hxx> -#include <libbutl/manifest-parser.mxx> -#include <libbutl/manifest-serializer.mxx> +#include <libbutl/manifest-parser.hxx> +#include <libbutl/manifest-serializer.hxx> #include <libbuild2/context.hxx> #include <libbuild2/filesystem.hxx> // path_perms() |