Git development
 help / color / mirror / Atom feed
* Re: [VOTE] git versus mercurial (for DragonflyBSD)
From: Nicolas Pitre @ 2008-10-28  1:28 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Brandon Casey, Arne Babenhauserheide, mercurial, SLONIK.AZ, git
In-Reply-To: <200810272137.07309.jnareb@gmail.com>

On Mon, 27 Oct 2008, Jakub Narebski wrote:

> On the other access from "smart" client (git://, ssh://, future "smart"
> HTTP server) results in creating a pack, so we cannot allow for too
> tight pack compression, or to be more exact too much CPU load taken.

Don't forget that, in those cases, the created pack for streaming is 
copying chunks of data from existing packs for most objects, effectively 
reusing for free the work that has previously been done to tightly 
compress those packs.


Nicolas

^ permalink raw reply

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
From: Junio C Hamano @ 2008-10-28  3:21 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Tuncer Ayaz, git
In-Reply-To: <20081027190816.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:
>
>> After fixing clone -q I noticed that pull -q does not do what
>> it's supposed to do and implemented --quiet/--verbose by
>> adding it to builtin-merge and fixing two places in builtin-fetch.
>
> Junio, may I ask what the status of this patch is? Maybe the patch was lost in the noise? The commit log message is written very differently from existing commits in the history of git, and I am thinking that maybe that is why you did not like the whole patch? Or is it lack of any test script?

Well, perhaps you've been with us long enough to get too picky like
myself, but this was genuinely lost in the noise and my scrambling to get
back to normal.  We do not typically say "I did this, I did that", but the
first paragraph in Tuncer's message is perfectly fine.  I would probably
liked the second paragraph better if it were after --- lines (it is more
about the possible enhancements in other areas, and does not belong to the
commit log message for this change), but it is not a grave enough offence
to get the patch rejected.

The patch itself looks Ok; the lack of test script additions does indeed
bother me somewhat, but it is not too big a deal.

P.S. We are having fun at GitTogether'08 in the first half of this week,
so please expect slower response than usual.

^ permalink raw reply

* Re: [RFC PATCH v2] fetch-pack: log(n)-transmission find_common()
From: Junio C Hamano @ 2008-10-28  3:24 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Thomas Rast, git
In-Reply-To: <20081027192923.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Thomas Rast <trast@student.ethz.ch>:
>
>> Replaces the existing simple history search with a more sophisticated
>> algorithm:
>> 
>> 1) Walk history with exponentially increasing stride lengths; i.e.,
>>    send the 1st commit, then the 2nd after that, then the 4th after
>>    that, and so on.
>> 
>> 2) Bisect the resulting intervals.
>
> Junio, may I ask what the status of this patch is? I see Nicolas responded and said "I gave this a quick try". Wasn't it a good enough review?

I took the "quick try" more about "first feel in performance" and not
"code review concentrating on correctness and trying to catch mistakes".

I like what the patch tries to solve, and the approach it takes to solve
it.  I haven't had a chance to have a block of time for me to concentrate
on this patch to assess where it could go wrong yet.

^ permalink raw reply

* Re: [PATCH] Only update the cygwin-related configuration during state auto-setup
From: Junio C Hamano @ 2008-10-28  3:27 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Alex Riesen, Mark Levedahl, spearce, dpotapov, git
In-Reply-To: <20081027192018.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Junio C Hamano <gitster@pobox.com>:
>
>> This is the answer to the question I asked in:
>> 
>>  http://thread.gmane.org/gmane.comp.version-control.git/97986/focus=98066
>> 
>> Perhaps we should use a separate variable as the original patch did, in:
>> 
>>   http://article.gmane.org/gmane.comp.version-control.git/97987
>> 
>> How about doing it like this instead?
>
> Junio, may I ask what the status of this patch is? I see you did not write tests nor commit message --- are you waiting for others to write them?

Heh, Alex's ack is good enough for me as far as the code itself is
concerned, but I do want these "fixes" accompanied by additional tests to
reproduce to avoid future regressions, and this being a Cygwin fix, I am
not really the right person to write tests nor run them.

^ permalink raw reply

* Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer
From: Junio C Hamano @ 2008-10-28  3:35 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Linus Torvalds
In-Reply-To: <20081027064537.GA4751@blimp.localdomain>

Alex Riesen <raa.lkml@gmail.com> writes:

> Junio C Hamano, Mon, Oct 27, 2008 06:07:24 +0100:
>> Where is git_snpath() used?
>
> Nowhere yet, but it should replace git_path in every call where the
> result is not used immediately. Which, as the story with cygwin
> porting shows, can be sometimes not quite trivial (who could suspect
> lstat(2) will have application local side effects?).
>
> Maybe I should resend the patches without it, following by patches
> introducing git_snpath and replacing calls to git_path.

I took the liberty of doing the first half of just that ;-)

I also think this series of fix is maint-worthy, and splitted the last one
into two so that maint and master can be fixed independently.

^ permalink raw reply

* Re: [PATCH] Add support for uintmax_t type on FreeBSD 4.9
From: Junio C Hamano @ 2008-10-28  4:14 UTC (permalink / raw)
  To: David Syzdek; +Cc: Junio C Hamano, git
In-Reply-To: <9a0027270810270623h4c0c34d0vcd92f61edff6da5@mail.gmail.com>

"David Syzdek" <syzdek@gmail.com> writes:

> The following should match against FreeBSD 4.x:
>
> 	FREEBSD_MAJOR := $(shell sh -c 'echo $(uname_R) |cut -d. -f1')
> 	ifeq ($(FREEBSD_MAJOR),4)
> 		NO_UINTMAX_T = YesPlease
> 		NO_STRTOUMAX = YesPlease
> 	endif
>
> Is the use of FREEBSD_MAJOR okay, or would another name be more appropriate?

Other parts of the Makefile seems to do something like this:

	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
        

^ permalink raw reply

* Re: [PATCH 2/2] Add a 'source' decorator for commits
From: Jeff King @ 2008-10-28  5:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.LFD.2.00.0810271306230.3386@nehalem.linux-foundation.org>

On Mon, Oct 27, 2008 at 01:07:10PM -0700, Linus Torvalds wrote:

> We already support decorating commits by tags or branches that point to
> them, but especially when we are looking at multiple branches together,
> we sometimes want to see _how_ we reached a particular commit.

I think this is a cool feature, but I have a few questions/complaints:

  - Does it make sense to have this _in addition_ to --decorate (since
    for any commit with a --decorate field, it would likely be the same
    as --source)? Should it be a different type of decorate instead,
    like --decorate=source or --decorate=branch?

  - Should this be triggered by the "%d" --pretty=format specifier? This
    two-liner:

diff --git a/pretty.c b/pretty.c
index f6ff312..bdaad19 100644
--- a/pretty.c
+++ b/pretty.c
@@ -487,6 +487,8 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
 	const char *prefix = " (";
 
 	load_ref_decorations();
+	if (commit->util)
+		printf("%s", (char *)commit->util);
 	d = lookup_decoration(&name_decoration, &commit->object);
 	while (d) {
 		strbuf_addstr(sb, prefix);

    works, but:

      - it doesn't check revs->show_source, so is it possible that
        commit->util is being used for something else?

      - using '%d' automatically turns on --decorate, so you end up with
        both the --source and --decorate values. More sensible semantics
        would be "%d turns on --decorate, unless you have done
        --decorate=<explicit format>".

        Alternatively, this should just be "%b" or "%S".

  - If you don't specify --all, you just get "HEAD" for everything.
    Which makes sense when you consider the implementation, but I think
    is probably a bit confusing for users.

> Of course, if the commit is reachable through multiple sources (which is
> common), our particular choice of "first" reachable is entirely random
> and depends on the particular path we happened to follow.

Hmm. It would be nice to keep even a simple counter to get a "distance"
from the ref and choose the one with the smallest distance (I think we
can get away without the complex rules that git-describe uses, since we
are not interested in describing the full commit, but rather finding a
"likely" branch).

However, that would require making multiple passes over the commit
graph, which might impact the startup speed.

-Peff

^ permalink raw reply related

* Re: problems with clone and .gitattributes
From: Jeff King @ 2008-10-28  5:50 UTC (permalink / raw)
  To: Leo Razoumov; +Cc: git
In-Reply-To: <ee2a733e0810271146r5b21213eg989045e4bf42d99a@mail.gmail.com>

On Mon, Oct 27, 2008 at 02:46:41PM -0400, Leo Razoumov wrote:

> There is a little catch-22 problem here. .gitattributes are stored
> in-tree and git clone does not know about these files existence until
> it checks the tree out, by that time it is already too late to apply
> filters.

Yes, this has been brought up on the list before.

> Of course, there could be several obvious workarounds:
> 
> (1) git clone can redo checkout when it finds files affected by gitattributes
> 
> (2) before doing checkout "git clone" inspects tree-object and looks
> for .gitattributes files. If found it checks them out first before all
> other files. Now it can apply the attributes found as the checkout
> process progresses.

I think (2) is closer to the right solution. Though instead of changing
checkout order, I think .gitattributes should simply be able to look in
an auxiliary tree (and checkout would feed the to-be-checked-out tree to
the attribute machinery). One concern, though, is how to handle
conflicts between the tree we're moving _to_ and what's already in the
working tree. I would think that the tree we're moving to would take
precedence.

I feel like Junio may have mentioned some of these issues in a mail the
last time this subject came up, but maybe I'm mis-remembering. Try
searching the archive.

-Peff

^ permalink raw reply

* alternate log --follow idea
From: Jeff King @ 2008-10-28  6:44 UTC (permalink / raw)
  To: git

At the GitTogether this morning, I mentioned an alternate idea for
looking at the history of a file that moved. Currently we have:

 1. git log <path>

    Show the commits, in reverse chronological order, that touched any
    content that at the time of touching resided at <path>.

 2. git log --follow <path>

    Show the commits, in reverse chronological order, for the content
    currently in <path>, where we find movement of content from one path
    to another by checking single commits, and say either "all of this
    content moved from one path to another" or not.

 3. git blame <path>

    Show the current content of <path>, but with each line of content
    showing the commit that introduced it.

In a repository with moving files, '1' is often insufficient or
annoying, since you get to the "root" of the file and discover that it
really came from somewhere else. In other words, you never really cared
about that path at all, but rather about the _content_ in it.

So you try '2', but that has its own drawbacks.  It only works on one
file at a time, and it doesn't work on many cases, including "git log
--follow git-gui/git-gui.sh".

So you move to '3', which sort of works. It shows you the commits that
introduced the current content, but not in the log format. That is, if
you don't care about "which commit introduced this particular line" but
rather "what are the commits that created this content", it is not very
useful. You see the same commits repeated (since they often change many
lines), the commits are shown in file order rather than reverse
chronological order, and you don't get the usual nicely formatted log.

So the new idea is very simple: look at the current content, get the
list of all commits which were involved in creating that content, and
then display them as a flattened history in the usual git-log way.

The script below implements a very naive version:

-- >8 --
#!/bin/sh

dashed() {
	for i in "$@"; do
		case "$i" in
		-*) echo $i ;;
		esac
	done
}

nondashed() {
	for i in "$@"; do
		case "$i" in
		-*) ;;
		*) echo $i ;;
		esac
	done
}

# hope we don't have spaces in our arguments
git ls-files `nondashed "$@"` |
while read file; do
	git blame --porcelain "$file" |
	egrep '^[0-9a-f]{40}' |
	cut -d' ' -f1
done |
sort -u |
xargs git log --no-walk `dashed "$@"`
-- >8 --

You can try:

  sh blame-log.sh git-gui/git-gui.sh

which should Just Work. Because it expands its arguments via ls-files,
you can also track the content of a whole directory:

  sh blame-log.sh -p contrib/examples

which finds all of the commits that touched those files, even when they
were not yet in contrib (in fact, it even traces some of the lines back
to git-tag-script!).

Of course it has its downsides (aside from this horrific
implementation). For one thing, it's _way_ slower than a regular log,
especially on a large chunk of content.  Some of that is in the
implementation, but mostly it is that blame takes a lot of computation.
We might be able to make things better with a specialized blame.

And as somebody (I think Steven) mentioned earlier, it only traces
content which survived to the end. So it won't mention commits which
were later eradicated, which may be useful to see.

And I'm sure there are probably other little problems, as I haven't
thought too deeply about this. But maybe it is worth adding to the
arsenal of exploration tools.

-Peff

^ permalink raw reply

* Re: Terminology question: "tracking" branches
From: Björn Steinbrink @ 2008-10-28  8:01 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Marc Branchaud, Peter Harris, git, Junio C Hamano
In-Reply-To: <20081027162840.GK3612@atjola.homenet>

On 2008.10.27 17:28:40 +0100, Björn Steinbrink wrote:
> On 2008.10.23 10:07:07 +0200, Michael J Gruber wrote:
> > That leaves open:
> > 
> > - What does "remote branch" mean, if it means anything at all? It could
> > be used for a branch in a remote repository, i.e. the other side of
> > fetch/push refspec (remote branch:tracking branch).
> 
> I prefer to say "the branch on the remote" there, but that's just to
> avoid confusion with "remote tracking branch".

So I just happened to use "git remote show origin" and that uses "remote
branch" as you described it:

* remote origin
  URL: git://git.kernel.org/pub/scm/git/git.git
  Remote branch merged with 'git pull' while on branch master
    master
  Tracked remote branches
    html
    maint
    ...

JFYI
Björn

^ permalink raw reply

* Re: Working with remotes; cloning remote references
From: Michael J Gruber @ 2008-10-28  8:12 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Peter Harris, git
In-Reply-To: <49061C6D.2070407@xiplink.com>

Marc Branchaud venit, vidit, dixit 27.10.2008 20:54:
> Michael J Gruber wrote:
>> Downside is that .gitremotes is tracked would show up as a file in the
>> repo, but I can't come up with a better way which is as simple as the
>> above. .gitremotes could be stored in a specially named branch, though.
> 
> That downside is a bit disappointing.  I might as well just make "git 
> remote export" simply generate a script of "git remote add" commands 
> based on the contents of .git/config, and check that script in.  Then I 
> could run the script in a clone to recreate the origin's remotes.

Yes, if you feel okay about running a script repeatedly which may change
due to being versioned; rather than running a script/command which uses
versioned input.

> It also seems awkward to have an export step in the origin repository. 
> I don't really see a need for an export step (except as an artifact of 
> the above implementation).

I had the impression that you want to configure as much as possible on
the "central", and have clones follow automatically. It's very likely
that you want your clones to see only a subset of central's remotes.
Both (individually) imply that there has to be a step on central which
determines which (if any) central remotes appear in clones automatically.

> It seems to me that this would be more natural if our hypothetical "git 
> remote import <X>" could just grab the remotes from repository <X> (or 
> the origin, if <X> is unspecified).  I assume that would involve 
> lower-level changes than what you described, but to me it seems like the 
> more usable approach.  (But then I know nothing of Git's internals, so 
> maybe this kind of change would be too much work?)

Feel free to hack the protocol and remote commands... Implementing
anything which exposes "server's" .git/config on the client without
interaction on the server side will most probably be rejected, though.
My suggestions were meant to be a minimal effort attempt within and
following (see submodules) current infrastructure. I guess now it's up
to you to pick the approach you deem most appropriate in your scenario
and follow through with it.

Michael

^ permalink raw reply

* Re: Problem with Git.pm bidi_pipe methods
From: Christian Jaeger @ 2008-10-28  9:12 UTC (permalink / raw)
  To: Philippe Bruhat (BooK); +Cc: git
In-Reply-To: <20081024001446.GE17717@plop>

Philippe Bruhat (BooK) wrote:
> Hi,
>
> To be able to call commit-tree from a Perl program, I had to use
> command_bidi_pipe() to pass the message on standard input, and get the
> new commit id on standard output. The only problem I have is that the
> method doesn't work (or at least, doesn't work like the other
> command_... methods).

Not really an answer on how to fix Git.pm, but: you may want to subclass 
Git.pm and add your own methods ad interim. See the thread "[PATCH] 
Git.pm: do not break inheritance" [1] about how to subclass (I see that 
this patch is actually in {next,pu} already, and anyway you can use the 
runtime patching approach I described there).

That approach of using subclasses has two benefits: (a) it will tend to 
make your code work with various versions of Git (i.e. one without a 
fixed command_bidi_pipe method), and (b) you will be much more free to 
extend the code. The drawback will be of course that your inventions 
won't necessarily make it back to Git.pm, but I'm tempted to think that 
some innovation taking place before settling on what should make it back 
to Git.pm (or, if incompatible, possibly a new Git.pm) would be worthwhile.

For an actual example in writing Git subclasses see "[ANNOUNCE] intergit 
repository-linking tool (early release)" [2].

Christian.

[1] http://article.gmane.org/gmane.comp.version-control.git/98568
[2] http://article.gmane.org/gmane.comp.version-control.git/99197

^ permalink raw reply

* Re: [PATCH] "git shell" won't work, need "git-shell"
From: Dmitry V. Levin @ 2008-10-28 11:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20080824202325.GA14930@eagain.net>

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

Hi,

Please apply this compatibility fix (commit v1.6.0.1-90-g27a6ed4)
to maint as well.

On Sun, Aug 24, 2008 at 11:23:25PM +0300, Tommi Virtanen wrote:
> >From 8e7935231e8a91d470b3a4a2310803031ef49fc4 Mon Sep 17 00:00:00 2001
> From: Tommi Virtanen <tv@eagain.net>
> Date: Sun, 24 Aug 2008 23:20:33 +0300
> Subject: [PATCH] Install git-shell in bindir, again.
> 
> /etc/passwd shell field must be something execable, you can't enter
> "/usr/bin/git shell" there. git-shell must be present as a separate
> executable, or it is useless.
> 
> Signed-off-by: Tommi Virtanen <tv@eagain.net>
> ---
> 
> Hi. Recent changes moved away from "git-foo" to "git foo", except for
> some commands that needed backwards compatibility. However, git-shell
> as a separate binary was removed. I hope you will reinstante git-shell
> as a publicly visible binary in bin. Here's why:
> 
> The shell field in /etc/passwd is *exec*ed, not interpreted via sh -c
> or some such. For example, source of Debian's shadow, containing
> /bin/login:
> 
> libmisc/shell.c:80:	execle (file, arg, (char *) 0, envp);
> 
> I also tested this for real, and having a
> 
> test:x:1001:1001:,,,:/home/test:/usr/bin/git-shell
> 
> line works, and
> 
> test:x:1001:1001:,,,:/home/test:/usr/bin/git shell
> 
> just makes ssh loop asking for a password, logging
> 
> "User test not allowed because shell /usr/bin/git shell does not exist"
> 
> So, as far as I understand, as it currently is, "git shell" is utterly
> useless for what it was meant to do. Restoring "git-shell" will fix
> it.
> 
>  Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 53ab4b5..24d5809 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1351,7 +1351,7 @@ install: all
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> -	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
> +	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
>  	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
>  ifndef NO_TCLTK
> -- 
> 1.6.0.2.g2ebc0.dirty


-- 
ldv

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

^ permalink raw reply

* Re: [PATCH 6/6] t9400, t9401: use "git cvsserver" without dash
From: Dmitry V. Levin @ 2008-10-28 11:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wtzvd8h.fsf@gitster.siamese.dyndns.org>

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

Hi,
 
On Wed, Sep 10, 2008 at 03:37:50PM -0700, Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
> > Subject: [PATCH] Install git-cvsserver in $(bindir)
> >
> > It is one of the server side programs and needs to be found on usual $PATH.
> >
> > Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
> > ...
> > -	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X '$(DESTDIR_SQ)$(bindir_SQ)'
> > +	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver$X '$(DESTDIR_SQ)$(bindir_SQ)'
> 
> Thanks.
> 
> Will queue but without $X at the end, as I do not think we want it even on
> Windows because cvsserver is a script.

Please apply this compatibility fix (commit v1.6.0.1-308-gede4caf)
to maint as well.


-- 
ldv

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

^ permalink raw reply

* Re: [PATCH] Only update the cygwin-related configuration during state auto-setup
From: Alex Riesen @ 2008-10-28 12:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Mark Levedahl, spearce, dpotapov, git
In-Reply-To: <7vhc6xh010.fsf@gitster.siamese.dyndns.org>

2008/10/28 Junio C Hamano <gitster@pobox.com>:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>> Quoting Junio C Hamano <gitster@pobox.com>:
>>
>>> This is the answer to the question I asked in:
>>>
>>>  http://thread.gmane.org/gmane.comp.version-control.git/97986/focus=98066
>>>
>>> Perhaps we should use a separate variable as the original patch did, in:
>>>
>>>   http://article.gmane.org/gmane.comp.version-control.git/97987
>>>
>>> How about doing it like this instead?
>>
>> Junio, may I ask what the status of this patch is? I see you did not write tests nor commit message --- are you waiting for others to write them?
>
> Heh, Alex's ack is good enough for me as far as the code itself is
> concerned, but I do want these "fixes" accompanied by additional tests to
> reproduce to avoid future regressions, and this being a Cygwin fix, I am
> not really the right person to write tests nor run them.

I suggest NOT writing the test for the workaround for just one platform I
personally call the most idiotic of the Microsoft's fallouts. As the
fix is located
in the code specific to that platform, it wont do any harm for anyone, whether
it works or not (even if the code in tree does not work). The Junio's fix is
definitely enough for me and, I'm very sure, for anyone still having to
deal with cygwin.

So for the Google's record: the patch to fix the --encoding option of git
format-patch broken on cygwin is in the Junio's mail, to be found, i.e.:
http://marc.info/?l=git&m=122482769817566&w=4

Nanako, as there is not many of such poor bastards left, it is not
even a problem
to keep the fix just in Git mailing list archives. There is just not
enough of a looser
base to press its development in whatever direction.

^ permalink raw reply

* Re: [VOTE]  git versus mercurial
From: walt @ 2008-10-28 12:31 UTC (permalink / raw)
  To: git
In-Reply-To: <ge0rla$mce$1@ger.gmane.org>

walt wrote:
> No, no, I'm not the one calling for a vote.  You old-timers here
> will know the name Matt Dillon, who is leading the dragonflybsd
> project (www.dragonflybsd.org).
>
> Matt is the one who is calling for the vote in his thread "Vote
> for your source control system" in the dragonfly.kernel group,
> accessible via nntp://nntp.dragonflybsd.org...

The official vote was 19 to 19, plus one for perforce and one
for svn.  Matt has proposed a primary git repository and a mirror
in hg, and that's being debated now.

I've already learned a lot from following this topic in both
lists and it seems this is a topic of great interest to many,
so I'll continue reading in both places.

Thanks!

^ permalink raw reply

* Per-hunk whitespace cleaning in pre-commit?
From: Jonathan del Strother @ 2008-10-28 12:45 UTC (permalink / raw)
  To: Git Mailing List

I like the idea of whitespace fixing in pre-commit, but I'd prefer if
it only cleaned whitespace on the hunks that have changed, rather than
on the entire file.
Has anyone come up with a pre-commit version that does this?

^ permalink raw reply

* Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer
From: Alex Riesen @ 2008-10-28 12:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7vd4hlgzo7.fsf@gitster.siamese.dyndns.org>

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

2008/10/28 Junio C Hamano <gitster@pobox.com>:
> Alex Riesen <raa.lkml@gmail.com> writes:
>> Maybe I should resend the patches without it, following by patches
>> introducing git_snpath and replacing calls to git_path.
>
> I took the liberty of doing the first half of just that ;-)
>

Thanks. And am sorry... I did that too, and stupidly forgot to send.
I also considered replacing xstrdup(mkpath) with a function which does
just that (patches 8-9). Patches 1 and 2 are unrelated, will send them
separately.

FWIW now, I'm sending the patches.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Add-mksnpath-which-allows-to-specify-the-output-buff.patch --]
[-- Type: text/x-diff; name=0003-Add-mksnpath-which-allows-to-specify-the-output-buff.patch, Size: 1653 bytes --]

From dcea4c409f4deadbc176b43e8cdbfd5cd9edac3f Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 10:18:06 +0100
Subject: [PATCH] Add mksnpath which allows to specify the output buffer

To be used as alternative to mkpath where the buffer for the created
path may not be be reused by subsequent calls of the function or will
be copied anyway.

It is actually just a vsnprintf's but additionally calls cleanup_path
on the result.
---
 cache.h |    2 ++
 path.c  |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index b0edbf9..9efa602 100644
--- a/cache.h
+++ b/cache.h
@@ -495,6 +495,8 @@ extern int check_repository_format(void);
 #define DATA_CHANGED    0x0020
 #define TYPE_CHANGED    0x0040
 
+extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
+	__attribute__((format (printf, 3, 4)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 76e8872..8b64878 100644
--- a/path.c
+++ b/path.c
@@ -32,6 +32,21 @@ static char *cleanup_path(char *path)
 	return path;
 }
 
+char *mksnpath(char *buf, size_t n, const char *fmt, ...)
+{
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(buf, n, fmt, args);
+	va_end(args);
+	if (len >= n) {
+		snprintf(buf, n, bad_path);
+		return buf;
+	}
+	return cleanup_path(buf);
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0004-Fix-mkpath-abuse-in-dwim_ref-sha1_name.c.patch --]
[-- Type: text/x-diff; name=0004-Fix-mkpath-abuse-in-dwim_ref-sha1_name.c.patch, Size: 1438 bytes --]

From 14fcf0fe5cd3baec1706a81876698a27adcda1fa Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Tue, 14 Oct 2008 18:23:49 +0200
Subject: [PATCH] Fix mkpath abuse in dwim_ref/sha1_name.c

Otherwise the function sometimes fail to resolve obviously correct refnames,
because the string data pointed to by "ref" argument were reused.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 sha1_name.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 41b6809..159c2ab 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -245,11 +245,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 
 	*ref = NULL;
 	for (p = ref_rev_parse_rules; *p; p++) {
+		char fullref[PATH_MAX];
 		unsigned char sha1_from_ref[20];
 		unsigned char *this_result;
 
 		this_result = refs_found ? sha1_from_ref : sha1;
-		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
+		mksnpath(fullref, sizeof(fullref), *p, len, str);
+		r = resolve_ref(fullref, this_result, 1, NULL);
 		if (r) {
 			if (!refs_found++)
 				*ref = xstrdup(r);
@@ -272,7 +274,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
-		strcpy(path, mkpath(*p, len, str));
+		mksnpath(path, sizeof(path), *p, len, str);
 		ref = resolve_ref(path, hash, 1, NULL);
 		if (!ref)
 			continue;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0005-Fix-potentially-dangerous-use-of-mkpath.patch --]
[-- Type: text/x-diff; name=0005-Fix-potentially-dangerous-use-of-mkpath.patch, Size: 2109 bytes --]

From 9f751c2a681aed2089ba30f64a0478ea3e68a81c Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Sun, 19 Oct 2008 20:17:17 +0200
Subject: [PATCH] Fix potentially dangerous use of mkpath

In the changed code a pointer to the buffer returned by mkpath is used
after a function is called which also uses mkpath or git_path.  As
both these functions use the same ring of buffers, the data pointed by
the pointer stored in the first function can be overwritten when the
function returns, not to mention the possibility that other code using
the same buffer ring can come in in the future.

Replace mkpath with mksnpath and a local buffer for the resulting
string.
---
 builtin-apply.c        |    4 ++--
 builtin-for-each-ref.c |    6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index cfd8fce..4c4d1e1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2841,8 +2841,8 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned
 		unsigned int nr = getpid();
 
 		for (;;) {
-			const char *newpath;
-			newpath = mkpath("%s~%u", path, nr);
+			char newpath[PATH_MAX];
+			mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
 			if (!try_create_file(newpath, mode, buf, size)) {
 				if (!rename(newpath, path))
 					return;
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index fa6c1ed..e46b7ad 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -620,14 +620,16 @@ static char *get_short_ref(struct refinfo *ref)
 		for (j = 0; j < i; j++) {
 			const char *rule = ref_rev_parse_rules[j];
 			unsigned char short_objectname[20];
+			char refname[PATH_MAX];
 
 			/*
 			 * the short name is ambiguous, if it resolves
 			 * (with this previous rule) to a valid ref
 			 * read_ref() returns 0 on success
 			 */
-			if (!read_ref(mkpath(rule, short_name_len, short_name),
-				      short_objectname))
+			mksnpath(refname, sizeof(refname),
+				 rule, short_name_len, short_name);
+			if (!read_ref(refname, short_objectname))
 				break;
 		}
 
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0006-Add-git_snpath-a-.git-path-formatting-routine-with.patch --]
[-- Type: text/x-diff; name=0006-Add-git_snpath-a-.git-path-formatting-routine-with.patch, Size: 1881 bytes --]

From cf5fe91d172aca73b79c05aef261fb7040749127 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 10:22:21 +0100
Subject: [PATCH] Add git_snpath: a .git path formatting routine with output buffer

The function's purpose is to replace git_path where the buffer of
formatted path may not be reused by subsequent calls of the function
or will be copied anyway.
---
 cache.h |    2 ++
 path.c  |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 9efa602..a9024db 100644
--- a/cache.h
+++ b/cache.h
@@ -497,6 +497,8 @@ extern int check_repository_format(void);
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
+extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+	__attribute__((format (printf, 3, 4)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 8b64878..85ab28a 100644
--- a/path.c
+++ b/path.c
@@ -47,6 +47,29 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	return cleanup_path(buf);
 }
 
+char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+{
+	const char *git_dir = get_git_dir();
+	va_list args;
+	size_t len;
+
+	len = strlen(git_dir);
+	if (n < len + 1)
+		goto bad;
+	memcpy(buf, git_dir, len);
+	if (len && !is_dir_sep(git_dir[len-1]))
+		buf[len++] = '/';
+	va_start(args, fmt);
+	len += vsnprintf(buf + len, n - len, fmt, args);
+	va_end(args);
+	if (len >= n)
+		goto bad;
+	return cleanup_path(buf);
+bad:
+	snprintf(buf, n, bad_path);
+	return buf;
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0007-Fix-potentially-dangerous-use-of-git_path-in-ref.c.patch --]
[-- Type: text/x-diff; name=0007-Fix-potentially-dangerous-use-of-git_path-in-ref.c.patch, Size: 1506 bytes --]

From 109d1b74e2b093bd63419fb1d0f720fa54a476f3 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:11:40 +0100
Subject: [PATCH] Fix potentially dangerous use of git_path in ref.c

---
 refs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 0a126fa..68189ba 100644
--- a/refs.c
+++ b/refs.c
@@ -413,7 +413,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		*flag = 0;
 
 	for (;;) {
-		const char *path = git_path("%s", ref);
+		char path[PATH_MAX];
 		struct stat st;
 		char *buf;
 		int fd;
@@ -421,6 +421,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		if (--depth < 0)
 			return NULL;
 
+		git_snpath(path, sizeof(path), "%s", ref);
 		/* Special case: non-existing file. */
 		if (lstat(path, &st) < 0) {
 			struct ref_list *list = get_packed_refs();
@@ -1130,13 +1131,14 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	int logfd, written, oflags = O_APPEND | O_WRONLY;
 	unsigned maxlen, len;
 	int msglen;
-	char *log_file, *logrec;
+	char log_file[PATH_MAX];
+	char *logrec;
 	const char *committer;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	log_file = git_path("logs/%s", ref_name);
+	git_snpath(log_file, sizeof(log_file), "logs/%s", ref_name);
 
 	if (log_all_ref_updates &&
 	    (!prefixcmp(ref_name, "refs/heads/") ||
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0008-git_pathdup-returns-xstrdup-ed-copy-of-the-formatte.patch --]
[-- Type: text/x-diff; name=0008-git_pathdup-returns-xstrdup-ed-copy-of-the-formatte.patch, Size: 2252 bytes --]

From bb22051c0925039b5161bc0f49e3098f9965a069 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:17:51 +0100
Subject: [PATCH] git_pathdup: returns xstrdup-ed copy of the formatted path

---
 cache.h |    2 ++
 path.c  |   24 ++++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index a9024db..981b4e6 100644
--- a/cache.h
+++ b/cache.h
@@ -499,6 +499,8 @@ extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
+extern char *git_pathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 85ab28a..092ce57 100644
--- a/path.c
+++ b/path.c
@@ -47,10 +47,9 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	return cleanup_path(buf);
 }
 
-char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list args)
 {
 	const char *git_dir = get_git_dir();
-	va_list args;
 	size_t len;
 
 	len = strlen(git_dir);
@@ -59,9 +58,7 @@ char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	memcpy(buf, git_dir, len);
 	if (len && !is_dir_sep(git_dir[len-1]))
 		buf[len++] = '/';
-	va_start(args, fmt);
 	len += vsnprintf(buf + len, n - len, fmt, args);
-	va_end(args);
 	if (len >= n)
 		goto bad;
 	return cleanup_path(buf);
@@ -70,6 +67,25 @@ bad:
 	return buf;
 }
 
+char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+{
+	va_list args;
+	va_start(args, fmt);
+	(void)git_vsnpath(buf, n, fmt, args);
+	va_end(args);
+	return buf;
+}
+
+char *git_pathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	va_start(args, fmt);
+	(void)git_vsnpath(path, sizeof(path), fmt, args);
+	va_end(args);
+	return xstrdup(path);
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0009-Use-git_pathdup-instead-of-xstrdup-git_path.patch --]
[-- Type: text/x-diff; name=0009-Use-git_pathdup-instead-of-xstrdup-git_path.patch, Size: 5483 bytes --]

From 3e142004e382ea0e05d030a3be6a8fc0e896f5d1 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:22:09 +0100
Subject: [PATCH] Use git_pathdup instead of xstrdup(git_path(...))

---
 builtin-config.c |    2 +-
 builtin-reflog.c |    4 ++--
 builtin-revert.c |    2 +-
 builtin-tag.c    |    2 +-
 config.c         |    6 +++---
 environment.c    |    2 +-
 refs.c           |    2 +-
 rerere.c         |    2 +-
 server-info.c    |    2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index 91fdc49..f710162 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -84,7 +84,7 @@ static int get_value(const char* key_, const char* regex_)
 	local = config_exclusive_filename;
 	if (!local) {
 		const char *home = getenv("HOME");
-		local = repo_config = xstrdup(git_path("config"));
+		local = repo_config = git_pathdup("config");
 		if (git_config_global() && home)
 			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
diff --git a/builtin-reflog.c b/builtin-reflog.c
index 6b3667e..d95f515 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -277,11 +277,11 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 	lock = lock_any_ref_for_update(ref, sha1, 0);
 	if (!lock)
 		return error("cannot lock ref '%s'", ref);
-	log_file = xstrdup(git_path("logs/%s", ref));
+	log_file = git_pathdup("logs/%s", ref);
 	if (!file_exists(log_file))
 		goto finish;
 	if (!cmd->dry_run) {
-		newlog_path = xstrdup(git_path("logs/%s.lock", ref));
+		newlog_path = git_pathdup("logs/%s.lock", ref);
 		cb.newlog = fopen(newlog_path, "w");
 	}
 
diff --git a/builtin-revert.c b/builtin-revert.c
index 7483a7a..4038b41 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -251,7 +251,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	int i, index_fd, clean;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
-	char *defmsg = xstrdup(git_path("MERGE_MSG"));
+	char *defmsg = git_pathdup("MERGE_MSG");
 	struct merge_options o;
 	struct tree *result, *next_tree, *base_tree, *head_tree;
 	static struct lock_file index_lock;
diff --git a/builtin-tag.c b/builtin-tag.c
index b13fa34..efd7723 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -283,7 +283,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 		int fd;
 
 		/* write the template message before editing: */
-		path = xstrdup(git_path("TAG_EDITMSG"));
+		path = git_pathdup("TAG_EDITMSG");
 		fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
 		if (fd < 0)
 			die("could not create file '%s': %s",
diff --git a/config.c b/config.c
index b8d289d..67cc1dc 100644
--- a/config.c
+++ b/config.c
@@ -649,7 +649,7 @@ int git_config(config_fn_t fn, void *data)
 		free(user_config);
 	}
 
-	repo_config = xstrdup(git_path("config"));
+	repo_config = git_pathdup("config");
 	ret += git_config_from_file(fn, repo_config, data);
 	free(repo_config);
 	return ret;
@@ -889,7 +889,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	if (config_exclusive_filename)
 		config_filename = xstrdup(config_exclusive_filename);
 	else
-		config_filename = xstrdup(git_path("config"));
+		config_filename = git_pathdup("config");
 
 	/*
 	 * Since "key" actually contains the section name and the real
@@ -1149,7 +1149,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 	if (config_exclusive_filename)
 		config_filename = xstrdup(config_exclusive_filename);
 	else
-		config_filename = xstrdup(git_path("config"));
+		config_filename = git_pathdup("config");
 	out_fd = hold_lock_file_for_update(lock, config_filename, 0);
 	if (out_fd < 0) {
 		ret = error("could not lock config file %s", config_filename);
diff --git a/environment.c b/environment.c
index 0693cd9..bf93a59 100644
--- a/environment.c
+++ b/environment.c
@@ -71,7 +71,7 @@ static void setup_git_env(void)
 	}
 	git_graft_file = getenv(GRAFT_ENVIRONMENT);
 	if (!git_graft_file)
-		git_graft_file = xstrdup(git_path("info/grafts"));
+		git_graft_file = git_pathdup("info/grafts");
 }
 
 int is_bare_repository(void)
diff --git a/refs.c b/refs.c
index 68189ba..01e4df4 100644
--- a/refs.c
+++ b/refs.c
@@ -1267,7 +1267,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	const char *lockpath;
 	char ref[1000];
 	int fd, len, written;
-	char *git_HEAD = xstrdup(git_path("%s", ref_target));
+	char *git_HEAD = git_pathdup("%s", ref_target);
 	unsigned char old_sha1[20], new_sha1[20];
 
 	if (logmsg && read_ref(ref_target, old_sha1))
diff --git a/rerere.c b/rerere.c
index 8e5532b..02931a1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -351,7 +351,7 @@ int setup_rerere(struct string_list *merge_rr)
 	if (!is_rerere_enabled())
 		return -1;
 
-	merge_rr_path = xstrdup(git_path("MERGE_RR"));
+	merge_rr_path = git_pathdup("MERGE_RR");
 	fd = hold_lock_file_for_update(&write_lock, merge_rr_path,
 				       LOCK_DIE_ON_ERROR);
 	read_rr(merge_rr);
diff --git a/server-info.c b/server-info.c
index c1c073b..66b0d9d 100644
--- a/server-info.c
+++ b/server-info.c
@@ -25,7 +25,7 @@ static int add_info_ref(const char *path, const unsigned char *sha1, int flag, v
 
 static int update_info_refs(int force)
 {
-	char *path0 = xstrdup(git_path("info/refs"));
+	char *path0 = git_pathdup("info/refs");
 	int len = strlen(path0);
 	char *path1 = xmalloc(len + 2);
 
-- 
1.6.0.3.549.gb475d


^ permalink raw reply related

* Re: [PATCH 2/2] Add a 'source' decorator for commits
From: Pierre Habouzit @ 2008-10-28 13:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Linus Torvalds, Git Mailing List, Junio C Hamano
In-Reply-To: <20081028054539.GA23195@sigill.intra.peff.net>

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

On Tue, Oct 28, 2008 at 05:45:40AM +0000, Jeff King wrote:
> On Mon, Oct 27, 2008 at 01:07:10PM -0700, Linus Torvalds wrote:
> > Of course, if the commit is reachable through multiple sources (which is
> > common), our particular choice of "first" reachable is entirely random
> > and depends on the particular path we happened to follow.
> 
> Hmm. It would be nice to keep even a simple counter to get a "distance"
> from the ref and choose the one with the smallest distance (I think we
> can get away without the complex rules that git-describe uses, since we
> are not interested in describing the full commit, but rather finding a
> "likely" branch).
> 
> However, that would require making multiple passes over the commit
> graph, which might impact the startup speed.

Actually I tried to do that (and you meant name-rev --contains rather
than describe actually ;p), and I stopped because it's too slow. I
believe the proper way to do that is to help git-log knowing which are
the short (topic) branches, and to crawl incrementally using a
date-based hack. This would basically work a bit like this. Let's
imaging you want to crawl "next" in git and know which topics from pu
are in it. You would say e.g.:

git-log --topics=*/* next (as pretty much every */* is a topic branch
for git.git).


Then one has to know which are the heads of every topic branches first,
then crawl next the usual way, except that when you arrive to a point
that is a topic branch head, you don't look that way. You remember the
date of that point, and continue to crawl "next" a bit further so that
you can start annotating the topic's commits you've stumbled upon. And
you do that, you look at jd/topic (as in John Doe topic branch) and mark
all the commits as being from jd/topic, until you either go back to some
commit from next, or your current commit date is younger than your
current "next" crawling front. In the former case, you're done with that
topic, in the latter you must continue to preprocess "next" a bit more.

That should allow incremental output, FSVO incremental (in the git.git
kind of integration, you have buckets of topic branches merges, and
basically we would have output being spit bucket per bucket I believe).


If you do that, you don't really need to keep distance scores, this
should, I believe, yield decent enough results, while being incremental,
which should yield almost instantaneous output.

Point is, I've no clue how to do that with our crawling primitives right
now, to be fair, I didn't looked at it because I'm lazy and don't have
enough topic branches to work with in my projects, so it's not an itch
yet ;)

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

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

^ permalink raw reply

* Re: [PATCH 2/2] Add a 'source' decorator for commits
From: Pierre Habouzit @ 2008-10-28 13:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Linus Torvalds, Git Mailing List, Junio C Hamano
In-Reply-To: <20081028131116.GA8272@artemis.googlewifi.com>

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

On Tue, Oct 28, 2008 at 01:11:16PM +0000, Pierre Habouzit wrote:
> On Tue, Oct 28, 2008 at 05:45:40AM +0000, Jeff King wrote:
> > On Mon, Oct 27, 2008 at 01:07:10PM -0700, Linus Torvalds wrote:
> > > Of course, if the commit is reachable through multiple sources (which is
> > > common), our particular choice of "first" reachable is entirely random
> > > and depends on the particular path we happened to follow.
> > 
> > Hmm. It would be nice to keep even a simple counter to get a "distance"
> > from the ref and choose the one with the smallest distance (I think we
> > can get away without the complex rules that git-describe uses, since we
> > are not interested in describing the full commit, but rather finding a
> > "likely" branch).
> > 
> > However, that would require making multiple passes over the commit
> > graph, which might impact the startup speed.
> 
> Actually I tried to do that (and you meant name-rev --contains rather
> than describe actually ;p), and I stopped because it's too slow. I
> believe the proper way to do that is to help git-log knowing which are
> the short (topic) branches, and to crawl incrementally using a
> date-based hack. This would basically work a bit like this. Let's
> imaging you want to crawl "next" in git and know which topics from pu
> are in it. You would say e.g.:
> 
> git-log --topics=*/* next (as pretty much every */* is a topic branch
> for git.git).
> 
> 
> Then one has to know which are the heads of every topic branches first,
> then crawl next the usual way, except that when you arrive to a point
> that is a topic branch head, you don't look that way. You remember the
> date of that point, and continue to crawl "next" a bit further so that
> you can start annotating the topic's commits you've stumbled upon. And
> you do that, you look at jd/topic (as in John Doe topic branch) and mark
> all the commits as being from jd/topic, until you either go back to some
> commit from next, or your current commit date is younger than your
> current "next" crawling front. In the former case, you're done with that
> topic, in the latter you must continue to preprocess "next" a bit more.

My description is clumsy, as we probably sometimes want to crawl topics
that aren't merged either like in git-log --topics=*/* --all.  But we
just have to mark the current "front" of the walk, and have it sorted
between things that are part of topic heads and the thing that are not.
We don't even need to decorate the things that are _not_ part of the
topics at all I'd say, because that's where you definitely need distance
based algorithms and multiple passes to yield correct results.

Whereas for the specific case of topic branches, which is what people
want (see Ingo's mails on the subject a week or so ago), the algorithm I
propose is, I guess, mostly O(n + m) in the number of walked commits (n)
and references (m). IOW fast.


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

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

^ permalink raw reply

* Re: new plan for cleaning up the worktree mess, was Re: [PATCH] rehabilitate 'git index-pack' inside the object store
From: Nguyen Thai Ngoc Duy @ 2008-10-28 13:52 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0810211955250.22125@pacific.mpi-cbg.de.mpi-cbg.de>

On 10/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>  On Tue, 21 Oct 2008, Jeff King wrote:
>
>  > On Tue, Oct 21, 2008 at 07:02:48PM +0200, Johannes Schindelin wrote:
>  >
>  > > So I propose this change in semantics:
>  > >
>  > > - setup_git_directory_gently(): rename to discover_git_directory(),
>  > >   and avoid any chdir() at all.
>  > > - setup_git_directory(): keep the semantics that it chdir()s to the
>  > >   worktree, or to the git directory for bare repositories.
>  > >
>  > > Using _gently() even for RUN_SETUP builtins should solve the long
>  > > standing pager problem, too.
>  >
>  > I'm not sure there aren't hidden problems lurking in that strategy
>  > (every time I look at this area of code, something unexpected prevents
>  > what I think should Just Work from Just Working), but I think that is a
>  > promising direction to go for clearing up some of the long-standing
>  > issues.
>
>  Same here.  I grew a pretty strong opinion about the whole worktree thing,
>  but maybe that is only because it was done trying to change as little as
>  possible.

I played a bit with code, extracted discover_git_directory() from
setup_git_directory_gently() then made the latter a wrapper of the
former with chdir(). Some more for thoughts from the experiment.

1. Because discover_git_directory() does not do chdir() until later in
setup_git_directory_gently(), setting GIT_DIR to a relative path seems
unsafe (or worse unset at all, in case .git is found in parent
directories). But making GIT_DIR absolute path breaks tests because
some of them expect "git rev-parse --git-dir" to return a relative
path. The approach used in 044bbbc (Make git_dir a path relative to
work_tree in setup_work_tree()) can be reused to performance loss, but
that won't solve the issue.

2. 044bbbc also brings up another issue: code duplication between
setup_git_directory_gently() and setup_work_tree(). The new
setup*gently() can be roughly like this if setup_work_tree() can
calculate prefix too:

const char *setup_git_directory_gently(int *nongit_ok)
{
        int nonworktree_ok;
        /*
         * Let's assume that we are in a git repository.
         * If it turns out later that we are somewhere else, the value will be
         * updated accordingly.
         */
        if (nongit_ok)
                *nongit_ok = 0;

        if (!discover_git_directory()) {
                if (nongit_ok) {
                        *nongit_ok = 1;
                        return NULL;
                }
                die("Not a git repository");
        }

        return setup_work_tree_gently(&nonworktree_ok); // gentle version
}

So I propose to make setup_work_tree() return a prefix, relative to
current cwd. The setup procedure then would become:

if (discover_git_directory())
    die("Git repository needed");
prefix = setup_work_tree(); // die() inside if cannot setup worktree

3. Dealing with cwd outside worktree. If cwd is inside a worktree,
prefix will be calculated correctly. If it is outside, the current
behavior (with both GIT_DIR and GIT_WORK_TREE set) is leave prefix as
NULL. I think that is not right. With a wrong prefix, git commands
will not be able to access on-disk files. I would propose either:
  - die() if cwd is outside worktree
  - setup*gently() discovers the situation and gives up, then lets git
commands handle themselves. Some commands, like git-archive, don't
care about on-disk files at all, they could just simply ignore the
prefix and keep going. Others may die() or handle it properly.

Again, this breaks things.
-- 
Duy

^ permalink raw reply

* Re: [VOTE]  git versus mercurial
From: Johannes Schindelin @ 2008-10-28 14:28 UTC (permalink / raw)
  To: walt; +Cc: git
In-Reply-To: <ge70nl$l6t$1@ger.gmane.org>

Hi,

On Tue, 28 Oct 2008, walt wrote:

> walt wrote:
> > No, no, I'm not the one calling for a vote.  You old-timers here will 
> > know the name Matt Dillon, who is leading the dragonflybsd project 
> > (www.dragonflybsd.org).
> >
> > Matt is the one who is calling for the vote in his thread "Vote for 
> > your source control system" in the dragonfly.kernel group, accessible 
> > via nntp://nntp.dragonflybsd.org...
> 
> The official vote was 19 to 19, plus one for perforce and one for svn.  
> Matt has proposed a primary git repository and a mirror in hg, and 
> that's being debated now.

While many may say that that is a half-baked solution, I actually like it.  
Mercurial and Git are pretty similar in their concept (if not in how the 
data is actually stored).

Note that with git fast-export and hg fast-import, it should be relatively 
simple to convert from one data format to the other, even incrementally.

And for the other direction, you could use hg fast-export from the 
fast-export.git repository (I am working on a better one at the moment, 
too, so that incremental fast-export would be possible, too).

Ciao,
Dscho

^ permalink raw reply

* Git/Mercurial interoperability (and what about bzr?) (was: Re: [VOTE] git versus mercurial)
From: Peter Krefting @ 2008-10-28 14:41 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0810281445190.22125@pacific.mpi-cbg.de.mpi-cbg.de>

Johannes Schindelin:

> While many may say that that is a half-baked solution, I actually
> like it. Mercurial and Git are pretty similar in their concept (if
> not in how the data is actually stored).

That touches on something that I have been thinking about for a while.

How difficult are the storage formats? Would it be possible, in a
reasonable amount of work, to add support for the Mercurial protocol
and format in "git clone", so that I could clone a Mercurial repository
and work on it with Git, and then possibly use "git push" to possibly
push the result back to Mercurial?

It seems to me that use of DVCS is polarising between Git, Mercurial
and Bzr. It would be nice to have easy interoperability between the
systems, at least as far as can be covered by the lowest common
denominator of what they support. I would love to be able to use Git to
clone a Bzr repository that I need to be able to access, since bzr is
just different enough from Git to be annoying. Same goes for Mercurial.
And I am sure that users of the other tools feel the same.

Would it be possible to design a common transfer format that could be
implemented by all three (and that would be a little smarter than
fast-export/fast-import)?

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: Git/Mercurial interoperability (and what about bzr?) (was: Re: [VOTE]  git versus mercurial)
From: Johannes Schindelin @ 2008-10-28 14:59 UTC (permalink / raw)
  To: Peter Krefting; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0810281536360.27029@ds9.cixit.se>

Hi,

On Tue, 28 Oct 2008, Peter Krefting wrote:

> How difficult are the storage formats? Would it be possible, in a
> reasonable amount of work, to add support for the Mercurial protocol
> and format in "git clone", so that I could clone a Mercurial repository
> and work on it with Git, and then possibly use "git push" to possibly
> push the result back to Mercurial?

There was talk about imitating Mercurial's wire protocol in order to have 
an efficient HTTP server.  Shawn is working on that front;

We discussed it briefly, and there might be some cute ways to copy it: 
since we are not append-only, we have to download the pack index first 
(which is not downloaded ATM, as we generate it from the downloaded pack 
while verifying it).  With that index, we can determine which parts we 
need in order to regenerate the pack; it would still be pretty stupid when 
there are a lot of branches and we are really only interested in one of 
them.

But I doubt that it will be possible to use the wire protocol to pull/push 
between different DVCSes.  I _strongly_ doubt that the SHA-1s in the 
Mercurial repositories could _ever_ be reused in Git mirrors of them, as 
our data format (on which the hash depends) is different.

> It would be nice to have easy interoperability between the systems, at 
> least as far as can be covered by the lowest common denominator of what 
> they support. I would love to be able to use Git to clone a Bzr 
> repository that I need to be able to access, since bzr is just different 
> enough from Git to be annoying. Same goes for Mercurial. And I am sure 
> that users of the other tools feel the same.

Wasn't bzr touting it as one of their major features that they could have 
foreign-scm remotes?  If I remembered that correctly, that might be the 
route you want to take.

Ciao,
Dscho

^ permalink raw reply

* Re: Git/Mercurial interoperability (and what about bzr?)
From: Matthieu Moy @ 2008-10-28 15:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Peter Krefting, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0810281551040.22125@pacific.mpi-cbg.de.mpi-cbg.de>

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

> Wasn't bzr touting it as one of their major features that they could have 
> foreign-scm remotes?  If I remembered that correctly, that might be the 
> route you want to take.

Yes, see http://bazaar-vcs.org/BzrForeignBranches . That can be
compared to git-svn for git, except that one uses the exact same
command set to interact with the remotes (i.e. you "bzr push" to an
svn repository, while you would "git svn dcommit" with git).

There are read-only implementations of Git and Mercurial foreign
branches. AFAIK, unfortunately, they're more proof of concepts than
real "production ready" plugins.

-- 
Matthieu

^ 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