Summary

[clients/svn] fix convertion to absolute path for removed files

Review Request #7913 — Created Jan. 28, 2016 and discarded Jan. 29, 2019, 1:41 p.m.

Information

Owner

mizhka

Repository

RBTools

Branch

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Hi,

Here is patch for convertion to absolute path for removed files. If file has been removed in repository, "svn info" returns error. So if somebody tries to generate diff for revision where file has been removed, it generates incorrect diff (without absolute path, i.e. with branch information). As result, such diff can't be submitted to ReviewBoard instance.

The fix is simple and safe. To fetch file path prefix it calls "svn info ." instead of "svn info file", i.e. get information for root folder which exists in any way. Then it concatenates prefix and relative path.

Please review it!

Thank you in advance!

Testing Done

Tool: Pyflakes
Processed Files:
    rbtools/clients/svn.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/clients/svn.py

I'm not totally following the issue you are describing. It might be helpful if you could present a more detailed example, where svn info returns an error, perhaps in the form a ticket at https://hellosplat.com/s/beanbag/tickets/new/.

In any event, I don't think your current solution is going to work. First of all it causes multiple unit tests to fail. I believe the primary problem is that you can not assume that '.' is the relevant path of interest. It is possible to operate on files in different directories from where rbt has been launched from (e.g. rbt diff --include ../foo.py), which breaks the '.' assumption.

mizhka Feb. 1, 2016, 4:37 a.m.

Hi,

Thank you for your reply! Here is ticket: https://hellosplat.com/s/beanbag/tickets/4098/
Your remark is absolutely correct, current solution doesn't work for inclusion of files. May be it's worth to call "svn info ." only in case of SVN error raised by "svn info $file".

Please feel free to ask any question.

Thanks!

gmyers Feb. 1, 2016, 6:01 p.m.

Thanks for the detailed example in ticket.  I wasn't considering the post-commit use case.  I'll try to dig into this a bit, but it won't be for a few days.

Status:: Discarded