develooper Front page | perl.qa | Postings from January 2017

Re: Test::Smoke failing to test most recent commit to Perl 5 blead

Thread Previous | Thread Next
From:
demerphq
Date:
January 29, 2017 16:37
Subject:
Re: Test::Smoke failing to test most recent commit to Perl 5 blead
Message ID:
CANgJU+UdGSG87NYP_vuOGg9SXsZyxW8hDaRY83+tg8-=8u80hw@mail.gmail.com
On 29 January 2017 at 15:07, James E Keenan <jkeen@verizon.net> wrote:
> On 01/27/2017 10:58 PM, demerphq wrote:
>>
>>
>> On 28 Jan 2017 6:32 a.m., "James E Keenan" <jkeen@verizon.net
>> <mailto:jkeen@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/"

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About