Git development
 help / color / mirror / Atom feed
* Re: metastore
From: martin f krafft @ 2007-10-02 20:23 UTC (permalink / raw)
  To: david, David Kastrup, David Härdeman, git, Daniel Barkalow
In-Reply-To: <Pine.LNX.4.64.0710021314370.24697@asgard.lang.hm>

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

also sprach david@lang.hm <david@lang.hm> [2007.10.02.2118 +0100]:
> the problem with this is dealing with the attributes outside of git 
> (especially when the filesystem can't store the attributes nativly, 
> specificly including things like owners when not running as root)

In which case you should not be able to manipulate them (as you
could not test the result) and any commits could not affect them,
meaning they'd just stay unchanged.

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
the unix philosophy basically involves
giving you enough rope to hang yourself.
and then some more, just to be sure.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: merging .gitignore
From: martin f krafft @ 2007-10-02 20:20 UTC (permalink / raw)
  To: Junio C Hamano, git, Andy Parkins, Johannes Schindelin
In-Reply-To: <7vodfhjnpe.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

also sprach Junio C Hamano <gitster@pobox.com> [2007.10.02.2055 +0100]:
> Perhaps you can use the existing union merge there.

Many thanks for the pointer, I did not even have time to look at
gitattributes.txt yet...

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
"self-denial is the shining sore
 on the leprous body of christianity."
                                                        -- oscar wilde
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: metastore
From: david @ 2007-10-02 20:18 UTC (permalink / raw)
  To: David Kastrup
  Cc: David Härdeman, martin f krafft, git, Daniel Barkalow,
	Johannes Schindelin, Thomas Harning Jr., Francis Moreau,
	Nicolas Vilz
In-Reply-To: <85lkalz3iv.fsf@lola.goethe.zz>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1461 bytes --]

On Tue, 2 Oct 2007, David Kastrup wrote:

> David Härdeman <david@hardeman.nu> writes:
>
>> On Tue, Oct 02, 2007 at 08:53:01PM +0100, martin f krafft wrote:
>>> also sprach David Härdeman <david@hardeman.nu> [2007.09.19.2016 +0100]:
>>>> But I agree, if any changes were made to git, I'd advocate adding
>>>> arbitrary attributes to files (much like xattrs) in name=value
>>>> pairs, then any extended metadata could be stored in those
>>>> attributes and external scripts/tools could use them in some way
>>>> that makes sense...and also make sure to only update them when it
>>>> makes sense.
>>>
>>> So where would those metdata be stored in your opinion?
>>
>> I'm not sufficiently versed in the internals of git to have an
>> informed opinion :)
>
> I think we have something like a length count for file names in index
> and/or tree.  We could just put the (sorted) attributes after a NUL
> byte in the file name and include them in the count.  It would also
> make those artificially longer file names work more or less when
> sorting them for deltification.

the problem with this is dealing with the attributes outside of git 
(especially when the filesystem can't store the attributes nativly, 
specificly including things like owners when not running as root)

this is one of the reasons for talking about useing a seperate file for 
the attributes (the other being the ability to minimize the impact to 
git-core of tracking attributes)

David Lang

^ permalink raw reply

* [PATCH] Fix typo in config.txt
From: Johannes Schindelin @ 2007-10-02 20:14 UTC (permalink / raw)
  To: gitster, git


There was an 'l' (ell) instead of a '1' (one) in one of the gitlinks.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 328487b..6b2fc82 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -599,7 +599,7 @@ merge.summary::
 
 merge.tool::
 	Controls which merge resolution program is used by
-	gitlink:git-mergetool[l].  Valid values are: "kdiff3", "tkdiff",
+	gitlink:git-mergetool[1].  Valid values are: "kdiff3", "tkdiff",
 	"meld", "xxdiff", "emerge", "vimdiff", "gvimdiff", and "opendiff".
 
 merge.verbosity::
-- 
1.5.3.3.1147.g47f44

^ permalink raw reply related

* Re: merging .gitignore
From: Pierre Habouzit @ 2007-10-02 20:13 UTC (permalink / raw)
  To: martin f krafft; +Cc: git, Andy Parkins, Johannes Schindelin
In-Reply-To: <20071002195148.GA14171@lapse.madduck.net>

[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]

On Tue, Oct 02, 2007 at 07:51:48PM +0000, martin f krafft wrote:
> Well, with gitignore I am ready to say that merges should be
> resolved in an additive way. Remember that I am talking about an
> intergration branch, and if feature branches A and B used to ignore
> .o files, and now B suddenly does not ignore them anymore, the only
> real reason I can think of is that it was rewritten in a languages
> other than C*. So then you *still* want to ignore .o files in the
> integration branch.
> 
> Basically I am saying that it should be
> 
>   cat $gitignore_files | sort -u

  Except that this would not work, just take that example (for the sake
of conciseness I put lines as members of a set):


  Common ancestor content: (bar, foo, quux)
  Left child: (bar, baz, foo, quux)
  Right child: (bar, quux)

  This one is a conflict, and if you apply your method, the merge always
"works" (as in has no cases where it fails) and would yield a result
like:

  (bar, baz, foo, quux) whereas it's probably (bar, baz, quux) that
would be the proper one (aka left branch added a new ignore `baz` and
the right one removed it).

  The proper way for gitignore is probably to work on the sets
operations, like diff does with lines, but without taking ordering into
account. What gets harder is when your lists are:

  Ancestor: (aa*, aaa, bbb)
  Left child: (aa*, bbb)   <-- remove aaa because aa* covers it
  Right child: (aaa, aabcd, bbb, cc*) <-- remove aa* and be explicit

  The proper result is probably: (aaa, aabcd, bbb, cc*) but is in fact a
case of conflict, because the "left" child could have used the fact that
aa* was present and hide say a aaXXX that the right child did not had,
and the merge would be wrong.

  Of course, .gitignore aren't _that_ important and if you ignore one
less file, or one too many, git will continue to behave properly, but
well, merge implementations aren't _that_ trivial.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: metastore
From: David Kastrup @ 2007-10-02 20:04 UTC (permalink / raw)
  To: David Härdeman
  Cc: martin f krafft, git, Daniel Barkalow, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz
In-Reply-To: <20071002195816.GA6759@hardeman.nu>

David Härdeman <david@hardeman.nu> writes:

> On Tue, Oct 02, 2007 at 08:53:01PM +0100, martin f krafft wrote:
>>also sprach David Härdeman <david@hardeman.nu> [2007.09.19.2016 +0100]:
>>> But I agree, if any changes were made to git, I'd advocate adding
>>> arbitrary attributes to files (much like xattrs) in name=value
>>> pairs, then any extended metadata could be stored in those
>>> attributes and external scripts/tools could use them in some way
>>> that makes sense...and also make sure to only update them when it
>>> makes sense.
>>
>>So where would those metdata be stored in your opinion?
>
> I'm not sufficiently versed in the internals of git to have an
> informed opinion :)

I think we have something like a length count for file names in index
and/or tree.  We could just put the (sorted) attributes after a NUL
byte in the file name and include them in the count.  It would also
make those artificially longer file names work more or less when
sorting them for deltification.

However, this requires implementing _policies_: it must be possible to
specify per repository exactly what will and what won't get tracked,
or one will get conflicts that are not necessary or appropriate.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: David Kastrup @ 2007-10-02 18:44 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071002173941.GA7187@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Oct 02, 2007 at 06:31:18PM +0200, David Kastrup wrote:
>
>> This does not actually require an actual merge _sort_ AFAICS: do the
>> "sort file.hashed" step using qsort.  The comparison step does not
>> actually need to produce merged output, but merely advances through
>> two hash arrays and generates statistics.
>
> Right, that's why I used "merge" in quotes.
>
>> This should already beat the pants off the current implementation,
>> even when the hash array is sparse, simply because our inner loop
>> then has perfect hash coherence.
>
> Yes, I hope so. We'll see. :)
>
>> Getting rid of this outer O(n^2) remains an interesting challenge,
>> though.  One way would be the following: fill a _single_ array with
>> entries containing _both_ hash and file number.  Sort this, and
>> then gather the statistics of hash runs by making a single pass
>> through.  That reduces the O(n^2) behavior to only those parts with
>> actual hash collisions.
>
> Interesting. Care to take a stab at implementing it?

I actually have worked through the last night on the day job, have
urgent stuff piling up in my freelance work queue, and the next thing
I need to finish for git is some smart stuff for delta packing.

So it's unlikely I'll get to _that_ anytime soon.  However, I had a
hilarious idea on the way home that kept me rather amused (perhaps my
programmer's humour is affected by sleep deprivation).

I was annoyed at needing double the space because of having to keep
score of both hash and file number.  So I came up with a rather cute
manner to avoid this: first do all files in isolation with full
precision, but store the resulting list of hash as difference to the
last value.  When merging the data of 2^k and 2^k (or somewhat less)
files, we multiply the values by two (this will not carry except for
utterly improbable cases or very small data sets which we can do
differently) and add one bit of identification.  When we have just a
single sequence remaining, undeltafying will tell us about collisions
in the high bits, and the affected files in the low bits.

Of course, using a merge-like algorithm means that we temporarily need
double space anyway.  Which takes some of the fun.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: David Härdeman @ 2007-10-02 19:58 UTC (permalink / raw)
  To: martin f krafft
  Cc: git, Daniel Barkalow, Johannes Schindelin, Thomas Harning Jr.,
	Francis Moreau, Nicolas Vilz
In-Reply-To: <20071002195301.GB14171@lapse.madduck.net>

On Tue, Oct 02, 2007 at 08:53:01PM +0100, martin f krafft wrote:
>also sprach David Härdeman <david@hardeman.nu> [2007.09.19.2016 +0100]:
>> But I agree, if any changes were made to git, I'd advocate adding
>> arbitrary attributes to files (much like xattrs) in name=value
>> pairs, then any extended metadata could be stored in those
>> attributes and external scripts/tools could use them in some way
>> that makes sense...and also make sure to only update them when it
>> makes sense.
>
>So where would those metdata be stored in your opinion?

I'm not sufficiently versed in the internals of git to have an informed 
opinion :)

-- 
David Härdeman

^ permalink raw reply

* Re: merging .gitignore
From: Junio C Hamano @ 2007-10-02 19:55 UTC (permalink / raw)
  To: martin f krafft; +Cc: git, Andy Parkins, Johannes Schindelin
In-Reply-To: <20071002195148.GA14171@lapse.madduck.net>

martin f krafft <madduck@madduck.net> writes:

> Basically I am saying that it should be
>
>   cat $gitignore_files | sort -u
>
> and obviously, this is something for a specific merge driver, as
> Johannes suggested.

Perhaps you can use the existing union merge there.

^ permalink raw reply

* Re: metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-10-02 19:53 UTC (permalink / raw)
  To: git
  Cc: David Härdeman, Daniel Barkalow, Johannes Schindelin,
	Thomas Harning Jr., Francis Moreau, Nicolas Vilz
In-Reply-To: <20070919191607.GE13683@hardeman.nu>

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

also sprach David Härdeman <david@hardeman.nu> [2007.09.19.2016 +0100]:
> But I agree, if any changes were made to git, I'd advocate adding
> arbitrary attributes to files (much like xattrs) in name=value
> pairs, then any extended metadata could be stored in those
> attributes and external scripts/tools could use them in some way
> that makes sense...and also make sure to only update them when it
> makes sense.

So where would those metdata be stored in your opinion?

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
seen on an advertising for an elaborate swiss men's watch:
  "almost as complicated as a woman. except it's on time"
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: merging .gitignore
From: martin f krafft @ 2007-10-02 19:51 UTC (permalink / raw)
  To: git; +Cc: Andy Parkins, Johannes Schindelin
In-Reply-To: <200710011448.17701.andyparkins@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

also sprach Johannes Schindelin <Johannes.Schindelin@gmx.de> [2007.10.01.1457 +0100]:
> You might be interested in writing a merge driver.  See 
> Documentation/gitattributes.txt.

This is an excellent idea; thanks.



also sprach Andy Parkins <andyparkins@gmail.com> [2007.10.01.1448 +0100]:
> Then, assuming the conflicts you get now occur for a reason, you
> will get conflicts within the .gitignore.d/ directory.  Let's say
> branchCignores adds *.o and branchFignores removes *.o from the
> ignores.  Who is right?  Who knows, and worse than that you didn't
> see the conflict when it happened so it wasn't resolved and the
> master branch was left with conflicts in it.

Well, with gitignore I am ready to say that merges should be
resolved in an additive way. Remember that I am talking about an
intergration branch, and if feature branches A and B used to ignore
.o files, and now B suddenly does not ignore them anymore, the only
real reason I can think of is that it was rewritten in a languages
other than C*. So then you *still* want to ignore .o files in the
integration branch.

Basically I am saying that it should be

  cat $gitignore_files | sort -u

and obviously, this is something for a specific merge driver, as
Johannes suggested.

Thanks for the feedback,

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
minchinhampton (n.): the expression on a man's face when he has just
zipped up his trousers without due care and attention.
                               -- douglas adams, the meaning of liff
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] Change "refs/" references to symbolic constants
From: Junio C Hamano @ 2007-10-02 19:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Andy Parkins, git
In-Reply-To: <20071002191104.GA7901@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Oct 02, 2007 at 07:16:43PM +0100, Andy Parkins wrote:
>
>> Changed repeated use of the same constants for the ref paths to be
>> symbolic constants.  I've defined them in refs.h
>
> I've manually inspected the patch. Comments are below.
>
>> -		if (prefixcmp(head, "refs/heads/"))
>> -			die("HEAD not found below refs/heads!");
>> -		head += 11;
>> +		if (prefixcmp(head, PATH_REFS_HEADS))
>> +			die("HEAD not found below " PATH_REFS_HEADS "!");
>> +		head += STRLEN_PATH_REFS_HEADS;
>
> This slightly changes the message (extra "/"), but I don't think that is
> a big deal...

        die("HEAD not found below %.*%s!",
             PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS-1)

>> -		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>> +		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)
>
> I find the 'PATH_REFS_TAGS "%s"' (with a space) you used earlier a
> little easier to read.

Even though we all know that PATH_REFS_* do not have any '%' in
them, it is somewhat unnerving to see such an opaque string in
the format specifier part of _any_printf() function.  It just
makes you think twice, disrupting the flow of thoughts.

This applies to die() and friends as well; see my above rewrite.

To me, the valid reasons for this kind of rewrite are if:

 - it makes typo harder to make and easier to spot
   (e.g. "refs/head/");

 - it makes miscount harder to make and easier to spot (e.g.
   what is this magic constant 11? Is it strlen("refs/heads/")?);

 - it makes reviewing the resulting code, and more importantly,
   future patches on the resulting code, easier.

 - it makes it easier for us to later revamp the strings
   wholesale (e.g. "refs/heads/" => "refs/branches/").

 - it saves us repeated instances of the same string constant;
   using C literal string as values for PATH_REFS_HEADS would
   not help and you would need (const char []) strings instead,
   but the compiler may be clever enough to do so.

Unquestionably, this series helps on the first two counts.

It however actively hurts on the third count.  These long
constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to
the eye, overwhelming the surrounding code.  I wonder if we can
do anything about this point to resurrect the first two
benefits, which I like very much.

The forth is a myth we shouldn't care about.  If we later would
want to change refs/heads to refs/branches, we would want to
rename PATH_REFS_HEADS to PATH_REFS_BRANCHES at the same time as
well, so the kind of rewrite this patch does does not buy us
anything there.  More importantly, such a change would need to
be made in a backward compatible way (e.g. "if we have heads
then keep using them but in new repositories we favor
branches"), so it won't be straight token replacement anyway.

And the fifth do not apply to us.  This matters only if we were
an embedded application on memory starved machine and string
constants are far smaller matter compared to the amount of other
data we use in-core.

^ permalink raw reply

* Re: git-diff not showing changes (corrupt repo?)
From: Jeff King @ 2007-10-02 19:17 UTC (permalink / raw)
  To: Dan Zwell; +Cc: git
In-Reply-To: <47029414.3080100@gmail.com>

On Tue, Oct 02, 2007 at 01:55:16PM -0500, Dan Zwell wrote:

> A small personal git repository has started lying to me about changed files. 
> git-diff sometimes tells me that the index has no changes from HEAD, while 
> other commands (git-status, at least) seem to tell the truth. It is the same 

Perhaps you are confused by the fact that "git-diff" with no options
shows the difference between the index and the working tree? Did you try
"git-diff --cached"?

> after I commit the new changes--at that point, "git-diff-tree HEAD^ HEAD -p" 
> spits out a nice patch, but "git-diff HEAD^ HEAD" gives nothing.

That doesn't seem right. Can you reproduce this, or at least show us the
command you used?

> $ git status
> # On branch bak_linear-checks3
> nothing to commit (working directory clean)
> $ echo "test" >> Makefile
> $ git status
> # On branch bak_linear-checks3
> # Changed but not updated:
> #   (use "git add <file>..." to update what will be committed)
> #
> #       modified:   Makefile
> #
> no changes added to commit (use "git add" and/or "git commit -a")
> $ git diff
> $

OK, that does seem wrong. If you run git-diff-files, does it produce the
expected output?

-Peff

^ permalink raw reply

* Re: git-diff not showing changes (corrupt repo?)
From: Junio C Hamano @ 2007-10-02 19:15 UTC (permalink / raw)
  To: Dan Zwell; +Cc: git
In-Reply-To: <47029414.3080100@gmail.com>

Dan Zwell <dzwell@gmail.com> writes:

> ... It is the same after I commit the new changes--at that point,
> "git-diff-tree HEAD^ HEAD -p" spits out a nice patch, but "git-diff
> HEAD^ HEAD" gives nothing.

This part is most interesting.  They are both about comparing
two commits and do not interact with anything in the work tree
nor your index.

> I have tried git versions 1.5.1.6-1.5.3.3, and they all act the same,

Do you mean 1.5.1.5 was Ok?  Can you bisect it?

> so I think the repository is corrupt. Does sound familiar to anybody?

Not to me; and I do not think repository is corrupt from the two
"HEAD^ vs HEAD" diff.  There is something entirely different
going on.

> ... I can
> restore from a recent backup and redo some changes, but I would like
> to help troubleshoot this behavior if it is a git bug.

Thanks.

^ permalink raw reply

* Re: [PATCH] Change "refs/" references to symbolic constants
From: Jeff King @ 2007-10-02 19:11 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git
In-Reply-To: <200710021916.44388.andyparkins@gmail.com>

On Tue, Oct 02, 2007 at 07:16:43PM +0100, Andy Parkins wrote:

> Changed repeated use of the same constants for the ref paths to be
> symbolic constants.  I've defined them in refs.h

I've manually inspected the patch. Comments are below.

> -		if (prefixcmp(head, "refs/heads/"))
> -			die("HEAD not found below refs/heads!");
> -		head += 11;
> +		if (prefixcmp(head, PATH_REFS_HEADS))
> +			die("HEAD not found below " PATH_REFS_HEADS "!");
> +		head += STRLEN_PATH_REFS_HEADS;

This slightly changes the message (extra "/"), but I don't think that is
a big deal...

> -	strcpy(path + len, "refs");
> +	strcpy(path + len, PATH_REFS);
>  	safe_create_dir(path, 1);
> -	strcpy(path + len, "refs/heads");
> +	strcpy(path + len, PATH_REFS_HEADS);
>  	safe_create_dir(path, 1);
> -	strcpy(path + len, "refs/tags");
> +	strcpy(path + len, PATH_REFS_TAGS);
>  	safe_create_dir(path, 1);

...but here it's not immediately obvious if the extra trailing "/" is
OK. Looks like the path just gets handed off to system calls trhough
safe_create_dir, and they are happy with the trailing slash. But it is a
behavior change.

> -		strcpy(path + len, "refs");
> +		strcpy(path + len, PATH_REFS);
>  		adjust_shared_perm(path);
> -		strcpy(path + len, "refs/heads");
> +		strcpy(path + len, PATH_REFS_HEADS);
>  		adjust_shared_perm(path);
> -		strcpy(path + len, "refs/tags");
> +		strcpy(path + len, PATH_REFS_TAGS);
>  		adjust_shared_perm(path);

And of course ditto here.

> -		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
> +		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)

I find the 'PATH_REFS_TAGS "%s"' (with a space) you used earlier a
little easier to read.

> -	if (len < 5 || memcmp(name, "refs/", 5))
> +	if (len < STRLEN_PATH_REFS || memcmp(name, PATH_REFS, STRLEN_PATH_REFS))

I imagine this was one of the times you mentioned before where prefixcmp
would be more readable. I would agree.

> -	strcpy(posn, "/objects/");
> +	strcpy(posn, "/" PATH_OBJECTS);
>  	posn += 9;

should be posn += 1 + STRLEN_PATH_OBJECTS ?

> -	url = xmalloc(strlen(repo->base) + 64);
> -	sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> +	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 56);
> +	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.idx", repo->base, hex);

The '56' is still quite hard to verify as correct ("/" + "pack/pack-" +
".idx" + "\0"). But I wonder if trying to fix that will just make it
harder to read (perhaps a comment is in order?).

Or maybe using a strbuf here would be much more obviously correct?

> -	url = xmalloc(strlen(base) + 31);
> -	sprintf(url, "%s/objects/info/http-alternates", base);
> +	url = xmalloc(strlen(base) + STRLEN_PATH_OBJECTS + 23);
> +	sprintf(url, "%s/" PATH_OBJECTS "info/http-alternates", base);

Also a potential strbuf. Ther are more of this same form, but I'm not
going to bother pointing out each one.

> -- 
> 1.5.3.rc5.11.g312e

Man that was tedious. But I think every other change is purely
syntactic, so there shouldn't be any bugs.

-Peff

^ permalink raw reply

* Re: Problems setting up bare repository (git 1.5.3.3)
From: Junio C Hamano @ 2007-10-02 19:09 UTC (permalink / raw)
  To: Sean; +Cc: Johannes Schindelin, Carl Worth, Barry Fishman, git
In-Reply-To: <BAYC1-PASMTP05AB6AE16E90C15710819EAEAE0@CEZ.ICE>

Sean <seanlkml@sympatico.ca> writes:

> On Tue, 02 Oct 2007 11:23:39 -0700
> Junio C Hamano <gitster@pobox.com> wrote:
>
>> If your push were "next~27^2:frotz", it becomes even less clear.
>> It may be that I am pushing out the tip of a topic branch I
>> usually do not push out, so it would be easier for some specific
>> person to build on top of.  Or maybe I am marking that place as
>> a lightweight tag.  They are equally likely.
>
> But you could pick a reasonable default in assuming that a new
> branch is desired with the above example.  If someone wants to
> push a tag, they can create the tag locally, and then push it.

I think you are on the same page.

We can pick _a_ default, and tell people that if they want a
non-default behaviour, they have to be explicit.  That goes
without saying.

The discussion between Johannes and I was about picking what
default is _reasonable_; Johannes made it sound like branches
are norm and tags are oddball.  I was merely pointing out that
it won't be so cut-and-dried.

^ permalink raw reply

* Re: Problems setting up bare repository (git 1.5.3.3)
From: Sean @ 2007-10-02 18:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Carl Worth, Barry Fishman, git
In-Reply-To: <7vwsu5l6j8.fsf@gitster.siamese.dyndns.org>

On Tue, 02 Oct 2007 11:23:39 -0700
Junio C Hamano <gitster@pobox.com> wrote:

> If your push were "next~27^2:frotz", it becomes even less clear.
> It may be that I am pushing out the tip of a topic branch I
> usually do not push out, so it would be easier for some specific
> person to build on top of.  Or maybe I am marking that place as
> a lightweight tag.  They are equally likely.

But you could pick a reasonable default in assuming that a new
branch is desired with the above example.  If someone wants to
push a tag, they can create the tag locally, and then push it.

Sean

^ permalink raw reply

* git-diff not showing changes (corrupt repo?)
From: Dan Zwell @ 2007-10-02 18:55 UTC (permalink / raw)
  To: git

Hi,
A small personal git repository has started lying to me about changed 
files. git-diff sometimes tells me that the index has no changes from 
HEAD, while other commands (git-status, at least) seem to tell the 
truth. It is the same after I commit the new changes--at that point, 
"git-diff-tree HEAD^ HEAD -p" spits out a nice patch, but "git-diff 
HEAD^ HEAD" gives nothing.

I have tried git versions 1.5.1.6-1.5.3.3, and they all act the same, so 
I think the repository is corrupt. Does sound familiar to anybody? If 
this is user error (for example, "git-reset --hard HEAD^" on a branch 
that had already been pulled into another branch), I can restore from a 
recent backup and redo some changes, but I would like to help 
troubleshoot this behavior if it is a git bug. Is this a known behavior? 
git-fsck doesn't reveal anything amiss, besides dangling objects. Does 
anybody know anything about this situation or what can cause it?


Example of behavior:

$ git status
# On branch bak_linear-checks3
nothing to commit (working directory clean)
$ echo "test" >> Makefile
$ git status
# On branch bak_linear-checks3
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   Makefile
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git diff
$

Thanks,
Dan

^ permalink raw reply

* Re: trailing whitespace problem
From: Junio C Hamano @ 2007-10-02 18:54 UTC (permalink / raw)
  To: lode leroy; +Cc: git
In-Reply-To: <b41dbf4a0710020507va40ef83u50c88094f8c2823b@mail.gmail.com>

"lode leroy" <lode.leroy@gmail.com> writes:

> when I use git commit -a, I get the following message:
>
> *
> * You have some suspicious patch lines:
> *
> * In src/test.c
> * trailing whitespace (line 60)
> src/test.c:60: }
>
> when I fix the whitespace problem,  and do "git commit -a" again,
> I still get the same message.
>
> Only after adding the file again, with "git add src/test.c",
> I can commit my changes.

Interesting.  Are you doing anything unusual, like "assume unchanged"?

> git version 1.5.3.2 (on cygwin)
>
> ps: not related to CR/LF...

^ permalink raw reply

* Re: [PATCH] Adding rebase merge strategy
From: Junio C Hamano @ 2007-10-02 18:40 UTC (permalink / raw)
  To: Tom Clarke; +Cc: Johannes Schindelin, Shawn O. Pearce, Carl Worth, git
In-Reply-To: <550f9510710020329m7917dc9m2bb6cfc4055fea84@mail.gmail.com>

"Tom Clarke" <tom@u2i.com> writes:

> So it's perhaps the question is whether rebasing should be treated as
> a kind of merging, or as an alternative to merging when pulling.
> Incidentally, are there any other cases other than pulling where using
> rebase as an alternative merge strategy is useful?

Very well put; I like concise summary of the point in a
discussion thread like this.

And the question in your last sentence is not merely incidental,
but I think is the most crucial one to decide which way we
should proceed.

I do not offhand think of a place other than "git pull" that
would make sense to sometimes be able to rebase when you
normally use merge, so I am inclined to say it would be easier
to teach that "'git pull' is usually a 'git fetch' followed by
'git merge', but in certain workflow it is handier to 'git
fetch' and then 'git rebase', and here are the ways to get the
rebasing behaviour...".

^ permalink raw reply

* Re: Problems setting up bare repository (git 1.5.3.3)
From: Junio C Hamano @ 2007-10-02 18:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Carl Worth, Barry Fishman, git
In-Reply-To: <Pine.LNX.4.64.0710021841300.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> But then I stepped back a little: What is most likely meant when you say 
> "master:blub" and there is no tag/branch of name "blub" on the remote 
> side?  Exactly, you want a branch to be created.

It's not that "exactly" for me.

If your push were "next~27^2:frotz", it becomes even less clear.
It may be that I am pushing out the tip of a topic branch I
usually do not push out, so it would be easier for some specific
person to build on top of.  Or maybe I am marking that place as
a lightweight tag.  They are equally likely.

On the other hand, what the user wants to do with "git push
$elsewhere frob" is reasonably clear.  If frob is locally a
branch, then the branch is pushed out.  If frob is a tag, the
tag is propagated.

^ permalink raw reply

* [PATCH] Change "refs/" references to symbolic constants
From: Andy Parkins @ 2007-10-02 18:16 UTC (permalink / raw)
  To: git
In-Reply-To: <20071002155800.GA6828@coredump.intra.peff.net>

Changed repeated use of the same constants for the ref paths to be
symbolic constants.  I've defined them in refs.h

  refs/ is now PATH_REFS
  refs/heads/ is now PATH_REFS_HEADS
  refs/tags/ is now PATH_REFS_TAGS
  refs/remotes/ is now PATH_REFS_REMOTES

I've changed all references to them and made constants for the string
lengths as well.  This has clarified the code in some places; for
example:

 - len = strlen(refs[i]) + 11;
 + len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;

In this case 11 isn't STRLEN_PATH_REFS_HEADS, as it is in most other
cases, it's TAGS + 1.  With the change to symbolic constants it's much
clearer what is happening.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 builtin-branch.c        |   28 ++++++++++++++--------------
 builtin-describe.c      |    2 +-
 builtin-fetch--tool.c   |   10 +++++-----
 builtin-fmt-merge-msg.c |    5 +++--
 builtin-fsck.c          |    4 ++--
 builtin-init-db.c       |   15 ++++++++-------
 builtin-name-rev.c      |   14 +++++++-------
 builtin-pack-refs.c     |    2 +-
 builtin-push.c          |    6 +++---
 builtin-show-branch.c   |   34 +++++++++++++++++-----------------
 builtin-show-ref.c      |    6 +++---
 builtin-tag.c           |    4 ++--
 connect.c               |   10 +++++-----
 fetch-pack.c            |    6 +++---
 http-fetch.c            |   31 ++++++++++++++++---------------
 http-push.c             |   42 +++++++++++++++++++++---------------------
 local-fetch.c           |   13 +++++++------
 path.c                  |    5 +++--
 receive-pack.c          |    4 ++--
 reflog-walk.c           |    6 +++---
 refs.c                  |   18 +++++++++---------
 refs.h                  |   17 +++++++++++++++++
 remote.c                |   14 +++++++-------
 setup.c                 |    5 +++--
 sha1_name.c             |   10 +++++-----
 wt-status.c             |    5 +++--
 26 files changed, 170 insertions(+), 146 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 5f5c182..b203b2a 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -92,12 +92,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 
 	switch (kinds) {
 	case REF_REMOTE_BRANCH:
-		fmt = "refs/remotes/%s";
+		fmt = PATH_REFS_REMOTES "%s";
 		remote = "remote ";
 		force = 1;
 		break;
 	case REF_LOCAL_BRANCH:
-		fmt = "refs/heads/%s";
+		fmt = PATH_REFS_HEADS "%s";
 		remote = "";
 		break;
 	default:
@@ -189,15 +189,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	int len;
 
 	/* Detect kind */
-	if (!prefixcmp(refname, "refs/heads/")) {
+	if (!prefixcmp(refname, PATH_REFS_HEADS)) {
 		kind = REF_LOCAL_BRANCH;
-		refname += 11;
-	} else if (!prefixcmp(refname, "refs/remotes/")) {
+		refname += STRLEN_PATH_REFS_HEADS;
+	} else if (!prefixcmp(refname, PATH_REFS_REMOTES)) {
 		kind = REF_REMOTE_BRANCH;
-		refname += 13;
-	} else if (!prefixcmp(refname, "refs/tags/")) {
+		refname += STRLEN_PATH_REFS_REMOTES;
+	} else if (!prefixcmp(refname, PATH_REFS_TAGS)) {
 		kind = REF_TAG;
-		refname += 10;
+		refname += STRLEN_PATH_REFS_TAGS;
 	}
 
 	/* Don't add types the caller doesn't want */
@@ -400,7 +400,7 @@ static void create_branch(const char *name, const char *start_name,
 	char *real_ref, ref[PATH_MAX], msg[PATH_MAX + 20];
 	int forcing = 0;
 
-	snprintf(ref, sizeof ref, "refs/heads/%s", name);
+	snprintf(ref, sizeof ref, PATH_REFS_HEADS "%s", name);
 	if (check_ref_format(ref))
 		die("'%s' is not a valid branch name.", name);
 
@@ -469,13 +469,13 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	if (!oldname)
 		die("cannot rename the current branch while not on any.");
 
-	if (snprintf(oldref, sizeof(oldref), "refs/heads/%s", oldname) > sizeof(oldref))
+	if (snprintf(oldref, sizeof(oldref), PATH_REFS_HEADS "%s", oldname) > sizeof(oldref))
 		die("Old branchname too long");
 
 	if (check_ref_format(oldref))
 		die("Invalid branch name: %s", oldref);
 
-	if (snprintf(newref, sizeof(newref), "refs/heads/%s", newname) > sizeof(newref))
+	if (snprintf(newref, sizeof(newref), PATH_REFS_HEADS "%s", newname) > sizeof(newref))
 		die("New branchname too long");
 
 	if (check_ref_format(newref))
@@ -602,9 +602,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		detached = 1;
 	}
 	else {
-		if (prefixcmp(head, "refs/heads/"))
-			die("HEAD not found below refs/heads!");
-		head += 11;
+		if (prefixcmp(head, PATH_REFS_HEADS))
+			die("HEAD not found below " PATH_REFS_HEADS "!");
+		head += STRLEN_PATH_REFS_HEADS;
 	}
 
 	if (delete)
diff --git a/builtin-describe.c b/builtin-describe.c
index 669110c..b6f5b45 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -53,7 +53,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	 * If --tags, then any tags are used.
 	 * Otherwise only annotated tags are used.
 	 */
-	if (!prefixcmp(path, "refs/tags/")) {
+	if (!prefixcmp(path, PATH_REFS_TAGS)) {
 		if (object->type == OBJ_TAG)
 			prio = 2;
 		else
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 24c7e6f..521b5fc 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -76,7 +76,7 @@ static int update_local_ref(const char *name,
 		char *msg;
 	just_store:
 		/* new ref */
-		if (!strncmp(name, "refs/tags/", 10))
+		if (!prefixcmp(name, PATH_REFS_TAGS))
 			msg = "storing tag";
 		else
 			msg = "storing head";
@@ -94,7 +94,7 @@ static int update_local_ref(const char *name,
 		return 0;
 	}
 
-	if (!strncmp(name, "refs/tags/", 10)) {
+	if (!prefixcmp(name, PATH_REFS_TAGS)) {
 		fprintf(stderr, "* %s: updating with %s\n", name, note);
 		show_new(type, sha1_new);
 		return update_ref_env("updating tag", name, sha1_new, NULL);
@@ -151,15 +151,15 @@ static int append_fetch_head(FILE *fp,
 		kind = "";
 		what = "";
 	}
-	else if (!strncmp(remote_name, "refs/heads/", 11)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_HEADS)) {
 		kind = "branch";
 		what = remote_name + 11;
 	}
-	else if (!strncmp(remote_name, "refs/tags/", 10)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_TAGS)) {
 		kind = "tag";
 		what = remote_name + 10;
 	}
-	else if (!strncmp(remote_name, "refs/remotes/", 13)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_REMOTES)) {
 		kind = "remote branch";
 		what = remote_name + 13;
 	}
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index ae60fcc..8700343 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -4,6 +4,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "tag.h"
+#include "refs.h"
 
 static const char *fmt_merge_msg_usage =
 	"git-fmt-merge-msg [--summary] [--no-summary] [--file <file>]";
@@ -282,8 +283,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
 	if (!current_branch)
 		die("No current branch");
-	if (!prefixcmp(current_branch, "refs/heads/"))
-		current_branch += 11;
+	if (!prefixcmp(current_branch, PATH_REFS_HEADS))
+		current_branch += STRLEN_PATH_REFS_HEADS;
 
 	while (fgets(line, sizeof(line), in)) {
 		i++;
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 8d12287..83a2d0c 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -629,14 +629,14 @@ static int fsck_head_link(void)
 	if (!strcmp(head_points_at, "HEAD"))
 		/* detached HEAD */
 		null_is_error = 1;
-	else if (prefixcmp(head_points_at, "refs/heads/"))
+	else if (prefixcmp(head_points_at, PATH_REFS_HEADS))
 		return error("HEAD points to something strange (%s)",
 			     head_points_at);
 	if (is_null_sha1(sha1)) {
 		if (null_is_error)
 			return error("HEAD: detached HEAD points at nothing");
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			head_points_at + 11);
+			head_points_at + STRLEN_PATH_REFS_HEADS);
 	}
 	return 0;
 }
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 763fa55..1f3c0dd 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "refs.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -194,11 +195,11 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	strcpy(path + len, "refs");
+	strcpy(path + len, PATH_REFS);
 	safe_create_dir(path, 1);
-	strcpy(path + len, "refs/heads");
+	strcpy(path + len, PATH_REFS_HEADS);
 	safe_create_dir(path, 1);
-	strcpy(path + len, "refs/tags");
+	strcpy(path + len, PATH_REFS_TAGS);
 	safe_create_dir(path, 1);
 
 	/* First copy the templates -- we might have the default
@@ -217,11 +218,11 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	if (shared_repository) {
 		path[len] = 0;
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs");
+		strcpy(path + len, PATH_REFS);
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs/heads");
+		strcpy(path + len, PATH_REFS_HEADS);
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs/tags");
+		strcpy(path + len, PATH_REFS_TAGS);
 		adjust_shared_perm(path);
 	}
 
@@ -232,7 +233,7 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	strcpy(path + len, "HEAD");
 	reinit = !read_ref("HEAD", sha1);
 	if (!reinit) {
-		if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
+		if (create_symref("HEAD", PATH_REFS_HEADS "master", NULL) < 0)
 			exit(1);
 	}
 
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 03083e9..56c13d0 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -96,7 +96,7 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	struct name_ref_data *data = cb_data;
 	int deref = 0;
 
-	if (data->tags_only && prefixcmp(path, "refs/tags/"))
+	if (data->tags_only && prefixcmp(path, PATH_REFS_TAGS))
 		return 0;
 
 	if (data->ref_filter && fnmatch(data->ref_filter, path, 0))
@@ -112,14 +112,14 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	if (o && o->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *)o;
 
-		if (!prefixcmp(path, "refs/heads/"))
-			path = path + 11;
+		if (!prefixcmp(path, PATH_REFS_HEADS))
+			path = path + STRLEN_PATH_REFS_HEADS;
 		else if (data->tags_only
 		    && data->name_only
-		    && !prefixcmp(path, "refs/tags/"))
-			path = path + 10;
-		else if (!prefixcmp(path, "refs/"))
-			path = path + 5;
+		    && !prefixcmp(path, PATH_REFS_TAGS))
+			path = path + STRLEN_PATH_REFS_TAGS;
+		else if (!prefixcmp(path, PATH_REFS))
+			path = path + STRLEN_PATH_REFS;
 
 		name_rev(commit, xstrdup(path), 0, 0, deref);
 	}
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 09df4e1..23b4c4e 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -39,7 +39,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
 	/* Do not pack the symbolic refs */
 	if ((flags & REF_ISSYMREF))
 		return 0;
-	is_tag_ref = !prefixcmp(path, "refs/tags/");
+	is_tag_ref = !prefixcmp(path, PATH_REFS_TAGS);
 
 	/* ALWAYS pack refs that were already packed or are tags */
 	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
diff --git a/builtin-push.c b/builtin-push.c
index 88c5024..2fdae7a 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -33,9 +33,9 @@ static void set_refspecs(const char **refs, int nr)
 			int len;
 			if (nr <= ++i)
 				die("tag shorthand without <tag>");
-			len = strlen(refs[i]) + 11;
+			len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;
 			tag = xmalloc(len);
-			strcpy(tag, "refs/tags/");
+			strcpy(tag, PATH_REFS_TAGS);
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
@@ -148,7 +148,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--tags")) {
-			add_refspec("refs/tags/*");
+			add_refspec(PATH_REFS_TAGS "*");
 			continue;
 		}
 		if (!strcmp(arg, "--force") || !strcmp(arg, "-f")) {
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 4fa87f6..7e39d60 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -380,36 +380,36 @@ static int append_ref(const char *refname, const unsigned char *sha1,
 static int append_head_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	unsigned char tmp[20];
-	int ofs = 11;
-	if (prefixcmp(refname, "refs/heads/"))
+	int ofs = STRLEN_PATH_REFS_HEADS;
+	if (prefixcmp(refname, PATH_REFS_HEADS))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
-		ofs = 5;
+		ofs = STRLEN_PATH_REFS;
 	return append_ref(refname + ofs, sha1, 0);
 }
 
 static int append_remote_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	unsigned char tmp[20];
-	int ofs = 13;
-	if (prefixcmp(refname, "refs/remotes/"))
+	int ofs = STRLEN_PATH_REFS_REMOTES;
+	if (prefixcmp(refname, PATH_REFS_REMOTES))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
-		ofs = 5;
+		ofs = STRLEN_PATH_REFS;
 	return append_ref(refname + ofs, sha1, 0);
 }
 
 static int append_tag_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
-	if (prefixcmp(refname, "refs/tags/"))
+	if (prefixcmp(refname, PATH_REFS_TAGS))
 		return 0;
-	return append_ref(refname + 5, sha1, 0);
+	return append_ref(refname + STRLEN_PATH_REFS, sha1, 0);
 }
 
 static const char *match_ref_pattern = NULL;
@@ -438,9 +438,9 @@ static int append_matching_ref(const char *refname, const unsigned char *sha1, i
 		return 0;
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
-	if (!prefixcmp(refname, "refs/heads/"))
+	if (!prefixcmp(refname, PATH_REFS_HEADS))
 		return append_head_ref(refname, sha1, flag, cb_data);
-	if (!prefixcmp(refname, "refs/tags/"))
+	if (!prefixcmp(refname, PATH_REFS_TAGS))
 		return append_tag_ref(refname, sha1, flag, cb_data);
 	return append_ref(refname, sha1, 0);
 }
@@ -465,12 +465,12 @@ static int rev_is_head(char *head, int headlen, char *name,
 	if ((!head[0]) ||
 	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
-	if (!prefixcmp(head, "refs/heads/"))
-		head += 11;
-	if (!prefixcmp(name, "refs/heads/"))
-		name += 11;
-	else if (!prefixcmp(name, "heads/"))
-		name += 6;
+	if (!prefixcmp(head, PATH_REFS_HEADS))
+		head += STRLEN_PATH_REFS_HEADS;
+	if (!prefixcmp(name, PATH_REFS_HEADS))
+		name += STRLEN_PATH_REFS_HEADS;
+	else if (!prefixcmp(name, PATH_HEADS))
+		name += STRLEN_PATH_HEADS;
 	return !strcmp(head, name);
 }
 
@@ -781,7 +781,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 				has_head++;
 		}
 		if (!has_head) {
-			int pfxlen = strlen("refs/heads/");
+			int pfxlen = STRLEN_PATH_REFS_HEADS;
 			append_one_rev(head + pfxlen);
 		}
 	}
diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index 65051d1..d463d80 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -29,8 +29,8 @@ static int show_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	if (tags_only || heads_only) {
 		int match;
 
-		match = heads_only && !prefixcmp(refname, "refs/heads/");
-		match |= tags_only && !prefixcmp(refname, "refs/tags/");
+		match = heads_only && !prefixcmp(refname, PATH_REFS_HEADS);
+		match |= tags_only && !prefixcmp(refname, PATH_REFS_TAGS);
 		if (!match)
 			return 0;
 	}
@@ -227,7 +227,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 		while (*pattern) {
 			unsigned char sha1[20];
 
-			if (!prefixcmp(*pattern, "refs/") &&
+			if (!prefixcmp(*pattern, PATH_REFS) &&
 			    resolve_ref(*pattern, sha1, 1, NULL)) {
 				if (!quiet)
 					show_one(*pattern, sha1);
diff --git a/builtin-tag.c b/builtin-tag.c
index 3a9d2ee..800e823 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -146,7 +146,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
 	unsigned char sha1[20];
 
 	for (p = argv; *p; p++) {
-		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
+		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)
 					>= sizeof(ref)) {
 			error("tag name too long: %.*s...", 50, *p);
 			had_error = 1;
@@ -440,7 +440,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (get_sha1(object_ref, object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
 
-	if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1)
+	if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS "%s", tag) > sizeof(ref) - 1)
 		die("tag name too long: %.*s...", 50, tag);
 	if (check_ref_format(ref))
 		die("'%s' is not a valid tag name.", tag);
diff --git a/connect.c b/connect.c
index 8b1e993..9d576bc 100644
--- a/connect.c
+++ b/connect.c
@@ -13,23 +13,23 @@ static int check_ref(const char *name, int len, unsigned int flags)
 	if (!flags)
 		return 1;
 
-	if (len < 5 || memcmp(name, "refs/", 5))
+	if (len < STRLEN_PATH_REFS || memcmp(name, PATH_REFS, STRLEN_PATH_REFS))
 		return 0;
 
 	/* Skip the "refs/" part */
-	name += 5;
-	len -= 5;
+	name += STRLEN_PATH_REFS;
+	len -= STRLEN_PATH_REFS;
 
 	/* REF_NORMAL means that we don't want the magic fake tag refs */
 	if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
 		return 0;
 
 	/* REF_HEADS means that we want regular branch heads */
-	if ((flags & REF_HEADS) && !memcmp(name, "heads/", 6))
+	if ((flags & REF_HEADS) && !memcmp(name, PATH_HEADS, STRLEN_PATH_HEADS))
 		return 1;
 
 	/* REF_TAGS means that we want tags */
-	if ((flags & REF_TAGS) && !memcmp(name, "tags/", 5))
+	if ((flags & REF_TAGS) && !memcmp(name, PATH_TAGS, STRLEN_PATH_TAGS))
 		return 1;
 
 	/* All type bits clear means that we are ok with anything */
diff --git a/fetch-pack.c b/fetch-pack.c
index 9c81305..c3b7ef6 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -344,11 +344,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
 
 	for (ref = *refs; ref; ref = next) {
 		next = ref->next;
-		if (!memcmp(ref->name, "refs/", 5) &&
-		    check_ref_format(ref->name + 5))
+		if (!memcmp(ref->name, PATH_REFS, STRLEN_PATH_REFS) &&
+		    check_ref_format(ref->name + STRLEN_PATH_REFS))
 			; /* trash */
 		else if (fetch_all &&
-			 (!depth || prefixcmp(ref->name, "refs/tags/") )) {
+			 (!depth || prefixcmp(ref->name, PATH_REFS_TAGS) )) {
 			*newtail = ref;
 			ref->next = NULL;
 			newtail = &ref->next;
diff --git a/http-fetch.c b/http-fetch.c
index 202fae0..85c46a8 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -3,6 +3,7 @@
 #include "pack.h"
 #include "fetch.h"
 #include "http.h"
+#include "refs.h"
 
 #define PREV_BUF_SIZE 4096
 #define RANGE_HEADER_SIZE 30
@@ -157,11 +158,11 @@ static void start_object_request(struct object_request *obj_req)
 
 	SHA1_Init(&obj_req->c);
 
-	url = xmalloc(strlen(obj_req->repo->base) + 51);
-	obj_req->url = xmalloc(strlen(obj_req->repo->base) + 51);
+	url = xmalloc(strlen(obj_req->repo->base) + STRLEN_PATH_OBJECTS + 43);
+	obj_req->url = xmalloc(strlen(obj_req->repo->base) + STRLEN_PATH_OBJECTS + 43);
 	strcpy(url, obj_req->repo->base);
 	posn = url + strlen(obj_req->repo->base);
-	strcpy(posn, "/objects/");
+	strcpy(posn, "/" PATH_OBJECTS);
 	posn += 9;
 	memcpy(posn, hex, 2);
 	posn += 2;
@@ -398,8 +399,8 @@ static int fetch_index(struct alt_base *repo, unsigned char *sha1)
 	if (get_verbosely)
 		fprintf(stderr, "Getting index for pack %s\n", hex);
 
-	url = xmalloc(strlen(repo->base) + 64);
-	sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.idx", repo->base, hex);
 
 	filename = sha1_pack_index_name(sha1);
 	snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename);
@@ -478,7 +479,7 @@ static void process_alternates_response(void *callback_data)
 
 			/* Try reusing the slot to get non-http alternates */
 			alt_req->http_specific = 0;
-			sprintf(alt_req->url, "%s/objects/info/alternates",
+			sprintf(alt_req->url, "%s/" PATH_OBJECTS "info/alternates",
 				base);
 			curl_easy_setopt(slot->curl, CURLOPT_URL,
 					 alt_req->url);
@@ -625,8 +626,8 @@ static void fetch_alternates(const char *base)
 	if (get_verbosely)
 		fprintf(stderr, "Getting alternates list for %s\n", base);
 
-	url = xmalloc(strlen(base) + 31);
-	sprintf(url, "%s/objects/info/http-alternates", base);
+	url = xmalloc(strlen(base) + STRLEN_PATH_OBJECTS + 23);
+	sprintf(url, "%s/" PATH_OBJECTS "info/http-alternates", base);
 
 	/* Use a callback to process the result, since another request
 	   may fail and need to have alternates loaded before continuing */
@@ -675,8 +676,8 @@ static int fetch_indices(struct alt_base *repo)
 	if (get_verbosely)
 		fprintf(stderr, "Getting pack list for %s\n", repo->base);
 
-	url = xmalloc(strlen(repo->base) + 21);
-	sprintf(url, "%s/objects/info/packs", repo->base);
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 13);
+	sprintf(url, "%s/" PATH_OBJECTS "info/packs", repo->base);
 
 	slot = get_active_slot();
 	slot->results = &results;
@@ -757,8 +758,8 @@ static int fetch_pack(struct alt_base *repo, unsigned char *sha1)
 			sha1_to_hex(sha1));
 	}
 
-	url = xmalloc(strlen(repo->base) + 65);
-	sprintf(url, "%s/objects/pack/pack-%s.pack",
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 57);
+	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.pack",
 		repo->base, sha1_to_hex(target->sha1));
 
 	filename = sha1_pack_name(target->sha1);
@@ -930,14 +931,14 @@ static char *quote_ref_url(const char *base, const char *ref)
 	int len, baselen, ch;
 
 	baselen = strlen(base);
-	len = baselen + 7; /* "/refs/" + NUL */
+	len = baselen + STRLEN_PATH_REFS + 2; /* "/" + "refs/" + NUL */
 	for (cp = ref; (ch = *cp) != 0; cp++, len++)
 		if (needs_quote(ch))
 			len += 2; /* extra two hex plus replacement % */
 	qref = xmalloc(len);
 	memcpy(qref, base, baselen);
-	memcpy(qref + baselen, "/refs/", 6);
-	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
+	memcpy(qref + baselen, "/" PATH_REFS, STRLEN_PATH_REFS + 1);
+	for (cp = ref, dp = qref + baselen + STRLEN_PATH_REFS + 1; (ch = *cp) != 0; cp++) {
 		if (needs_quote(ch)) {
 			*dp++ = '%';
 			*dp++ = hex((ch >> 4) & 0xF);
diff --git a/http-push.c b/http-push.c
index 7c3720f..1d26b9b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -272,12 +272,12 @@ static void start_fetch_loose(struct transfer_request *request)
 
 	SHA1_Init(&request->c);
 
-	url = xmalloc(strlen(remote->url) + 50);
-	request->url = xmalloc(strlen(remote->url) + 50);
+	url = xmalloc(strlen(remote->url) + 42 + STRLEN_PATH_OBJECTS);
+	request->url = xmalloc(strlen(remote->url) + 42 + STRLEN_PATH_OBJECTS);
 	strcpy(url, remote->url);
 	posn = url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	*(posn++) = '/';
@@ -357,11 +357,11 @@ static void start_mkcol(struct transfer_request *request)
 	struct active_request_slot *slot;
 	char *posn;
 
-	request->url = xmalloc(strlen(remote->url) + 13);
+	request->url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 5);
 	strcpy(request->url, remote->url);
 	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	strcpy(posn, "/");
@@ -415,8 +415,8 @@ static void start_fetch_packed(struct transfer_request *request)
 	snprintf(request->tmpfile, sizeof(request->tmpfile),
 		 "%s.temp", filename);
 
-	url = xmalloc(strlen(remote->url) + 64);
-	sprintf(url, "%sobjects/pack/pack-%s.pack",
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.pack",
 		remote->url, sha1_to_hex(target->sha1));
 
 	/* Make sure there isn't another open request for this pack */
@@ -519,11 +519,11 @@ static void start_put(struct transfer_request *request)
 	request->buffer.posn = 0;
 
 	request->url = xmalloc(strlen(remote->url) +
-			       strlen(request->lock->token) + 51);
+			       strlen(request->lock->token) + STRLEN_PATH_OBJECTS + 43);
 	strcpy(request->url, remote->url);
 	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	*(posn++) = '/';
@@ -922,8 +922,8 @@ static int fetch_index(unsigned char *sha1)
 	struct slot_results results;
 
 	/* Don't use the index if the pack isn't there */
-	url = xmalloc(strlen(remote->url) + 64);
-	sprintf(url, "%sobjects/pack/pack-%s.pack", remote->url, hex);
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.pack", remote->url, hex);
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
@@ -945,7 +945,7 @@ static int fetch_index(unsigned char *sha1)
 	if (push_verbosely)
 		fprintf(stderr, "Getting index for pack %s\n", hex);
 
-	sprintf(url, "%sobjects/pack/pack-%s.idx", remote->url, hex);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.idx", remote->url, hex);
 
 	filename = sha1_pack_index_name(sha1);
 	snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename);
@@ -1029,8 +1029,8 @@ static int fetch_indices(void)
 	if (push_verbosely)
 		fprintf(stderr, "Getting pack list\n");
 
-	url = xmalloc(strlen(remote->url) + 20);
-	sprintf(url, "%sobjects/info/packs", remote->url);
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 12);
+	sprintf(url, "%s" PATH_OBJECTS "info/packs", remote->url);
 
 	slot = get_active_slot();
 	slot->results = &results;
@@ -1615,7 +1615,7 @@ static void remote_ls(const char *path, int flags,
 
 static void get_remote_object_list(unsigned char parent)
 {
-	char path[] = "objects/XX/";
+	char path[] = PATH_OBJECTS "XX/";
 	static const char hex[] = "0123456789abcdef";
 	unsigned int val = parent;
 
@@ -1925,7 +1925,7 @@ static void get_local_heads(void)
 static void get_dav_remote_heads(void)
 {
 	remote_tail = &remote_refs;
-	remote_ls("refs/", (PROCESS_FILES | PROCESS_DIRS | RECURSIVE), process_ls_ref, NULL);
+	remote_ls(PATH_REFS, (PROCESS_FILES | PROCESS_DIRS | RECURSIVE), process_ls_ref, NULL);
 }
 
 static int is_zero_sha1(const unsigned char *sha1)
@@ -2069,7 +2069,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 	buffer.buffer = xcalloc(1, 4096);
 	buffer.size = 4096;
 	buffer.posn = 0;
-	remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
+	remote_ls(PATH_REFS, (PROCESS_FILES | RECURSIVE),
 		  add_remote_info_ref, &buffer);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
@@ -2375,7 +2375,7 @@ int main(int argc, char **argv)
 	/* Check whether the remote has server info files */
 	remote->can_update_info_refs = 0;
 	remote->has_info_refs = remote_exists("info/refs");
-	remote->has_info_packs = remote_exists("objects/info/packs");
+	remote->has_info_packs = remote_exists(PATH_OBJECTS "info/packs");
 	if (remote->has_info_refs) {
 		info_ref_lock = lock_remote("info/refs", LOCK_TIME);
 		if (info_ref_lock)
diff --git a/local-fetch.c b/local-fetch.c
index bf7ec6c..67dc065 100644
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -4,6 +4,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "fetch.h"
+#include "refs.h"
 
 static int use_link;
 static int use_symlink;
@@ -23,7 +24,7 @@ static void setup_index(unsigned char *sha1)
 	struct packed_git *new_pack;
 	char filename[PATH_MAX];
 	strcpy(filename, path);
-	strcat(filename, "/objects/pack/pack-");
+	strcat(filename, "/" PATH_OBJECTS "pack/pack-");
 	strcat(filename, sha1_to_hex(sha1));
 	strcat(filename, ".idx");
 	new_pack = parse_pack_index_file(sha1, filename);
@@ -37,7 +38,7 @@ static int setup_indices(void)
 	struct dirent *de;
 	char filename[PATH_MAX];
 	unsigned char sha1[20];
-	sprintf(filename, "%s/objects/pack/", path);
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/", path);
 	dir = opendir(filename);
 	if (!dir)
 		return -1;
@@ -122,11 +123,11 @@ static int fetch_pack(const unsigned char *sha1)
 		fprintf(stderr, " which contains %s\n",
 			sha1_to_hex(sha1));
 	}
-	sprintf(filename, "%s/objects/pack/pack-%s.pack",
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/pack-%s.pack",
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_name(target->sha1),
 		  sha1_to_hex(target->sha1), 1);
-	sprintf(filename, "%s/objects/pack/pack-%s.idx",
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/pack-%s.idx",
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_index_name(target->sha1),
 		  sha1_to_hex(target->sha1), 1);
@@ -143,7 +144,7 @@ static int fetch_file(const unsigned char *sha1)
 
 	if (object_name_start < 0) {
 		strcpy(filename, path); /* e.g. git.git */
-		strcat(filename, "/objects/");
+		strcat(filename, "/" PATH_OBJECTS);
 		object_name_start = strlen(filename);
 	}
 	filename[object_name_start+0] = hex[0];
@@ -169,7 +170,7 @@ int fetch_ref(char *ref, unsigned char *sha1)
 	int ifd;
 
 	if (ref_name_start < 0) {
-		sprintf(filename, "%s/refs/", path);
+		sprintf(filename, "%s/" PATH_REFS, path);
 		ref_name_start = strlen(filename);
 	}
 	strcpy(filename + ref_name_start, ref);
diff --git a/path.c b/path.c
index 4260952..d330bbc 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "refs.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -99,7 +100,7 @@ int validate_headref(const char *path)
 	/* Make sure it is a "refs/.." symlink */
 	if (S_ISLNK(st.st_mode)) {
 		len = readlink(path, buffer, sizeof(buffer)-1);
-		if (len >= 5 && !memcmp("refs/", buffer, 5))
+		if (len >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buffer, STRLEN_PATH_REFS))
 			return 0;
 		return -1;
 	}
@@ -123,7 +124,7 @@ int validate_headref(const char *path)
 		len -= 4;
 		while (len && isspace(*buf))
 			buf++, len--;
-		if (len >= 5 && !memcmp("refs/", buf, 5))
+		if (len >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buf, STRLEN_PATH_REFS))
 			return 0;
 	}
 
diff --git a/receive-pack.c b/receive-pack.c
index d3c422b..114ea38 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -165,7 +165,7 @@ static const char *update(struct command *cmd)
 	unsigned char *new_sha1 = cmd->new_sha1;
 	struct ref_lock *lock;
 
-	if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
+	if (!prefixcmp(name, PATH_REFS) && check_ref_format(name + STRLEN_PATH_REFS)) {
 		error("refusing to create funny ref '%s' locally", name);
 		return "funny refname";
 	}
@@ -177,7 +177,7 @@ static const char *update(struct command *cmd)
 	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
-	    !prefixcmp(name, "refs/heads/")) {
+	    !prefixcmp(name, PATH_REFS_HEADS)) {
 		struct commit *old_commit, *new_commit;
 		struct commit_list *bases, *ent;
 
diff --git a/reflog-walk.c b/reflog-walk.c
index ee1456b..98cf8ef 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -55,11 +55,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 	}
 	if (reflogs->nr == 0) {
 		int len = strlen(ref);
-		char *refname = xmalloc(len + 12);
-		sprintf(refname, "refs/%s", ref);
+		char *refname = xmalloc(len + STRLEN_PATH_REFS_HEADS + 1);
+		sprintf(refname, PATH_REFS "%s", ref);
 		for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		if (reflogs->nr == 0) {
-			sprintf(refname, "refs/heads/%s", ref);
+			sprintf(refname, PATH_REFS_HEADS "%s", ref);
 			for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		}
 		free(refname);
diff --git a/refs.c b/refs.c
index 7fb3350..840a433 100644
--- a/refs.c
+++ b/refs.c
@@ -409,7 +409,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		/* Follow "normalized" - ie "refs/.." symlinks by hand */
 		if (S_ISLNK(st.st_mode)) {
 			len = readlink(path, buffer, sizeof(buffer)-1);
-			if (len >= 5 && !memcmp("refs/", buffer, 5)) {
+			if (len >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buffer, STRLEN_PATH_REFS)) {
 				buffer[len] = 0;
 				strcpy(ref_buffer, buffer);
 				ref = ref_buffer;
@@ -561,22 +561,22 @@ int head_ref(each_ref_fn fn, void *cb_data)
 
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/", fn, 0, cb_data);
+	return do_for_each_ref(PATH_REFS, fn, 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/tags/", fn, 10, cb_data);
+	return do_for_each_ref(PATH_REFS_TAGS, fn, STRLEN_PATH_REFS_TAGS, cb_data);
 }
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/heads/", fn, 11, cb_data);
+	return do_for_each_ref(PATH_REFS_HEADS, fn, STRLEN_PATH_REFS_HEADS, cb_data);
 }
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/remotes/", fn, 13, cb_data);
+	return do_for_each_ref(PATH_REFS_REMOTES, fn, STRLEN_PATH_REFS_REMOTES, cb_data);
 }
 
 /* NEEDSWORK: This is only used by ssh-upload and it should go; the
@@ -588,7 +588,7 @@ int get_ref_sha1(const char *ref, unsigned char *sha1)
 {
 	if (check_ref_format(ref))
 		return -1;
-	return read_ref(mkpath("refs/%s", ref), sha1);
+	return read_ref(mkpath(PATH_REFS "%s", ref), sha1);
 }
 
 /*
@@ -824,7 +824,7 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
 	char refpath[PATH_MAX];
 	if (check_ref_format(ref))
 		return NULL;
-	strcpy(refpath, mkpath("refs/%s", ref));
+	strcpy(refpath, mkpath(PATH_REFS "%s", ref));
 	return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
 }
 
@@ -1078,8 +1078,8 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	log_file = git_path("logs/%s", ref_name);
 
 	if (log_all_ref_updates &&
-	    (!prefixcmp(ref_name, "refs/heads/") ||
-	     !prefixcmp(ref_name, "refs/remotes/") ||
+	    (!prefixcmp(ref_name,  PATH_REFS_HEADS) ||
+	     !prefixcmp(ref_name, PATH_REFS_REMOTES) ||
 	     !strcmp(ref_name, "HEAD"))) {
 		if (safe_create_leading_directories(log_file) < 0)
 			return error("unable to create directory for %s",
diff --git a/refs.h b/refs.h
index 6eb98a4..1025d04 100644
--- a/refs.h
+++ b/refs.h
@@ -13,6 +13,23 @@ struct ref_lock {
 #define REF_ISSYMREF 01
 #define REF_ISPACKED 02
 
+#define PATH_OBJECTS             "objects/"
+#define STRLEN_PATH_OBJECTS      8
+#define PATH_REFS                "refs/"
+#define STRLEN_PATH_REFS         5
+#define PATH_HEADS               "heads/"
+#define STRLEN_PATH_HEADS        6
+#define PATH_TAGS                "tags/"
+#define STRLEN_PATH_TAGS         5
+#define PATH_REMOTES             "remotes/"
+#define STRLEN_PATH_REMOTES      8
+#define PATH_REFS_HEADS          PATH_REFS PATH_HEADS
+#define STRLEN_PATH_REFS_HEADS   (STRLEN_PATH_REFS+STRLEN_PATH_HEADS)
+#define PATH_REFS_TAGS           PATH_REFS PATH_TAGS
+#define STRLEN_PATH_REFS_TAGS    (STRLEN_PATH_REFS+STRLEN_PATH_TAGS)
+#define PATH_REFS_REMOTES        PATH_REFS PATH_REMOTES
+#define STRLEN_PATH_REFS_REMOTES (STRLEN_PATH_REFS+STRLEN_PATH_REMOTES)
+
 /*
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
diff --git a/remote.c b/remote.c
index bb774d0..b8922c7 100644
--- a/remote.c
+++ b/remote.c
@@ -211,8 +211,8 @@ static void read_config(void)
 	current_branch = NULL;
 	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
 	if (head_ref && (flag & REF_ISSYMREF) &&
-	    !prefixcmp(head_ref, "refs/heads/")) {
-		current_branch = head_ref + strlen("refs/heads/");
+	    !prefixcmp(head_ref, PATH_REFS_HEADS)) {
+		current_branch = head_ref + STRLEN_PATH_REFS_HEADS;
 		current_branch_len = strlen(current_branch);
 	}
 	git_config(handle_config);
@@ -398,9 +398,9 @@ static int count_refspec_match(const char *pattern,
 		 * at the remote site.
 		 */
 		if (namelen != patlen &&
-		    patlen != namelen - 5 &&
-		    prefixcmp(name, "refs/heads/") &&
-		    prefixcmp(name, "refs/tags/")) {
+		    patlen != namelen - STRLEN_PATH_REFS &&
+		    prefixcmp(name, PATH_REFS_HEADS) &&
+		    prefixcmp(name, PATH_REFS_TAGS)) {
 			/* We want to catch the case where only weak
 			 * matches are found and there are multiple
 			 * matches, and where more than one strong
@@ -511,7 +511,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
+		if (!prefixcmp(dst_value, PATH_REFS))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else
 			error("dst refspec %s does not match any "
@@ -594,7 +594,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			if (!pat)
 				continue;
 		}
-		else if (prefixcmp(src->name, "refs/heads/"))
+		else if (prefixcmp(src->name, PATH_REFS_HEADS))
 			/*
 			 * "matching refs"; traditionally we pushed everything
 			 * including refs outside refs/heads/ hierarchy, but
diff --git a/setup.c b/setup.c
index 06004f1..c8912d2 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "dir.h"
+#include "refs.h"
 
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
@@ -158,12 +159,12 @@ static int is_git_directory(const char *suspect)
 			return 0;
 	}
 	else {
-		strcpy(path + len, "/objects");
+		strcpy(path + len, "/" PATH_OBJECTS);
 		if (access(path, X_OK))
 			return 0;
 	}
 
-	strcpy(path + len, "/refs");
+	strcpy(path + len, "/" PATH_REFS);
 	if (access(path, X_OK))
 		return 0;
 
diff --git a/sha1_name.c b/sha1_name.c
index 2d727d5..649e438 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -241,11 +241,11 @@ static int ambiguous_path(const char *path, int len)
 
 static const char *ref_fmt[] = {
 	"%.*s",
-	"refs/%.*s",
-	"refs/tags/%.*s",
-	"refs/heads/%.*s",
-	"refs/remotes/%.*s",
-	"refs/remotes/%.*s/HEAD",
+	PATH_REFS "%.*s",
+	PATH_REFS_TAGS "%.*s",
+	PATH_REFS_HEADS "%.*s",
+	PATH_REFS_REMOTES "%.*s",
+	PATH_REFS_REMOTES "%.*s/HEAD",
 	NULL
 };
 
diff --git a/wt-status.c b/wt-status.c
index 10ce6ee..93dee72 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -7,6 +7,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "diffcore.h"
+#include "refs.h"
 
 int wt_status_use_color = 0;
 static char wt_status_colors[][COLOR_MAXLEN] = {
@@ -311,8 +312,8 @@ void wt_status_print(struct wt_status *s)
 	if (s->branch) {
 		const char *on_what = "On branch ";
 		const char *branch_name = s->branch;
-		if (!prefixcmp(branch_name, "refs/heads/"))
-			branch_name += 11;
+		if (!prefixcmp(branch_name, PATH_REFS_HEADS))
+			branch_name += STRLEN_PATH_REFS_HEADS;
 		else if (!strcmp(branch_name, "HEAD")) {
 			branch_name = "";
 			on_what = "Not currently on any branch.";
-- 
1.5.3.rc5.11.g312e

^ permalink raw reply related

* Re: [PATCH 1/2] Change "refs/" references to symbolic constants
From: Junio C Hamano @ 2007-10-02 17:59 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git, Jeff King
In-Reply-To: <200710020941.05288.andyparkins@gmail.com>

Andy Parkins <andyparkins@gmail.com> writes:

> I noticed a couple of places where memcmp() has been used where prefixcmp() 
> would work fine.  I'm tempted to change them too - what do you think?  
> Perhaps a separate patch?

In general, probably it is preferable to have a separate
"preliminary patch" to normalize the existing code without using
the new infrastructure (i.e. REF_* macros), and then to have the
main patch.  That way would make the main patch more about
mechanical conversion, which would be easier to verify
independently.

^ permalink raw reply

* Re: [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t.
From: Junio C Hamano @ 2007-10-02 17:54 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Johannes Sixt, git
In-Reply-To: <4700B8FC.70704@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
> ...
>> As to error indication, it somehow does not feel right to return
>> something called "child _process_" structure when we want to
>> tell the caller that there is no process to wait for in the
>> no-error case, although the fact that we can use .in/.out fd in
>> the structure when we _do_ have child process is attractive.
>
> Did you mean: "even if we don't have a child process"?
>
> How about a typedef if you dislike the name?
>
>> As an alternative, we could keep the "NULL return means there
>> was no need to fork" semantics of git_connect(), and instead add
>> "int *status_ret" parameter for the caller to check.
>
> Seriously? Add an *out* parameter when we can get rid of one and have
> a return value, too?

Ah, I somehow got confused and thought that the caller decides
not to do the waitpid business at the end of the connection, but
as everybody calls finish_connect() with what it got from
git_connect(), as long as the fake "child_process" structure
records something to let finish_connect() know that it should
not waitpid() on the process, all is well.

It might make sense to teach finish_command() that a magic value
of (cmd->pid == 0) means there is no process to wait for and
this "child_process" structure is only about the in/out stream
to the other side.

^ permalink raw reply

* Re: Problems setting up bare repository (git 1.5.3.3)
From: Johannes Schindelin @ 2007-10-02 17:45 UTC (permalink / raw)
  To: Carl Worth; +Cc: Junio C Hamano, Barry Fishman, git
In-Reply-To: <87641psey8.wl%cworth@cworth.org>

Hi,

On Tue, 2 Oct 2007, Carl Worth wrote:

> On Tue, 2 Oct 2007 10:46:56 +0100 (BST), Johannes Schindelin wrote:
> > On Mon, 1 Oct 2007, Carl Worth wrote:
> > > And why is that?
> >
> > Well, if the OP had used "git push <bla> master" instead of "... 
> > master:master", it would have worked.  I am unaware of any tutorial 
> > that suggests the latter, only of tutorials that suggest the former.
> 
> OK. I was wrong. Somehow I got stuck thinking that "git push <bla> 
> master" wouldn't create a new remote master branch if it didn't 
> previously exist. (It's bizarre that I forgot since I've used that for a 
> long time).
> 
> Sorry about the noise.

Nothing to be sorry about.  It got me thinking.  People propose that "git 
push <nick> master:blub" should create the branch "refs/heads/blub" on the 
remote side.

My initial reaction was "then you have to be precise, because we do not 
know if you want to push it as a branch, or as a lightweight tag".

But then I stepped back a little: What is most likely meant when you say 
"master:blub" and there is no tag/branch of name "blub" on the remote 
side?  Exactly, you want a branch to be created.

_Except_ if you had a typo, such as "git push ko master:po" where you want 
to be warned that that ref is not present on the remote side.

So I am less opposed to making "master:blub" automatically create a branch 
"blub" if it does not exist yet.  But opposed nevertheless.

Ciao,
Dscho

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox