Git development
 help / color / mirror / Atom feed
* Re: Install issues
From: Junio C Hamano @ 2008-11-11 21:11 UTC (permalink / raw)
  To: H.Merijn Brand; +Cc: Miklos Vajna, git
In-Reply-To: <20081110173101.3d76613b@pc09.procura.nl>

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>  	./test-sha1.sh
>  
>  check: common-cmds.h
> +	@`sparse </dev/null 2>/dev/null` || (\
> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> +	    echo "Did you mean 'make test' instead?" ;\
> +	    exit 1 )

When you mean "grouping", using {} is much clearer to convey your
intention.  Use of needless (subshell) forces the reader to wonder if you
wanted to do something that affects the environment for later commands
inside, and in this case you didn't.

Why do you have sparse check inside a backtick to produce a string to be
interpreted as a command to be executed?

How about doing this instead?  'sparse' without any parameter exits with
success status silently; when you do not have the command, the shell will
complain with "sparse: command not found" anyway, so you only need to
suggest "make 'test'" and nothing else.

-- >8 --
Subject: Makefile: help people who run 'make check' by mistake

The target to run self test is 'make test', but there are people who try
'make check' and worse yet do not have sparse installed.

Suggest 'make test' target when they do not have 'sparse'.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git c/Makefile w/Makefile
index 40309e1..d3137ca 100644
--- c/Makefile
+++ w/Makefile
@@ -1355,7 +1355,16 @@ check-sha1:: test-sha1$X
 	./test-sha1.sh
 
 check: common-cmds.h
-	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
+	if sparse; \
+	then \
+		for i in *.c; \
+		do \
+			sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; \
+		done; \
+	else \
+		echo 2>&1 "Did you mean 'make test'?"; \
+		exit 1; \
+	fi
 
 remove-dashes:
 	./fixup-builtins $(BUILT_INS) $(PROGRAMS) $(SCRIPTS)

^ permalink raw reply related

* [PATCH] git-submodule: Avoid printing a spurious message.
From: Alexandre Julliard @ 2008-11-11 21:09 UTC (permalink / raw)
  To: git

Fix 'git submodule update' to avoid printing a spurious "Maybe you want
to use 'update --init'?" once for every uninitialized submodule it
encounters.

Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b63e5c3..220d94e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -323,7 +323,7 @@ cmd_update()
 			# Only mention uninitialized submodules when its
 			# path have been specified
 			test "$#" != "0" &&
-			say "Submodule path '$path' not initialized"
+			say "Submodule path '$path' not initialized" &&
 			say "Maybe you want to use 'update --init'?"
 			continue
 		fi
-- 
1.6.0.4.637.ga2ff4

-- 
Alexandre Julliard
julliard@winehq.org

^ permalink raw reply related

* [PATCH (GITK)] gitk: Fix transient windows on Win32 and MacOS.
From: Alexander Gavrilov @ 2008-11-11 20:55 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras

Transient windows cause problems on these platforms:

- On Win32 the windows appear in the top left corner
  of the screen. In order to fix it, this patch causes
  them to be explicitly centered on their parents by
  an idle handler.

- On MacOS with Tk 8.4 they appear without a title bar.
  Since it is clearly unacceptable, this patch disables
  transient on that platform.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	NOTE: This patch DEPRECATES the earlier one, called
		"Explicitly position popup windows"

	Alexander

 gitk |   44 +++++++++++++++++++++++++++++++-------------
 1 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/gitk b/gitk
index 9b2a6e5..e6aafe8 100755
--- a/gitk
+++ b/gitk
@@ -1739,6 +1739,24 @@ proc removehead {id name} {
     unset headids($name)
 }
 
+proc make_transient {window origin} {
+    global have_tk85
+
+    # In MacOS Tk 8.4 transient appears to work by setting
+    # overrideredirect, which is utterly useless, since the
+    # windows get no border, and are not even kept above
+    # the parent.
+    if {!$have_tk85 && [tk windowingsystem] eq {aqua}} return
+
+    wm transient $window $origin
+
+    # Windows fails to place transient windows normally, so
+    # schedule a callback to center them on the parent.
+    if {[tk windowingsystem] eq {win32}} {
+	after idle [list tk::PlaceWindow $window widget $origin]
+    }
+}
+
 proc show_error {w top msg} {
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
@@ -1754,7 +1772,7 @@ proc show_error {w top msg} {
 proc error_popup {msg {owner .}} {
     set w .error
     toplevel $w
-    wm transient $w $owner
+    make_transient $w $owner
     show_error $w $w $msg
 }
 
@@ -1763,7 +1781,7 @@ proc confirm_popup {msg {owner .}} {
     set confirm_ok 0
     set w .confirm
     toplevel $w
-    wm transient $w $owner
+    make_transient $w $owner
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
     button $w.ok -text [mc OK] -command "set confirm_ok 1; destroy $w"
@@ -2558,7 +2576,7 @@ proc about {} {
     }
     toplevel $w
     wm title $w [mc "About gitk"]
-    wm transient $w .
+    make_transient $w .
     message $w.m -text [mc "
 Gitk - a commit viewer for git
 
@@ -2587,7 +2605,7 @@ proc keys {} {
     }
     toplevel $w
     wm title $w [mc "Gitk key bindings"]
-    wm transient $w .
+    make_transient $w .
     message $w.m -text "
 [mc "Gitk key bindings:"]
 
@@ -3669,7 +3687,7 @@ proc vieweditor {top n title} {
 
     toplevel $top
     wm title $top $title
-    wm transient $top .
+    make_transient $top .
 
     # View name
     frame $top.nfr
@@ -7912,7 +7930,7 @@ proc mkpatch {} {
     set patchtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Generate patch"]
     grid $top.title - -pady 10
     label $top.from -text [mc "From:"]
@@ -7999,7 +8017,7 @@ proc mktag {} {
     set mktagtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Create tag"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8102,7 +8120,7 @@ proc writecommit {} {
     set wrcomtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Write commit to file"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8159,7 +8177,7 @@ proc mkbranch {} {
     set top .makebranch
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Create new branch"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8322,7 +8340,7 @@ proc resethead {} {
     set confirm_ok 0
     set w ".confirmreset"
     toplevel $w
-    wm transient $w .
+    make_transient $w .
     wm title $w [mc "Confirm reset"]
     message $w.m -text \
 	[mc "Reset branch %s to %s?" $mainhead [string range $rowmenuid 0 7]] \
@@ -8502,7 +8520,7 @@ proc showrefs {} {
     }
     toplevel $top
     wm title $top [mc "Tags and heads: %s" [file tail [pwd]]]
-    wm transient $top .
+    make_transient $top .
     text $top.list -background $bgcolor -foreground $fgcolor \
 	-selectbackground $selectbgcolor -font mainfont \
 	-xscrollcommand "$top.xsb set" -yscrollcommand "$top.ysb set" \
@@ -9844,7 +9862,7 @@ proc choosefont {font which} {
 	font create sample
 	eval font config sample [font actual $font]
 	toplevel $top
-	wm transient $top $prefstop
+	make_transient $top $prefstop
 	wm title $top [mc "Gitk font chooser"]
 	label $top.l -textvariable fontparam(which)
 	pack $top.l -side top
@@ -9961,7 +9979,7 @@ proc doprefs {} {
     }
     toplevel $top
     wm title $top [mc "Gitk preferences"]
-    wm transient $top .
+    make_transient $top .
     label $top.ldisp -text [mc "Commit list display options"]
     grid $top.ldisp - -sticky w -pady 10
     label $top.spacer -text " "
-- 
1.6.0.3.15.gb8d36

^ permalink raw reply related

* Re: [PATCH] git send-email: edit recipient addresses with the --compose flag
From: Francis Galiegue @ 2008-11-11 20:53 UTC (permalink / raw)
  To: Ian Hilt; +Cc: Tait, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811111542230.24205@sys-0.hiltweb.site>

Le Tuesday 11 November 2008 21:47:55 Ian Hilt, vous avez écrit :
> On Tue, 11 Nov 2008, Francis Galiegue wrote:
> > Le Tuesday 11 November 2008 02:49:19 Tait, vous avez écrit :
> > > > > > +	if ($c_file =~ /^To:\s*+(.+)\s*\nCc:/ism) {
> > > > >
> > > > > Greedy operators are only supported with perl 5.10 or more... I
> > > > > think it's a bad idea to use them...
> > > >
> > > > The problem here was that a space should follow the field, but it may
> > > > not.  The user may unwarily backup over it.  "\s*" would match this
> > > > case.
> > > >
> > > > But if there is a space, it is included in the "(.+)".
> > >
> > > Not in any version of Perl to which I have access.
> >
> > And if you see a space in (.+), your regex engine is buggy anyway.
>
> So what does this script produce on your systems?
>
>
> #!/usr/bin/perl -Tw
> --8<--
> use strict;
> my $ws = "To: \nCc:";
>
> $ws =~ /^To:\s*(.+)\s*\nCc:/ism;
>
> if ($1 eq ' ') {
> 	print "\$1 is equal to a space.\n";
> }
> -->8--
>
> On mine, it prints the message.  So it seems it is matching _a_ space.

Which is perfectly normal. The first \s* wanted spaces, it got them. But it 
left nothing for the capturing .+ behind. And any quantifier (except when it 
is possessive) _MUST_ backtrack in order for the full regex to complete. This 
is why the .+ captured the space: the first \s* was perfectly fine with no 
space at all, and the second, well, didn't find any space but it didn't care 
either.

-- 
Francis Galiegue
ONE2TEAM
Ingénieur système
Mob : +33 (0) 6 83 87 78 75
Tel : +33 (0) 1 78 94 55 52
fge@one2team.com
40 avenue Raymond Poincaré
75116 Paris

^ permalink raw reply

* Re: [PATCH] git send-email: edit recipient addresses with the --compose flag
From: Ian Hilt @ 2008-11-11 20:53 UTC (permalink / raw)
  To: Francis Galiegue; +Cc: Tait, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811111542230.24205@sys-0.hiltweb.site>

On Tue, 11 Nov 2008, Ian Hilt wrote:

Hm, how about keeping the path to perl in the snippet,

--8<--
#!/usr/bin/perl -Tw
use strict;
my $ws = "To: \nCc:";

$ws =~ /^To:\s*(.+)\s*\nCc:/ism;

if ($1 eq ' ') {
	print "\$1 is equal to a space.\n";
}
-->8--

^ permalink raw reply

* Re: [PATCH] git send-email: edit recipient addresses with the --compose flag
From: Ian Hilt @ 2008-11-11 20:47 UTC (permalink / raw)
  To: Francis Galiegue; +Cc: Tait, Git Mailing List
In-Reply-To: <200811111230.28076.fg@one2team.com>

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

On Tue, 11 Nov 2008, Francis Galiegue wrote:
> Le Tuesday 11 November 2008 02:49:19 Tait, vous avez écrit :
> > > > > +	if ($c_file =~ /^To:\s*+(.+)\s*\nCc:/ism) {
> > > >
> > > > Greedy operators are only supported with perl 5.10 or more... I think
> > > > it's a bad idea to use them...
> > >
> > > The problem here was that a space should follow the field, but it may
> > > not.  The user may unwarily backup over it.  "\s*" would match this
> > > case.
> > >
> > > But if there is a space, it is included in the "(.+)".
> >
> > Not in any version of Perl to which I have access.
> >
> 
> And if you see a space in (.+), your regex engine is buggy anyway.

So what does this script produce on your systems?


#!/usr/bin/perl -Tw
--8<--
use strict;
my $ws = "To: \nCc:";

$ws =~ /^To:\s*(.+)\s*\nCc:/ism;

if ($1 eq ' ') {
	print "\$1 is equal to a space.\n";
}
-->8--

On mine, it prints the message.  So it seems it is matching _a_ space.
This resulted in an illegal recipient field.  Junio's suggestion to
change this to

  /^To:\s*(\S.+?)\s*\nCc:/ism

worked beautifully.

^ permalink raw reply

* Newbie questions regarding jgit
From: Farrukh Najmi @ 2008-11-11 20:44 UTC (permalink / raw)
  To: git


Hi all,

I am git newbie and looking to use jgit in a servlet endpoint.

Where can I find a public maven repo for gjit? It seems there is one 
somewhere because of the following file in src tree:

jgit-maven/jgit/pom.xml

For now I have built the jar using /make_jgit.sh and installed the pom 
manually using m

mvn install:install-file -DpomFile=jgit-maven/jgit/pom.xml -Dfile=jgit.jar

I have added the following dependency to my maven project's pom.xml:

   <dependency>
     <groupId>org.spearce</groupId>
     <artifactId>jgit</artifactId>
     <version>0.4-SNAPSHOT</version>
   </dependency>

Now I am wondering where to begin to learn how to do the equivalent of 
the following commands via the gjit Java API:

    * git add /file/
    * git rm /file/
    * git mv /file
    * Whatever is the git way to get a specific version of a file


I am hoping that there aremore docs, samples, tutorials etc. somewhere 
that I am missing. Thanks for any help you can provide. Some pointers or 
code fragments would be terrific.

-- 
Regards,
Farrukh Najmi

Web: http://www.wellfleetsoftware.com

^ permalink raw reply

* Re: [take 2] git send-email updates
From: Junio C Hamano @ 2008-11-11 20:30 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <1226361242-2516-1-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

> The last patch is dropped for now (the automatic --compose stuff)
> because I'm not sure which option to add, and that I don't care enough
> about it to spend more time on it.
>
> I think I've incorporated most of the stuff people asked about in this
> series.
>
>  [PATCH 1/4] git send-email: make the message file name more specific.
>  [PATCH 2/4] git send-email: interpret unknown files as revision lists
>  [PATCH 3/4] git send-email: add --annotate option
>  [PATCH 4/4] git send-email: ask less questions when --compose is used.

Thanks.

It is somewhat unfortunate that an explicit --no-format-patch works
exactly the same way as not giving the option at all.  I would have
expected that it would guess and warn if you did not give either, and it
would not even guess (i.e. file is mbox, dir is maildir) and error out if
there is a leftover option in @rev_list_opts if the user explicitly asked
the command not act as a frontend to format patch.

I will queue the series in 'pu' because I suspect you would like a chance
to amend out a "print foo" from the second commit ;-)

^ permalink raw reply

* Possible bug: "git log" ignores "--encoding=UTF-8" option if --pretty=format:%e%n%s%n is used
From: Constantine Plotnikov @ 2008-11-11 19:12 UTC (permalink / raw)
  To: git

If encoding is forced for git log with "--encoding=UTF-8", it seems to
be ineffective for the formatted output. Is it intentional?

Lets consider the following scenario (script is for bash, git 1.5.4.3
and 1.6.0.2):

git init
echo initial >test.txt
git add test.txt
echo -e \\0320\\0242\\0320\\0265\\0321\\0201\\0321\\0202 >msg-UTF-8.txt
git commit -F msg-UTF-8.txt
echo updated >test.txt
git add test.txt
echo -e \\0322\\0345\\0361\\0362 >msg-windows-1251.txt
git config i18n.commitencoding Windows-1251
git commit -F msg-windows-1251.txt
git log --encoding=Windows-1251 >log1.txt
git log --encoding=UTF-8 >log2.txt
git log --encoding=Windows-1251 --pretty=format:%e%n%s%n >log3.txt
git log --encoding=UTF-8 --pretty=format:%e%n%s%n >log4.txt

In the both cases the string is the russian string meaning test in
different encoding.

If we compare log1.txt and log2.txt, we will see that the same
encoding specified is used for both commits in the log.

If we compare log3.txt and log4.txt, we will see that the same the
contents of the file is completely identical. The native encoding is
used for each commit. So the first listed commit
is encoded using Windows-1251 and the second one using UTF-8. However
in the description of the %s %b options nothing is said about which
encoding is used and implied behavior is that they are affected by
--encoding option.

I suggest documenting that the placeholders %s and %b use native
commit encoding and introducing the placeholders %S and %B options
that use encoding specified on the command line or the default log
encoding.

I also suggest adding %g and %G placeholders (%m placeholder is
already occupied) that print the entire commit message instead of just
the subject or the body. Currently the tools have to join the entire
message from two parts when they are just interested in the entire
message.

Regards,
Constantine

^ permalink raw reply

* Re: [EGIT PATCH 6/7] Add tags to the graphical history display.
From: Shawn O. Pearce @ 2008-11-11 18:32 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1226095664-13759-7-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Both the SWT (Eclipse) drawing and Swing versions are updated.
> The coloring and shapes are intentionally not the same as for gitk.

Despite my comments about the series, I really like the rendering
in SWT.  I'm looking forward to getting this in-tree, but I would
like to get a better definition of the Ref peeling API.
 
-- 
Shawn.

^ permalink raw reply

* Re: [EGIT PATCH 6/7] Add tags to the graphical history display.
From: Shawn O. Pearce @ 2008-11-11 18:28 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1226095664-13759-7-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -92,7 +104,64 @@ protected void drawText(final String msg, final int x, final int y) {
>  		g.drawString(msg, cellX + x, cellY + texty, true);
>  	}

My SWTPlotRenderer doesn't have this context line.  Am I missing
a patch in the series?
  
> +	@Override
> +	protected int drawLabel(int x, int y, Ref ref) {

-- 
Shawn.

^ permalink raw reply

* Re: [EGIT PATCH 5/7] Add a method to get refs by object Id
From: Shawn O. Pearce @ 2008-11-11 18:26 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1226095664-13759-6-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -952,6 +955,37 @@ public Ref peel(final Ref ref) {
>  	}
>  
>  	/**
> +	 * @return a map with all objects referenced by a peeled ref.
> +	 */
> +	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
> +		Map<String, Ref> allRefs = getAllRefs();
> +		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
> +		for (Ref ref : allRefs.values()) {
> +			if (ref == null)
> +				continue;

How did we get a null Ref inside the allRefs collection?

> +			if (!ref.isPeeled()) {
> +				ref = peel(ref);
> +				allRefs.put(ref.getOrigName(), ref);

Hmm.  Mutating a HashMap while you are traversing it with an Iterator
is *not* a good idea.  Its a ConcurrentModificationException waiting
to happen.

-- 
Shawn.

^ permalink raw reply

* Re: [EGIT PATCH 4/7] Handle peeling of loose refs.
From: Shawn O. Pearce @ 2008-11-11 18:23 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1226095664-13759-5-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -238,10 +235,19 @@ public ObjectId getObjectId() {
>  	 *         refer to an annotated tag.
>  	 */
>  	public ObjectId getPeeledObjectId() {
> +		if (peeledObjectId == ObjectId.zeroId())
> +			return null;
>  		return peeledObjectId;
>  	}
>  
>  	/**
> +	 * @return whether the Ref represents a peeled tag
> +	 */
> +	public boolean isPeeled() {
> +		return peeledObjectId != null && peeledObjectId != ObjectId.zeroId();
> +	}

Huh.  So peeledObjectId == ObjectId.zeroId() means what, exactly?
That we tried to peel this but we cannot because its not a tag?

What does a packed-refs record which has no peel data but which
is in a packed-refs with-peeled file have for peeledObjectId?
null or ObjectId.zeroId()?

> @@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
>  		}
>  	}
>  
> +	Ref peel(final Ref ref) {
> +		if (ref.isPeeled())
> +			return ref;

I think you mean something more like:

	if (ref.peeledObjectId != null)
		return ref;

as the ref is already peeled, or at least attempted.

> +		try {
> +			Object tt = db.mapObject(ref.getObjectId(), ref.getName());
> +			if (tt != null && tt instanceof Tag) {
> +				Tag t = (Tag)tt;
> +				while (t != null && t.getType().equals(Constants.TYPE_TAG))
> +					t = db.mapTag(t.getTag(), t.getObjId());
> +				if (t != null)
> +					ref.setPeeledObjectId(t.getObjId());
> +				else
> +					ref.setPeeledObjectId(null);
> +			} else
> +				ref.setPeeledObjectId(ref.getObjectId());
> +		} catch (IOException e) {
> +			// Serious error. Caller knows a ref should never be null
> +			ref.setPeeledObjectId(null);
> +		}
> +		return ref;

This whole block is simpler to write as:

	try {
		Object target = db.mapObject(ref.getObjectId(), ref.getName());
		while (target instanceof Tag) {
			final Tag tag = (Tag)target;
			ref.setPeeledObjectId(tag.getObjId());
			if (Constants.TYPE_TAG.equals(tag.getType()))
				target = db.mapObject(tag.getObjId(), ref.getName());
			else
				break;
		}
	} catch (IOException e) {
		// Ignore a read error.  Callers will also get the same error
		// if they try to use the result of getPeeledObjectId.
	}

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> index 26748e2..4d6e6fd 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -939,6 +939,19 @@ public String getBranch() throws IOException {
>  	}
>  
>  	/**
> +	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
> +	 * the peeled id is set to {@link ObjectId#zeroId()}

But applications never see ObjectId.zeroId() because you earlier defined
getPeeledObjectId() to return null in that case.  So why is this part of
the documentation relevant? 

> +	 * 
> +	 * @param ref
> +	 *            The ref to peel
> +	 * @return The same, an updated ref with peeled info or a new instance with
> +	 *         more information
> +	 */
> +	public Ref peel(final Ref ref) {
> +		return refs.peel(ref);

Can we tighten this contract?  Are we always going to update the
current ref in-place, or are we going to return the caller a new
Ref instance?  I'd like it to be consistent one way or the other.

If we are updating in-place then the caller needs to realize there
may be some cache synchronization issues when using multiple threads
to access the same Ref instances and peeling them.  I.e. they may
need to go through their own synchornization barrier to ensure the
processor caches are coherent.

Given that almost everywhere else we treat the Ref as immutable
I'm tempted to say this should always return a new Ref object if we
peeled; but return the parameter if the parameter is already peeled
(or is known to be unpeelable, e.g. a branch head).

-- 
Shawn.

^ permalink raw reply

* Re: problem with git gui on cygwin.
From: Alex Riesen @ 2008-11-11 18:08 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jim Jensen, git
In-Reply-To: <20081111170557.GI2932@spearce.org>

Shawn O. Pearce, Tue, Nov 11, 2008 18:05:57 +0100:
> Jim Jensen <jhjjhjjhj@gmail.com> wrote:
> > I have been trying to use git for a small project using cygwin.  I copied my
> > repository from one windows XP system to a Vista system using a usb drive.  On
> > the new system when I use "git gui" I get a pop up that says "Git directory not
> > found: .git"
> > 
> > This happens with git gui and gitk.  The command line programs I tried, git show
> > and git status appear to work.
> 
> Huh.  Sounds like the Tcl/Tk process isn't spawning the Cygwin git
> process in the right directory or something.  Weird that both git-gui
> and gitk are giving you errors; usually its git-gui that is being
> paranoid about the repository structure and gitk works just fine.
> 
> You can try starting `git gui --trace` and see if that tells you
> any more detail, but it doesn't show cwd so it may not be all that
> useful to help you debug it.
> 

Maybe the fact that wish is not a cygwin application (doesn't use use
cygwin1.dll) is a hint?

^ permalink raw reply

* Re: Install issues
From: H.Merijn Brand @ 2008-11-11 18:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <7vhc6e17fv.fsf@gitster.siamese.dyndns.org>

On Tue, 11 Nov 2008 09:47:00 -0800, Junio C Hamano <gitster@pobox.com>
wrote:

> Miklos Vajna <vmiklos@frugalware.org> writes:
> 
> > On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> >> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> >> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> >> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
> >>  	./test-sha1.sh
> >>  
> >>  check: common-cmds.h
> >> +	@`sparse </dev/null 2>/dev/null` || (\
> >> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> >> +	    echo "Did you mean 'make test' instead?" ;\
> >> +	    exit 1 )
> >>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
> >
> > Please read Documentation/SubmittingPatches, your patch lacks a signoff
> > and a commit message.
> 
> Heh, for something small and obvious like this, that's asking a tad too
> much, although a properly formatted message does reduce my workload and is
> appreciated.

Junio++

> I said "obvious" not in the sense that it is "obviously good".  It is
> obvious what issue the patch wants to address.
> 
> Having said that, it is far from clear if special casing "make check" like
> this is a good thing, though.  The crufts resulting from "Four extra lines
> won't hurt" kind of reasoning can accumulate and snowball.  Is reading the
> Makefile when your build fails in order to see if the target was what you
> really wanted to invoke (ideally, it should rater be "_before_ running
> make, reading the Makefile to find out what you want to run") a lost art
> these days?

Not at all, and for me it was clear from the start, so I typed 'make
test' and went ahead.

It was that I am just all to aware of the GNU world that I can easily
imagine other people making the same mistake, and just thought it
end-user-friendly to do as I proposed.

I'm by now way offended or scared away if you reject these kind of
patches

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

^ permalink raw reply

* Re: [EGIT PATCH 1/7] Test the origName part of the key vs the ref itself
From: Shawn O. Pearce @ 2008-11-11 18:05 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1226095664-13759-2-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -127,4 +129,12 @@ public void testDeleteEmptyDirs() throws IOException {
>  	private void assertExists(final boolean expected, final String name) {
>  		assertEquals(expected, new File(db.getDirectory(), name).exists());
>  	}
> +
> +	public void testRefKeySameAsOrigName() {
> +		Map<String, Ref> allRefs = db.getAllRefs();
> +		for (Entry<String, Ref> e : allRefs.entrySet()) {
> +			assertEquals(e.getKey(), e.getValue().getOrigName());
> +
> +		}
> +	}

Hmm, doesn't this have to go after "Keep original ref name ..."
so getOrigName() is actually defined?

-- 
Shawn.

^ permalink raw reply

* Re: Stgit and refresh-temp
From: Catalin Marinas @ 2008-11-11 17:59 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <20081107054419.GA27146@diana.vm.bytemark.co.uk>

2008/11/7 Karl Hasselström <kha@treskal.com>:
> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>
>> I hit a case when refreshing a buried patch that needed a merge
>> conflict sorted out. I'm unable to recover out of the state.
>
> Hmm, so what you're saying is basically that you did something with
> "stg refresh -p" that caused a merge conflict, and that messed things
> up so that you needed to run "stg repair". Is that right?
>
> Have you been able to reproduce it? (I would like to add the failing
> case to the test suite.)

Could be related to this - if I run 'stg goto some-patch' and it fails
with a conflict, the HEAD points to the previous patch though the
stack has the conflicting patch as empty (which is normal) and the
conflicts in the index. Anything after that says HEAD and top not
equal and 'stg repair' is needed.

-- 
Catalin

^ permalink raw reply

* Re: Install issues
From: Junio C Hamano @ 2008-11-11 17:47 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: H.Merijn Brand, git
In-Reply-To: <20081110175123.GV24201@genesis.frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
>> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>  	./test-sha1.sh
>>  
>>  check: common-cmds.h
>> +	@`sparse </dev/null 2>/dev/null` || (\
>> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>> +	    echo "Did you mean 'make test' instead?" ;\
>> +	    exit 1 )
>>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>
> Please read Documentation/SubmittingPatches, your patch lacks a signoff
> and a commit message.

Heh, for something small and obvious like this, that's asking a tad too
much, although a properly formatted message does reduce my workload and is
appreciated.

I said "obvious" not in the sense that it is "obviously good".  It is
obvious what issue the patch wants to address.

Having said that, it is far from clear if special casing "make check" like
this is a good thing, though.  The crufts resulting from "Four extra lines
won't hurt" kind of reasoning can accumulate and snowball.  Is reading the
Makefile when your build fails in order to see if the target was what you
really wanted to invoke (ideally, it should rater be "_before_ running
make, reading the Makefile to find out what you want to run") a lost art
these days?

^ permalink raw reply

* Re: [PATCH (GIT-GUI)] git-gui: Request blame metadata in utf-8.
From: Shawn O. Pearce @ 2008-11-11 17:35 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200811091853.09498.angavrilov@gmail.com>

Alexander Gavrilov <angavrilov@gmail.com> wrote:
> The blame builtin now supports automatic conversion of
> metadata encoding. By default it is converted to the
> character set specified by i18n.logoutputencoding.
> 
> Since gui blame expects the data in utf-8, it is
> necessary to specify the desired encoding directly.
> An old version of the blame command will simply
> ignore the option.

Thanks; all 3 applied.
 
-- 
Shawn.

^ permalink raw reply

* Re: JGIT: discuss: diff/patch implementation
From: Sverre Rabbelier @ 2008-11-11 17:31 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Raimund Bauer, Junio C Hamano, Francis Galiegue, Git Mailing List,
	Robin Rosenberg
In-Reply-To: <20081111171816.GK2932@spearce.org>

On Tue, Nov 11, 2008 at 18:18, Shawn O. Pearce <spearce@spearce.org> wrote:
> I don't know how the Eclipse foundation feels about distributing
> LGPL in the IDE.  One of our major reasons for going with a BSD
> license on JGit was so the Eclipse Git team provider plugin could be
> distributed alongside the CVS team provider, as part of the basic IDE
> team provider package.  We're clearly not ready for that wide of a
> distribution, but it was a goal Robin and I set out for the project.

Why not keep that as a goall? For now, you can stick with one of the
existing LGPL implementations, later, when you want to have JGit
distributed with Eclipse, you (or Johanness Schindelin when he has the
time) write up your own Java version of it and license it BSD

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] git push: Interpret $GIT_DIR/branches in a Cogito compatible way
From: Junio C Hamano @ 2008-11-11 17:25 UTC (permalink / raw)
  To: Martin Koegler; +Cc: git
In-Reply-To: <20081111083704.GA4075@auto.tuwien.ac.at>

mkoegler@auto.tuwien.ac.at (Martin Koegler) writes:

> Nobody else seemd to be interessted in this topic, so its difficult to
> start a discussion.
>
> I would suggest to queue it in pu for some time and wait for other
> feedback.
>
>> While I'd personally agree matching with whatever cg-push used to do might
>> make the most sense in the end, I am not sure changing of behaviour
>> abruptly like this is a good idea.
>
> I'm open for any suggestion, how to make this change smoothly. I
> think, that doing such a behaviour change should be OK for 1.6.1 or
> 1.6.2:
>
> Until v1.5.4 (9 month ago), git-push with # was totally broken. Daniel
> Barkalow fixed the fetch case in v1.5.4. As a side effect, git-push no
> longer fails, but ignores everything after #. So in my option, the
> current behaviour was created by chance.
>
>> I am also not so sure url#branch is illogical; I'd suggest dropping that
>> line from the commit log message in any case.
>
> OK, I'll drop it in the next version.

Thanks for detailed explanation of the background.

Everything you said makes sense.  You convinced me that this is just a
bugfix that does not warrant a deprecation notice nor waiting for a flag
day release.  I'm even inclined to say that we should apply this
eventually to 'maint' (which means we'd fork a topic branch from maint,
merge that first to next and then to master and eventually to maint to be
included in 1.6.0.X, while the change will go to 1.6.1 as well).

Does the patch apply to 'maint' right now?

^ permalink raw reply

* Re: force a merge conflict
From: Caleb Cushing @ 2008-11-11 17:24 UTC (permalink / raw)
  To: Constantine Plotnikov; +Cc: git
In-Reply-To: <85647ef50811110914w7e1d36afh23b43b44ba0e2cf3@mail.gmail.com>

> Is not the following option of the merge just for it?
>
>  --no-commit
>     Perform the merge but pretend the merge failed and do not
>  autocommit, to give the user a chance to inspect and further tweak the
>  merge result before committing.

yes and no... it doesn't auto-magically allow me to do what  a merge
conflict does. which is put together a diff. all it really did was not
commit the auto merge.

as far as --no-ff I'm not saying it commits the same way but I didn't
see any difference in the resulting diffs, which, imho, all that
matters is whether the resulting merged code is correct.


-- 
Caleb Cushing

^ permalink raw reply

* Re: JGIT: discuss: diff/patch implementation
From: Shawn O. Pearce @ 2008-11-11 17:18 UTC (permalink / raw)
  To: Raimund Bauer
  Cc: Junio C Hamano, Francis Galiegue, Git Mailing List,
	Robin Rosenberg
In-Reply-To: <1226398000.7541.11.camel@minastirith.xtradesoft.lan>

Raimund Bauer <ray007@gmx.net> wrote:
> On Mon, 2008-11-10 at 12:50 -0800, Junio C Hamano wrote:
> > Francis Galiegue <fg@one2team.net> writes:
> > 
> > > A very nice git feature, without even going as far as merges, is the cherry 
> > > pick feature.
> > 
> > I thought cherry-picking needs to be done in terms of 3-way merge, not
> > diff piped to patch, for correctness's sake.
> 
> What about http://sourceforge.net/projects/jlibdiff ?
> Maybe a bit old, but claims to have diff3 and is under LGPL.

I hadn't looked at that library before.

We've generally tried to avoid LGPL diff implementations, but partly
because any I found were ports from a GPL C based code tree to Java,
but then the guy who did the port went and changed the license
to LGPL.  Slightly dubious if you ask me.  ;-)

LGPL plays nicely with BSD, especially in Java where runtime
relinking is possible.  But it does screw with jgit.pgm's little
idea of "shove *everything* into a single shell script", as then
its not runtime re-linkable by the user.

I don't know how the Eclipse foundation feels about distributing
LGPL in the IDE.  One of our major reasons for going with a BSD
license on JGit was so the Eclipse Git team provider plugin could be
distributed alongside the CVS team provider, as part of the basic IDE
team provider package.  We're clearly not ready for that wide of a
distribution, but it was a goal Robin and I set out for the project.

-- 
Shawn.

^ permalink raw reply

* Re: git commit -v does not removes the patch
From: Junio C Hamano @ 2008-11-11 17:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <20081111102914.GA30330@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> But I have to wonder if there is some more robust solution. It seems
> like this can have false positives if you include diff output in your
> commit message, and a potential false negative if you delete the newline
> (e.g., delete everything up to "diff --git", making it the first line).

Actually, I recall (from my googling quite some time ago) people noticing
this, but they learnt to live with it.

We may want to change this.  We can say "# Everything under this line is
deleted." at the beginning of the "#" block we produce in the commit log
message editor, replacing the "Lines starting with '#' will be ignored, "
we currently have.  When reading back the editor result, make "git commit
-v" scan for the "# Everything ..." line.  We remove it and everything
that follows, but we do not touch anything above that line (including the
ones that begin with "diff" or "#") except the usual trailing whitespace
removal.  That way, people can leave a sample shell session with root
prompt, and sample diff, in their message.

If we do not see "# Everything ..." when we read it back, we can do what
we currently do as a fallback.

^ permalink raw reply

* Re: JGIT: discuss: diff/patch implementation
From: Shawn O. Pearce @ 2008-11-11 17:13 UTC (permalink / raw)
  To: Rogan Dawes; +Cc: Francis Galiegue, Git Mailing List, Robin Rosenberg
In-Reply-To: <491933DF.3060307@dawes.za.net>

Rogan Dawes <lists@dawes.za.net> wrote:
> Francis Galiegue wrote:
>>
>> For this to be doable from within the Eclipse Git plugin, a diff/patch  
>> implementation needs to be found, in a license compatible with the 
>> current JGit license (3-clause BSD, as far as I can tell). Or a new 
>> implementation can be rewritten from scratch, of course.
>
> Shouldn't Eclipse already *have* a diff/patch implementation, for its  
> other "team work" plugins?

Err, uhm, sort of.

Eclipse has patch available as an internal API, but it is exposed
in the UI for any team provider (or no team provider at all) to
use to apply patches to a project in the workspace.

The team provider API assumes the VCS implementation has its own
diff, and therefore the diff implementation inside Eclipse is only
used for the native Compare view

I've dug around that part of the text compare plugin and its mostly
internal APIs, and mostly still low-level LCS generation from
arbitrary object input.  It doesn't seem well suited to producing
fast diffs of text.

Its under the EPL.  We could take the code and simplify it down,
but I think by that point we'd mostly just want to rewrite it, or
use a different library anyway.  At which point we wouldn't want
to bring in the EPL baggage if we can have a BSD implementation.

So yea, there's some implementation in there, but its not easy to
use or get to...

-- 
Shawn.

^ 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