darcs

Issue 1877 pull --dry-run --xml-output => "Would pull from..." noise (2.4.4)

Title pull --dry-run --xml-output => "Would pull from..." noise (2.4.4)
Priority bug Status resolved
Milestone Resolved in 2.5.0
Superseder Nosy List dmitry.kurochkin, galbolle, kowey, lele
Assigned To galbolle
Topics Regression, UI

Created on 2010-06-20.08:46:45 by lele, last changed 2010-07-01.21:21:39 by galbolle.

Messages
msg11491 (view) Author: lele Date: 2010-06-20.08:46:44
Hi all,

latest darcs (>2.4) reintroduced a bug that was fixed back in
2008 ([1]_): the output of "darcs pull --dry-run --xml-output" starts
with a pointless (in XML context) verbosity "Would pull from XXX...".

The defect imposes 3rd party consumers (such as Tailor, to
mention one :) to "massage" the output removing non-XML before feeding
it to a XML parser. Not a big deal ([2]_), but annoying to say the
least.

Ideally, when --xml-output is given, darcs should emit a clean and
consistent XML stream on its output (maybe redirecting the
informational messages to stderr?).

Consider the following transcript:

 1. I create a sample repository
 
    $ cd /tmp/
    $ mkdir test
    $ cd test
    $ darcs init
    $ darcs --version
    2.4.4 (release)
    $ touch a
    $ darcs add a
    $ darcs rec -a -m "First"
    Finished recording patch 'First'

 2. I create an empty repository and pull from the sample
 
    $ cd ..
    $ mkdir other
    $ cd other
    $ darcs init
    $ darcs pull --dry-run --xml-output ../test
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
        </patch>
    </patches>

    Look ma, no noisy lines... Good!

 3. But tailor needs more info
    
    $ darcs pull --summary --dry-run --xml-output ../test
    Would pull from "/tmp/test"...
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
            <summary>
            <add_file>
            a
            </add_file>
            </summary>
        </patch>
    </patches>

    Gasp! Need to drop the first line before parsing the output...

 4. Repeat the previous command
 
    $ darcs pull --dry-run --xml-output ../test
    Would pull from "/tmp/test"...
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
        </patch>
    </patches>

    Uh, what the heck is going on here?? This confused me a lot trying
    to distill the test case... Apparently, executing "darcs changes
    --summary" in the third step alters the behaviour of succeeding
    commands??

 5. Darcs 2.3.1 did behave differently

    $ darcs-2.3.1 pull --dry-run --xml-output ../test
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
        </patch>
    </patches>
    $ darcs-2.3.1 pull --summary --dry-run --xml-output ../test
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
            <summary>
            <add_file>
            a
            </add_file>
            </summary>
        </patch>
    </patches>
    $ darcs-2.3.1 pull --dry-run --xml-output ../test
    <patches>
        <patch author='lele@nautilus.homeip.net' date='20100620083122' local_date='Sun Jun 20 10:31:22 CEST 2010' inverted='False' hash='20100620083122-97f81-152958635c941142ea3e00c861f3eab23beb4718.gz'>
            <name>First</name>
            <comment>Ignore-this: b4c104a51d2623519ab352ce6d8eccde</comment>
        </patch>
    </patches>

I hope to be back with a test script.

thank you,
ciao, lele.

.. [1] See patch 'Honour --xml-output when printing the patches in the
       "will do"/"would do" message'
.. [2] http://progetti.arstecnica.it/tailor/changeset/1669       
-- 
nickname: Lele Gaifax    | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas    | comincerò ad aver paura di chi mi copia.
lele@nautilus.homeip.net |                 -- Fortunato Depero, 1929.
msg11506 (view) Author: kowey Date: 2010-06-21.17:11:17
This is a regression introduced by Darcs 2.4.

Hi Florent, I think you might be a good candidate to investigate this bug.

Some bits and pieces I've found.
The desired behaviour was implemented by Lele in this pre Darcs 2.0.2 darcs:
$ darcs changes --match 'author lele && name Honour' -v
Sat May 17 12:08:20 BST 2008  lele@nautilus.homeip.net
  * Honour --xml-output when printing the patches in the "will
do"/"would do" message
    hunk ./src/Darcs/Arguments.lhs 881
    -              $$ (vsep $ mapFL (showFriendly opts) patches)
    +              $$ put_mode
    hunk ./src/Darcs/Arguments.lhs 887
    -              $$ (vsep $ mapFL (showFriendly opts) patches)
    +              $$ put_mode
    +     where put_mode = if XMLOutput `elem` opts
    +                      then (text "<patches>" $$
    +                            vcat (mapFL (to_xml . info) patches) $$
    +                            text "</patches>")
    +                      else (vsep $ mapFL (showFriendly opts) patches)

But it looks like the new messages are coming from some new unrelated
code in Darcs.Commands.Pull.fetchPatches.  Also it looks like (cf
patch284) the behaviour is contingent on the defaultrepo being set/unset
(which also points at fetchPatches).

Would you have a moment to look into this?  Thanks!
msg11543 (view) Author: galbolle Date: 2010-06-22.09:55:10
The following patch updated issue issue1877 with status=resolved;resolvedin=2.6.0 HEAD

* resolve issue1877: make pull --dry-run --xml terse 
Ignore-this: 25bcdaa4e30f3c82b7e5a74900a905c8
msg11546 (view) Author: kowey Date: 2010-06-22.10:20:02
Hi Florent,

Unfortunately, this still fails the test (see buildbot).
Could you have another look, please?  Thanks!

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg11550 (view) Author: kowey Date: 2010-06-22.12:05:12
Properly resolved this time.  Thanks, Florent!
msg11669 (view) Author: galbolle Date: 2010-07-01.21:21:39
The following patch updated issue issue1877 with status=resolved;resolvedin=2.5.0 CURRENT

* resolve issue1877: make pull --dry-run --xml terse 
Ignore-this: 25bcdaa4e30f3c82b7e5a74900a905c8
History
Date User Action Args
2010-06-20 08:46:45lelecreate
2010-06-21 17:11:19koweysetstatus: unknown -> needs-reproduction
topic: + UI, Regression
title: The strange case of noisy lines reappeared in the output of "pull --dry-run --xml-output" -> pull --dry-run --xml-output => "Would pull from..." noise (2.4.4)
nosy: + galbolle, kowey
messages: + msg11506
priority: bug
assignedto: galbolle
2010-06-22 09:55:10galbollesetstatus: needs-reproduction -> resolved
messages: + msg11543
resolvedin: 2.8.0
2010-06-22 10:20:03koweysetstatus: resolved -> needs-reproduction
messages: + msg11546
2010-06-22 10:20:16koweysetresolvedin: 2.8.0 -> (no value)
2010-06-22 12:05:13koweysetstatus: needs-reproduction -> resolved
messages: + msg11550
resolvedin: 2.8.0
2010-07-01 21:21:39galbollesetmessages: + msg11669
resolvedin: 2.8.0 -> 2.5.0