diff options
| -rw-r--r-- | bpkg/package.hxx | 12 | ||||
| -rw-r--r-- | bpkg/pkg-fetch.cxx | 85 | ||||
| -rw-r--r-- | bpkg/pkg-fetch.hxx | 2 | ||||
| -rw-r--r-- | bpkg/pkg-purge.cxx | 4 | ||||
| -rw-r--r-- | bpkg/pkg-unpack.cxx | 2 | ||||
| -rw-r--r-- | bpkg/utility.cxx | 24 | ||||
| -rw-r--r-- | bpkg/utility.hxx | 3 | ||||
| -rw-r--r-- | tests/pkg-fetch.testscript | 9 |
8 files changed, 132 insertions, 9 deletions
diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 02d2b07..947393b 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -1318,6 +1318,18 @@ namespace bpkg std::string string (database&) const; + // Return the relative archive path completed using the configuration + // directory. Return the absolute archive path as is. + // + path + effective_archive (const dir_path& configuration) const + { + // Cast for compiling with ODB (see above). + // + assert (static_cast<bool> (archive)); + return archive->absolute () ? *archive : configuration / *archive; + } + // Return the relative source directory completed using the configuration // directory. Return the absolute source directory as is. // diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx index 009ddf9..837c968 100644 --- a/bpkg/pkg-fetch.cxx +++ b/bpkg/pkg-fetch.cxx @@ -44,17 +44,24 @@ namespace bpkg // normalize (a, "archive"); + // Only purge the existing archive if its path differs from the new path. + // + shared_ptr<selected_package> p (db.find<selected_package> (n)); + + bool purge_archive (p != nullptr && + p->archive && + p->effective_archive (db.config) != a); + if (a.sub (db.config)) a = a.leaf (db.config); - shared_ptr<selected_package> p (db.find<selected_package> (n)); if (p != nullptr) { // 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. + // replacing. Once this is done, there is no going back. If things go + // badly, we can't simply abort the transaction. // - pkg_purge_fs (db, t, p, simulate); + pkg_purge_fs (db, t, p, simulate, purge_archive); // Note that if the package name spelling changed then we need to update // it, to make sure that the subsequent commands don't fail and the @@ -197,6 +204,8 @@ namespace bpkg bool replace, bool simulate) { + assert (session::has_current ()); + tracer trace ("pkg_fetch"); tracer_guard tg (pdb, trace); // NOTE: sets tracer for the whole cluster. @@ -248,10 +257,76 @@ namespace bpkg << "from " << pl->repository_fragment->name; auto_rmfile arm; - path a (pdb.config_orig / pl->location.leaf ()); + path an (pl->location.leaf ()); + path a (pdb.config_orig / an); + + // Note that in the replace mode we first fetch the new package version + // archive and then update the existing selected package object, dropping + // the previous package version archive, if present. This way we, in + // particular, keep the existing selected package/archive intact if the + // fetch operation fails. However, this approach requires to handle + // re-fetching (potentially from a different repository) of the same + // package version specially. + // + // Specifically, if we need to overwrite the package archive file, then we + // stash the existing archive in the temporary directory and remove it on + // success. On failure, we try to move the stashed archive to the original + // place. Failed that either, we mark the package as broken. + // + // (If you are wondering why don't we instead always fetch into a + // temporary file, the answer is Windows, where moving a newly created + // file may not succeed because it is being scanned by Windows Defender + // or some such.) + // + auto_rmfile earm; + shared_ptr<selected_package> sp; + + auto g ( + make_exception_guard ( + [&arm, &a, &earm, &sp, &pdb, &t] () + { + // Restore stashed archive. + // + if (!earm.path.empty () && exists (earm.path)) + { + if (mv (earm.path, a, true /* ignore_error */)) + { + earm.cancel (); + arm.cancel (); // Note: may not be armed yet, which is ok. + } + // + // Note: can already be marked as broken by pkg_purge_fs(). + // + else if (sp->state != package_state::broken) + { + sp->state = package_state::broken; + pdb.update (sp); + t.commit (); + + // Here we assume that mv() has already issued the diagnostics. + // + info << "package " << sp->name << pdb << " is now broken; " + << "use 'pkg-purge --force' to remove"; + } + } + })); if (!simulate) { + // Stash the existing package archive if it needs to be overwritten (see + // above for details). + // + // Note: compare the archive absolute paths. + // + if (replace && + (sp = pdb.find<selected_package> (n)) != nullptr && + sp->archive && + sp->effective_archive (pdb.config) == pdb.config / an) + { + earm = tmp_file (pdb.config_orig, n.string () + '-' + v.string ()); + mv (a, earm.path); + } + pkg_fetch_archive ( co, pl->repository_fragment->location, pl->location, a); diff --git a/bpkg/pkg-fetch.hxx b/bpkg/pkg-fetch.hxx index 5d698d5..8607178 100644 --- a/bpkg/pkg-fetch.hxx +++ b/bpkg/pkg-fetch.hxx @@ -37,6 +37,8 @@ namespace bpkg // Note that both package and repository information configurations need to // be passed. // + // Also note that it should be called in session. + // shared_ptr<selected_package> pkg_fetch (const common_options&, database& pdb, diff --git a/bpkg/pkg-purge.cxx b/bpkg/pkg-purge.cxx index 07951a6..e031b6a 100644 --- a/bpkg/pkg-purge.cxx +++ b/bpkg/pkg-purge.cxx @@ -58,7 +58,7 @@ namespace bpkg { if (!simulate) { - path a (p->archive->absolute () ? *p->archive : c / *p->archive); + path a (p->effective_archive (c)); if (exists (a)) rm (a); @@ -192,7 +192,7 @@ namespace bpkg if (p->purge_archive) { - path a (p->archive->absolute () ? *p->archive : c / *p->archive); + path a (p->effective_archive (c)); if (exists (a)) fail << "archive file of broken package " << n << " still exists" << diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index 323f296..22ff02f 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -381,7 +381,7 @@ namespace bpkg // If the archive path is not absolute, then it must be relative // to the configuration. // - path a (p->archive->absolute () ? *p->archive : c / *p->archive); + path a (p->effective_archive (c)); l4 ([&]{trace << "archive: " << a;}); diff --git a/bpkg/utility.cxx b/bpkg/utility.cxx index ec95264..d084b76 100644 --- a/bpkg/utility.cxx +++ b/bpkg/utility.cxx @@ -313,6 +313,30 @@ namespace bpkg return true; } + bool + mv (const path& from, const path& to, bool ie) + { + if (verb >= 3) + text << "mv " << from << ' ' << to; + + try + { + mvfile (from, to, + cpflags::overwrite_content | cpflags::overwrite_permissions); + } + catch (const system_error& e) + { + error << "unable to move file " << from << " to " << to << ": " << e; + + if (ie) + return false; + + throw failed (); + } + + return true; + } + dir_path change_wd (const dir_path& d) { diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx index f36185a..7a51948 100644 --- a/bpkg/utility.hxx +++ b/bpkg/utility.hxx @@ -214,6 +214,9 @@ namespace bpkg bool mv (const dir_path& from, const dir_path& to, bool ignore_errors = false); + bool + mv (const path& from, const path& to, bool ignore_errors = false); + // Set (with diagnostics at verbosity level 3 or higher) the new and return // the previous working directory. // diff --git a/tests/pkg-fetch.testscript b/tests/pkg-fetch.testscript index 7d32523..5046c5d 100644 --- a/tests/pkg-fetch.testscript +++ b/tests/pkg-fetch.testscript @@ -160,7 +160,14 @@ $* libfoo/1.0.0 2>>/EOE != 0 $* -e $src/t1/libfoo-1.0.0.tar.gz 2>'using libfoo/1.0.0 (external)'; $pkg_status libfoo/1.0.0 1>'libfoo fetched 1.0.0'; - $pkg_purge libfoo 2>'purged libfoo/1.0.0' + $* libfoo/1.1.0 2>'fetched libfoo/1.1.0'; + $pkg_unpack libfoo 2>'unpacked libfoo/1.1.0'; + test -d cfg/libfoo-1.1.0; + $* libfoo/1.1.0 2>'fetched libfoo/1.1.0'; + test -d cfg/libfoo-1.1.0 == 1; + $pkg_status libfoo/1.1.0 1>'libfoo fetched 1.1.0'; + + $pkg_purge libfoo 2>'purged libfoo/1.1.0' } : purge-existing |