Strip symlink mode information from files on Git-style Mercurial diffs.

 






 

 


  Review Request #12061
  —
  Created Feb. 17, 2022 and submitted 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-4.0.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Git-style Mercurial diffs, like normal Git diffs, contain symlink mode

information. We've hit issues in the past with the server applying

patches containing symlink modes.


Since a Git-style diff otherwise represents a symlink change as a

standard file change, with the content being the old/new target names,

we addressed this problem by stripping out the symlink mode and treating

it as a normal file when patching for display in the diff viewer.


This is by no means a perfect solution (that is in development, with

initial steps having been done in Review Board 4.0.5), but it addresses

the issue in Git.


This change brings that fix over to Mercurial when working with

Git-style diffs. The code in Git has been split out into a common

function that both can call, simplifying maintenance and allowing both

to benefit from any future bug fixes.

 








  


 
 

  
Unit tests pass.


Tested with a customer who hit this problem. Verified that they were able to

review symlinks in Mercurial.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Strip symlink mode information from files on Git-style Mercurial diffs.
      
Git-style Mercurial diffs, like normal Git diffs, contain symlink mode
information. We've hit issues in the past with the server applying
patches containing symlink modes.

Since a Git-style diff otherwise represents a symlink change as a
standard file change, with the content being the old/new target names,
we addressed this problem by stripping out the symlink mode and treating
it as a normal file when patching for display in the diff viewer.

This is by no means a perfect solution (that is in development, with
initial steps having been done in Review Board 4.0.5), but it addresses
the issue in Git.

This change brings that fix over to Mercurial when working with
Git-style diffs. The code in Git has been split out into a common
function that both can call, simplifying maintenance and allowing both
to benefit from any future bug fixes.

     


    		
    b0f0b5324db02a8d20e78b2394f02b213be68794

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
F401 'stat' imported but unused

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
How does the performance of this particular usage compare to just using b'diff -r' in data?

      		
      
       daviddavid
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/scmtools/hg.py
      


       (Diff revision 2)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    How does the performance of this particular usage compare to just using b'diff -r' in data?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      The important bit is the ordering. diff --git must be before a diff -r for it to be a git-style diff, and not after (it might be testing data within a diff).
      

      And that's the bigger problem. It doesn't account for whether things are at the beginning of a line, or data within a diff.
      

      This logic is not new, though, just moved. I don't feel comfortable redoing it for this change.
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to release-4.0.x (c567608)





 


 










  

 









     

    

   

  

  

   
   Loading...