Implement posting for shelved perforce changes.

Review Request #5172 — Created Dec. 27, 2013 and submitted

Information

RBTools
master

Reviewers

Implement posting for shelved perforce changes.

The perforce shelf is kind of like a shared "git stash". When you shelve a
change, it copies the data from your working directory over to the server. What
I didn't expect is that it also leaves your working directory the same, with
the same open files (which can then be reverted).

We can get the file list for a shelved change via `p4 files //...@=CLN`, and
the file contents via `p4 print //path/to/file@=CLN`. This lets us use a very
similar implementation to pending changes, with the exception that the "new"
files are fetched from the repository instead of just using the working copy.

The perforce documentation is full of lies. Among other fantastical claims, it
asserts that shelved changes have a 'Status' field of 'shelved', when in
practice, they're still 'pending'. I've made changes to parse_revision_spec to
treat shelved changes the same as pending ones, and then made some changes to
the pending diff code path to look on the shelf if there are no open files in
the changeset.

The one major limitation that I wasn't able to work around is that when you
shelve a change containing file moves, there doesn't seem to be any way to know
which "move/add" corresponds to which "move/delete". The usual ways of doing
this (looking for "movedFile" in fstat, or looking in the filelog) don't work
for shelved changesets. I suspect that this is just an oversight that will be
corrected in the course of time, but for now, I've made it so that when you
post such a change, you'll see an old-school deleted file and an added file,
even when the Review Board server supports moved files.

  • Posted a variety of shelved changes with different types of changes contained
    within.
  • Ran rbt diff N with a shelved changeset, which is what prompted the addition
    of the options to that command.
  • Ran unit tests.
Description From Last Updated

I think you need the :: right up against note. At least, we do that everywhere else, so it should …

chipx86chipx86

One blank line.

chipx86chipx86
david
chipx86
  1. Logic seems good from what I understand. Might be worth reporting the moved files issue to Perforce.

    Just a couple very small things.

  2. docs/rbtools/rbt/commands/post.txt (Diff revision 2)
     
     
    Show all issues

    I think you need the :: right up against note. At least, we do that everywhere else, so it should be consistent.

    1. It's not necessary but I'll change it for consistency.

  3. rbtools/clients/perforce.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    One blank line.

  4. rbtools/clients/perforce.py (Diff revision 2)
     
     

    This seems pretty similar to _extract_add_files. Maybe you could do:

    def _extract_edit_files(...):
        old_filename, new_filename = self._extract_add_files(...)
        self._write_file(..., old_filename)
    
        return old_filename, new_filename
    

    No big deal either way.

  5. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (2c6abfc).
Loading...