Add support for reading and writing history metadata

Review Request #8278 — Created July 11, 2016 and submitted

Information

RBTools
dvcs
4f3c6e5...

Reviewers

For server-side detection of rebases and commit amends, we need to keep
track of some metadata indicating the original SHA1 of commits. With
git, we can store this metadata in git notes.

git notes allows for storing arbitrary metadata associated with each
commit. We store the SHA1 of the commit in a special notes ref,
rbt-history-metadata. This way, we do not interfere with notes that
users may be using. We also add this ref to the list of refs to be
rewritten during rebases and amends.

When an amend occurs, the SHA1 of the commit will change, but the
stored SHA1 will not, so we will be able to detect that commit was
amended. Likewise, when a squash occurs during a rebase, the SHA1 will
change but the stored metadata will be the SHA1 hashes of all the
squashed commits, so we can also know when a squash occurs. We can then
upload this metadata when doing rbt post so that Review Board can
detect history changes server side.

Using https://reviews.reviewboard.org/r/8276/, I was able to do the
following:

  • Upload a review request with multiple commits.
  • Squash the commits together locally.
  • Update the review request.

I observed that the original commits were set in the Review Board
database.

Description From Last Updated

Can we make this if commits is None instead?

daviddavid

Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two …

daviddavid

Instead of the early return, I think it would be better to put the logging statement into the conditional and …

daviddavid
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
  2. 
      
brennie
david
  1. 
      
  2. rbtools/clients/git.py (Diff revision 1)
     
     
    Show all issues

    Can we make this if commits is None instead?

  3. rbtools/clients/git.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two cases because the remedy is the same:

    if rewrite_mode.strip() != 'concatenate':
        logging.warn('notes.rewriteMode is not set to concatenate. '
                     'Review Board will not be able to correctly track '
                     'rebased and amended commits without this. To fix '
                     'this, run:\n'
                     '\tgit config notes.rewriteMode concatenate')
    
  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
  2. 
      
david
  1. 
      
  2. rbtools/clients/git.py (Diff revision 2)
     
     
     
    Show all issues

    Instead of the early return, I think it would be better to put the logging statement into the conditional and just let control flow go all the way to the end of the function. This is simplest if you don't use none_on_ignored_error in the execute call, in which case you should always have a string, but you could also test for rewrite_mode is None or rewrite_mode.strip() != 'concatenate'

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/post.py
        rbtools/clients/__init__.py
        rbtools/api/resource.py
        rbtools/clients/git.py
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to dvcs (53e61d5)