Change 'rbt diff' and 'rbt post' to take revisions as arguments.

Review Request #5226 — Created Jan. 9, 2014 and submitted

david
RBTools
master
rbtools

Change 'rbt diff' and 'rbt post' to take revisions as arguments.

This change updates the 'diff' and 'post' commands to treat extra arguments as
the revision spec, and adds a new -I/--include flag to specify any files to
limit the diff to.

This is done for all SCMs except ClearCase. I need to work through all the
pending ClearCase patches and read through the proposal on how to fit it into
this new world order.

This also includes some changes to the way that changenums are pulled out for
perforce and plastic. Ideally I'd like to get rid of the server-side
summary/description fetch and implement the --guess-* options, but that would
likely piss off a lot of users. Instead, I've just moved the changenum
extraction into the diff methods, so that we don't duplicate the work of
parsing revisions in two different ways. This also returns 'None' for submitted
changelists (for now) because the server-side code is broken at the moment.

Whether or not files are supported is currently client-dependent. Most already
did, and I've added it for git. It's still not done for Plastic, since my
Plastic expertise is exactly zero.

  • Ran unit tests
  • Checked diffs and extracted summaries for:
  • bzr backend with zero, one, and two-arg forms.
  • cvs backend with zero arguments. I haven't tested the two-arg form because
    after trying four different public CVS repos, I have yet to find one that
    (a) has a CVSROOT, (b) allows me to check it out, (c) allows me to do a
    diff between two tags, and (d) have that diff not contain broken text
    encodings.
  • hg backend with zero, one, and two-arg forms.
  • hgsubversion backend with zero, one, and two-arg forms.
  • git backend with zero, one, and two-arg forms.
  • git-p4 backend with zero, one, and two-arg forms.
  • git-svn backend with zero, one, and two-arg forms.
  • perforce backend with zero, one, and two-arg forms, including submitted and
    shelved changesets.
  • svn backend with zero, one, and two-arg forms, including changelists.
Description From Last Updated

I think RBTools' current approach for Mercurial is wrong. This patch (along with the previous code) is very happy about ...

IN indygreg

"Subversion"

chipx86chipx86

Should add a trailing period.

chipx86chipx86

Same comments as with diff.

chipx86chipx86
chipx86
  1. 
      
  2. rbtools/clients/tests.py (Diff revision 1)
     
     

    What's this addition for?

    1. It's needed because I moved the call to _depot_to_local into the diff routines themselves. We now get the local filename even for deleted files, so that it can be tested against the -I parameters.

  3. rbtools/commands/diff.py (Diff revision 1)
     
     

    "Subversion"

  4. rbtools/commands/diff.py (Diff revision 1)
     
     

    Should add a trailing period.

  5. rbtools/commands/post.py (Diff revision 1)
     
     
     

    Same comments as with diff.

  6. 
      
IN
  1. 
      
  2. rbtools/clients/mercurial.py (Diff revision 1)
     
     

    I think RBTools' current approach for Mercurial is wrong. This patch (along with the previous code) is very happy about using 'tip' and the revlog revision range to construct outgoing changesets and diffs.

    Please note that the revlog revision order is different from the DAG. The only guaranteed similarity is that a single revision's parent must come before the child in the revlog.

    I have a set of outstanding patches (which will conflict significantly with this patch) that do away with blanket hg out (without -r). This ensures that every changeset under consideration by RBTools chains up through the DAG to a specific parent. Otherwise, we may have changesets belonging to other heads creaping in.

    FWIW, your refactoring of adding revspec to all the functions is very similar to what I've done. I think we share many of the same opinions on what is needed to improve Mercurial support. There's just a few issues to work out.

    Perhaps the Mercurial refactoring should be done as a standalone patch before the API change on rbt diff and post?

    1. I wasn't aware that revlog (or even hg out) wasn't traversing the DAG. I guess I've been in the bowels of git so much that I make assumptions that don't hold true. I agree that traversing the DAG up from the desired head is much better.

      I anticipate that this change will be pending for a while, since there's really a ton of testing that needs to be done with all the different backends (plus I haven't even tried to do anything with clearcase, yet). Feel free to post your changes as you think they're ready, and we can figure out the right time to merge our work together.

    2. Will do!

      FWIW, I posted more comments on the hackpad on what I think Mercurial's behavior should be. Not sure if they are reviewed/posted yet.

    3. I haven't received a moderation email about it yet. I think I'll just unmoderate it for now.

  3. 
      
IN
  1. I have a few patches that conflict with this and your other patch. This patch makes me very happy and is larger in scope than just Mercurial. I'm totally fine with waiting for this patch to land and rebasing my work on top of it.

  2. 
      
david
david
david
david
david
david
david
david
david
chipx86
  1. Looks good!

    Was there really nothing more to do with the Bazaar support?

  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (cf70a4c)
Loading...