• 
      

    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:
    Completed
    Change Summary:
    Pushed to master (2c6abfc).