From 35b19af5bb585276821dc17f0f487d6cd2ece598 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Sat, 15 Jun 2019 23:48:59 +0300 Subject: Fix failure to fetch git repository location with tagged commit id for git 2.22 Also make some cleanups (always probe URLs prior to git-ls-remote, use peeled reference ids to identify repository fragments, etc). --- bpkg/fetch-git.cxx | 98 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 30 deletions(-) (limited to 'bpkg') diff --git a/bpkg/fetch-git.cxx b/bpkg/fetch-git.cxx index 63ee09d..5ffc9a6 100644 --- a/bpkg/fetch-git.cxx +++ b/bpkg/fetch-git.cxx @@ -107,10 +107,9 @@ namespace bpkg } // Start git process. On the first call check that git version is 2.11.0 or - // above, and fail if that's not the case. Note that the full functionality - // (such as being able to fetch unadvertised commits) requires 2.14.0. And - // supporting versions prior to 2.11.0 doesn't seem worth it (plus other - // parts of the toolchain also requires 2.11.0). + // above, and fail if that's not the case. Note that supporting earlier + // versions doesn't seem worth it (plus other parts of the toolchain also + // require 2.11.0). // // Also note that git is executed in the "sanitized" environment, having the // environment variables that are local to the repository being unset (all @@ -790,6 +789,31 @@ namespace bpkg return r; } + + // Return the peeled reference for an annotated tag and the original + // reference if it is not an annotated tag or is already peeled. + // + const ref& + peel (const ref& rf) const + { + if (rf.name.compare (0, 10, "refs/tags/") == 0 && !rf.peeled) + { + for (const ref& r: *this) + { + if (r.peeled && r.name == rf.name) + return r; + } + + // Fall through. + // + // Presumably is a lightweight tag reference containing the commit id. + // Note that git-ls-remote output normally contains peeled references + // for all annotated tags. + // + } + + return rf; + } }; // Map of repository URLs to their advertized refs/commits. @@ -971,6 +995,13 @@ namespace bpkg } case capabilities::smart: { + // Note that the git server communicating with the client using the + // protocol version 2 always supports unadvertised refs fetch (see the + // "advertised commit fetch using commit id fails" git bug report for + // details). We ignore this fact (because currently this is disabled + // by default, even if both support version 2) but may rely on it in + // the future. + // return !rf.commit || commit_advertized (co, url, *rf.commit); } case capabilities::unadv: @@ -1020,16 +1051,27 @@ namespace bpkg return *cap; }; - auto references = [&co, &url] (const string& refname, bool abbr_commit) + auto references = [&co, &url, &caps] (const string& refname, + bool abbr_commit) -> refs::search_result { + // Make sure the URL is probed before running git-ls-remote (see + // load_refs() for details). + // + caps (); + return load_refs (co, url ()).search_names (refname, abbr_commit); }; // Return the default reference set (see repository-types(1) for details). // - auto default_references = [&co, &url] () -> refs::search_result + auto default_references = [&co, &url, &caps] () -> refs::search_result { + // Make sure the URL is probed before running git-ls-remote (see + // load_refs() for details). + // + caps (); + refs::search_result r; for (const ref& rf: load_refs (co, url ())) { @@ -1146,7 +1188,9 @@ namespace bpkg ? default_references () : references (*rf.name, true /* abbr_commit */)) { - const string& c (r.get ().commit); + // Reduce the reference to the commit id. + // + const string& c (load_refs (co, url ()).peel (r).commit); if (!rf.exclusion) { @@ -1180,7 +1224,7 @@ namespace bpkg { assert (!rf.exclusion); // Already handled. - add_spec (*rf.commit, strings ({*rf.commit}), true); + add_spec (*rf.commit, strings ({*rf.commit}), true /* shallow */); } // If the shallow fetch is not possible for the commit but the refname // containing the commit is specified, then we fetch the whole history @@ -1313,41 +1357,35 @@ namespace bpkg // Fetch the refspecs. If no refspecs are specified, then fetch the // whole repository history. // - auto fetch = [&co, &url, &dir] (const strings& refspecs, bool shallow) + auto fetch = [&co, &url, &dir, &caps] (const strings& refspecs, + bool shallow) { // We don't shallow fetch the whole repository. // assert (!refspecs.empty () || !shallow); - // Prior to 2.14.0 the git-fetch command didn't accept commit id as a - // refspec: - // - // $ git fetch --no-recurse-submodules --depth 1 origin 5e8245ee3526530a3467f59b0601bbffb614f45b - // error: Server does not allow request for unadvertised object 5e8245ee3526530a3467f59b0601bbffb614f45b - // - // We will try to remap commits back to git refs (tags, branches, etc) - // based on git-ls-remote output and fail if unable to do so (which - // should only happen for unadvertised commits). - // - // Note that in this case we will fail only for servers supporting - // unadvertised refs fetch. For other protocols we have already fallen - // back to fetching some history, passing to fetch() either advertised - // commit ids (of branches, tags, etc) or an empty refspecs list (the - // whole repository history). So we could just reduce the server - // capabilities from 'unadv' to 'smart' for such old clients. + // Note that peeled references present in git-ls-remote output are not + // advertised, so we need to "unpeel" them back to the annotated tag + // references. Also note that prior to 2.14.0 the git-fetch command + // didn't accept commit id as a refspec for advertised commits except + // for servers with dumb or unadv capabilities. Seems that remapping + // reference ids back to git refs (tags, branches, etc) is harmless and + // works consistently across different git versions. That's why we will + // always remap except for the server with the unadv capabilities, where + // we may not have a name to remap to. That seems OK as no issues with + // using commit ids for fetching from such servers were encountered so + // far. // optional remapped_refspecs; - if (!refspecs.empty () && git_ver < semantic_version {2, 14, 0}) + + if (!refspecs.empty () && caps () != capabilities::unadv) { remapped_refspecs = strings (); for (const string& c: refspecs) { const ref* r (load_refs (co, url ()).find_commit (c)); - - if (r == nullptr) - fail << "git version is too old for specified location" << - info << "consider upgrading git to 2.14.0 or above"; + assert (r != nullptr); // Otherwise we would fail earlier. remapped_refspecs->push_back (r->name); } -- cgit v1.1