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 ++++++++++++++++++++++---------- tests/common/git/init | 4 +- tests/common/git/state0/libbar.tar | Bin 174080 -> 174080 bytes tests/common/git/state0/libfoo.tar | Bin 440320 -> 450560 bytes tests/common/git/state0/libfox.tar | Bin 245760 -> 245760 bytes tests/common/git/state0/links.tar | Bin 276480 -> 276480 bytes tests/common/git/state0/style-basic.tar | Bin 81920 -> 81920 bytes tests/common/git/state0/style.tar | Bin 143360 -> 143360 bytes tests/common/git/state1/libbaz.tar | Bin 61440 -> 61440 bytes tests/common/git/state1/libfoo.tar | Bin 501760 -> 512000 bytes tests/common/git/state1/libfox.tar | Bin 245760 -> 245760 bytes tests/common/git/state1/style-basic.tar | Bin 81920 -> 81920 bytes tests/common/git/state1/style.tar | Bin 143360 -> 143360 bytes tests/publish | 2 +- tests/remote-git.testscript | 4 +- tests/rep-fetch-git-commit.testscript | 35 ++++++++++++ tests/rep-fetch.testscript | 2 +- 17 files changed, 110 insertions(+), 35 deletions(-) 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); } diff --git a/tests/common/git/init b/tests/common/git/init index 9519779..8cac2da 100755 --- a/tests/common/git/init +++ b/tests/common/git/init @@ -114,8 +114,10 @@ git -C style-basic.git checkout master # git -C libbar.git init git -C libbar.git add '*' -git -C libbar.git submodule add -b stable ../style-basic.git extras git -C libbar.git commit -am 'Create' +git -C libbar.git tag -a 'v1.0.0' -m 'Tag version 1.0.0' +git -C libbar.git submodule add -b stable ../style-basic.git extras +git -C libbar.git commit -am 'Add extras' # Create master branch for libfoo.git, adding style.git and libbar.git as # submodules. diff --git a/tests/common/git/state0/libbar.tar b/tests/common/git/state0/libbar.tar index b63a7b5..cf00747 100644 Binary files a/tests/common/git/state0/libbar.tar and b/tests/common/git/state0/libbar.tar differ diff --git a/tests/common/git/state0/libfoo.tar b/tests/common/git/state0/libfoo.tar index 200ebf5..05281ee 100644 Binary files a/tests/common/git/state0/libfoo.tar and b/tests/common/git/state0/libfoo.tar differ diff --git a/tests/common/git/state0/libfox.tar b/tests/common/git/state0/libfox.tar index df77539..b024532 100644 Binary files a/tests/common/git/state0/libfox.tar and b/tests/common/git/state0/libfox.tar differ diff --git a/tests/common/git/state0/links.tar b/tests/common/git/state0/links.tar index 25c4089..3769fd1 100644 Binary files a/tests/common/git/state0/links.tar and b/tests/common/git/state0/links.tar differ diff --git a/tests/common/git/state0/style-basic.tar b/tests/common/git/state0/style-basic.tar index 7223c52..01eadbc 100644 Binary files a/tests/common/git/state0/style-basic.tar and b/tests/common/git/state0/style-basic.tar differ diff --git a/tests/common/git/state0/style.tar b/tests/common/git/state0/style.tar index 93007d5..042684e 100644 Binary files a/tests/common/git/state0/style.tar and b/tests/common/git/state0/style.tar differ diff --git a/tests/common/git/state1/libbaz.tar b/tests/common/git/state1/libbaz.tar index d46b25c..5238b4a 100644 Binary files a/tests/common/git/state1/libbaz.tar and b/tests/common/git/state1/libbaz.tar differ diff --git a/tests/common/git/state1/libfoo.tar b/tests/common/git/state1/libfoo.tar index 683f5ab..c0e7523 100644 Binary files a/tests/common/git/state1/libfoo.tar and b/tests/common/git/state1/libfoo.tar differ diff --git a/tests/common/git/state1/libfox.tar b/tests/common/git/state1/libfox.tar index 49c4752..e502974 100644 Binary files a/tests/common/git/state1/libfox.tar and b/tests/common/git/state1/libfox.tar differ diff --git a/tests/common/git/state1/style-basic.tar b/tests/common/git/state1/style-basic.tar index 5ab346d..1ef7399 100644 Binary files a/tests/common/git/state1/style-basic.tar and b/tests/common/git/state1/style-basic.tar differ diff --git a/tests/common/git/state1/style.tar b/tests/common/git/state1/style.tar index 71be292..2a0120b 100644 Binary files a/tests/common/git/state1/style.tar and b/tests/common/git/state1/style.tar differ diff --git a/tests/publish b/tests/publish index df355eb..bd1f327 100755 --- a/tests/publish +++ b/tests/publish @@ -60,7 +60,7 @@ for r in $(find test -type d -regex '.*/git/.*/[^/]+\.git'); do # Push local branches and tags to the remote repository, if it exists. # - if git ls-remote "$url" 2>/dev/null >&2; then + if git ls-remote --refs "$url" 2>/dev/null >&2; then # Point the bare repository origin to the remote repository. # git -C $br config remote.origin.url "$url" diff --git a/tests/remote-git.testscript b/tests/remote-git.testscript index 40f7ae5..56c33b7 100644 --- a/tests/remote-git.testscript +++ b/tests/remote-git.testscript @@ -63,8 +63,8 @@ else rep_git_https_dumb = "https://build2.org/bpkg/git/$cmd" rep_git_https_smart = "https://git.build2.org/testing/bpkg/advonly/$cmd" rep_git_https_smart_unadv = "https://git.build2.org/testing/bpkg/unadv/$cmd" - rep_git_git = "git://git.build2.org/testing/bpkg/unadv/$cmd" - rep_git_ssh = "ssh://git.build2.org/var/scm/testing/bpkg/unadv/$cmd" + rep_git_git = "git://git.build2.org/testing/bpkg/advonly/$cmd" + rep_git_ssh = "ssh://git.build2.org/var/scm/testing/bpkg/advonly/$cmd" rep_git = $rep_git_https_dumb # Default remote repository URL. end diff --git a/tests/rep-fetch-git-commit.testscript b/tests/rep-fetch-git-commit.testscript index 3f0fbe7..8af9ea1 100644 --- a/tests/rep-fetch-git-commit.testscript +++ b/tests/rep-fetch-git-commit.testscript @@ -3,6 +3,7 @@ # license : MIT; see accompanying LICENSE file +git clone "$rep_git/state0/style-basic.git" 2>! &style-basic/*** ++git clone "$rep_git/state0/libbar.git" 2>! &libbar/*** : unadvertised : @@ -94,3 +95,37 @@ if ($git_fully_supported || $git_protocol != 'https-smart-unadv') EOE } } + +: peeled +: +{ + +git -C ../libbar log '--pretty=format:%H' --all --grep='Create' | \ + set commit + + : remap + : + : Test that the commit id is properly remapped back to the advertised tag + : reference. + : + { + $clone_root_cfg && $rep_add "$rep/state0/libbar.git#@$commit"; + + $* 2>>~"%EOE%" + %fetching git:.+libbar#@$commit% + %.+ + EOE + } + + : peel + : + : Test that the tag reference is properly peeled into the commit id. + : + { + $clone_root_cfg && $rep_add "$rep/state0/libbar.git#v1.0.0,-$commit"; + + $* 2>>~%EOE% + %.+ + 0 package(s) in 1 repository(s) + EOE + } +} diff --git a/tests/rep-fetch.testscript b/tests/rep-fetch.testscript index 8e7011a..c2fd4cc 100644 --- a/tests/rep-fetch.testscript +++ b/tests/rep-fetch.testscript @@ -809,7 +809,7 @@ else { $clone_root_cfg && $rep_add "$rep_git_git/state0/libfoo.git#master"; $* 2>! &cfg/.bpkg/repos/*/***; - $rep_add "$rep_git_https_smart_unadv/state0/libfoo.git#master"; + $rep_add "$rep_git_https_smart/state0/libfoo.git#master"; $* 2>>~%EOE% %fetching git:.+libfoo#master% -- cgit v1.1