From 0eed5982bbca328cc6319d36708d64a285160972 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 30 Apr 2018 12:27:49 +0200 Subject: Implement multi-project synchronization Now the same configuration can be shared by multiple projects and a sync command from any of them (or from the build system hook) will synchronize everyting. --- bdep/init.cxx | 60 +++---- bdep/sync.cli | 4 +- bdep/sync.cxx | 477 +++++++++++++++++++++++++++++++++++++++---------------- bdep/utility.cxx | 17 ++ bdep/utility.hxx | 8 + bdep/utility.txx | 10 +- 6 files changed, 397 insertions(+), 179 deletions(-) diff --git a/bdep/init.cxx b/bdep/init.cxx index 303c475..f03b7cc 100644 --- a/bdep/init.cxx +++ b/bdep/init.cxx @@ -170,38 +170,40 @@ namespace bdep return 0; } - // Make sure everyone refers to the same objects across all the - // transactions. - // - session s; - - // --config-add/create - // configurations cfgs; - if (ca || cc) { - cfgs.push_back ( - cmd_init_config ( - o, - o, - prj, - db, - ca ? o.config_add () : o.config_create (), - args, - ca, - cc)); - - // Fall through. - } + // Make sure everyone refers to the same objects across all the + // transactions. + // + session s; - // If this is the default mode, then find the configurations the user - // wants us to use. - // - if (cfgs.empty ()) - { - transaction t (db.begin ()); - cfgs = find_configurations (prj, t, o); - t.commit (); + // --config-add/create + // + if (ca || cc) + { + cfgs.push_back ( + cmd_init_config ( + o, + o, + prj, + db, + ca ? o.config_add () : o.config_create (), + args, + ca, + cc)); + + // Fall through. + } + + // If this is the default mode, then find the configurations the user + // wants us to use. + // + if (cfgs.empty ()) + { + transaction t (db.begin ()); + cfgs = find_configurations (prj, t, o); + t.commit (); + } } // Initialize each package in each configuration. diff --git a/bdep/sync.cli b/bdep/sync.cli index 61d15cb..838a01c 100644 --- a/bdep/sync.cli +++ b/bdep/sync.cli @@ -206,7 +206,9 @@ namespace bdep configurations are synchronized. To improve performance, especially for the \"everything is already synchronized\" case, \cb{sync} executed in this mode assumes that no configuration flags (see \l{bdep-config(1)}) - have changed since the last explicit synchronization. + have changed since the last explicit synchronization. It also does not + search for a project in the current working directory \- if any, its + directory should be specified explicitly with \c{\b{--config}|\b{-c}}. To avoid recursive re-synchronization, the \cb{sync} command maintains the \cb{BDEP_SYNCED_CONFIGS} environment variable. It contains a diff --git a/bdep/sync.cxx b/bdep/sync.cxx index b95aa54..994d76e 100644 --- a/bdep/sync.cxx +++ b/bdep/sync.cxx @@ -8,6 +8,7 @@ #include #include +#include #include @@ -15,6 +16,114 @@ using namespace std; namespace bdep { + // Project to be synchronized. + // + struct project + { + dir_path path; + shared_ptr config; + + bool implicit; + bool fetch; + }; + + using projects = small_vector; + + // Append the list of additional (to origin, if not empty) projects that are + // using this configuration. + // + static void + load_implicit (const common_options& co, + const dir_path& cfg, + const dir_path& origin_prj, + projects& r) + { + tracer trace ("load_implicit"); + + // Use bpkg-rep-list to discover the list of project directories. + // + process pr; + bool io (false); + try + { + fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate. + + pr = start_bpkg (co, + pipe /* stdout */, + 2 /* stderr */, + "rep-list", + "-d", cfg); + + pipe.out.close (); + ifdstream is (move (pipe.in), fdstream_mode::skip, ifdstream::badbit); + + for (string l; !eof (getline (is, l)); ) + { + // The repository type must be 'dir'. + // + if (l.compare (0, 4, "dir:") != 0) + continue; + + size_t p (l.find (' ')); + if (p == string::npos) + fail << "invalid bpkg-rep-list output: no repository location"; + + // Paths that we add are absolute and normilized but who knows what + // else the user might have added. + // + dir_path d (l, p + 1, string::npos); + + if (d.relative ()) + continue; + + d.normalize (); // For good measure. + + if (d == origin_prj) + continue; + + // Next see if it looks like a bdep-managed project. + // + if (!exists (d / bdep_file)) + continue; + + shared_ptr c; + { + using query = bdep::query; + + database db (open (d, trace)); + + transaction t (db.begin ()); + c = db.query_one (query::path == cfg.string ()); + t.commit (); + } + + // If the project is a repository of this configuration but the bdep + // database has no knowledge of this configuration, then assume it is + // not managed by bdep (i.e., the user added the project manually or + // some such). + // + if (c == nullptr) + continue; + + r.push_back (project {move (d), + move (c), + true /* implicit */, + true /* fetch */}); + } + + is.close (); // Detect errors. + } + catch (const io_error&) + { + // Presumably the child process failed and issued diagnostics so let + // finish_bpkg() try to deal with that first. + // + io = true; + } + + finish_bpkg (co, pr, io); + } + // Sync with optional upgrade. // // If upgrade is not nullopt, then: If there are dep_pkgs, then we are @@ -22,8 +131,9 @@ namespace bdep // static void cmd_sync (const common_options& co, - const dir_path& prj, - const shared_ptr& c, + const dir_path& cfg, + const dir_path& origin_prj, + const shared_ptr& origin_config, bool implicit, bool fetch, bool yes, @@ -32,73 +142,69 @@ namespace bdep const package_locations& prj_pkgs, const strings& dep_pkgs) { - assert (!c->packages.empty ()); + assert (origin_config == nullptr || !origin_config->packages.empty ()); assert (prj_pkgs.empty () || dep_pkgs.empty ()); // Can't have both. - // Do a separate fetch instead of letting pkg-build do it. This way we get - // better control of the diagnostics (no "fetching ..." for the project - // itself). We also make sure that if the user specifies a repository for - // a dependency to upgrade, then that repository is listed as part of the - // project prerequisites/complements. Or, in other words, we only want to - // allow specifying the location as a proxy for specifying version (i.e., - // "I want this dependency upgraded to the latest version available from - // this repository"). - // - // We also use the repository name rather than the location as a sanity - // check (the repository must have been added as part of init). - // - // @@ For implicit fetch this will print diagnostics/progress before - // "synchronizing :". Maybe rep-fetch also needs something - // like --plan but for progress? Plus there might be no sync at all. + projects prjs; + + if (origin_config != nullptr) + prjs.push_back (project {origin_prj, origin_config, implicit, fetch}); + + // Load other projects that might be using the same configuration -- we + // have to synchronize everything at once. // - if (fetch) - run_bpkg (co, - "fetch", - "-d", c->path, - "--shallow", - "dir:" + prj.string ()); - - // Prepare the package list. + load_implicit (co, cfg, origin_prj, prjs); + + // Prepare the list of packages to build and repositories to fetch. // strings args; + strings reps; - for (const package_state& p: c->packages) + for (const project& prj: prjs) { - if (upgrade) + if (prj.fetch) + reps.push_back ("dir:" + prj.path.string ()); + + for (const package_state& pkg: prj.config->packages) { - // We synchronize all the init'ed packages but only upgrade the - // specified. - // - if (find_if (prj_pkgs.begin (), - prj_pkgs.end (), - [&p] (const package_location& pl) - { - return pl.name == p.name; - }) != prj_pkgs.end ()) + if (upgrade && !prj.implicit) { - // The project package itself must always be upgraded to the latest - // version/iteration. So we have to translate our options to their - // dependency-only --{upgrade,patch}-{recursive,immediate}. + // We synchronize all the init'ed packages but only upgrade the + // specified. // - args.push_back ("{"); - args.push_back ( - *upgrade - ? *recursive ? "--upgrade-recursive" : "--upgrade-immediate" - : *recursive ? "--patch-recursive" : "--patch-immediate"); - args.push_back ("}+"); + if (find_if (prj_pkgs.begin (), + prj_pkgs.end (), + [&pkg] (const package_location& pl) + { + return pl.name == pkg.name; + }) != prj_pkgs.end ()) + { + // The project package itself must always be upgraded to the + // latest version/iteration. So we have to translate to + // dependency-only --{upgrade,patch}-{recursive,immediate}. + // + args.push_back ("{"); + args.push_back ( + *upgrade + ? *recursive ? "--upgrade-recursive" : "--upgrade-immediate" + : *recursive ? "--patch-recursive" : "--patch-immediate"); + args.push_back ("}+"); + } } - } - // We need to add the explicit location qualification (@) since - // there is no guarantee a higher version isn't available from another - // repository. - // - args.push_back (p.name + '@' + prj.string ()); + // We need to add the explicit location qualification (@) + // since there is no guarantee a higher version isn't available from + // another repository. + // + args.push_back (pkg.name + '@' + prj.path.string ()); + } } + // Add dependencies to upgrade (if any). + // if (upgrade) { - for (const string& p: dep_pkgs) + for (const string& n: dep_pkgs) { // Unless this is the default "non-recursive upgrade" we need to add a // group. @@ -106,17 +212,43 @@ namespace bdep if (recursive || !*upgrade) { args.push_back ("{"); - args.push_back (*upgrade ? "-u" : "-p"); - if (recursive) args.push_back (*recursive ? "-r" : "-i"); + + string o (*upgrade ? "-u" : "-p"); + if (recursive) o += *recursive ? 'r' : 'i'; + args.push_back (move (o)); + args.push_back ("}+"); } // Make sure it is treated as a dependency. // - args.push_back ('?' + p); + args.push_back ('?' + n); } } + // We do a separate fetch instead of letting pkg-build do it. This way we + // get better control of the diagnostics (no "fetching ..." for the + // project itself). We also make sure that if the user specifies a + // repository for a dependency to upgrade, then that repository is listed + // as part of the project prerequisites/complements. Or, in other words, + // we only want to allow specifying the location as a proxy for specifying + // version (i.e., "I want this dependency upgraded to the latest version + // available from this repository"). + // + // Note also that we use the repository name rather than the location as a + // sanity check (the repository must have been added as part of init). + // Plus, without 'dir:' it will be treated as version control-based. + // + // @@ For implicit fetch this will print diagnostics/progress before + // "synchronizing :". Maybe rep-fetch also needs something + // like --plan but for progress? Plus there might be no sync at all. + // + // @@ TODO: remember to rep-remove in deinit if there are no more + // init'ed packages in this configuration. + // + if (!reps.empty ()) + run_bpkg (co, "fetch", "-d", cfg, "--shallow", reps); + // For implicit sync (normally performed on one configuration at a time) // add the configuration directory to the plan header. // @@ -125,12 +257,12 @@ namespace bdep // will normally be no "originating project" (unlike with explicit sync). // string plan (implicit - ? "synchronizing " + c->path.representation () + ':' + ? "synchronizing " + cfg.representation () + ':' : "synchronizing:"); run_bpkg (co, "build", - "-d", c->path, + "-d", cfg, "--no-fetch", "--configure-only", "--keep-out", @@ -153,70 +285,73 @@ namespace bdep // implemented by just changing the flag on the configuration and then // requiring an explicit sync to configure/disfigure forwards. // - package_locations pls (load_packages (prj)); - - for (const package_state& p: c->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); + package_locations pls (load_packages (prj.path)); + + for (const package_state& pkg: prj.config->packages) { - auto i (find_if (pls.begin (), - pls.end (), - [&p] (const package_location& pl) - { - return p.name == pl.name; - })); + // 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; + })); - if (i == pls.end ()) - fail << "package " << p.name << " is not listed in " << prj; + if (i == pls.end ()) + fail << "package " << pkg.name << " is not listed in " << prj.path; - src /= i->path; - } + src /= i->path; + } - // We could run 'b info' and used the 'forwarded' value but this is - // both faster and simpler. - // - path f (src / "build" / "bootstrap" / "out-root.build"); - bool e (exists (f)); + // We could run 'b info' and used the 'forwarded' value but this is + // both faster and simpler. + // + path f (src / "build" / "bootstrap" / "out-root.build"); + bool e (exists (f)); - const char* o (nullptr); - if (c->forward) - { - bool changed (true); + const char* o (nullptr); + if (prj.config->forward) + { + bool changed (true); - if (changed || !e) - o = "configure:"; - } - else if (!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 (changed || !e) + 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 (e) - // o = "disfigure:"; - } + //if (e) + // o = "disfigure:"; + } - if (o != nullptr) - { - dir_path out (dir_path (c->path) /= p.name); - string arg (src.representation () + '@' + out.representation () + - ",forward"); - run_b (co, o, arg); + if (o != nullptr) + { + dir_path out (dir_path (cfg) /= pkg.name); + string arg (src.representation () + '@' + out.representation () + + ",forward"); + run_b (co, o, arg); + } } } // Add/remove auto-synchronization build system hook. // - if (!implicit) + if (origin_config != nullptr && !implicit) { - path f (c->path / "build" / "bootstrap" / "pre-bdep-sync.build"); + path f (cfg / "build" / "bootstrap" / "pre-bdep-sync.build"); bool e (exists (f)); - if (c->auto_sync) + if (origin_config->auto_sync) { if (!e) { @@ -242,8 +377,8 @@ namespace bdep << " $build.meta_operation != 'configure' && \\" << endl << " $build.meta_operation != 'disfigure')" << endl << " run " << argv0 << " sync --hook=1 " << - "--config \"$out_root\" " << - "-d '" << prj.string () << "'" << endl; + "--verbose $build.verbosity " << + "--config \"$out_root\"" << endl; os.close (); } @@ -270,6 +405,7 @@ namespace bdep bool yes) { cmd_sync (co, + c->path, prj, c, implicit, @@ -316,6 +452,9 @@ namespace bdep fail << "unsupported build system hook protocol" << info << "project requires re-initialization"; + if (o.directory_specified ()) + fail << "--hook specified with project directory"; + o.implicit (true); // Implies --implicit. } @@ -329,40 +468,90 @@ namespace bdep if (!dep_pkgs.empty ()) fail << "--implicit specified with dependency package"; + + if (!o.directory_specified ()) + { + // All of these options require an originating project. + // + if (o.fetch () || o.fetch_full ()) + fail << "--implicit specified with --fetch"; + + if (o.config_id_specified ()) + fail << "--implicit specified with --config-id"; + + if (o.config_name_specified ()) + fail << "--implicit specified with --config-name"; + + if (!o.config_specified ()) + fail << "no project or configuration specified with --implicit"; + } } - // We could be running from a package directory (or the user specified one - // with -d) that has not been init'ed in this configuration. Unless we are - // upgrading specific dependencies, we want to diagnose that since such a - // package will not be present in the bpkg configuration. But if we are - // running from the project, then we don't want to treat all the available - // packages as specified by the user (thus load_packages=false). + dir_path prj; // Empty if we have no originating project. + package_locations prj_pkgs; + configurations cfgs; + dir_paths cfg_dirs; + + // In the implicit mode we don't search the current working directory + // for a project. // - project_packages pp ( - find_project_packages (o, - !dep_pkgs.empty () /* ignore_packages */, - false /* load_packages */)); + if (o.directory_specified () || !o.implicit ()) + { + // We could be running from a package directory (or the user specified + // one with -d) that has not been init'ed in this configuration. Unless + // we are upgrading specific dependencies, we want to diagnose that + // since such a package will not be present in the bpkg configuration. + // But if we are running from the project, then we don't want to treat + // all the available packages as specified by the user (thus + // load_packages is false). + // + project_packages pp ( + find_project_packages (o, + !dep_pkgs.empty () /* ignore_packages */, + false /* load_packages */)); - const dir_path& prj (pp.project); - const package_locations& prj_pkgs (pp.packages); + // Load project configurations. + // + { + database db (open (pp.project, trace)); - database db (open (prj, trace)); + transaction t (db.begin ()); + cfgs = find_configurations (pp.project, t, o); + t.commit (); + } - transaction t (db.begin ()); - configurations cfgs (find_configurations (prj, t, o)); - t.commit (); + // If specified, verify packages are present in each configuration. + // + if (!pp.packages.empty ()) + verify_project_packages (pp, cfgs); - // If specified, verify packages are present in each configuration. - // - if (!prj_pkgs.empty ()) - verify_project_packages (pp, cfgs); + prj = move (pp.project); + prj_pkgs = move (pp.packages); + } + else + { + // Implicit sync without an originating project. + // + for (dir_path d: o.config ()) + { + d.complete (); + d.normalize (); + + cfgs.push_back (nullptr); + cfg_dirs.push_back (move (d)); + } + } - // Synchronize each configuration skipping empty ones. + // Synchronize each configuration. // - bool first (true); - for (const shared_ptr& c: cfgs) + for (size_t i (0), n (cfgs.size ()); i != n; ++i) { - if (c->packages.empty ()) + const shared_ptr& c (cfgs[i]); // Can be NULL. + const dir_path& cd (c != nullptr ? c->path : cfg_dirs[i]); + + // Skipping empty ones. + // + if (c != nullptr && c->packages.empty ()) { if (verb) info << "skipping empty configuration " << *c; @@ -373,14 +562,10 @@ namespace bdep // If we are synchronizing multiple configurations, separate them with a // blank line and print the configuration name/directory. // - if (verb && cfgs.size () > 1) - { - text << (first ? "" : "\n") + if (verb && n > 1) + text << (i == 0 ? "" : "\n") << "in configuration " << *c << ':'; - first = false; - } - bool fetch (o.fetch () || o.fetch_full ()); if (fetch) @@ -393,6 +578,7 @@ namespace bdep // Only prompt if upgrading their dependencies. // cmd_sync (o, + cd, prj, c, false /* implicit */, @@ -410,6 +596,7 @@ namespace bdep // (immediate by default, recursive if requested). // cmd_sync (o, + cd, prj, c, false /* implicit */, @@ -438,7 +625,7 @@ namespace bdep const char n[] = "BDEP_SYNCED_CONFIGS"; string v; - const string& p (c->path.string ()); + const string& p (cd.string ()); if (const char* e = getenv (n)) { @@ -461,7 +648,7 @@ namespace bdep if (o.implicit ()) return 0; // Ignore. else - fail << "explicit re-synchronization of " << c->path; + fail << "explicit re-synchronization of " << cd; } } @@ -479,7 +666,17 @@ namespace bdep #endif } - cmd_sync (o, prj, c, o.implicit (), !fetch, true /* yes */); + cmd_sync (o, + cd, + prj, + c, + o.implicit (), + !fetch, + true /* yes */, + nullopt /* upgrade */, + nullopt /* recursive */, + package_locations () /* prj_pkgs */, + strings () /* dep_pkgs */); } } diff --git a/bdep/utility.cxx b/bdep/utility.cxx index e048a5a..15fb599 100644 --- a/bdep/utility.cxx +++ b/bdep/utility.cxx @@ -109,6 +109,23 @@ namespace bdep : "bpkg" BDEP_EXE_SUFFIX; } + void + finish_bpkg (const common_options& co, process& pr, bool io) + { + if (!pr.wait ()) + { + const process_exit& e (*pr.exit); + + if (e.normal ()) + throw failed (); // Assume the child issued diagnostics. + + fail << "process " << name_bpkg (co) << " " << e; + } + + if (io) + fail << "error reading " << name_bpkg (co) << " output"; + } + const char* name_b (const common_options& co) { diff --git a/bdep/utility.hxx b/bdep/utility.hxx index 09e0e45..1ecdc6a 100644 --- a/bdep/utility.hxx +++ b/bdep/utility.hxx @@ -15,6 +15,7 @@ #include #include // casecmp(), reverse_iterate(), etc +#include #include #include @@ -54,6 +55,10 @@ namespace bdep using butl::auto_rmfile; using butl::auto_rmdir; + // + // + using butl::fdopen_pipe; + // Empty string and path. // extern const string empty_string; @@ -118,6 +123,9 @@ namespace bdep process start_bpkg (const common_options&, O&& out, E&& err, A&&... args); + void + finish_bpkg (const common_options&, process&, bool io_error = false); + template void run_bpkg (const common_options&, A&&... args); diff --git a/bdep/utility.txx b/bdep/utility.txx index aa75952..f09983e 100644 --- a/bdep/utility.txx +++ b/bdep/utility.txx @@ -133,15 +133,7 @@ namespace bdep 1 /* stdout */, 2 /* stderr */, forward (args)...)); - if (!pr.wait ()) - { - const process_exit& e (*pr.exit); - - if (e.normal ()) - throw failed (); // Assume the child issued diagnostics. - - fail << "process " << name_bpkg (co) << " " << e; - } + finish_bpkg (co, pr); } // *_b() -- cgit v1.1