Use `p4 print -o` for getting the contents of perforce files.
Review Request #9469 — Created Jan. 4, 2018 and submitted
The command which we use to get the contents of perforce files at a
specific revision,p4 print
, tries to be smart about UTF-8 BOM
characters. If the output is to stdout, even if stdout isn't a terminal,
it will strip the BOM. Unfortunately, we had a difference between
RBTools, which usesp4 print -o
, and Review Board, which uses
p4 print
, so that the diffs produced by RBTools might not apply
cleanly to the files fetched by Review Board.This change makes Review Board use
p4 print -o
so that the source file
will match the one used to create the diffs.
Ran unit tests.
Description | From | Last Updated |
---|---|---|
p4 print sets the file readonly and that causes a later call to unlink fail. See https://github.com/reviewboard/rbtools/blob/master/rbtools/clients/perforce.py#L1312 """ Grabs a … |
EI eight.ring | |
os.chmod() follows symlink*, so if filename is a symlink, os.chmod(filename) will affect the target filename points to, which is not … |
EI eight.ring |
-
-
reviewboard/scmtools/perforce.py (Diff revision 1) p4 print sets the file readonly and that causes a later call to unlink fail. See https://github.com/reviewboard/rbtools/blob/master/rbtools/clients/perforce.py#L1312 """ Grabs a file from Perforce and writes it to a temp file. p4 print sets the file readonly and that causes a later call to unlink fail. So we make the file read/write. """
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+14 -3) |
Checks run (2 succeeded)
-
-
reviewboard/scmtools/perforce.py (Diff revision 2) os.chmod() follows symlink*, so if filename is a symlink, os.chmod(filename) will affect the target filename points to, which is not correct. Other issues with chmod() on symlink can be found in rbtools - https://github.com/reviewboard/rbtools/blob/master/rbtools/clients/perforce.py#L1319 # The output of 'p4 print' will be a symlink if that's what version # control contains. There's a few reasons to skip these files... # # * Relative symlinks will likely be broken, causing an unexpected # OSError. # * File that's symlinked to isn't necessarily in version control. # * Users expect that this will only process files under version # control. If I can replace a file they opened with a symlink to # private keys in '~/.ssh', then they'd probably be none too happy # when rbt uses their credentials to publish its contents. Can we just be same as rbtools? * In python 3, a 'follow_symlinks' parameter is added for os.chmod(), but to be back compatible with python 2, it's default value is True. Both Python 2 and 3 have os.lchmod() which does not follow symlink.
Change Summary:
Only chmod in thee non-symlink case.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+17 -3) |