Properly generate parent diffs in git

 






 

 


  Review Request #5825
  —
  Created May 14, 2014 and submitted 

 


















 

  

   Information

  





 
 

  sgallagh
 









 
 

  RBTools
 









 
 

  release-0.6.x
 









 
 

  
 









 
 

  
 









 
 

  8647517...
 






 




 

  

   Reviewers

  





 
 

  rbtools
 









 
 

  
 






 








 





  


 
 

  
The old behavior was to only populate the 'parent_base' attribute

if the 'base' of the review range did not show up with

git branch -r --contains <base>


This fails for many common environments (such as having pushed to

your private github repository prior to posting the review).

Presumably, the assumption that was made is that if the base is

present on a remote branch, it must be already available to

Review Board. With distributed version-control systems, this is

not a safe assumption to make.


With this change, we will always look up the merge base and

compare it to the review base. If they are different, we will pass

along the parent_base so that the parent diff can be generated

correctly.

 








  


 
 

  
1) Set up a series of patches that depended on one another. (Patch 2 depends on changes from patch 1)

2) Pushed the patches to a remote git repository

3) Ran 'rbt post patch1:patch2'


Previous behavior:

The file was not found in the repository (HTTP 400, API Error 207)




Behavior after this patch:

Review was properly uploaded.

 







 









 





  








 









 






  

 








 














 



 

  

   




sgallagh




   

   

  

 



 















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Just to check my understanding of what's going on, when you pushed, you pushed to a remote other than the one which is connected to the Review Board server?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Yes, it was a remote other than the one linked by Review Board. Even if it was, this check would still be wrong (merely harmlessly so) if the commit existed on the linked repository but in an unrelated branch. (Harmless because in that case, RB would still be able to look it up by hash even though it wasn't logically related to the patch being applied).
      

      However, if this hash appears only in remote branches that are unaccessable to Review Board, it's going to fail. I don't know of any fool-proof way to know ahead of time on the client which remote necessarily matches the one Review Board is aware of, so I solved the problem more simplistically (just checking whether the base and parent_base were the same). In the worst-case (where the diff does exist in some other branch that RB has access to), we've generated an unnecessary parent diff. In the more common case, this will just be faster and safer.
      
 
      

      2. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




sgallagh




   

   

  

 



 















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Oops, closed the wrong thing.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




sgallagh




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to master (5b8159e)





 


 










  

 









     

    

   

  

  

   
   Loading...