From feb93edb4820feceb602b1e081f782e2e0bc45bc Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 18 Aug 2021 10:57:59 +0200 Subject: Avoid reconfiguring forwards in noop implicit sync --- bdep/sync.cxx | 138 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 71 insertions(+), 67 deletions(-) diff --git a/bdep/sync.cxx b/bdep/sync.cxx index cc0894a..f855c60 100644 --- a/bdep/sync.cxx +++ b/bdep/sync.cxx @@ -908,6 +908,7 @@ namespace bdep // 5. Re-run bpkg-pkg-build and if it turns out that (another) build-time // dependency configuration is required, then go to p.1. // + bool noop (false); for (;;) { // Run bpkg with the --no-private-config option, so that it reports the @@ -928,6 +929,7 @@ namespace bdep "--no-fetch", "--no-refinement", "--no-private-config", 125, + "--noop-exit", 124, "--configure-only", "--keep-out", "--plan", plan, @@ -970,10 +972,12 @@ namespace bdep if (e.normal ()) { - need_config = (e.code () == 125); - - if (!need_config) - throw failed (); // Assume the child issued diagnostics. + switch (e.code ()) + { + case 124: noop = true; break; + case 125: need_config = true; break; + default: throw failed (); // Assume the child issued diagnostics. + } } else fail << "process " << name_bpkg (co) << " " << e; @@ -1015,86 +1019,86 @@ namespace bdep // Handle configuration forwarding. // // We do it here (instead of, say init) because a change in a package may - // introduce new subprojects. Though it would be nice to only do this if - // the package was upgraded. - // - // @@ TODO: changed flag below: Probably by comparing before/after - // versions. Or, even simpler, if pkg-build signalled that - // nothing has changed (the --exit-result idea) -- then we - // can skip the whole thing. + // introduce new subprojects. For an implicit sync we can skip all of this + // if no packages were upgraded (since our version revision tracking takes + // into account changes to the subproject set). // // Also, the current thinking is that config set --[no-]forward is best // implemented by just changing the flag on the configuration and then // requiring an explicit sync to configure/disfigure forwards. // - for (const project& prj: prjs) + if (!implicit || !noop) { - package_locations pls (load_packages (prj.path)); - - for (const package_state& pkg: prj.config->packages) + for (const project& prj: prjs) { - // If this is a forwarded configuration, make sure forwarding is - // configured and is up-to-date. Otherwise, make sure it is disfigured - // (the config set --no-forward case). - // - dir_path src (prj.path); - { - auto i (find_if (pls.begin (), - pls.end (), - [&pkg] (const package_location& pl) - { - return pkg.name == pl.name; - })); + package_locations pls (load_packages (prj.path)); - if (i == pls.end ()) - fail << "package " << pkg.name << " is not listed in " << prj.path; + for (const package_state& pkg: prj.config->packages) + { + // If this is a forwarded configuration, make sure forwarding is + // configured and is up-to-date. Otherwise, make sure it is + // disfigured (the config set --no-forward case). + // + dir_path src (prj.path); + { + auto i (find_if (pls.begin (), + pls.end (), + [&pkg] (const package_location& pl) + { + return pkg.name == pl.name; + })); - src /= i->path; - } + if (i == pls.end ()) + fail << "package " << pkg.name << " is not listed in " + << prj.path; - // We could run 'b info' and used the 'forwarded' value but this is - // both faster and simpler. Or at least it was until we got the - // alternative naming scheme. - // - auto check = [&src] () - { - path f (src / "build2" / "bootstrap" / "out-root.build2"); - bool e (exists (f)); + src /= i->path; + } - if (!e) +#if 0 + // We could run 'b info' and used the 'forwarded' value but this is + // both faster and simpler. Or at least it was until we got the + // alternative naming scheme. + // + auto check = [&src] () { - f = src / "build" / "bootstrap" / "out-root.build"; - e = exists (f); - } + path f (src / "build" / "bootstrap" / "out-root.build"); + bool e (exists (f)); - return e; - }; + if (!e) + { + f = src / "build2" / "bootstrap" / "out-root.build2"; + e = exists (f); + } - const char* o (nullptr); - if (prj.config->forward) - { - bool changed (true); + return e; + }; +#endif - if (changed || !check ()) + const char* o (nullptr); + if (prj.config->forward) + { o = "configure:"; - } - else if (!prj.implicit) // Requires explicit sync. - { - //@@ This is broken: we will disfigure forwards to other configs. - // Looks like we will need to test that the forward is to this - // config. 'b info' here we come? - - //if (check ()) - // o = "disfigure:"; - } + } + else if (!prj.implicit) // Requires explicit sync. + { + //@@ This is broken: we will disfigure forwards to other configs. + // Looks like we will need to test that the forward is to this + // config. 'b info' here we come? +#if 0 + if (check ()) + o = "disfigure:"; +#endif + } - if (o != nullptr) - { - dir_path out (dir_path (cfg) /= pkg.name.string ()); - run_b (co, - o, - "'" + src.representation () + "'@'" + out.representation () + - "',forward"); + if (o != nullptr) + { + dir_path out (dir_path (cfg) /= pkg.name.string ()); + run_b (co, + o, + "'" + src.representation () + "'@'" + out.representation () + + "',forward"); + } } } } -- cgit v1.1