From ec931aa6550b47461e92062a703e6ef9f4c24b17 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sat, 10 Oct 2015 09:30:37 +0200 Subject: Implement --replace|-r mode for pkg-unpack; improve in pkg-fetch --- bpkg/pkg-fetch-options.cli | 8 ++-- bpkg/pkg-fetch.cxx | 33 +++++++------- bpkg/pkg-purge | 12 ++--- bpkg/pkg-purge.cxx | 107 +++++++++++++++++++------------------------- bpkg/pkg-unpack-options.cli | 13 +++++- bpkg/pkg-unpack.cxx | 77 ++++++++++++++++++++++--------- bpkg/test.sh | 32 +++++++++++-- 7 files changed, 174 insertions(+), 108 deletions(-) diff --git a/bpkg/pkg-fetch-options.cli b/bpkg/pkg-fetch-options.cli index 4cfeb35..a064da5 100644 --- a/bpkg/pkg-fetch-options.cli +++ b/bpkg/pkg-fetch-options.cli @@ -17,9 +17,9 @@ bpkg pkg-fetch [] (/)|(-e )" The \cb{pkg-fetch} command fetches the archive for the specified package name and version from one of the configuration's repositories. If the \cb{-r|--replace} option is specified, then \cb{pkg-fetch} will replace the -archive of a package that is already in the \cb{fetched} state. Otherwise, -\cb{pkg-fetch} expects the package to not exist in the configuration's -database. +archive of a package that is already in the \cb{fetched} or \cb{unpacked} +state. Otherwise, \cb{pkg-fetch} expects the package to not exist in the +configuration's database. If the \cb{-e|--existing} option is used, then instead of the name and version argument, \cb{pkg-fetch} expects a local path to the existing @@ -36,7 +36,7 @@ namespace bpkg { bool --replace|-r { - "Replace the archive if the package is already fetched." + "Replace the archive if the package is already fetched or unpacked." }; bool --existing|-e diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx index c8b8543..fe8eca7 100644 --- a/bpkg/pkg-fetch.cxx +++ b/bpkg/pkg-fetch.cxx @@ -52,21 +52,24 @@ namespace bpkg { shared_ptr p (db.find (n)); - if (p == nullptr || - (p->state == package_state::fetched && o.replace ())) - return p; - + if (p != nullptr) { - diag_record dr (error); + bool s (p->state == package_state::fetched || + p->state == package_state::unpacked); + + if (!o.replace () || !s) + { + diag_record dr (fail); - dr << "package " << n << " already exists in configuration " << c << - info << "version: " << p->version << ", state: " << p->state; + dr << "package " << n << " already exists in configuration " << c << + info << "version: " << p->version << ", state: " << p->state; - if (p->state == package_state::fetched) - dr << info << "use 'pkg-fetch --replace|-r' to replace its archive"; + if (s) // Suitable state for replace? + dr << info << "use 'pkg-fetch --replace|-r' to replace"; + } } - throw failed (); + return p; }; if (o.existing ()) @@ -167,14 +170,14 @@ namespace bpkg if (sp != nullptr) { - // Clean up the archive we are replacing. Once this is done, there - // is no going back. If things go badly, we can't simply abort the - // transaction. + // Clean up the source directory and archive of the package we are + // replacing. Once this is done, there is no going back. If things + // go badly, we can't simply abort the transaction. // - if (sp->purge_archive) - pkg_purge_archive (c, t, sp); + pkg_purge_fs (c, t, sp); sp->version = move (m.version); + sp->state = package_state::fetched; sp->repository = move (rl); sp->archive = move (a); sp->purge_archive = purge; diff --git a/bpkg/pkg-purge b/bpkg/pkg-purge index 4765e60..0c88e19 100644 --- a/bpkg/pkg-purge +++ b/bpkg/pkg-purge @@ -14,13 +14,15 @@ namespace bpkg void pkg_purge (const pkg_purge_options&, cli::scanner& args); - // Remove package archive. If this fails, set the package state to - // broken, commit the transaction, and fail. + // Remove package's filesystem objects (the source directory and, if + // the archive argument is true, the package archive). If this fails, + // set the package state to broken, commit the transaction, and fail. // void - pkg_purge_archive (const dir_path& configuration, - transaction&, - const shared_ptr&); + pkg_purge_fs (const dir_path& configuration, + transaction&, + const shared_ptr&, + bool archive = true); } #endif // BPKG_PKG_PURGE diff --git a/bpkg/pkg-purge.cxx b/bpkg/pkg-purge.cxx index 91c4c5b..dd92857 100644 --- a/bpkg/pkg-purge.cxx +++ b/bpkg/pkg-purge.cxx @@ -17,26 +17,42 @@ using namespace butl; namespace bpkg { void - pkg_purge_archive (const dir_path& c, - transaction& t, - const shared_ptr& p) + pkg_purge_fs (const dir_path& c, + transaction& t, + const shared_ptr& p, + bool archive) { - assert (p->purge_archive && p->state != package_state::broken); + assert (p->state == package_state::fetched || + p->state == package_state::unpacked); tracer trace ("pkg_purge_archive"); database& db (t.database ()); tracer_guard tg (db, trace); - path a (p->archive->absolute () ? *p->archive : c / *p->archive); - try { - if (exists (a)) - rm (a); + if (p->purge_src) + { + dir_path d (p->src_root->absolute () ? *p->src_root : c / *p->src_root); + + if (exists (d)) // Don't complain if someone did our job for us. + rm_r (d); + + p->src_root = nullopt; + p->purge_src = false; + } + + if (p->purge_archive && archive) + { + path a (p->archive->absolute () ? *p->archive : c / *p->archive); - p->archive = nullopt; - p->purge_archive = false; + if (exists (a)) + rm (a); + + p->archive = nullopt; + p->purge_archive = false; + } } catch (const failed&) { @@ -108,66 +124,30 @@ namespace bpkg } } - // First clean up the package source directory. + // For a broken package we just verify that all the filesystem objects + // were cleaned up by the user. // - if (p->purge_src) + if (p->state == package_state::broken) { - dir_path d (p->src_root->absolute () ? *p->src_root : c / *p->src_root); - - if (p->state != package_state::broken) + if (p->out_root) { - try - { - if (exists (d)) // Don't complain if someone did our job for us. - rm_r (d); - - p->src_root = nullopt; - p->purge_src = false; - } - catch (const failed&) - { - p->state = package_state::broken; - db.update (p); - t.commit (); - - info << "package " << n << " is now broken; " - << "use 'pkg-purge --force' to remove"; - throw; - } + dir_path d (c / *p->out_root); // Always relative. + + if (exists (d)) + fail << "broken package " << n << " output directory still exists" << + info << "remove " << d << " manually then re-run pkg-purge"; } - else + + if (p->purge_src) { - // If we are broken, simply make sure the user cleaned things up - // manually. - // + dir_path d (p->src_root->absolute () ? *p->src_root : c / *p->src_root); + if (exists (d)) fail << "broken package " << n << " source directory still exists" << info << "remove " << d << " manually then re-run pkg-purge"; } - } - - // Also check the output directory of broken packages. - // - if (p->out_root) - { - // Can only be present if broken. - // - assert (p->state == package_state::broken); - dir_path d (c / *p->out_root); // Always relative. - - if (exists (d)) - fail << "broken package " << n << " output directory still exists" << - info << "remove " << d << " manually then re-run pkg-purge"; - } - - // Now the archive. Pretty much the same code as above but for a file. - // - if (p->purge_archive && !o.keep ()) - { - if (p->state != package_state::broken) - pkg_purge_archive (c, t, p); - else + if (p->purge_archive) { path a (p->archive->absolute () ? *p->archive : c / *p->archive); @@ -176,7 +156,14 @@ namespace bpkg info << "remove " << a << " manually then re-run pkg-purge"; } } + else + { + assert (!p->out_root); + pkg_purge_fs (c, t, p, !o.keep ()); + } + // Finally, update the database state. + // if (o.keep ()) { if (p->state != package_state::fetched) // No-op we were talking about. diff --git a/bpkg/pkg-unpack-options.cli b/bpkg/pkg-unpack-options.cli index c9101a9..97386a1 100644 --- a/bpkg/pkg-unpack-options.cli +++ b/bpkg/pkg-unpack-options.cli @@ -22,7 +22,12 @@ a local path to the existing package source directory. In this case, configuration or package cache directories. Also, unless the \cb{-p|--purge} option is specified, \cb{bpkg} will not attempt to remove this directory when the package is purged with the \cb{pkg-purge} -command." +command. + +If \cb{-e|--existing} is specified together with the \cb{-r|--replace} +option, then \cb{pkg-unpack} will replace the archive and/or source +directory of a package that is already in the \cb{unpacked} or +\cb{fetched} state." */ namespace bpkg @@ -39,5 +44,11 @@ namespace bpkg { "Remove the existing package directory when the package is purged." }; + + bool --replace|-r + { + "Replace the source directory if the package is already unpacked or + fetched. Can only be specified together with \cb{-e|--existing}." + }; }; } diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index a7c53b6..7100809 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -15,6 +15,7 @@ #include #include +#include #include using namespace std; @@ -23,7 +24,11 @@ using namespace butl; namespace bpkg { static shared_ptr - pkg_unpack (database& db, const dir_path& c, const dir_path& d, bool purge) + pkg_unpack (database& db, + const dir_path& c, + const dir_path& d, + bool replace, + bool purge) { tracer trace ("pkg_unpack(dir)"); tracer_guard tg (db, trace); @@ -36,16 +41,6 @@ namespace bpkg package_manifest m (pkg_verify (d)); level4 ([&]{trace << d << ": " << m.name << " " << m.version;}); - const auto& n (m.name); - - transaction t (db.begin ()); - - // See if this package already exists in this configuration. - // - if (shared_ptr p = db.find (n)) - fail << "package " << n << " already exists in configuration " << c << - info << "version: " << p->version << ", state: " << p->state; - // Make the package and configuration paths absolute and normalized. // If the package is inside the configuration, use the relative path. // This way we can move the configuration around. @@ -57,25 +52,63 @@ namespace bpkg if (ad.sub (ac)) ad = ad.leaf (ac); - // Add the package to the configuration. Use the special root - // repository as the repository of this package. + transaction t (db.begin ()); + + // See if this package already exists in this configuration. // - shared_ptr p (new selected_package { + const string& n (m.name); + shared_ptr p (db.find (n)); + + if (p != nullptr) + { + bool s (p->state == package_state::fetched || + p->state == package_state::unpacked); + + if (!replace || !s) + { + diag_record dr (fail); + + dr << "package " << n << " already exists in configuration " << c << + info << "version: " << p->version << ", state: " << p->state; + + if (s) // Suitable state for replace? + dr << info << "use 'pkg-unpack --replace|-r' to replace"; + } + + // Clean up the source directory and archive of the package we are + // replacing. Once this is done, there is no going back. If things + // go badly, we can't simply abort the transaction. + // + pkg_purge_fs (c, t, p); + + // Use the special root repository as the repository of this package. + // + p->version = move (m.version); + p->state = package_state::unpacked; + p->repository = repository_location (); + p->src_root = move (ad); + p->purge_src = purge; + + db.update (p); + } + else + { + p.reset (new selected_package { move (m.name), move (m.version), package_state::unpacked, - repository_location (), + repository_location (), // Root repository. nullopt, // No archive false, // Don't purge archive. move (ad), purge, nullopt, // No output directory yet. - {} // No prerequisites captured yet. - }); + {}}); // No prerequisites captured yet. - db.persist (p); - t.commit (); + db.persist (p); + } + t.commit (); return p; } @@ -184,6 +217,9 @@ namespace bpkg { tracer trace ("pkg_unpack"); + if (o.replace () && !o.existing ()) + fail << "-r|--replace can only be specified with -e|--existing"; + const dir_path& c (o.directory ()); level4 ([&]{trace << "configuration: " << c;}); @@ -199,7 +235,8 @@ namespace bpkg fail << "package directory argument expected" << info << "run 'bpkg help pkg-unpack' for more information"; - p = pkg_unpack (db, c, dir_path (args.next ()), o.purge ()); + p = pkg_unpack ( + db, c, dir_path (args.next ()), o.replace (), o.purge ()); } else { diff --git a/bpkg/test.sh b/bpkg/test.sh index f787baa..4a99347 100755 --- a/bpkg/test.sh +++ b/bpkg/test.sh @@ -212,9 +212,11 @@ test pkg-purge libfoo test pkg-fetch -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz stat libfoo/1.0.0 fetched test pkg-unpack libfoo -fail pkg-fetch -r libfoo/1.0.0 -fail pkg-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz -test pkg-purge -k libfoo +test pkg-fetch -r libfoo/1.1.0 +stat libfoo/1.1.0 fetched +test pkg-unpack libfoo +test pkg-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz +stat libfoo/1.0.0 fetched test pkg-fetch -r libfoo/1.1.0 stat libfoo/1.1.0 fetched test pkg-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz @@ -231,6 +233,7 @@ test pkg-fetch libhello/1.0.0 test pkg-unpack libhello test pkg-purge libhello + ## @@ ## ## @@ -246,6 +249,29 @@ stat fetched test pkg-purge $pkg stat unknown +## +## pkg-unpack +## + +# @@ TODO + +# replace +# +test cfg-create --wipe +test rep-add ../tests/repository/1/fetch/t1 +test rep-fetch +test pkg-fetch libfoo/1.0.0 +fail pkg-unpack -e ../tests/repository/1/fetch/libfoo-1.1.0 +test pkg-unpack -r -e ../tests/repository/1/fetch/libfoo-1.1.0 +stat libfoo/1.1.0 unpacked +test pkg-purge libfoo +test pkg-fetch libfoo/1.0.0 +test pkg-unpack libfoo +fail pkg-unpack -e ../tests/repository/1/fetch/libfoo-1.1.0 +test pkg-unpack -r -e ../tests/repository/1/fetch/libfoo-1.1.0 +stat libfoo/1.1.0 unpacked +test pkg-purge libfoo + ## ## pkg-purge -- cgit v1.1