Kallithea issues archive

In kallithea/lib/middleware/simplegit.py:

def is_git(environ):
    path_info = environ['PATH_INFO']
    log.debug('pathinfo: path_info is %s' % path_info)
    isgit_path = GIT_PROTO_PAT.match(path_info)
    log.debug('pathinfo: %s detected as Git %s',
        path_info, isgit_path is not None
    )
    return isgit_path

path_info is /infrastructure/testgit2.

Looking at the re.compile line:

GIT_PROTO_PAT = re.compile(r'^/(.+)/(info/refs|git-upload-pack|git-receive-pack)')

the path_info clearly (that is, if I understand the regex), it's checking to see if info/refs or git-upload-pack or git-receive-pack is at the end of it.

and since it doesn't, it doesn't detect it as a git repo.

I think I've missed the point of that regex. That or something was supposed to return the path_info (that contains the /info/refs part) but didn't.

Actually.. my mistake in understanding the code. Apparently the two simple* detections don't work, even for Mercurial repos. This means that the git detection is sometime after that.

Sorry.. apparently this is another problem with the system's package version. Kallithea expects a git version that supports '-c'. v1.7 (which was installed on this system from the packages) doesn't know anything about '-c'. (This is from line 118.:

  _copts = ['-c', 'core.quotepath=false', ]

My apologies for the noise.

I would expect the logs to show the output from

    req_ver = StrictVersion('1.7.4')

    log.debug('Git executable: "%s" version %s detected: %s',
              settings.GIT_EXECUTABLE_PATH, ver, stdout)
    if stderr:
        log.warning('Error detecting git version: %r', stderr)
    elif ver < req_ver:
        log.warning('Kallithea detected git version %s, which is too old '
                    'for the system to function properly. '
                    'Please upgrade to version %s or later.' % (ver, req_ver))

@ewongcc Please confirm whether you see this warning message in your log (while you had git v1.7).

If that is the case, I would classify this issue #342 as a user error: I don't see what we can do more. Making it a blocking error is not desired because some users may not need git at all.

But we could perhaps also help the user by complaining more loud and clear, for example when kallithea-cli is invoked?

@patrickdepinguin Yes. Please do classify it as user error. It does show the warning message.. and I missed it. It was only when I looked through the long apache logs does it show. My humblest apologies.

@kiilerix That would actually help a lot. As it would be the first thing a user sees when setting it up instead of finding the error in a long log.

Perhaps we could add and document a new kallithea-cli command, like 'systemcheck' that would verify certain things, like the version of git and maybe others. Same command could perhaps be used as a command-line alternative to the 'system info' tab in the Admin UI, i.e. listing the versions of tools used.

It seems cleaner to me than adding unrelated checks to exiting commands, like config-create, db-create, etc.

That would be something like hg debuginstall.

The problem is that those with problems often not will run that command. We need something that really is thrown in the face of people and give them a hint about what is wrong or where to look.

One alternative could be to stop the server process after logging such severe errors.

But the error is only a problem if you actually use git repos. Right now the git backend can be disabled by editing some python file, but not by editing just the ini file.

Stopping the server when git is too old could be done if we tell users to explicitly disable git if they want to proceed without updating git itself.

For gists git is also used, I don't know if it has the same requirements.

An other alternative is to require a "i know what I'm doing" setting to allow continuing with too old git.

We could have that through using existing knobs: Fail badly if git is too old, and people can specify another / non-existing git to work around it.

Like this?

diff --git a/kallithea/lib/utils.py b/kallithea/lib/utils.py
--- a/kallithea/lib/utils.py
+++ b/kallithea/lib/utils.py
@@ -606,7 +606,7 @@ def check_git_version():
     if stderr:
         log.warning('Error detecting git version: %r', stderr)
     elif ver < req_ver:
-        log.warning('Kallithea detected git version %s, which is too old '
+        raise Exception('Kallithea detected git version %s, which is too old '
                     'for the system to function properly. '
                     'Please upgrade to version %s or later.' % (ver, req_ver))
     return ver

Yes, it seems to me like that would be better (and very simple and low risk).

But perhaps also change the message to say something like "Please upgrade the Git configured in git_path (${config.git_path}) to yada yada ... or make it not point at an old Git if you don't care about Git support."

What's your thoughts about pros and cons about that?

But pointing at a non-existing git should perhaps ideally have the same effect as the mentioned "the git backend can be disabled by editing some python file"?

I don't know if that is feasible or a good idea, but it seems like a potential option ...

It would then actually be even better to automatically remove 'git' from BACKENDS (kallithea/init.py) if the version is not suitable. Then either the user has suitable git binary, and git can be used. Or not, and then git is disabled. The 'log.warning' should then perhaps be 'log.error' (but not an exception).

Right. And then log an error whenever someone try to access a git repo without having a suitable git available.

Should we make such change on stable, or rather on default?

It is only a matter of more clear error reporting. There is a workaround, so it is not essential. Also, the fix might be disruptive, and I think it will be problematic to introduce it in a minor update. I thus suggest doing “fail on error” on default branch.

Fixed with commit df275f701d53 .