[PATCH] guess default branch from HEAD

Julius Plenz plenz at cis.fu-berlin.de
Wed Jul 13 17:10:37 CEST 2011


Hi!

* chris <jugg at hotmail.com> [2011-07-09 10:12]:
> I've applied the following patch:
> 
> http://hjemli.net/git/cgit/patch/?id=795118d4042d53566d8375dcabe20515e9135351
> 
> ontop of:
> 
> http://hjemli.net/git/cgit/commit/?id=2a8f553163d642e60092ced20631e1020581273b
> 
> and the index page 'idle' times no longer show up (the idle column
> is blank). Each project's summary page 'Age' column still works just
> fine.

Do you get any error messages in your webserver's logs?

> This patch does not have this problem:
> 
> http://hjemli.net/git/cgit/patch/?id=d711de55280c3c9c10cfda4e24c8f3b3015217b2

In principle, the patch d711de5 (by Lars) does the same as 795118d
(mine). Consider the following program:

    #include <stdio.h>
    #include <git/refs.h>

    int main(int argc, char *argv[]) {
        const char *ref;
        unsigned char sha1[20];

        ref = resolve_ref("HEAD", sha1, 0, NULL);
        printf("%s\n", ref+11);

        return 0;
    }

If you compile that thing, it will correctly resolve the HEAD ref.

When Lars proposed the patch, I didn't actually try it out. Now,
reading over it again, I see two ovious quirks: The first is trivial
to fix: return "master" will return a stack address, but you have to
allocate memory for that:

diff --git a/cgit.c b/cgit.c
index d51885b..f9be05c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -427,7 +427,7 @@ static char *guess_defbranch(const char *repo_path)
 
        ref = resolve_ref("HEAD", sha1, 0, NULL);
        if (!ref || prefixcmp(ref, "refs/heads/"))
-               return "master";
+               return xstrdup("master");
        return xstrdup(ref + 11);
 }
 

Now, the second thing is a more severe thing. guess_defbranch will get
the repo_path as an argument; however, it's not used. The reason why
this works *once* (in my little program above) is that you explictly
set GIT_DIR on the shell, or you are in a git repository already.

However, when iterating over multiple repositories, you change your
gitdir. When writing the patch I discovered resolve_ref at some point,
BUT it cannot be used in this case. The key point is in the original
patch at http://hjemli.net/git/cgit/commit/?id=d711de55280c3c9c10cfda4e24c8f3b3015217b2

    head = fmt("%s/HEAD", repo_path);

This will look at repo_path/HEAD. resolve_ref cannot do this, because
it expects to find an already set up gitdir.

So, Lars, I propose to revert your patch and use my original one
instead. I agree it would be nice to use as much functionality from
libgit.a -- in this case, however, I can't see a way to do it properly.

Cheers,
Julius




More information about the CGit mailing list