| Submitter | Siddharth Agarwal |
|---|---|
| Date | March 17, 2015, 10:57 p.m. |
| Message ID | <8229e988837bc3a89a26.1426633050@devbig136.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/8131/ |
| State | Superseded |
| Headers | show |
Comments
On Tue, Mar 17, 2015 at 4:00 PM Siddharth Agarwal <sid0@fb.com> wrote: > # HG changeset patch > # User Siddharth Agarwal <sid0@fb.com> > # Date 1426624884 25200 > # Tue Mar 17 13:41:24 2015 -0700 > # Node ID 8229e988837bc3a89a26b09378e8351e2cab0146 > # Parent 5f0dfcf33d3c1c06856c9271f8006e9eb7898368 > patch.diff: add support for diffs relative to a subdirectory > > For now this implementation is pretty naive -- it filters out files right > before passing them into trydiff. In upcoming patches we'll add some more > smarts. > > diff --git a/mercurial/patch.py b/mercurial/patch.py > --- a/mercurial/patch.py > +++ b/mercurial/patch.py > @@ -2034,7 +2034,7 @@ > return mdiff.diffopts(**buildopts) > > def diff(repo, node1=None, node2=None, match=None, changes=None, > opts=None, > - losedatafn=None, prefix=''): > + losedatafn=None, prefix='', relroot=''): > '''yields diff of changes to files between two nodes, or node and > working directory. > > @@ -2051,7 +2051,9 @@ > > prefix is a filename prefix that is prepended to all filenames on > display (used for subrepos). > - ''' > + > + relroot, if not empty, must be normalized with a trailing /. Any match > + patterns that fall outside it will be ignored.''' > > if opts is None: > opts = mdiff.defaultopts > @@ -2095,9 +2097,21 @@ > if opts.git or opts.upgrade: > copy = copies.pathcopies(ctx1, ctx2) > > + if relroot is not None: > + # XXX this would ideally be done in the matcher rather than here > So why isn't it? I guess I'm missing an explanation for when relroot would be used in the first place. > + def filterrel(l): > + return [f for f in l if f.startswith(relroot)] > + modified = filterrel(modified) > + added = filterrel(added) > + removed = filterrel(removed) > + # filter out copies where either side isn't inside the relative > root > + copy = dict(((dst, src) for (dst, src) in copy.iteritems() > + if dst.startswith(relroot) > + and src.startswith(relroot))) > + > def difffn(opts, losedata): > return trydiff(repo, revs, ctx1, ctx2, modified, added, removed, > - copy, getfilectx, opts, losedata, prefix, '') > + copy, getfilectx, opts, losedata, prefix, relroot) > if opts.upgrade and not opts.git: > try: > def losedata(fn): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel >
On 03/17/2015 08:46 PM, Martin von Zweigbergk wrote: > > + if relroot is not None: > + # XXX this would ideally be done in the matcher rather > than here > > > So why isn't it? A patch I haven't sent out yet has more details, but supporting this in the matcher in general is non-trivial. The matcher is generally designed to support 'or'ing patterns, not 'and'ing them. In a later patch I'll add an optimization for the match.always() case, creating a new matcher where the only pattern is the relative root. > I guess I'm missing an explanation for when relroot would be used in > the first place. relroot will be used to produce patches relative to a directory. e.g. hg diff --relative mercurial will produce a diff where the filenames are e.g. a/patch.py and b/patch.py.
On Tue, Mar 17, 2015 at 9:10 PM Siddharth Agarwal <sid@less-broken.com> wrote: > On 03/17/2015 08:46 PM, Martin von Zweigbergk wrote: > > > > + if relroot is not None: > > + # XXX this would ideally be done in the matcher rather > > than here > > > > > > So why isn't it? > > A patch I haven't sent out yet has more details, but supporting this in > the matcher in general is non-trivial. The matcher is generally designed > to support 'or'ing patterns, not 'and'ing them. > Yep, I'll try to clean up some old patches that add support for that. They only mutate an existing matcher. Is that good enough for your needs? The first three patches look good to me. Can we wait with the rest until I get those patches cleaned up? I think I should be able to do that today. > > In a later patch I'll add an optimization for the match.always() case, > creating a new matcher where the only pattern is the relative root. > Actually, wouldn't that work with any pattern that doesn't already have an "include"? I.e., can you just set "include=['$relroot']" if no includes were passed on the command line? > > > I guess I'm missing an explanation for when relroot would be used in > > the first place. > > relroot will be used to produce patches relative to a directory. e.g. > > hg diff --relative mercurial > > will produce a diff where the filenames are e.g. a/patch.py and b/patch.py. > And if there are paths outside "mercurial/", they are ignored? I found that a little surprising. I see that git made the same decision. I would probably have expected it to fail. What do people want this feature for?
On 03/18/2015 06:29 AM, Martin von Zweigbergk wrote: > Yep, I'll try to clean up some old patches that add support for that. > They only mutate an existing matcher. Is that good enough for your needs? In this case it would be more preferable to create a new matcher with all the patterns of the existing matcher anded with some new ones. > The first three patches look good to me. Can we wait with the rest > until I get those patches cleaned up? I think I should be able to do > that today. OK.
On 03/18/2015 06:29 AM, Martin von Zweigbergk wrote: > And if there are paths outside "mercurial/", they are ignored? Yeah. That's the same as saying hg diff mercurial/ so I don't think it's surprising. > I found that a little surprising. I see that git made the same > decision. I would probably have expected it to fail. What do people > want this feature for? We're merging a bunch of repos into a single one (mirroring them as subdirectories), and during the transition have patches that are generated from the unified repo that need to be applied onto the individual repos, and the other way round.
Patch
diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -2034,7 +2034,7 @@ return mdiff.diffopts(**buildopts) def diff(repo, node1=None, node2=None, match=None, changes=None, opts=None, - losedatafn=None, prefix=''): + losedatafn=None, prefix='', relroot=''): '''yields diff of changes to files between two nodes, or node and working directory. @@ -2051,7 +2051,9 @@ prefix is a filename prefix that is prepended to all filenames on display (used for subrepos). - ''' + + relroot, if not empty, must be normalized with a trailing /. Any match + patterns that fall outside it will be ignored.''' if opts is None: opts = mdiff.defaultopts @@ -2095,9 +2097,21 @@ if opts.git or opts.upgrade: copy = copies.pathcopies(ctx1, ctx2) + if relroot is not None: + # XXX this would ideally be done in the matcher rather than here + def filterrel(l): + return [f for f in l if f.startswith(relroot)] + modified = filterrel(modified) + added = filterrel(added) + removed = filterrel(removed) + # filter out copies where either side isn't inside the relative root + copy = dict(((dst, src) for (dst, src) in copy.iteritems() + if dst.startswith(relroot) + and src.startswith(relroot))) + def difffn(opts, losedata): return trydiff(repo, revs, ctx1, ctx2, modified, added, removed, - copy, getfilectx, opts, losedata, prefix, '') + copy, getfilectx, opts, losedata, prefix, relroot) if opts.upgrade and not opts.git: try: def losedata(fn):