| Message ID | 20180412092855.30621-2-ricardo.martincoski@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | fix some corner cases for download/git v1 | expand |
Ricardo, All, On 2018-04-12 06:28 -0300, Ricardo Martincoski spake thusly: > With old versions of git, when the shallow fetch was not enough, the > download fails like this: > Fetching all references > Could not fetch special ref 'sha1'; assuming it is not special. > fatal: reference is not a tree: sha1 > > At git version 1.9.0, the semantics of the option -t for git fetch > changed, see upstream commit [1]: > < 1.9.0: "fetch tags without also fetching other references"; > >=1.9.0: "fetch tags *in addition to* other stuff". > > Fall back to explicit refspec to support distros that use ancient > versions of git. [--SNIP--] > diff --git a/support/download/git b/support/download/git > index 381f3ceeb3..1b09e5c750 100755 > --- a/support/download/git > +++ b/support/download/git > @@ -76,7 +76,7 @@ if [ -n "$(_git ls-remote origin "'${cset}'" 2>&1)" ]; then > fi > if [ ${git_done} -eq 0 ]; then > printf "Fetching all references\n" > - _git fetch origin -t > + _git fetch origin -u "'+refs/tags/*:refs/tags/*'" "'+refs/heads/*:refs/remotes/origin/*'" Why don't we just run: git fetch origin git fetch origin -t As I understand it: ancient git | new git -------------------------------------------------------------------- git fetch | fetch all refs but tags | fetches all refs but tags git fetch -t | fetches only tags | fetch all refs and tags Right? Regards, Yann E. MORIN. > fi > > # Try to get the special refs exposed by some forges (pull-requests for > -- > 2.14.1 >
Hello, Thank you for your review of the series. On Thu, Apr 12, 2018 at 02:42 PM, Yann E. MORIN wrote: >> - _git fetch origin -t >> + _git fetch origin -u "'+refs/tags/*:refs/tags/*'" "'+refs/heads/*:refs/remotes/origin/*'" > > Why don't we just run: > > git fetch origin > git fetch origin -t One extra connection to the server ... but much more readable/maintainable! I will do. I will change this patch to Changes Requested. > As I understand it: > > ancient git | new git > -------------------------------------------------------------------- > git fetch | fetch all refs but tags | fetches all refs but tags > git fetch -t | fetches only tags | fetch all refs and tags > > Right? Yes. Nit: actually 'git fetch' does fetch some tags, but not necessarily all. Only tags in commits also reachable by branches are fetched. Regards, Ricardo
Ricardo, All, On 2018-04-13 15:23 -0300, Ricardo Martincoski spake thusly: > On Thu, Apr 12, 2018 at 02:42 PM, Yann E. MORIN wrote: > >> - _git fetch origin -t > >> + _git fetch origin -u "'+refs/tags/*:refs/tags/*'" "'+refs/heads/*:refs/remotes/origin/*'" > > Why don't we just run: > > git fetch origin > > git fetch origin -t > > One extra connection to the server ... > but much more readable/maintainable! > I will do. I will change this patch to Changes Requested. Thanks. I don't care much about an extra round-trip to the server, especially if that makes the code easier to read. > > As I understand it: > > ancient git | new git > > -------------------------------------------------------------------- > > git fetch | fetch all refs but tags | fetches all refs but tags > > git fetch -t | fetches only tags | fetch all refs and tags > Nit: actually 'git fetch' does fetch some tags, but not necessarily all. Only > tags in commits also reachable by branches are fetched. OK, but overall, we would cover all cases with the two git-fetch, whether we use an old git or a newer one, and that's the important info. Thanks! :-) Regards, Yann E. MORIN.
Hello, > On 2018-04-13 15:23 -0300, Ricardo Martincoski spake thusly: >> On Thu, Apr 12, 2018 at 02:42 PM, Yann E. MORIN wrote: >> >> - _git fetch origin -t >> >> + _git fetch origin -u "'+refs/tags/*:refs/tags/*'" "'+refs/heads/*:refs/remotes/origin/*'" >> > Why don't we just run: >> > git fetch origin >> > git fetch origin -t >> >> One extra connection to the server ... >> but much more readable/maintainable! >> I will do. I will change this patch to Changes Requested. > > Thanks. I don't care much about an extra round-trip to the server, > especially if that makes the code easier to read. I agree. And a few years from now, when we stop supporting distros with git <1.9.0 we can remove 'git fetch origin' if we wish. BTW, the order of the commands is important here. Some dumb http servers don't work well with old git versions if 'git fetch origin -t' is used first, see: https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/62895510 But in the order you propose (and I see you used in your series) it works fine: https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/62897927 So we are good. > OK, but overall, we would cover all cases with the two git-fetch, > whether we use an old git or a newer one, and that's the important > info. Agreed. Regards, Ricardo
diff --git a/support/download/git b/support/download/git index 381f3ceeb3..1b09e5c750 100755 --- a/support/download/git +++ b/support/download/git @@ -76,7 +76,7 @@ if [ -n "$(_git ls-remote origin "'${cset}'" 2>&1)" ]; then fi if [ ${git_done} -eq 0 ]; then printf "Fetching all references\n" - _git fetch origin -t + _git fetch origin -u "'+refs/tags/*:refs/tags/*'" "'+refs/heads/*:refs/remotes/origin/*'" fi # Try to get the special refs exposed by some forges (pull-requests for