Re: Test::Smoke failing to test most recent commit to Perl 5 blead
[email protected] (demerphq) Sat, 28 Jan 2017 04:58:22 +0100
| Newsgroups | perl.qa |
|---|---|
| Message-ID | <CANgJU+UnkepMS3G_8uFfu3eYXg+u5D2w-t7CGJxjpteBM9yPJQ@mail.gmail.com> |
--94eb2c1cb94a00553105471f988e Content-Type: text/plain; charset=UTF-8 On 28 Jan 2017 6:32 a.m., "James E Keenan" <[email protected]> wrote: > On 01/14/2017 10:10 AM, James E Keenan wrote: > Here is the first part of the smokecurrent.log for this run -- with some > comments: > > ##### > [2017-01-27 08:11:22-0500] Read configuration from: > /usr/home/jkeenan/p5smoke/install/smokecurrent_config > [2017-01-27 08:11:22-0500] Commitlevel before sync: > d6115793d6cc41755a3ed4baaa38d30653656f41 > > # d611579 was HEAD in the previous branch being smoked: > smoke-me/jkeenan/130635-storable > > [2017-01-27 08:11:22-0500] ==> Starting synctree > [2017-01-27 08:11:22-0500] Reading branch to smoke from: > '/usr/home/jkeenan/p5smoke/install/smokecurrent.gitbranch' > [2017-01-27 08:11:22-0500] In pwd(/usr/home/jkeenan/p5smoke/git-perl) > running: > [2017-01-27 08:11:22-0500] qx[/usr/local/bin/git pull --all] > From git://perl5.git.perl.org/perl > 1f74a12..9a7b7fb blead -> origin/blead > [2017-01-27 08:11:37-0500] Fetching origin > [2017-01-27 08:11:37-0500] Already up-to-date. > [2017-01-27 08:11:37-0500] In pwd(/usr/home/jkeenan/p5smoke/git-perl) > running: > [2017-01-27 08:11:37-0500] qx[/usr/local/bin/git remote prune origin] > [2017-01-27 08:11:38-0500] In pwd(/usr/home/jkeenan/p5smoke/git-perl) > running: > [2017-01-27 08:11:38-0500] qx[/usr/local/bin/git checkout blead > [2017-01-27 08:11:38-0500] 2>&1] > Switched to branch 'blead' > [2017-01-27 08:11:41-0500] Your branch is behind 'origin/blead' by 7 > commits, and can be fast-forwarded. > [2017-01-27 08:11:41-0500] (use "git pull" to update your local branch) > > # Note: No indication that 'git pull' was actually run! Why not? > > [2017-01-27 08:11:41-0500] In pwd(/usr/home/jkeenan/p5smoke/perl-current) > running: > [2017-01-27 08:11:41-0500] qx[/usr/local/bin/git reset --hard] > > # Note: Although 'man git-reset' is not explicit about this, we can > probably assume > # that 'git reset --hard' with no <commit> resets to HEAD -- i.e., no > update to checkout. > > Yes. It throws away any changes to the current branch. That should say git reset --hard origin/blead > [2017-01-27 08:11:43-0500] HEAD is now at d611579 Fix stack buffer > overflow in deserialization of hooks. > ##### > > So why does Test-Smoke not update the branch being tested in cases like > this? > > This is an educated guess: whoever wrote that code did not understand git well and got confused about what git pull does, and what git pull --all do. Seeing git pull in a script like this is a red flag, seeing --all is a bigger red flag for me. ( It does not "pull all", it does a fetch against all remotes.) IMO git pull is not really suitable for scripting as it can trigger a rebase or merge, and trigger an editor. I would expect to see a sequence like this: git remote update -p git checkout $branch git reset --hard origin/$branch or like this: git remote prune --all git fetch --all git checkout $branch git reset --hard origin/$branch or even like this: git fetch origin git checkout $branch git reset --hard origin/$branch The difference between the variants being about whether the code should be managing multiple upstream repos or not. The --all introduces ambiguity about this, as it means "fetch code from --all remotes", so one might guess someone wanted to support multiple upstreams. On the other hand, my guess is that the person who coded it to do a 'git pull --all' thought that the -all would update --all branches, which it does not. Bolstering this view is that it seems to make little sense to smoke branches from multiple upstreams, especially for Perl. I would have expected only the canonical branches in the master upstream repo should be smoked, so the --all probably was a bug. What appears to be happening is that when the pull is executed it is in a different branch, so it fetches and then updates *THAT* branch only (via a merge!). When it then checks-out the new branch branch it is on an old commit, and requires a reset to the latest code. I would recommend that the smoke scripts do not use pull *ever*. They should never ever modify code, so doing a pull makes no sense. git pull is basically a smart wrapper around git fetch + git merge. A smoker should *NEVER* be doing git-merge, so it should *never* be using git-pull. Note, I am fully aware that under perfect circumstances you /can/ script this using git pull. I would recommend not to. First it is confusing. Second, if circumstances are less than perfect then it could lead to unwanted behavior. For instance someone tinkers in a branch used by the smoker, then one could imaging the script breaking because of uncommitted changes, or breaking because pull has triggered a merge which requires a edited text message. FWIW, i have in the past recommended that new users to git do NOT use "git pull", until they have mastered using "git fetch" and "git rebase" or "git merge" as independent commands. Only once having experience of the three basic commands should people use git pull. Since it is a wrapper around three distinct commands it easily leads to confusion in the inexperienced. yves --94eb2c1cb94a00553105471f988e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"auto"><br></div><div class=3D"gmail_extra"><di= v class=3D"gmail_quote">On 28 Jan 2017 6:32 a.m., "James E Keenan"= ; <<a href=3D"mailto:[email protected]" target=3D"_blank">jkeen@verizon.= net</a>> wrote:<br type=3D"attribution"><blockquote class=3D"gmail_quote= " style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);= padding-left:1ex">On 01/14/2017 10:10 AM, James E Keenan wrote:<br> Here is the first part of the smokecurrent.log for this run -- with some co= mments:<br> <br> #####<br> [2017-01-27 08:11:22-0500] Read configuration from: /usr/home/jkeenan/p5smo= ke/inst<wbr>all/smokecurrent_config<br> [2017-01-27 08:11:22-0500] Commitlevel before sync: d6115793d6cc41755a3ed4b= aaa38d3<wbr>0653656f41<br> <br> # d611579 was HEAD in the previous branch being smoked: smoke-me/jkeenan/13= 0635-storab<wbr>le<br> <br> [2017-01-27 08:11:22-0500] =3D=3D> Starting synctree<br> [2017-01-27 08:11:22-0500] Reading branch to smoke from: '/usr/home/jke= enan/p5smoke/ins<wbr>tall/smokecurrent.gitbranch'<br> [2017-01-27 08:11:22-0500] In pwd(/usr/home/jkeenan/p5smoke/<wbr>git-perl) = running:<br> [2017-01-27 08:11:22-0500] qx[/usr/local/bin/git pull --all]<br> From git://<a href=3D"http://perl5.git.perl.org/perl" rel=3D"noreferrer" ta= rget=3D"_blank">perl5.git.perl.org/perl</a><br> =C2=A0 =C2=A01f74a12..9a7b7fb=C2=A0 blead=C2=A0 =C2=A0 =C2=A0 -> origin/= blead<br> [2017-01-27 08:11:37-0500] Fetching origin<br> [2017-01-27 08:11:37-0500] Already up-to-date.<br> [2017-01-27 08:11:37-0500] In pwd(/usr/home/jkeenan/p5smoke/<wbr>git-perl) = running:<br> [2017-01-27 08:11:37-0500] qx[/usr/local/bin/git remote prune origin]<br> [2017-01-27 08:11:38-0500] In pwd(/usr/home/jkeenan/p5smoke/<wbr>git-perl) = running:<br> [2017-01-27 08:11:38-0500] qx[/usr/local/bin/git checkout blead<br> [2017-01-27 08:11:38-0500]=C2=A0 2>&1]<br> Switched to branch 'blead'<br> [2017-01-27 08:11:41-0500] Your branch is behind 'origin/blead' by = 7 commits, and can be fast-forwarded.<br> [2017-01-27 08:11:41-0500]=C2=A0 =C2=A0(use "git pull" to update = your local branch)<br> <br> # Note: No indication that 'git pull' was actually run!=C2=A0 Why n= ot?<br> <br> [2017-01-27 08:11:41-0500] In pwd(/usr/home/jkeenan/p5smoke/<wbr>perl-curre= nt) running:<br> [2017-01-27 08:11:41-0500] qx[/usr/local/bin/git reset --hard]<br> <br> # Note: Although 'man git-reset' is not explicit about this, we can= probably assume<br> # that 'git reset --hard' with no <commit> resets to HEAD -- = i.e., no update to checkout.<br> <br></blockquote><div><br></div><div>Yes. It throws away any changes to the= current branch. That should say=C2=A0</div><div><br></div><div>git reset -= -hard origin/blead</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" = style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);pa= dding-left:1ex"> [2017-01-27 08:11:43-0500] HEAD is now at d611579 Fix stack buffer overflow= in deserialization of hooks.<br> #####<br> <br> So why does Test-Smoke not update the branch being tested in cases like thi= s?<br><br></blockquote><div><br></div><div>This is an educated guess: whoev= er wrote that code did not understand git well and got confused about what = git pull does, and what git pull --all do.</div><div><br></div><div>Seeing = git pull in a script like this is a red flag, seeing =C2=A0--all is a bigge= r red flag for me. ( It does not "pull all", it does a fetch agai= nst all remotes.)</div><div><br></div><div>IMO git pull is not really suita= ble for scripting as it can trigger a rebase or merge, and trigger an edito= r.</div><div><br></div><div>I would expect to see a sequence like this:</di= v><div><br></div><div>git remote update -p</div><div>git checkout $branch</= div><div>git reset --hard origin/$branch</div><div><br></div><div>or like t= his:</div><div><br></div><div><div>git remote prune --all=C2=A0</div><div>g= it fetch --all</div><div>git checkout $branch</div><div>git reset --hard or= igin/$branch</div></div><div><br></div><div>or even like this:</div><div><b= r></div><div>git fetch origin</div><div>git checkout $branch</div><div>git = reset --hard origin/$branch</div><div><br></div><div>The difference between= the variants being about whether the code should be managing multiple upst= ream repos or not. The --all introduces ambiguity about this, as it means &= quot;fetch code from --all remotes", so one might guess someone wanted= to support multiple upstreams. On the other hand, my guess is that the per= son who coded it to do a 'git pull --all' thought that the -all wou= ld update --all branches, which it does not. Bolstering this view is that i= t seems to make little sense to smoke branches from multiple upstreams, esp= ecially for Perl. I would have expected only the canonical branches in the = master upstream repo should be smoked, so the --all probably was a bug.=C2= =A0</div><div><br></div><div>What appears to be happening is that when the = pull is executed it is in a different branch, so it fetches and then update= s *THAT* branch only (via a merge!). When it then checks-out the new branch= branch it is on an old commit, and requires a reset to the latest code.</d= iv><div><br></div><div>I would recommend that the smoke scripts do not use = pull *ever*. They should never ever modify code, so doing a pull makes no s= ense. =C2=A0git pull is basically a smart wrapper around git fetch + git me= rge. A smoker should *NEVER* be doing git-merge, so it should *never* be us= ing git-pull.</div><div><br></div><div>Note, I am fully aware that under pe= rfect circumstances you /can/ script this using git pull. I would recommend= not to. First it is confusing. Second, if circumstances are less than perf= ect then it could lead to unwanted behavior. For instance someone tinkers i= n a branch used by the smoker, then one could imaging the script breaking b= ecause of uncommitted changes, or breaking because pull has triggered a mer= ge which requires a edited text message.</div><div><br></div><div>FWIW, i h= ave in the past recommended that new users to git do NOT use "git pull= ", until they have mastered using "git fetch" and "git = rebase" or "git merge" as independent commands. Only once ha= ving experience of the three basic commands should people use git pull. Sin= ce it is a wrapper around three distinct commands it easily leads to confus= ion in the inexperienced.</div><div><br></div><div>yves=C2=A0</div></div></= div> </div> --94eb2c1cb94a00553105471f988e--