[PATCH] D32452: [git-llvm] Make `push` work on CRLF files with svn:eol-style=native

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/utils/git-svn/git-llvm:92
+        # Silence errors if requested.
+        err_pipe = open(os.devnull, 'w')
+
----------------
jlebar wrote:
> Do we need to close this at some point?
We could, but what do you think about opening it once in a global and never closing it?


================
Comment at: llvm/utils/git-svn/git-llvm:195
+                sr).split('\n')
+    files = [f.split('/', 1)[1] for f in files]
+    # Use ignore_errors because 'svn propget' prints errors if the file doesn't
----------------
jlebar wrote:
> Would prefer to call some os.path function if there's one that DTRT for this, but if not, whatever.
git seems to use forward slashes in its output on Windows, so I hope this is safe. I could probably do `f.split('\\/', 1)[1]`, or try to combine os.path.sep and os.path.altsep, but that doesn't seem better.


================
Comment at: llvm/utils/git-svn/git-llvm:202
+    for eol_prop in eol_props:
+        (f, eol_style) = eol_prop.strip().split(' - ')
+        if eol_style == 'native':
----------------
jlebar wrote:
> Nit, probably want something to guard against the case where the filename contains ' - '.  Probably rsplit(' - ', 1)?
Done. I created such a file, added it to svn, gave it a property, listed it, and rsplit does the right thing.


https://reviews.llvm.org/D32452