List for cgit developers and users
 help / color / mirror / Atom feed
From: plenz at cis.fu-berlin.de (Julius Plenz)
Subject: [PATCH] guess default branch from HEAD
Date: Wed, 13 Jul 2011 17:10:37 +0200	[thread overview]
Message-ID: <20110713151037.GK5875@plenz.com> (raw)
In-Reply-To: <loom.20110709T095610-848@post.gmane.org>

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




  reply	other threads:[~2011-07-13 15:10 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-10 16:03 [PATCH 1/5] " plenz
2011-03-10 16:03 ` [PATCH 2/5] get_commit_date() obtains newest commit date plenz
2011-03-10 16:03 ` [PATCH 3/5] make enable-log-linecount independent of -filecount plenz
2011-03-10 17:25   ` hjemli
2011-03-26 14:43     ` hjemli
2011-03-10 16:03 ` [PATCH 4/5] fix two encoding bugs plenz
2011-03-10 17:29   ` hjemli
2011-03-10 16:03 ` [PATCH 5/5] Add advice about scan-path in cgitrc.5.txt plenz
2011-03-10 17:33   ` hjemli
2011-03-10 17:22 ` [PATCH 1/5] guess default branch from HEAD hjemli
2011-03-16 10:53   ` plenz
2011-03-26 10:08     ` hjemli
2011-03-30 16:00       ` [PATCH v2] " plenz
2011-04-07  9:44         ` hjemli
2011-04-07 10:49           ` plenz
2011-04-07 10:59             ` [PATCH] " plenz
2011-06-10  6:42               ` jugg
2011-06-13 10:16                 ` larsh
2011-06-13 10:16                   ` hjemli
2011-06-16 14:59                     ` plenz
2011-06-20 19:25                       ` hjemli
2011-07-09  8:09                         ` jugg
2011-07-13 15:10                           ` plenz [this message]
2011-07-19  7:08                             ` larsh
2011-07-19  9:40                             ` jugg
2011-07-19  6:30                           ` larsh
2011-07-19  9:35                             ` jugg
2011-07-19  9:56                               ` larsh
2011-07-19 10:13                                 ` jugg
2011-07-21 10:34                                   ` larsh
2011-04-07 11:05             ` [PATCH v2] " hjemli
2011-06-09  4:40               ` jugg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110713151037.GK5875@plenz.com \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).