[PATCH 3 of 5] patch: introduce changedfiles

Patrick Mézard pmezard at gmail.com
Sat May 7 12:47:06 CDT 2011


Le 07/05/11 17:56, Idan Kamara a écrit :
> On Sat, May 7, 2011 at 3:34 PM, Patrick Mézard <pmezard at gmail.com <mailto:pmezard at gmail.com>> wrote:
> 
>     Le 06/05/11 20:09, Matt Mackall a écrit :
>     > On Fri, 2011-05-06 at 19:58 +0300, Idan Kamara wrote:
>     >> # HG changeset patch
>     >> # User Idan Kamara <idankk86 at gmail.com <mailto:idankk86 at gmail.com>>
>     >> # Date 1304697821 -10800
>     >> # Node ID 1365953fe513ea8b9a68d238df5c65565475480c
>     >> # Parent  7766361172826971d52df6c19a300d72e7f2abfc
>     >> patch: introduce changedfiles
>     >>
>     >> returns the set of all changed files in a given patch
>     >
>     > I've queued 1, 2, and 4 from your series, I'd like an opinion from
>     > Patrick on this one though.
> 
>     I told Idan on IRC this solution was correct but suboptimal. The alternative is to write yet another parser, faster but less reliable. The reliability is not a problem with qpush because it would only generate false positives, not false negatives. But it will reduce its usefulness for sure. And to make it correct we need to copy most of iterhunks() and hunk parsers without decoding/storing the data.
> 
> 
> I also think extending the existing mq patchheader parser will eventually result in copying most of the logic from iterhunks. Looking at the results below, not sure it's worth the trouble.
>  
> 
>     The question is: is it slow enough to be problem?
> 
>     Idan, could you measure how slower is qpush with a large diff (for instance, diff two hg tags or something like that) with this call?
> 
> 
> $ hg diff -r 1.8 -r 1.8.3 > /tmp/diff
> 
> after qimporting it on top of 1.8:
> 
> $ touch z
> $ hg add z
> 
> old:
> 
> $ time hg qpush -f
> (working directory not at a head)
> applying diff
> now at: diff
> 
> real0m0.646s
> user0m0.576s
> sys0m0.068s
> 
> new:
> 
> $ time hg qpush
> (working directory not at a head)
> applying diff
> now at: diff
> 
> real0m0.826s
> user0m0.736s
> sys0m0.088s 

Ok, not super fast but the feature is worth it in my opinion.

Because of the hackish/complicated nature of patch.selectfile(), patch.changedfiles() will stat the working directory which is probably unexpected. But at least it will return the list of files that patch.patch() would actually touch.

So, this looks fine for now.

--
Patrick Mézard


More information about the Mercurial-devel mailing list