Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] clone: respect the settings in $HOME/.gitconfig and /etc/gitconfig
From: Daniel Barkalow @ 2008-06-29 18:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Pieter de Bie, Git Mailinglist
In-Reply-To: <7vvdzuo61b.fsf@gitster.siamese.dyndns.org>

On Fri, 27 Jun 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Fri, 27 Jun 2008, Johannes Schindelin wrote:
> >
> >> After initializing the config in the newly-created repository, we
> >> need to unset GIT_CONFIG so that the global configs are read again.
> >
> > This seems fine to me. OTOH, I'm not sure the environment variable should 
> > be needed in the first place; I think the config stuff should look in 
> > git_path("config") without it, and we set the git dir to the one we're 
> > initializing. So I think the use of the environment variable is just an 
> > artifact of how the shell script did it and how I was originally calling 
> > the init_db stuff.
> >
> > Just removing the "setenv()" line survives all of the tests for me, and I 
> > remember some of them failing before I'd gotten some sort of solution for 
> > the config stuff.
> 
> Ok, I take that you are Ok with 2/2 but you have a better replacement
> patch coming for this 1/2?

I think so. Did we even make a commitment on whether:

GIT_CONFIG=foo git clone bar

must ignore the environment variable, or simply doesn't necessarily obey 
it? IIRC, the test scripts used to set GIT_CONFIG to something 
problematic (and would fail if clone didn't ignore it), but this was 
deemed incorrect usage and fixed. 

If it's okay to obey it (i.e., it gives the location of the config file 
for the clone):

------
Use all of the normal config-file handling in builtin-clone.c

There's no need to use the environment variable to direct the generation 
of the config file in the C version of clone, and having it not defined 
means that global and per-user configuration is available. This is 
necessary for the fetching portion, and could be useful someday for the 
init portion as well. It is unlikely that the user would want the 
repository's configuration in some non-default location, but no less 
likely than with any other git command besides "git config".

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---

diff --git a/builtin-clone.c b/builtin-clone.c
index f13845f..6dd58ac 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -415,8 +415,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	atexit(remove_junk);
 	signal(SIGINT, remove_junk_on_signal);
 
-	setenv(CONFIG_ENVIRONMENT, xstrdup(mkpath("%s/config", git_dir)), 1);
-
 	if (safe_create_leading_directories_const(git_dir) < 0)
 		die("could not create leading directories of '%s'", git_dir);
 	set_git_dir(make_absolute_path(git_dir));

^ permalink raw reply related

* Re: [JGIT PATCH 16/21] Add Robert Harder's public domain Base64 encoding utility
From: Shawn O. Pearce @ 2008-06-29 18:06 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Marek Zawirski, git
In-Reply-To: <200806291551.08166.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> 
> Dragging in apache commons libraries seems to be too much at this
> point, one class, so this choice is understandable at this point.
> 
> Other that that Apache Commons are almost ubiquotous these days
> and so be considered at every point when we need external code.

True.  But I have a nearly allergic reaction to Apache code;
for some reason its always sort of not quite there.  Which also
describes jgit so I shouldn't say anything.  ;-)

We can always rip this implementation out if we do wind up taking
in the Apache Commons libraries for other support.  There is only
1 or two calls sites and it should be easy enough to change over
to the Apache Commons implementation.

-- 
Shawn.

^ permalink raw reply

* Re: [JGIT PATCH 09/21] Remember how a Ref was read in from disk and created
From: Shawn O. Pearce @ 2008-06-29 18:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Robin Rosenberg, Marek Zawirski, git
In-Reply-To: <alpine.DEB.1.00.0806291516540.9925@racer>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sun, 29 Jun 2008, Robin Rosenberg wrote:
> 
> > söndagen den 29 juni 2008 09.59.19 skrev Shawn O. Pearce:
> > > To efficiently deleted or update a ref we need to know where it came 
> > > from when it was read into the process.  If the ref is being updated 
> > > we can usually just write the loose file, but if it is being deleted 
> > > we may need to remove not just a loose file but also delete it from 
> > > the packed-refs.
> > 
> > One could argue that we should not normally just delete a ref, but mark 
> > it as deleted and let git gc delete it when it expires, just like any 
> > old ref, but then we should try to get C Git to do the same. There was a 
> > thread relating to this recently.
> 
> ... but it petered out, so you should consider any ideas in that thread 
> rejected.

Right.  Its a nice idea, but until there is a really solid agreement in
the community about how this should be stored on disk, I don't want to
try and implement it in jgit, or in C Git for that matter.  And I don't
really care enough to come up with something and reopen the thread myself.

I just realized that the dumb transport push support doesn't delete
the reflog when it deletes the ref.  Whoops.  That's a problem
if you later try to create a ref where a directory used to be.
Either git will run into errors trying to create the reflog.

-- 
Shawn.

^ permalink raw reply

* Re: gitweb/gitk: empty merge diffs
From: Jakub Narebski @ 2008-06-29 17:21 UTC (permalink / raw)
  To: Lea Wiemann; +Cc: Git Mailing List
In-Reply-To: <4867B105.7070208@gmail.com>

Lea Wiemann <lewiemann@gmail.com> writes:

> I'm trying to get the diff of a merge ("git diff master^1 master")
> with gitk and gitweb, but both of them only show empty diffs (you can
> try yourself using the test script below).

Note that if you are handcrafting gitweb URL, gitweb does not allow
"master^1" as value of 'hp' ($hash_parent) parameter due to overly
strict validation (checking) of this parameter: only values suitable
as refnames, or (shortened) SHA-1 identifiers are accepted.

> Gitk doesn't seem to allow me to view the merge diff at all.
> 
> Gitweb only shows "Trivial merge" in the commitdiff view.  

Because it _is_ trivial (tree level) merge, see documentation of diff
format for merges in git-diff(1) or Documentation/diff-format.txt:

  Note that 'combined diff' lists only files which were modified from
  all parents.

If combined diff raw (difftree) format lists nothing, gitweb denotes
this as "Trivial merge".

> In the commit view, I can get the merge diff I want by clicking the
> "diff" link behind the first parent.

You can also click on the "commitdiff" link in the navbar (at top of
the page), or you can click on commit title to switch between 'commit'
and 'commitdiff' views.
 
> Why is this a "trivial merge", and what's the reason for gitweb not
> displaying the diff to the first parent in the commitdiff view?  I'm
> kinda confused here.

Lets take git.git repository as an example.  Gitweb can display both
combined diff output for merge commit, and compact combined (--cc)
output (default); if you however provide both parents it would always
display ordinary diff.  The "diff" links in the 'commit' view you have
mentioned, and "1"/"2"/... links in treediff part of combined merge
output in 'commitdiff' view, always display ordinary diff.

Default compact combined diff of a merge commit (c.f. "git show pu")
  http://repo.or.cz/w/git.git?a=commitdiff;h=58829d79 ('pu' branch)

Diff to first parent (c.f. "git diff pu^1 pu")
  http://repo.or.cz/w/git.git?a=commitdiff;h=58829d79;hp=c944eebd

Diff to second parent (c.f. "git diff pu^2 pu")
Warning: large diff, and it takes some time to be displayed!
  http://repo.or.cz/w/git.git?a=commitdiff;h=58829d79;hp=f1646543

> ===== Graph: =====
> 
> *   master      merge mybranch into master
> |\
> | * mybranch    change file2 on mybranch
> | * mybranch~1  change file2 on mybranch
> * | master~1    change file1 on master
> * | master~2    change file1 on master
> |/
> * mybranch~2    initial commit
> 
> 
> ===== Script to set up the repository: =====

Well, you can display diff to first parent in gitweb, see

  http://localhost/gitweb.cgi/test-repo.git?a=commitdiff;h=36c2b5d0;hp=62868699

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [RFC/PATCH] Documentation: Don't assume git-sh-setup and git-parse-remote are in the PATH
From: jrnieder @ 2008-06-29 16:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Alex Riesen, Jeff King
In-Reply-To: <20080629053823.GA8262@sigill.intra.peff.net>

On Sun, 29 Jun 2008, Jeff King wrote:

> I think the right question is:
> 
>   PATH=$PATH:$(git --exec-path)
>   . git-sh-setup
> 
> or
> 
>   . "$(git --exec-path)/git-sh-setup"
> 
> ?
> 
> Generally, I would prefer the latter because it has no side effects.

On Sun, 28 Jun 2008, Alex Riesen wrote:

> . "$(git --exec-path)/git-sh-setup"

I agree. I was blindly following the instructions from the release notes
to Git 1.5.4. After thinking about it a little and reading git-sh-setup.sh
and git-parse-remote.sh, it does seem safe not to add the exec-path to PATH.

How about this documentation patch? (This time tested! With hg-fast-export
appropriately modified:
  $ hg clone http://hg.sharesource.org/asciidoc
  $ mkdir asciidoc-git && cd asciidoc-git
  $ git init && hg-fast-export.sh -r /path/to/asciidoc --quiet
  $ git checkout
  $ sudo ./install.sh
  $ cd /path/to/git/Documentation && make git-{sh-setup,parse-remote}.html
I haven't tried rebuilding manpages, though.)

Junio: I stole the commit message from you. I hope you don't mind.

-- %< --

Subject: [PATCH] Documentation: don't assume git-sh-setup and git-parse-remote are in PATH

When git-parse-remote and git-sh-setup are not installed in
$(bindir) anymore, the shell script library won't be found on
user's $PATH in general.

Signed-off-by: Jonathan Nieder <jrnieder@uchicago.edu>
---
 Documentation/git-parse-remote.txt |    2 +-
 Documentation/git-sh-setup.txt     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-parse-remote.txt b/Documentation/git-parse-remote.txt
index 951dbd6..421312e 100644
--- a/Documentation/git-parse-remote.txt
+++ b/Documentation/git-parse-remote.txt
@@ -8,7 +8,7 @@ git-parse-remote - Routines to help parsing remote repository access parameters
 
 SYNOPSIS
 --------
-'. git-parse-remote'
+'. "$(git --exec-path)/git-parse-remote"'
 
 DESCRIPTION
 -----------
diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index c543170..6731f9a 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -7,7 +7,7 @@ git-sh-setup - Common git shell script setup code
 
 SYNOPSIS
 --------
-'git-sh-setup'
+'. "$(git --exec-path)/git-sh-setup"'
 
 DESCRIPTION
 -----------
-- 
1.5.5.1.328.gbfcc6

^ permalink raw reply related

* gitweb/gitk: empty merge diffs
From: Lea Wiemann @ 2008-06-29 15:57 UTC (permalink / raw)
  To: Git Mailing List

I'm trying to get the diff of a merge ("git diff master^1 master") with 
gitk and gitweb, but both of them only show empty diffs (you can try 
yourself using the test script below).

Gitk doesn't seem to allow me to view the merge diff at all.

Gitweb only shows "Trivial merge" in the commitdiff view.  In the commit 
view, I can get the merge diff I want by clicking the "diff" link behind 
the first parent.

Why is this a "trivial merge", and what's the reason for gitweb not 
displaying the diff to the first parent in the commitdiff view?  I'm 
kinda confused here.


===== Graph: =====

*   master      merge mybranch into master
|\
| * mybranch    change file2 on mybranch
| * mybranch~1  change file2 on mybranch
* | master~1    change file1 on master
* | master~2    change file1 on master
|/
* mybranch~2    initial commit


===== Script to set up the repository: =====

rm -rf test-repo.tmp
mkdir test-repo.tmp
cd test-repo.tmp

git init

echo File 1 > file1
echo File 2 > file2
git add file1 file2
git commit -m 'initial commit'

git branch mybranch

echo Addition to file 1 >> file1
git commit -a -m 'change file1 on master'
echo Another addition to file 1 >> file1
git commit -a -m 'change file1 on master'

git checkout mybranch
echo Addition to file 2 >> file2
git commit -a -m 'change file2 on mybranch'
echo Another addition to file 2 >> file2
git commit -a -m 'change file2 on mybranch'

git checkout master
git merge 'merge mybranch into master' HEAD mybranch

^ permalink raw reply

* Re: [PATCH] Add new test to ensure git-merge handles pull.twohead and pull.octopus
From: Miklos Vajna @ 2008-06-29 15:11 UTC (permalink / raw)
  To: Olivier Marin; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <1214751098-547-1-git-send-email-vmiklos@frugalware.org>

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

On Sun, Jun 29, 2008 at 04:51:38PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Oops, I forgot to remove this Signed-off-by. Fixed in my fork.

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

^ permalink raw reply

* [PATCH] Add new test to ensure git-merge handles pull.twohead and pull.octopus
From: Miklos Vajna @ 2008-06-29 14:51 UTC (permalink / raw)
  To: Olivier Marin; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <48678E90.1030608@free.fr>

Test if the given strategies are used and test the case when multiple
strategies are configured using a space separated list.

Also test if the best strategy is picked if none is specified.  This is
done by adding a simple test case where recursive detects a rename, but
resolve does not, and verify that finally merge will pick up the
previous.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

On Sun, Jun 29, 2008 at 03:30:56PM +0200, Olivier Marin <dkr+ml.git@free.fr> wrote:
> > +   git tag c2
>
> Missing &&?

Fixed.

> > +   test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)"
>
> Duplicate the next line?
>
> > +   test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&

Yes, it was duplicated.

> > +   test "$auto_count" = "$recursive_count"
> > +'
>
> Should not "$auto_count" != "$resolve_count" also be tested to be
> sure that recursive has been choosen?

Sure, changed.

Updated patch below.

 t/t7601-merge-pull-config.sh |  129 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 129 insertions(+), 0 deletions(-)
 create mode 100755 t/t7601-merge-pull-config.sh

diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
new file mode 100755
index 0000000..32585f8
--- /dev/null
+++ b/t/t7601-merge-pull-config.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing pull.* configuration parsing.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo c0 >c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 >c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	git reset --hard c0 &&
+	echo c2 >c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2 &&
+	git reset --hard c0 &&
+	echo c3 >c3.c &&
+	git add c3.c &&
+	git commit -m c3 &&
+	git tag c3
+'
+
+test_expect_success 'merge c1 with c2' '
+	git reset --hard c1 &&
+	test -f c0.c &&
+	test -f c1.c &&
+	test ! -f c2.c &&
+	test ! -f c3.c &&
+	git merge c2 &&
+	test -f c1.c &&
+	test -f c2.c
+'
+
+test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
+	git reset --hard c1 &&
+	git config pull.twohead ours &&
+	git merge c2 &&
+	test -f c1.c &&
+	! test -f c2.c
+'
+
+test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
+	git reset --hard c1 &&
+	git config pull.octopus "recursive" &&
+	test_must_fail git merge c2 c3 &&
+	test "$(git rev-parse c1)" = "$(git rev-parse HEAD)"
+'
+
+test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octopus)' '
+	git reset --hard c1 &&
+	git config pull.octopus "recursive octopus" &&
+	git merge c2 c3 &&
+	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
+	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
+	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
+	git diff --exit-code &&
+	test -f c0.c &&
+	test -f c1.c &&
+	test -f c2.c &&
+	test -f c3.c
+'
+
+conflict_count()
+{
+	eval $1=`{
+		git diff-files --name-only
+		git ls-files --unmerged
+	} | wc -l`
+}
+
+# c4 - c5
+#    \ c6
+#
+# There are two conflicts here:
+#
+# 1) Because foo.c is renamed to bar.c, recursive will handle this,
+# resolve won't.
+#
+# 2) One in conflict.c and that will always fail.
+
+test_expect_success 'setup conflicted merge' '
+	git reset --hard c0 &&
+	echo A >conflict.c &&
+	git add conflict.c &&
+	echo contents >foo.c &&
+	git add foo.c &&
+	git commit -m c4 &&
+	git tag c4 &&
+	echo B >conflict.c &&
+	git add conflict.c &&
+	git mv foo.c bar.c &&
+	git commit -m c5 &&
+	git tag c5 &&
+	git reset --hard c4 &&
+	echo C >conflict.c &&
+	git add conflict.c &&
+	echo secondline >> foo.c &&
+	git add foo.c &&
+	git commit -m c6 &&
+	git tag c6
+'
+
+# First do the merge with resolve and recursive then verify that
+# recusive is choosen.
+
+test_expect_success 'merge picks up the best result' '
+	git config pull.twohead "recursive resolve" &&
+	git reset --hard c5 &&
+	git merge -s resolve c6
+	conflict_count resolve_count &&
+	git reset --hard c5 &&
+	git merge -s recursive c6
+	conflict_count recursive_count &&
+	git reset --hard c5 &&
+	git merge c6
+	conflict_count auto_count &&
+	test "$auto_count" = "$recursive_count" &&
+	test "$auto_count" != "$resolve_count"
+'
+
+test_done
-- 
1.5.6

^ permalink raw reply related

* Re: [PATCH 01/15] Move split_cmdline() to alias.c
From: Johannes Schindelin @ 2008-06-29 14:43 UTC (permalink / raw)
  To: Olivier Marin; +Cc: Miklos Vajna, Junio C Hamano, git, Olivier Marin
In-Reply-To: <48679C44.30702@free.fr>

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

Hi,

On Sun, 29 Jun 2008, Olivier Marin wrote:

> Johannes Schindelin a écrit :
> > 
> > As the string comes either from the config (where it is trimmed), or 
> > from the command line (where the user can be stup^W^Wask for 
> > whitespace explicitely), I do not see much merit in this patch.
> 
> You are right, today it is not a problem because the usage is really 
> limited but Miklos's intention seems to make the function usable by 
> everyone. As we do not know how it will be used in the future, I think 
> it is safer with my patch.

I am generally not a fan of crossing bridges miles before you reach them.

But if you want to keep running with this, you should add at least a few 
tests to show what you patch solves, and that it solves it.  Preferably 
with a "test_expect_failure" patch, and then a patch fixing it and 
changing that _failure to _success.

Here's a starter:

-- snipsnap --
diff --git a/Makefile b/Makefile
index d3e339a..83a9a30 100644
--- a/Makefile
+++ b/Makefile
@@ -1229,7 +1229,7 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X test-strbuf$X
+TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X test-strbuf$X test-cmdline$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/test-cmdline.c b/test-cmdline.c
new file mode 100644
index 0000000..2f6d9eb
--- /dev/null
+++ b/test-cmdline.c
@@ -0,0 +1,15 @@
+#include "cache.h"
+
+int main(int argc, char **argv)
+{
+	int i = 0;
+	const char **new_argv;
+
+	if (argc < 2 || (argc = split_cmdline(argv[1], &new_argv)) < 0)
+		return 1;
+
+	while (i < argc)
+		printf("arg %d: '%s'\n", i++, *new_argv++);
+
+	return 0;
+}

^ permalink raw reply related

* Re: [PATCH 01/15] Move split_cmdline() to alias.c
From: Olivier Marin @ 2008-06-29 14:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Miklos Vajna, Junio C Hamano, git, Olivier Marin
In-Reply-To: <alpine.DEB.1.00.0806291513450.9925@racer>

Johannes Schindelin a écrit :
> 
> As the string comes either from the config (where it is trimmed), or from 
> the command line (where the user can be stup^W^Wask for whitespace 
> explicitely),  I do not see much merit in this patch.

You are right, today it is not a problem because the usage is really limited
but Miklos's intention seems to make the function usable by everyone. As we
do not know how it will be used in the future, I think it is safer with my
patch.

Olivier.

^ permalink raw reply

* Re: [JGIT PATCH 09/21] Remember how a Ref was read in from disk and created
From: Johannes Schindelin @ 2008-06-29 14:17 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Shawn O. Pearce, Marek Zawirski, git
In-Reply-To: <200806291551.06201.robin.rosenberg@dewire.com>

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

Hi,

On Sun, 29 Jun 2008, Robin Rosenberg wrote:

> söndagen den 29 juni 2008 09.59.19 skrev Shawn O. Pearce:
> > To efficiently deleted or update a ref we need to know where it came 
> > from when it was read into the process.  If the ref is being updated 
> > we can usually just write the loose file, but if it is being deleted 
> > we may need to remove not just a loose file but also delete it from 
> > the packed-refs.
> 
> One could argue that we should not normally just delete a ref, but mark 
> it as deleted and let git gc delete it when it expires, just like any 
> old ref, but then we should try to get C Git to do the same. There was a 
> thread relating to this recently.

... but it petered out, so you should consider any ideas in that thread 
rejected.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 01/15] Move split_cmdline() to alias.c
From: Johannes Schindelin @ 2008-06-29 14:15 UTC (permalink / raw)
  To: Olivier Marin; +Cc: Miklos Vajna, Junio C Hamano, git, Olivier Marin
In-Reply-To: <486796C3.7040304@free.fr>

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

Hi,

On Sun, 29 Jun 2008, Olivier Marin wrote:

> Miklos Vajna a écrit :
> > split_cmdline() is currently used for aliases only, but later it can be
> > useful for other builtins as well. Move it to alias.c for now,
> > indicating that originally it's for aliases, but we'll have it in libgit
> > this way.
> 
> This function does not trim cmdline. 

As the string comes either from the config (where it is trimmed), or from 
the command line (where the user can be stup^W^Wask for whitespace 
explicitely),  I do not see much merit in this patch.

Unless you can provide an example where it fails, of course,
Dscho

^ permalink raw reply

* Re: [PATCH 01/15] Move split_cmdline() to alias.c
From: Olivier Marin @ 2008-06-29 14:05 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git, Johannes Schindelin, Olivier Marin
In-Reply-To: <58b2c36de6a6f51a562da303695482bea567f4bf.1214581610.git.vmiklos@frugalware.org>

Miklos Vajna a écrit :
> split_cmdline() is currently used for aliases only, but later it can be
> useful for other builtins as well. Move it to alias.c for now,
> indicating that originally it's for aliases, but we'll have it in libgit
> this way.

This function does not trim cmdline. 

Perhaps, the following patch can be inserted after 1/15.

-- >8 --
From: Olivier Marin <dkr@freesurf.fr>
Date: Sat, 28 Jun 2008 13:06:21 +0200
Subject: [PATCH] split_cmdline(): ignore whitespace at start/end of cmdline

Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---
 alias.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/alias.c b/alias.c
index ccb1108..7b69d18 100644
--- a/alias.c
+++ b/alias.c
@@ -24,14 +24,16 @@ char *alias_lookup(const char *alias)
 
 int split_cmdline(char *cmdline, const char ***argv)
 {
-       int src, dst, count = 0, size = 16;
+       int src = 0, dst, count = 0, size = 16;
        char quoted = 0;
 
        *argv = xmalloc(sizeof(char*) * size);
 
        /* split alias_string */
-       (*argv)[count++] = cmdline;
-       for (src = dst = 0; cmdline[src];) {
+       while (cmdline[src] && isspace(cmdline[src]))
+               src++;
+       (*argv)[count++] = cmdline + src;
+       for (dst = src; cmdline[src];) {
                char c = cmdline[src];
                if (!quoted && isspace(c)) {
                        cmdline[dst++] = 0;
@@ -42,7 +44,8 @@ int split_cmdline(char *cmdline, const char ***argv)
                                size += 16;
                                *argv = xrealloc(*argv, sizeof(char*) * size);
                        }
-                       (*argv)[count++] = cmdline + dst;
+                       if (cmdline[src])
+                               (*argv)[count++] = cmdline + dst;
                } else if (!quoted && (c == '\'' || c == '"')) {
                        quoted = c;
                        src++;
-- 
1.5.6.1.103.g191a2.dirty

^ permalink raw reply related

* Re: [JGIT PATCH 16/21] Add Robert Harder's public domain Base64 encoding utility
From: Robin Rosenberg @ 2008-06-29 13:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Marek Zawirski, git
In-Reply-To: <1214726371-93520-17-git-send-email-spearce@spearce.org>


Dragging in apache commons libraries seems to be too much at this point, one class, so this choice is
understandable at this point.

Other that that Apache Commons are almost ubiquotous these days and so be considered at every
point when we need external code.

-- robin

^ permalink raw reply

* Re: [JGIT PATCH 09/21] Remember how a Ref was read in from disk and created
From: Robin Rosenberg @ 2008-06-29 13:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Marek Zawirski, git
In-Reply-To: <1214726371-93520-10-git-send-email-spearce@spearce.org>

söndagen den 29 juni 2008 09.59.19 skrev Shawn O. Pearce:
> To efficiently deleted or update a ref we need to know where
> it came from when it was read into the process.  If the ref
> is being updated we can usually just write the loose file,
> but if it is being deleted we may need to remove not just a
> loose file but also delete it from the packed-refs.

One could argue that we should not normally just delete a ref, but
mark it as deleted and let git gc delete it when it expires, just like
any old ref, but then we should try to get C Git to do the same. There
was a thread relating to this recently.

-- robi

^ permalink raw reply

* Re: [StGit PATCH 1/2] Convert "stg refresh" to the new infrastructure
From: Karl Hasselström @ 2008-06-29 13:46 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0806290407h5eecd27bg510dd09e0188abca@mail.gmail.com>

On 2008-06-29 12:07:42 +0100, Catalin Marinas wrote:

> 2008/6/29 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-06-29 10:42:32 +0100, Catalin Marinas wrote:
> >
> > > Does it make sense to refresh an unapplied patch? Maybe adding a
> > > new file to the patch but I don't really see a need for this.
> >
> > But no, I don't have a strong sense that this is super useful. It
> > was just easy to allow, so I allowed it.
>
> It seems harmless, unless someone finds some unusual behaviour. What
> is the conflict behaviour? Is the refresh aborted? For unapplied
> patches, it is more complicated to let the user solve the conflict.

First, the refresh is stored in a temp patch. Then, that patch is
popped. Then we try to coalesce the temp patch and the target patch;
if this fails due to conflicts that we can't represent because we have
no index and worktree to work with for unapplied patches, we just
leave the temp patch in the stack for the user to deal with. (E.g. by
trying to apply the two patches and sort out the conflicts, by keeping
the temp patch separate, or by coalescing it with a different patch.)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf()
From: Johannes Schindelin @ 2008-06-29 13:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git, Olivier Marin
In-Reply-To: <7vskuwek9w.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, 29 Jun 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Anyway, I'm fine with having them only in my personal fork.
> 
> It will cost you some "trust point" next time you try to sneak something
> in as a part of a largely unrelated topic.  Please don't.

Fine.  Fine!

I thought it made the code more readable, and that it also might have 
helped other sites where you have to strbuf_init() first, and often need 
curly brackets only because of that.  Making for ultra-ugly code.

Please note that I also added a simple test case for the vaddf() function, 
so your "trying to sneak through" is only half as bad as you made it 
sound.

Also, please note that the situation with SNPRINTF_RETURNS_BOGUS works, 
but is a thorn in this developers eye.

Anyway, I will not try to post this patch again,
Dscho

^ permalink raw reply

* Re: [PATCH 04/15] Add new test to ensure git-merge handles pull.twohead and pull.octopus
From: Olivier Marin @ 2008-06-29 13:30 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git, Johannes Schindelin, Olivier Marin
In-Reply-To: <99b931de28ab3017269d15c334c83e242d025489.1214581610.git.vmiklos@frugalware.org>

Miklos Vajna a écrit :
> 

[...]

> +test_expect_success 'setup' '
> +	echo c0 >c0.c &&
> +	git add c0.c &&
> +	git commit -m c0 &&
> +	git tag c0 &&
> +	echo c1 >c1.c &&
> +	git add c1.c &&
> +	git commit -m c1 &&
> +	git tag c1 &&
> +	git reset --hard c0 &&
> +	echo c2 >c2.c &&
> +	git add c2.c &&
> +	git commit -m c2 &&
> +	git tag c2

Missing &&?

> +	git reset --hard c0 &&
> +	echo c3 >c3.c &&
> +	git add c3.c &&
> +	git commit -m c3 &&
> +	git tag c3
> +'

[...]

> +test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octopus)' '
> +	git reset --hard c1 &&
> +	git config pull.octopus "recursive octopus" &&
> +	git merge c2 c3 &&
> +	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
> +	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> +	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> +	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)"

Duplicate the next line?

> +	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> +	git diff --exit-code &&
> +	test -f c0.c &&
> +	test -f c1.c &&
> +	test -f c2.c &&
> +	test -f c3.c
> +'
> +
> +conflict_count()
> +{
> +	eval $1=`{
> +		git diff-files --name-only
> +		git ls-files --unmerged
> +	} | wc -l`
> +}
> +
> +# c4 - c5
> +#    \ c6
> +#
> +# There are two conflicts here:
> +#
> +# 1) Because foo.c is renamed to bar.c, recursive will handle this,
> +# resolve won't.
> +#
> +# 2) One in conflict.c and that will always fail.
> +
> +test_expect_success 'setup conflicted merge' '
> +	git reset --hard c0 &&
> +	echo A >conflict.c &&
> +	git add conflict.c &&
> +	echo contents >foo.c &&
> +	git add foo.c &&
> +	git commit -m c4 &&
> +	git tag c4 &&
> +	echo B >conflict.c &&
> +	git add conflict.c &&
> +	git mv foo.c bar.c &&
> +	git commit -m c5 &&
> +	git tag c5 &&
> +	git reset --hard c4 &&
> +	echo C >conflict.c &&
> +	git add conflict.c &&
> +	echo secondline >> foo.c &&
> +	git add foo.c &&
> +	git commit -m c6 &&
> +	git tag c6
> +'
> +
> +# First do the merge with resolve and recursive then verify that
> +# recusive is choosen.
> +
> +test_expect_success 'merge picks up the best result' '
> +	git config pull.twohead "recursive resolve" &&
> +	git reset --hard c5 &&
> +	git merge -s resolve c6
> +	conflict_count resolve_count &&
> +	git reset --hard c5 &&
> +	git merge -s recursive c6
> +	conflict_count recursive_count &&
> +	git reset --hard c5 &&
> +	git merge c6
> +	conflict_count auto_count &&
> +	test "$auto_count" = "$recursive_count"
> +'

Should not "$auto_count" != "$resolve_count" also be tested to be
sure that recursive has been choosen?

Olivier.

^ permalink raw reply

* Re: Is there a way to diff between a local repository and a remote one?
From: Jakub Narebski @ 2008-06-29 12:38 UTC (permalink / raw)
  To: Erez Zilber; +Cc: git@vger.kernel.org
In-Reply-To: <ce513bcc0806290429r1982fbf2i4c9a8258bc8db3c@mail.gmail.com>

"Erez Zilber" <erezzi.list@gmail.com> writes:

> Is there a way to diff between a local repository and a remote one?
>
> I could clone the remote repository and use Linux's diff, but I prefer
> to do it the right way (if possible).

There is no way to do this remotely; you have to download either
snapshot (tarball) of remote repository (either from gitweb interface
or other web interface if snapshot feature is enabled, or using 
"git archive --remote=<repo> <version>" (where <version> might be "HEAD")
if it is enabled on remote), or fetch / shallow fetch / shallow clone
of repository.

If local repository you want to diff is related to remote repository,
the simplest way would be to fetch (or shallow fetch) the repository
you want to compare into your repository, either using

  $ git remote add -f <nick> <URL>
  $ git diff HEAD <nick> #; or <nick>/HEAD, or <nick>/master

or

  $ git fetch --no-tags <URL> <branch> #; you can add --depth=1
  $ git diff HEAD FETCH_HEAD

(of course all above being in local repository; you can replace HEAD
by local revision you want to compare)
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Is there a way to diff between a local repository and a remote one?
From: Erez Zilber @ 2008-06-29 12:19 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: git@vger.kernel.org
In-Reply-To: <4867783C.1010408@panasas.com>

On Sun, Jun 29, 2008 at 2:55 PM, Boaz Harrosh <bharrosh@panasas.com> wrote:
> Erez Zilber wrote:
>> I could clone the remote repository and use Linux's diff, but I prefer
>> to do it the right way (if possible).
>>
>> Thanks,
>> Erez
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> I use git-remote add
>
> and git-diff that_remote/that_branch
>
> then from time to time "git-remote update" to update on remote progress
>
> Boaz

thanks!

^ permalink raw reply

* Re: Is there a way to diff between a local repository and a remote one?
From: Boaz Harrosh @ 2008-06-29 11:55 UTC (permalink / raw)
  To: Erez Zilber; +Cc: git@vger.kernel.org
In-Reply-To: <ce513bcc0806290429r1982fbf2i4c9a8258bc8db3c@mail.gmail.com>

Erez Zilber wrote:
> I could clone the remote repository and use Linux's diff, but I prefer
> to do it the right way (if possible).
> 
> Thanks,
> Erez
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

I use git-remote add

and git-diff that_remote/that_branch

then from time to time "git-remote update" to update on remote progress

Boaz

^ permalink raw reply

* Re: [PATCH] git cat-file: Fix memory leak in batch mode
From: Björn Steinbrink @ 2008-06-29 11:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Avery Pennarun, Eric Wong, Adam Roben, Samuel Bronson, git
In-Reply-To: <7v1w2gkj3l.fsf@gitster.siamese.dyndns.org>

On 2008.06.28 20:36:46 -0700, Junio C Hamano wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> 
> > When run in batch mode, git cat-file never frees the memory for the blob
> > contents it is printing. This quickly adds up and causes git-svn to be
> > hardly usable for imports of large svn repos, because it uses cat-file in
> > batch mode and cat-file's memory usage easily reaches several hundred MB
> > without any good reason.
> >
> > Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> > ---
> >  builtin-cat-file.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/builtin-cat-file.c b/builtin-cat-file.c
> > index bd343ef..f966dcb 100644
> > --- a/builtin-cat-file.c
> > +++ b/builtin-cat-file.c
> > @@ -183,6 +183,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
> >  		fflush(stdout);
> >  	}
> >  
> > +	free(contents);
> >  	return 0;
> >  }
> 
> Thanks, except that it should go inside the "if (print_contents == BATCH)"
> block to avoid freeing an uninitialized pointer.

Ah crap, I even wondered about the kill-a-warning initialization of
"contents", but my brain was already asleep.

Thanks,
Björn

^ permalink raw reply

* Is there a way to diff between a local repository and a remote one?
From: Erez Zilber @ 2008-06-29 11:29 UTC (permalink / raw)
  To: git@vger.kernel.org

I could clone the remote repository and use Linux's diff, but I prefer
to do it the right way (if possible).

Thanks,
Erez

^ permalink raw reply

* Re: [StGit PATCH 1/2] Convert "stg refresh" to the new infrastructure
From: Catalin Marinas @ 2008-06-29 11:07 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <20080629102147.GA5098@diana.vm.bytemark.co.uk>

2008/6/29 Karl Hasselström <kha@treskal.com>:
> On 2008-06-29 10:42:32 +0100, Catalin Marinas wrote:
>
>> 2008/6/25 Karl Hasselström <kha@treskal.com>:
>>
>> > And in the process, make it more powerful: it will now first
>> > create a temp patch containing the updates, and then try to merge
>> > it into the patch to be updated. If that patch is applied, this is
>> > done by popping, pushing, and coalescing; if it is unapplied, it
>> > is done with an in-index merge.
>>
>> Does it make sense to refresh an unapplied patch? Maybe adding a new
>> file to the patch but I don't really see a need for this.
>
> A change in a different part of the same file should work as well, I
> believe.
>
> But no, I don't have a strong sense that this is super useful. It was
> just easy to allow, so I allowed it.

It seems harmless, unless someone finds some unusual behaviour. What
is the conflict behaviour? Is the refresh aborted? For unapplied
patches, it is more complicated to let the user solve the conflict.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] Teach git-merge to pass -X<option> to the backend strategy module
From: Jakub Narebski @ 2008-06-29 10:32 UTC (permalink / raw)
  To: git
In-Reply-To: <7vwsk8d3q5.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:

> diff --git a/git-merge.sh b/git-merge.sh
> index 39b5cd9..d475852 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -17,6 +17,7 @@ commit               perform a commit if the merge succeeds (default)
>  ff                   allow fast forward (default)
>  s,strategy=          merge strategy to use
>  m,message=           message to be used for the merge commit (if any)
> +X=                   pass merge strategy specific options
>  "

You have updated usage for git-merge, but didn't update manpages
(Documentation).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ 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