From 4e3faacbc3c27e1d01ca95697b34db82cdecdb9d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sat, 10 Oct 2015 08:12:50 +0200 Subject: Implement --replace|-r mode for pkg-fetch --- bpkg/pkg-fetch-options.cli | 22 ++++-- bpkg/pkg-fetch.cxx | 85 ++++++++++++++++----- bpkg/pkg-purge | 9 +++ bpkg/pkg-purge-options.cli | 4 +- bpkg/pkg-purge.cxx | 59 ++++++++------ bpkg/test.sh | 37 ++++++--- tests/repository/1/fetch/libfoo-1.0.0.tar.gz | Bin 0 -> 348 bytes tests/repository/1/fetch/libfoo-1.1.0.tar.gz | Bin 0 -> 349 bytes .../1/fetch/libfoo-1.1.0/build/bootstrap.build | 2 + tests/repository/1/fetch/libfoo-1.1.0/buildfile | 1 + tests/repository/1/fetch/libfoo-1.1.0/manifest | 7 ++ tests/repository/1/fetch/repositories | 1 + tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz | 1 + tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz | 1 + tests/repository/1/fetch/t1/repositories | 1 + .../1/satisfy/libfoo-1.1.0/build/bootstrap.build | 2 +- 16 files changed, 171 insertions(+), 61 deletions(-) create mode 100644 tests/repository/1/fetch/libfoo-1.0.0.tar.gz create mode 100644 tests/repository/1/fetch/libfoo-1.1.0.tar.gz create mode 100644 tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build create mode 100644 tests/repository/1/fetch/libfoo-1.1.0/buildfile create mode 100644 tests/repository/1/fetch/libfoo-1.1.0/manifest create mode 100644 tests/repository/1/fetch/repositories create mode 120000 tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz create mode 120000 tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz create mode 120000 tests/repository/1/fetch/t1/repositories diff --git a/bpkg/pkg-fetch-options.cli b/bpkg/pkg-fetch-options.cli index afd75fb..4cfeb35 100644 --- a/bpkg/pkg-fetch-options.cli +++ b/bpkg/pkg-fetch-options.cli @@ -16,12 +16,17 @@ 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{-e|--existing} option is used, then instead of the name and version -argument, \cb{pkg-fetch} expects a local path to the existing package -archive. In this case, \cb{bpkg} will use the archive in place, without -copying it to the configuration or package cache directories. Also, -unless the \cb{-p|--purge} option is specified, \cb{bpkg} will not -attempt to remove the archive when the package is purged with 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. + +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 +package archive. In this case, \cb{bpkg} will use the archive in place, +without copying it to the configuration or package cache directories. +Also, unless the \cb{-p|--purge} option is specified, \cb{bpkg} will +not attempt to remove the archive when the package is purged with the \cb{pkg-purge} command." */ @@ -29,6 +34,11 @@ namespace bpkg { class pkg_fetch_options: configuration_options { + bool --replace|-r + { + "Replace the archive if the package is already fetched." + }; + bool --existing|-e { "Treat the argument as an existing package archive path rather than diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx index daa2d5e..c8b8543 100644 --- a/bpkg/pkg-fetch.cxx +++ b/bpkg/pkg-fetch.cxx @@ -15,6 +15,7 @@ #include #include +#include #include using namespace std; @@ -38,6 +39,35 @@ namespace bpkg auto_rm arm; bool purge; repository_location rl; + shared_ptr sp; + + // Check if the package already exists in this configuration and + // diagnose all the illegal cases. We want to do this as soon as + // the package name is known which happens at different times + // depending on whether we are dealing with an existing archive + // or fetching one. + // + auto check = [&o, &c, &db] (const string& n) + -> shared_ptr + { + shared_ptr p (db.find (n)); + + if (p == nullptr || + (p->state == package_state::fetched && o.replace ())) + return p; + + { + diag_record dr (error); + + 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"; + } + + throw failed (); + }; if (o.existing ()) { @@ -71,6 +101,10 @@ namespace bpkg fail << "package version expected" << info << "run 'bpkg help pkg-fetch' for more information"; + // Check/diagnose an already existing package. + // + sp = check (n); + if (db.query_value () == 0) fail << "configuration " << c << " has no repositories" << info << "use 'bpkg rep-add' to add a repository"; @@ -79,18 +113,18 @@ namespace bpkg fail << "configuration " << c << " has no available packages" << info << "use 'bpkg rep-fetch' to fetch available packages list"; - shared_ptr p ( + shared_ptr ap ( db.find (available_package_id (n, v))); - if (p == nullptr) + if (ap == nullptr) fail << "package " << n << " " << v << " is not available"; - // Pick a repository. Preferring local ones over the remote seems + // Pick a repository. Preferring a local one over the remotes seems // like a sensible thing to do. // - const package_location* pl (&p->locations.front ()); + const package_location* pl (&ap->locations.front ()); - for (const package_location& l: p->locations) + for (const package_location& l: ap->locations) { if (!l.repository.load ()->location.remote ()) { @@ -116,13 +150,10 @@ namespace bpkg package_manifest m (pkg_verify (o, a)); level4 ([&]{trace << m.name << " " << m.version;}); - const auto& n (m.name); - - // See if this package already exists in this configuration. + // Check/diagnose an already existing package. // - if (shared_ptr p = db.find (n)) - fail << "package " << n << " already exists in configuration " << c << - info << "version: " << p->version << ", state: " << p->state; + if (o.existing ()) + sp = check (m.name); // Make the archive and configuration paths absolute and normalized. // If the archive is inside the configuration, use the relative path. @@ -134,9 +165,27 @@ namespace bpkg if (a.sub (c)) a = a.leaf (c); - // Add the package to the configuration. - // - shared_ptr p (new selected_package { + 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. + // + if (sp->purge_archive) + pkg_purge_archive (c, t, sp); + + sp->version = move (m.version); + sp->repository = move (rl); + sp->archive = move (a); + sp->purge_archive = purge; + + db.update (sp); + } + else + { + // Add the package to the configuration. + // + sp.reset (new selected_package { move (m.name), move (m.version), package_state::fetched, @@ -146,15 +195,15 @@ namespace bpkg nullopt, // No source directory yet. false, nullopt, // No output directory yet. - {} // No prerequisites captured yet. - }); + {}}); // No prerequisites captured yet. - db.persist (p); + db.persist (sp); + } t.commit (); arm.cancel (); if (verb) - text << "fetched " << p->name << " " << p->version; + text << "fetched " << sp->name << " " << sp->version; } } diff --git a/bpkg/pkg-purge b/bpkg/pkg-purge index 7ec365d..4765e60 100644 --- a/bpkg/pkg-purge +++ b/bpkg/pkg-purge @@ -6,12 +6,21 @@ #define BPKG_PKG_PURGE #include +#include // transaction, selected_package #include 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. + // + void + pkg_purge_archive (const dir_path& configuration, + transaction&, + const shared_ptr&); } #endif // BPKG_PKG_PURGE diff --git a/bpkg/pkg-purge-options.cli b/bpkg/pkg-purge-options.cli index f5fef62..a4d7633 100644 --- a/bpkg/pkg-purge-options.cli +++ b/bpkg/pkg-purge-options.cli @@ -16,8 +16,8 @@ bpkg pkg-purge [] " The \cb{pkg-purge} command removes the package directory and archive from the filesystem and removes the package from the configuration's -database. Only packages in the \cb{fetched} and \cb{unpacked} state -can be purged plus broken packages if the \cb{-f|--force} options is +database. Only packages in the \cb{fetched} and \cb{unpacked} state can +be purged plus \cb{broken} packages if the \cb{-f|--force} options is specified (see this option's description for details on purging broken packages). If the \cb{-k|--keep} option is specified, then the package archive is not removed (see this option's description for details on diff --git a/bpkg/pkg-purge.cxx b/bpkg/pkg-purge.cxx index 5e14a2b..91c4c5b 100644 --- a/bpkg/pkg-purge.cxx +++ b/bpkg/pkg-purge.cxx @@ -17,6 +17,40 @@ using namespace butl; namespace bpkg { void + pkg_purge_archive (const dir_path& c, + transaction& t, + const shared_ptr& p) + { + assert (p->purge_archive && p->state != package_state::broken); + + 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); + + p->archive = nullopt; + p->purge_archive = false; + } + catch (const failed&) + { + p->state = package_state::broken; + db.update (p); + t.commit (); + + info << "package " << p->name << " is now broken; " + << "use 'pkg-purge --force' to remove"; + throw; + } + } + + void pkg_purge (const pkg_purge_options& o, cli::scanner& args) { tracer trace ("pkg_purge"); @@ -131,31 +165,12 @@ namespace bpkg // if (p->purge_archive && !o.keep ()) { - path a (p->archive->absolute () ? *p->archive : c / *p->archive); - if (p->state != package_state::broken) - { - try - { - if (exists (a)) - rm (a); - - p->archive = nullopt; - p->purge_archive = 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; - } - } + pkg_purge_archive (c, t, p); else { + path a (p->archive->absolute () ? *p->archive : c / *p->archive); + if (exists (a)) fail << "broken package " << n << " archive still exists" << info << "remove " << a << " manually then re-run pkg-purge"; diff --git a/bpkg/test.sh b/bpkg/test.sh index a806a25..f787baa 100755 --- a/bpkg/test.sh +++ b/bpkg/test.sh @@ -183,30 +183,43 @@ test rep-fetch ## ## pkg-fetch ## - +test rep-create ../tests/repository/1/fetch/t1 test cfg-create --wipe fail pkg-fetch -e # archive expected fail pkg-fetch -e ./no-such-file # archive does not exist -fail pkg-fetch # package name expected -fail pkg-fetch libhello # package version expected -fail pkg-fetch libhello/1/2/3 # invalid package version +fail pkg-fetch # package name expected +fail pkg-fetch libfoo # package version expected +fail pkg-fetch libfoo/1/2/3 # invalid package version -fail pkg-fetch libhello/1.0.0 # no repositories -test rep-add $rep -fail pkg-fetch libhello/1.0.0 # no packages +fail pkg-fetch libfoo/1.0.0 # no repositories +test rep-add ../tests/repository/1/fetch/t1 +fail pkg-fetch libfoo/1.0.0 # no packages test rep-fetch -fail pkg-fetch libhello/2+1.0.0 # not available +fail pkg-fetch libfoo/2+1.0.0 # not available # local # test cfg-create --wipe -test rep-add $rep +test rep-add ../tests/repository/1/fetch/t1 test rep-fetch -test pkg-fetch libhello/1.0.0 -test pkg-unpack libhello -test pkg-purge libhello +test pkg-fetch libfoo/1.0.0 +stat libfoo/1.0.0 fetched +fail pkg-fetch libfoo/1.0.0 +fail pkg-fetch -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz +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-fetch -r -e ../tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz +stat libfoo/1.0.0 fetched +test pkg-purge libfoo # remote # diff --git a/tests/repository/1/fetch/libfoo-1.0.0.tar.gz b/tests/repository/1/fetch/libfoo-1.0.0.tar.gz new file mode 100644 index 0000000..28a6a90 Binary files /dev/null and b/tests/repository/1/fetch/libfoo-1.0.0.tar.gz differ diff --git a/tests/repository/1/fetch/libfoo-1.1.0.tar.gz b/tests/repository/1/fetch/libfoo-1.1.0.tar.gz new file mode 100644 index 0000000..e03481f Binary files /dev/null and b/tests/repository/1/fetch/libfoo-1.1.0.tar.gz differ diff --git a/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build b/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build new file mode 100644 index 0000000..54f267e --- /dev/null +++ b/tests/repository/1/fetch/libfoo-1.1.0/build/bootstrap.build @@ -0,0 +1,2 @@ +project = fetch-libfoo +using config diff --git a/tests/repository/1/fetch/libfoo-1.1.0/buildfile b/tests/repository/1/fetch/libfoo-1.1.0/buildfile new file mode 100644 index 0000000..b3ec74f --- /dev/null +++ b/tests/repository/1/fetch/libfoo-1.1.0/buildfile @@ -0,0 +1 @@ +.: diff --git a/tests/repository/1/fetch/libfoo-1.1.0/manifest b/tests/repository/1/fetch/libfoo-1.1.0/manifest new file mode 100644 index 0000000..3453757 --- /dev/null +++ b/tests/repository/1/fetch/libfoo-1.1.0/manifest @@ -0,0 +1,7 @@ +: 1 +name: libfoo +version: 1.1.0 +summary: libfoo +license: MIT +url: http://example.org +email: pkg@example.org diff --git a/tests/repository/1/fetch/repositories b/tests/repository/1/fetch/repositories new file mode 100644 index 0000000..5b70556 --- /dev/null +++ b/tests/repository/1/fetch/repositories @@ -0,0 +1 @@ +: 1 diff --git a/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz b/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz new file mode 120000 index 0000000..32e5a3c --- /dev/null +++ b/tests/repository/1/fetch/t1/libfoo-1.0.0.tar.gz @@ -0,0 +1 @@ +../libfoo-1.0.0.tar.gz \ No newline at end of file diff --git a/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz b/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz new file mode 120000 index 0000000..c004b2a --- /dev/null +++ b/tests/repository/1/fetch/t1/libfoo-1.1.0.tar.gz @@ -0,0 +1 @@ +../libfoo-1.1.0.tar.gz \ No newline at end of file diff --git a/tests/repository/1/fetch/t1/repositories b/tests/repository/1/fetch/t1/repositories new file mode 120000 index 0000000..d965b15 --- /dev/null +++ b/tests/repository/1/fetch/t1/repositories @@ -0,0 +1 @@ +../repositories \ No newline at end of file diff --git a/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build b/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build index eb90fee..b24ee6a 100644 --- a/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build +++ b/tests/repository/1/satisfy/libfoo-1.1.0/build/bootstrap.build @@ -1,2 +1,2 @@ -project = libfoo +project = satisfy-libfoo using config -- cgit v1.1