Merge pull request gitpython-developers#530 fixing some gitpython-developers#525 Windows errors

+ git-daemon:
  + Use git-daemon PORT above 10k; on Windows all below need Admin rights.
  + Used relative daemon-paths with `--base-pth`.
  + Simplify git-daemon start/stop/ex-hanlding.
  +FIXED git-daemon  @with_rw_and_rw_remote_repo():
  + "Polish" most remote & config urls, converting \-->/.
  + test_base.test_with_rw_remote_and_rw_repo() PASS.
+ Remote: 
  + test_remote: apply polish-urls on `_do_test_fetch()` checking function.
  + test_remote.test_base() now freezes on Windows! (so still hidden win_err).
pump fetch-infos instead of GIL-reading stderr.
  + Push-cmd also keep (and optionally raise) any error messages.
+ `cmd.handle_process_output()` accepts null-finalizer, to pump completely
stderr before raising any errors.
+ test: Enable `TestGit.test_environment()` on Windows (to checks stderr
consumption).
+ util: delete unused `absolute_project_path()`.
+ Control separately *freezing* TCs on Windows with `git.util.HIDE_WINDOWS_FREEZE_ERRORS` flag.

Author: ankostis

git/cmd.py

@@ -42,12 +42,12 @@
 )
 
 
-execute_kwargs = set(('istream', 'with_keep_cwd', 'with_extended_output',
+execute_kwargs = set(('istream', 'with_extended_output',
                       'with_exceptions', 'as_process', 'stdout_as_string',
                       'output_stream', 'with_stdout', 'kill_after_timeout',
                       'universal_newlines', 'shell'))
 
-log = logging.getLogger('git.cmd')
+log = logging.getLogger(__name__)
 log.addHandler(logging.NullHandler())
 
 __all__ = ('Git',)
@@ -59,7 +59,8 @@
 # Documentation
 ## @{
 
-def handle_process_output(process, stdout_handler, stderr_handler, finalizer, decode_streams=True):
+def handle_process_output(process, stdout_handler, stderr_handler,
+                          finalizer=None, decode_streams=True):
     """Registers for notifications to lean that process output is ready to read, and dispatches lines to
     the respective line handlers.
     This function returns once the finalizer returns
@@ -108,10 +109,13 @@ def pump_stream(cmdline, name, stream, is_decode, handler):
         t.start()
         threads.append(t)
 
+    ## FIXME: Why Join??  Will block if `stdin` needs feeding...
+    #
     for t in threads:
         t.join()
 
-    return finalizer(process)
+    if finalizer:
+        return finalizer(process)
 
 
 def dashify(string):
@@ -186,14 +190,18 @@ def __setstate__(self, d):
     # Override this value using `Git.USE_SHELL = True`
     USE_SHELL = False
 
-    class AutoInterrupt(object):
+    @classmethod
+    def polish_url(cls, url):
+        return url.replace("\\\\", "\\").replace("\\", "/")
 
+    class AutoInterrupt(object):
         """Kill/Interrupt the stored process instance once this instance goes out of scope. It is
         used to prevent processes piling up in case iterators stop reading.
         Besides all attributes are wired through to the contained process object.
 
         The wait method was overridden to perform automatic status code checking
         and possibly raise."""
+
         __slots__ = ("proc", "args")
 
         def __init__(self, proc, args):
@@ -239,7 +247,7 @@ def __del__(self):
         def __getattr__(self, attr):
             return getattr(self.proc, attr)
 
-        def wait(self, stderr=b''):
+        def wait(self, stderr=b''):  # TODO: Bad choice to mimic `proc.wait()` but with different args.
             """Wait for the process and return its status code.
 
             :param stderr: Previously read value of stderr, in case stderr is already closed.
@@ -418,7 +426,6 @@ def version_info(self):
 
     def execute(self, command,
                 istream=None,
-                with_keep_cwd=False,
                 with_extended_output=False,
                 with_exceptions=True,
                 as_process=False,
@@ -441,11 +448,6 @@ def execute(self, command,
         :param istream:
             Standard input filehandle passed to subprocess.Popen.
 
-        :param with_keep_cwd:
-            Whether to use the current working directory from os.getcwd().
-            The cmd otherwise uses its own working_dir that it has been initialized
-            with if possible.
-
         :param with_extended_output:
             Whether to return a (status, stdout, stderr) tuple.
 
@@ -518,10 +520,7 @@ def execute(self, command,
             log.info(' '.join(command))
 
         # Allow the user to have the command executed in their working dir.
-        if with_keep_cwd or self._working_dir is None:
-            cwd = os.getcwd()
-        else:
-            cwd = self._working_dir
+        cwd = self._working_dir or os.getcwd()
 
         # Start the process
         env = os.environ.copy()
@@ -544,6 +543,9 @@ def execute(self, command,
                 cmd_not_found_exception = OSError
         # end handle
 
+        stdout_sink = (PIPE
+                       if with_stdout
+                       else getattr(subprocess, 'DEVNULL', open(os.devnull, 'wb')))
         log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s)",
                   command, cwd, universal_newlines, shell)
         try:
@@ -553,9 +555,9 @@ def execute(self, command,
                          bufsize=-1,
                          stdin=istream,
                          stderr=PIPE,
-                         stdout=PIPE if with_stdout else open(os.devnull, 'wb'),
+                         stdout=stdout_sink,
                          shell=shell is not None and shell or self.USE_SHELL,
-                         close_fds=(is_posix),  # unsupported on windows
+                         close_fds=is_posix,  # unsupported on windows
                          universal_newlines=universal_newlines,
                          creationflags=PROC_CREATIONFLAGS,
                          **subprocess_kwargs
@@ -647,10 +649,7 @@ def as_text(stdout_value):
         # END handle debug printing
 
         if with_exceptions and status != 0:
-            if with_extended_output:
-                raise GitCommandError(command, status, stderr_value, stdout_value)
-            else:
-                raise GitCommandError(command, status, stderr_value)
+            raise GitCommandError(command, status, stderr_value, stdout_value)
 
         if isinstance(stdout_value, bytes) and stdout_as_string:  # could also be output_stream
             stdout_value = safe_decode(stdout_value)

git/objects/submodule/base.py

@@ -41,6 +41,7 @@
 from unittest.case import SkipTest
 from git.util import HIDE_WINDOWS_KNOWN_ERRORS
 from git.objects.base import IndexObject, Object
+from git.cmd import Git
 
 __all__ = ["Submodule", "UpdateProgress"]
 
@@ -394,6 +395,9 @@ def add(cls, repo, name, path, url=None, branch=None, no_checkout=False):
             mrepo = cls._clone_repo(repo, url, path, name, **kwargs)
         # END verify url
 
+        ## See #525 for ensuring git urls in config-files valid under Windows.
+        url = Git.polish_url(url)
+
         # It's important to add the URL to the parent config, to let `git submodule` know.
         # otherwise there is a '-' character in front of the submodule listing
         #  a38efa84daef914e4de58d1905a500d8d14aaf45 mymodule (v0.9.0-1-ga38efa8)

git/remote.py

@@ -27,9 +27,8 @@
 )
 from git.util import (
     join_path,
-    finalize_process
 )
-from git.cmd import handle_process_output
+from git.cmd import handle_process_output, Git
 from gitdb.util import join
 from git.compat import (defenc, force_text, is_win)
 import logging
@@ -570,7 +569,7 @@ def create(cls, repo, name, url, **kwargs):
         :raise GitCommandError: in case an origin with that name already exists"""
         scmd = 'add'
         kwargs['insert_kwargs_after'] = scmd
-        repo.git.remote(scmd, name, url, **kwargs)
+        repo.git.remote(scmd, name, Git.polish_url(url), **kwargs)
         return cls(repo, name)
 
     # add is an alias
@@ -630,25 +629,19 @@ def _get_fetch_info_from_stderr(self, proc, progress):
         cmds = set(PushInfo._flag_map.keys()) & set(FetchInfo._flag_map.keys())
 
         progress_handler = progress.new_message_handler()
+        handle_process_output(proc, None, progress_handler, finalizer=None, decode_streams=False)
 
-        stderr_text = None
+        stderr_text = progress.error_lines and '\n'.join(progress.error_lines) or ''
+        proc.wait(stderr=stderr_text)
+        if stderr_text:
+            log.warning("Error lines received while fetching: %s", stderr_text)
 
-        for line in proc.stderr:
+        for line in progress.other_lines:
             line = force_text(line)
-            for pline in progress_handler(line):
-                # END handle special messages
-                for cmd in cmds:
-                    if len(line) > 1 and line[0] == ' ' and line[1] == cmd:
-                        fetch_info_lines.append(line)
-                        continue
-                    # end find command code
-                # end for each comand code we know
-            # end for each line progress didn't handle
-        # end
-        if progress.error_lines():
-            stderr_text = '\n'.join(progress.error_lines())
-
-        finalize_process(proc, stderr=stderr_text)
+            for cmd in cmds:
+                if len(line) > 1 and line[0] == ' ' and line[1] == cmd:
+                    fetch_info_lines.append(line)
+                    continue
 
         # read head information
         with open(join(self.repo.git_dir, 'FETCH_HEAD'), 'rb') as fp:
@@ -687,16 +680,19 @@ def stdout_handler(line):
             try:
                 output.append(PushInfo._from_line(self, line))
             except ValueError:
-                # if an error happens, additional info is given which we cannot parse
+                # If an error happens, additional info is given which we parse below.
                 pass
-            # END exception handling
-        # END for each line
 
+        handle_process_output(proc, stdout_handler, progress_handler, finalizer=None, decode_streams=False)
+        stderr_text = progress.error_lines and '\n'.join(progress.error_lines) or ''
         try:
-            handle_process_output(proc, stdout_handler, progress_handler, finalize_process, decode_streams=False)
+            proc.wait(stderr=stderr_text)
         except Exception:
-            if len(output) == 0:
+            if not output:
                 raise
+            elif stderr_text:
+                log.warning("Error lines received while fetching: %s", stderr_text)
+
         return output
 
     def _assert_refspec(self):

git/repo/base.py

@@ -62,8 +62,11 @@
 import os
 import sys
 import re
+import logging
 from collections import namedtuple
 
+log = logging.getLogger(__name__)
+
 DefaultDBType = GitCmdObjectDB
 if sys.version_info[:2] < (2, 5):     # python 2.4 compatiblity
     DefaultDBType = GitCmdObjectDB
@@ -871,46 +874,15 @@ def _clone(cls, git, url, path, odb_default_type, progress, **kwargs):
         if progress is not None:
             progress = to_progress_instance(progress)
 
-        # special handling for windows for path at which the clone should be
-        # created.
-        # tilde '~' will be expanded to the HOME no matter where the ~ occours. Hence
-        # we at least give a proper error instead of letting git fail
-        prev_cwd = None
-        prev_path = None
         odbt = kwargs.pop('odbt', odb_default_type)
-        if is_win:
-            if '~' in path:
-                raise OSError("Git cannot handle the ~ character in path %r correctly" % path)
-
-            # on windows, git will think paths like c: are relative and prepend the
-            # current working dir ( before it fails ). We temporarily adjust the working
-            # dir to make this actually work
-            match = re.match("(\w:[/\\\])(.*)", path)
-            if match:
-                prev_cwd = os.getcwd()
-                prev_path = path
-                drive, rest_of_path = match.groups()
-                os.chdir(drive)
-                path = rest_of_path
-                kwargs['with_keep_cwd'] = True
-            # END cwd preparation
-        # END windows handling
-
-        try:
-            proc = git.clone(url, path, with_extended_output=True, as_process=True,
-                             v=True, **add_progress(kwargs, git, progress))
-            if progress:
-                handle_process_output(proc, None, progress.new_message_handler(), finalize_process)
-            else:
-                (stdout, stderr) = proc.communicate()  # FIXME: Will block of outputs are big!
-                finalize_process(proc, stderr=stderr)
-            # end handle progress
-        finally:
-            if prev_cwd is not None:
-                os.chdir(prev_cwd)
-                path = prev_path
-            # END reset previous working dir
-        # END bad windows handling
+        proc = git.clone(url, path, with_extended_output=True, as_process=True,
+                         v=True, **add_progress(kwargs, git, progress))
+        if progress:
+            handle_process_output(proc, None, progress.new_message_handler(), finalize_process)
+        else:
+            (stdout, stderr) = proc.communicate()  # FIXME: Will block of outputs are big!
+            log.debug("Cmd(%s)'s unused stdout: %s", getattr(proc, 'args', ''), stdout)
+            finalize_process(proc, stderr=stderr)
 
         # our git command could have a different working dir than our actual
         # environment, hence we prepend its working dir if required
@@ -922,10 +894,10 @@ def _clone(cls, git, url, path, odb_default_type, progress, **kwargs):
         # that contains the remote from which we were clones, git stops liking it
         # as it will escape the backslashes. Hence we undo the escaping just to be
         # sure
-        repo = cls(os.path.abspath(path), odbt=odbt)
+        repo = cls(path, odbt=odbt)
         if repo.remotes:
             with repo.remotes[0].config_writer as writer:
-                writer.set_value('url', repo.remotes[0].url.replace("\\\\", "\\").replace("\\", "/"))
+                writer.set_value('url', Git.polish_url(repo.remotes[0].url))
         # END handle remote repo
         return repo