On 29 January 2017 at 15:07, James E Keenan <jk...@verizon.net> wrote: > On 01/27/2017 10:58 PM, demerphq wrote: >> >> >> On 28 Jan 2017 6:32 a.m., "James E Keenan" <jk...@verizon.net >> <mailto:jk...@verizon.net>> 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 <http://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 > > > Thanks for the feedback. Pull request filed: > https://github.com/abeltje/Test-Smoke/pull/26
I have some further comments in the other thread. Nothing that needs to stop your patch from being applied, but I noticed some stuff I found quite surprising, although I only did a quick scan so I very well may be talking out my ass in that thread. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
