When the svn diff output (on Windows) is sent as a single POST and...

When the svn diff output (on Windows) is sent as a single POST and
some of the files have attribute changes, the server responds with a
list of files where the files with attribute changes have backslashes
(as they do in the svn diff output).  Fix this in the client.

A server-side fix might also be nice; where does the server-side code live?

BUG=none
TEST=none

Patch contributed by Joi.

Committed: 16688.

[Jói Sigurðsson, 2009-05-21 22:23:23]

NB, I had to use the --no_presubmit flag to upload this change as the presubmit
is failing as follows:


C:\s\dt\depot_tools>gcl upload q6q1 -r maruel@chromium.org --send_mail
Traceback (most recent call last):
  File "C:\s\dt\depot_tools\gcl.py", line 1147, in ?
    sys.exit(main())
  File "C:\s\dt\depot_tools\gcl.py", line 1120, in main
    UploadCL(change_info, argv[3:])
  File "C:\s\dt\depot_tools\gcl.py", line 723, in UploadCL
    if not DoPresubmitChecks(change_info, committing=False):
  File "C:\s\dt\depot_tools\gcl.py", line 1030, in DoPresubmitChecks
    input_stream=sys.stdin)
  File "c:\s\dt\depot_tools\presubmit_support.py", line 637, in
DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "c:\s\dt\depot_tools\presubmit_support.py", line 586, in
ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 0, in ?
  File "<string>", line 14, in CheckChangeOnUpload
NameError: global name 'RunUnitTests' is not defined


Any ideas on this?  I can investigate but maybe you know off the top of your
head.

[Jói Sigurðsson, 2009-05-21 22:44:22]

Thanks John.  AFAICT, that code handles it for one code path, but not for the
code path where the patch is uploaded as a single POST request to the server. 
There definitely was a bug before I made this change locally.

On Thu, May 21, 2009 at 6:40 PM, John Abd-El-Malek <jam@chromium.org> wrote:
> I thought this was already handled on the client (upload.py)?  i.e. look at 
> def GetBaseFiles(self, diff):
>     """Helper that calls GetBase file for each file in the patch.
>
>     Returns:
>       A dictionary that maps from filename to GetBaseFile's tuple.
>  Filenames
>       are retrieved based on lines that start with "Index:" or
>       "Property changes on:".
>     """
>     files = {}
>     for line in diff.splitlines(True):
>       if line.startswith('Index:') or line.startswith('Property changes
> on:'):
>         unused, filename = line.split(':', 1)
>         # On Windows if a file has property changes its filename uses '\'
>         # instead of '/'.
>         filename = filename.strip().replace('\\', '/')
>         files[filename] = self.GetBaseFile(filename)
>     return files and
> def SplitPatch(data):
>   """Splits a patch into separate pieces for each file.
>
>   Args:
>     data: A string containing the output of svn diff.
>
>   Returns:
>     A list of 2-tuple (filename, text) where text is the svn diff output
>       pertaining to filename.
>   """
>   patches = []
>   filename = None
>   diff = []
>   for line in data.splitlines(True):
>     new_filename = None
>     if line.startswith('Index:'):
>       unused, new_filename = line.split(':', 1)
>       new_filename = new_filename.strip()
>     elif line.startswith('Property changes on:'):
>       unused, temp_filename = line.split(':', 1)
>       # When a file is modified, paths use '/' between directories, however
>       # when a property is modified '\' is used on Windows.  Make them the
> same
>       # otherwise the file shows up twice.
>       temp_filename = temp_filename.strip().replace('\\', '/')
> On Thu, May 21, 2009 at 3:22 PM, <joi.sigurdsson@gmail.com> wrote:
>>
>> Reviewers: M-A,
>>
>> Description:
>> When the svn diff output (on Windows) is sent as a single POST and
>> some of the files have attribute changes, the server responds with a
>> list of files where the files with attribute changes have backslashes
>> (as they do in the svn diff output).  Fix this in the client.
>>
>> A server-side fix might also be nice; where does the server-side code
>> live?
>>
>> BUG=none
>> TEST=none
>>
>>
>>
>> Please review this at http://codereview.chromium.org/112050
>>
>> SVN Base: http://src.chromium.org/svn/trunk/tools/depot_tools/
>>
>> Affected files:
>>  M     upload.py
>>
>>
>> Index: upload.py
>> ===================================================================
>> --- upload.py   (revision 16669)
>> +++ upload.py   (working copy)
>> @@ -1346,6 +1346,12 @@
>>       msg = lines[0]
>>       patchset = lines[1].strip()
>>       patches = [x.split(" ", 1) for x in lines[2:]]
>> +      for patch_pair in patches:
>> +        # On Windows if a file has property changes its filename uses '\'
>> +        # instead of '/'.  Perhaps this change should be made (also) on
>> the
>> +        # server when it is decoding the patch file sent by the client,
>> but
>> +        # we do it here as well to be safe.
>> +        patch_pair[1] = patch_pair[1].replace('\\', '/')
>>     else:
>>       msg = response_body
>>   else:
>>
>>
>
>

[Jói Sigurðsson, 2009-05-21 22:59:20]

jam wrote:
> I believe you're seeing a bug, can you give me repro steps though?  

I used 'svn move' to move a couple of files between directory, and this bug
occurred for one of them, but not the other.  It occurred for the one that I did
not modify in any way after moving, but did not occur for the one that I edited
after moving.

[M-A Ruel, 2009-05-21 23:53:23]

On 2009/05/21 22:23:23, Jói wrote:
> NB, I had to use the --no_presubmit flag to upload this change as the
presubmit
> is failing as follows:
> 
> 
> C:\s\dt\depot_tools>gcl upload q6q1 -r maruel@chromium.org --send_mail
> Traceback (most recent call last):
>   File "C:\s\dt\depot_tools\gcl.py", line 1147, in ?
>     sys.exit(main())
>   File "C:\s\dt\depot_tools\gcl.py", line 1120, in main
>     UploadCL(change_info, argv[3:])
>   File "C:\s\dt\depot_tools\gcl.py", line 723, in UploadCL
>     if not DoPresubmitChecks(change_info, committing=False):
>   File "C:\s\dt\depot_tools\gcl.py", line 1030, in DoPresubmitChecks
>     input_stream=sys.stdin)
>   File "c:\s\dt\depot_tools\presubmit_support.py", line 637, in
> DoPresubmitChecks
>     results += executer.ExecPresubmitScript(presubmit_script, filename)
>   File "c:\s\dt\depot_tools\presubmit_support.py", line 586, in
> ExecPresubmitScript
>     result = eval(function_name + '(*__args)', context)
>   File "<string>", line 0, in ?
>   File "<string>", line 14, in CheckChangeOnUpload
> NameError: global name 'RunUnitTests' is not defined
> 
> 
> Any ideas on this?  I can investigate but maybe you know off the top of your
> head.

Ignore this error for now, will fix.

patch lgtm.