Git development
 help / color / mirror / Atom feed
* Re: [JGIT] Request for help
From: Jonas Fonseca @ 2009-09-04 12:41 UTC (permalink / raw)
  To: Mark Struberg; +Cc: git, Gabe McArthur
In-Reply-To: <554991.93608.qm@web27807.mail.ukl.yahoo.com>

On Fri, Sep 4, 2009 at 03:33, Mark Struberg<struberg@yahoo.de> wrote:
>> From: Gabe McArthur <gabriel.mcarthur@gmail.com>
> >
>> I'll try to submit a full patch later, using your
>> conventions.

I have a question as well:

Support for using find bug is part of the Eclipse configuration (see
org.spearce.jgit/findBugs/), and I know that there's a find bug plugin
for Maven. From looking at sonatype's JGit repositories it is not
integrated. Have you managed to include it?

-- 
Jonas Fonseca

^ permalink raw reply

* Re: [PATCH 5/8] Allow programs to not depend on remotes having urls
From: Johannes Schindelin @ 2009-09-04 12:45 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909032213290.28290@iabervon.org>

Hi,

On Thu, 3 Sep 2009, Daniel Barkalow wrote:

> For fetch and ls-remote, which use the first url of a remote, have
> transport_get() determine this by passing a remote and passing NULL
> for the url. For push, which uses every url of a remote, use each url
> in turn if there are any, and use NULL if there are none.
> 
> This will allow the transport code to do something different if the
> location is not specified with a url.

For the record: you did not convince me.  Not that you tried, to...

Ciao,
Dscho

^ permalink raw reply

* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 12:47 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git, Gabe McArthur
In-Reply-To: <2c6b72b30909040541t4c781378g3f35aeb70021050c@mail.gmail.com>

as an old saying tells us: how to climb a mountain? step after step! ;)

I suggest we create a fresh branch based on the Shawns current version and add all the features incrementally.

1.) move the directory structure over to mavens std layout
2.) create the scm section and try releases
3.) improve site generation and documentation
tbc


LieGrue,
strub


--- On Fri, 9/4/09, Jonas Fonseca <jonas.fonseca@gmail.com> wrote:

> From: Jonas Fonseca <jonas.fonseca@gmail.com>
> Subject: Re: [JGIT] Request for help
> To: "Mark Struberg" <struberg@yahoo.de>
> Cc: git@vger.kernel.org, "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> Date: Friday, September 4, 2009, 2:41 PM
> On Fri, Sep 4, 2009 at 03:33, Mark
> Struberg<struberg@yahoo.de>
> wrote:
> >> From: Gabe McArthur <gabriel.mcarthur@gmail.com>
> > >
> >> I'll try to submit a full patch later, using your
> >> conventions.
> 
> I have a question as well:
> 
> Support for using find bug is part of the Eclipse
> configuration (see
> org.spearce.jgit/findBugs/), and I know that there's a find
> bug plugin
> for Maven. From looking at sonatype's JGit repositories it
> is not
> integrated. Have you managed to include it?
> 
> -- 
> Jonas Fonseca
> 


      

^ permalink raw reply

* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Gerrit Pape @ 2009-09-04 12:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0909041434310.4605@intel-tinevez-2-302>

On Fri, Sep 04, 2009 at 02:35:00PM +0200, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 4 Sep 2009, Gerrit Pape wrote:
> 
> > On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > > -- snipsnap --
> > > [PATCH] git help -a: do not look for a repository
> > 
> > Perfect, thanks.
> > 
> > Acked-by: Gerrit Pape <pape@smarden.org>
> 
> Hmm... I haven't checked if 'git help -a' wants to discover the (possibly 
> repository-specific) aliases.  Have you?

Yes, it doesn't.  Regards, Gerrit.

^ permalink raw reply

* Re: [PATCH 2/2] git-completion.bash: prevent 'git help' from searching for git repository
From: Johannes Schindelin @ 2009-09-04 12:52 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <20090904124904.22288.qmail@103db479e31ab7.315fe32.mid.smarden.org>

Hi,

On Fri, 4 Sep 2009, Gerrit Pape wrote:

> On Fri, Sep 04, 2009 at 02:35:00PM +0200, Johannes Schindelin wrote:
> > Hi,
> > 
> > On Fri, 4 Sep 2009, Gerrit Pape wrote:
> > 
> > > On Fri, Sep 04, 2009 at 12:22:36PM +0200, Johannes Schindelin wrote:
> > > > -- snipsnap --
> > > > [PATCH] git help -a: do not look for a repository
> > > 
> > > Perfect, thanks.
> > > 
> > > Acked-by: Gerrit Pape <pape@smarden.org>
> > 
> > Hmm... I haven't checked if 'git help -a' wants to discover the 
> > (possibly repository-specific) aliases.  Have you?
> 
> Yes, it doesn't.  Regards, Gerrit.

Thanks.

Junio, I am serious about this patch (i.e. it is meant for inclusion now).  
Want me to resend?

Ciao,
Dscho

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Tay Ray Chuan @ 2009-09-04 13:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysgit
In-Reply-To: <7viqfzvwf1.fsf@alter.siamese.dyndns.org>

Hi,

On Fri, Sep 4, 2009 at 6:25 PM, Junio C Hamano<gitster@pobox.com> wrote:
> codesite-noreply@google.com writes:
>
>> Status: New
>> Owner: ----
>>
>> New issue 323 by bjelli: Can't clone over http
>> http://code.google.com/p/msysgit/issues/detail?id=323

Junio, thanks for bringing this issue to the list's and my attention.

> This does not seem to be an msysgit issue.  Even on a Linux host, v1.6.2.5
> seems to work Ok but 'maint', 'master', nor 'next' does not clone this one
> correctly.

Releases including and after v1.6.4 will have this issue:

>> error: Unable to verify pack 382c25c935b744e909c749532578112d72a4aff9 is
>> available
>> error: Unable to find 0a41ac04d56ccc96491989dc71d9875cd804fc6b under
>> http://github.com/tekkub/addontemplate.git

The issue at hand is due to git checking the http repository for the
pack file before commencing the transfer; failing which, the transfer
aborts.

Right now, git chokes on the 500 error that github.com gives it, which
shouldn't be the case, even though that's a weird response.

--
Cheers,
Ray Chuan

-- >8 --
Subject: [PATCH] http.c: clarify missing-pack-check

Abort the pack transfer only if the pack is not available in the HTTP-
served repository; otherwise, allow the transfer to continue, even if
the check failed.

This addresses an issue raised by bjelli:

  http://code.google.com/p/msysgit/issues/detail?id=323

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 http.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index 5926c5b..cba7e9a 100644
--- a/http.c
+++ b/http.c
@@ -864,6 +864,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
 static int fetch_pack_index(unsigned char *sha1, const char *base_url)
 {
 	int ret = 0;
+	int result;
 	char *hex = xstrdup(sha1_to_hex(sha1));
 	char *filename;
 	char *url;
@@ -874,11 +875,14 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)
 	strbuf_addf(&buf, "objects/pack/pack-%s.pack", hex);
 	url = strbuf_detach(&buf, 0);

-	if (http_get_strbuf(url, NULL, 0)) {
-		ret = error("Unable to verify pack %s is available",
+	result = http_get_strbuf(url, NULL, 0);
+	if (result == HTTP_MISSING_TARGET) {
+		ret = error("Unable to find pack %s",
 			    hex);
 		goto cleanup;
-	}
+	} else if (result && http_is_verbose)
+		fprintf(stderr, "Unable to verify pack %s is available\n",
+			hex);

 	if (has_pack_index(sha1)) {
 		ret = 0;
--
1.6.4.2

^ permalink raw reply related

* tracking branch for a rebase
From: Pete Wyckoff @ 2009-09-04 13:54 UTC (permalink / raw)
  To: git

I received an interesting question today.

If "git pull" knows what remote tracking brach to pull from,
why doesn't "git rebase" automatically know I want to rebase
on top of my remote tracking branch?

He had done a "git remote update", reviewed the changes, and now
wants to rebase his changes on top of the new upstream.

I told him about git pull --rebase, and branch.name.rebase,
but didn't have an easy answer to the "what is my remote tracking
branch" question in this case.

All the information is in git config branch.name.{remote,merge},
but I can't find anything that just answers that question.  Nor
did I have a good answer for why "git rebase" doesn't just default
to rebasing the current branch on top of the tracking branch.

Thoughts?

		-- Pete

^ permalink raw reply

* Re: tracking branch for a rebase
From: Michael J Gruber @ 2009-09-04 14:31 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git
In-Reply-To: <20090904135414.GA3728@honk.padd.com>

Pete Wyckoff venit, vidit, dixit 04.09.2009 15:54:
> I received an interesting question today.
> 
> If "git pull" knows what remote tracking brach to pull from,
> why doesn't "git rebase" automatically know I want to rebase
> on top of my remote tracking branch?
> 
> He had done a "git remote update", reviewed the changes, and now
> wants to rebase his changes on top of the new upstream.
> 
> I told him about git pull --rebase, and branch.name.rebase,
> but didn't have an easy answer to the "what is my remote tracking
> branch" question in this case.

I guess he knows that branch if he reviewed it... More seriously,

git for-each-ref --format='%(upstream)' refs/heads/hisbranch

will return the upstream of hisbranch.

> 
> All the information is in git config branch.name.{remote,merge},
> but I can't find anything that just answers that question.  Nor
> did I have a good answer for why "git rebase" doesn't just default
> to rebasing the current branch on top of the tracking branch.

Maybe because nobody has cared. Doing a "git pull --rebase" is not
equivalent (it may fetch more refs), but close enough for many people.

There was an initiative by Dscho a while ago to introduce a shortcut for
the upstream, which was dwarfed by the lack reserved characters for
refnames. Maybe 1.7 time frame would be a good time to change this?

Making this the default for rebase without arguments may meet some
objections (oh no, I didn't mean to rebase), but I guess it's worth trying.

Michael

^ permalink raw reply

* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 14:49 UTC (permalink / raw)
  To: Douglas Campos; +Cc: Jonas Fonseca, git, Gabe McArthur
In-Reply-To: <ed88cb980909040744k2fa372fapb7ee457c745b9aa0@mail.gmail.com>

Hi Douglas!

http://github.com/sonatype/JGit

The branch will be called mavenizing or so.

Will post this after I got the tests running.

LieGrue,
strub

--- On Fri, 9/4/09, Douglas Campos <douglas@theros.info> wrote:

> From: Douglas Campos <douglas@theros.info>
> Subject: Re: [JGIT] Request for help
> To: "Mark Struberg" <struberg@yahoo.de>
> Cc: "Jonas Fonseca" <jonas.fonseca@gmail.com>, git@vger.kernel.org, "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> Date: Friday, September 4, 2009, 4:44 PM
> On Fri, Sep 4, 2009
> at 9:47 AM, Mark Struberg <struberg@yahoo.de>
> wrote:
> 
> 
> as an old saying tells us: how to climb a mountain? step
> after step! ;)
> 
> 
> 
> I suggest we create a fresh branch based on the Shawns
> current version and add all the features incrementally.
> 
> 
> 
> please point out where this branch will happen, I want to
> give some help too.
> 
> 


      

^ permalink raw reply

* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special  case
From: Daniel Barkalow @ 2009-09-04 15:40 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Junio C Hamano, git
In-Reply-To: <fabb9a1e0909032229k5e6e2ed5mc11e8ff9c16dfcc0@mail.gmail.com>

On Fri, 4 Sep 2009, Sverre Rabbelier wrote:

> Heya,
> 
> On Fri, Sep 4, 2009 at 04:13, Daniel Barkalow<barkalow@iabervon.org> wrote:
> > Instead of trying to make http://, https://, and ftp:// URLs
> > indicative of some sort of pattern of transport helper usage, make
> > them a special case which runs the "curl" helper, and leave the
> > mechanism by which arbitrary helpers will be chosen entirely to future
> > work.
> 
> I'm sorry, I missed a few emails I think :(. Would you mind explaining
> why we chose to special-case the curl helpers instead of the symlink
> scheme?

It turns out that the method used to form URLs that use a helper doesn't 
generalize well to other cases, because it interferes with the ssh-style 
locations. Instead, some different mechanism needs to be made up to handle 
arbitrary handlers that git doesn't know about. Since we want to keep 
supporting "http://something", that'll have to be a special case anyway, 
and so we might as well handle it by having git know what helpers to use 
for things that we've always supported, and use a single descriptive name 
for the helper that handles that collection of URLs.

As of this version, the idea is that there will be three ways helpers get 
selected:

 - git selects a helper based on the URL being something traditionally 
   supported internally; that is, git recognizes the URL and knows what to 
   run, if possible, to handle it

 - git uses the "vcs" option if it is set

 - something with the URL that we don't understand well enough yet to 
   design, but which doesn't seem to be possible to fit in as a single 
   rule with the first item.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Commit to wrong branch. How to fix?
From: Howard Miller @ 2009-09-04 15:54 UTC (permalink / raw)
  To: git@vger.kernel.org

I commited to the wrong branch and I can't figure out what to do. To  
make matters worse I then did 'git reset HEAD^' which has made things  
much worse. It didn't remove the commit and now I can't change  
branches. I'm utterly confused. Any help much appreciated!

Moral - use git status liberally and read it carefully before doing  
anything. A 'git undo" command would be great is someone is feeling  
generous :-) 

^ permalink raw reply

* Re: Commit to wrong branch. How to fix?
From: Michael J Gruber @ 2009-09-04 16:19 UTC (permalink / raw)
  To: Howard Miller; +Cc: git@vger.kernel.org
In-Reply-To: <88A0A7C2-7DCB-45A3-B196-BED2A8AC9405@googlemail.com>

Howard Miller venit, vidit, dixit 04.09.2009 17:54:
> I commited to the wrong branch and I can't figure out what to do. To  
> make matters worse I then did 'git reset HEAD^' which has made things  
> much worse. It didn't remove the commit and now I can't change  
> branches. I'm utterly confused. Any help much appreciated!
> 
> Moral - use git status liberally and read it carefully before doing  
> anything. A 'git undo" command would be great is someone is feeling  
> generous :-) 

Whatever happens, don't panic ;)

Let's say "geesh" is the branch on which you committed by mistake, and
which you have reset.

git reflog geesh

which show you what has happened to that branch lately. In particular,
it will list the "lost" commit. (Most probably it is the same as geesh@{1}.)

git tag sigh sha1ofthatcommit

will assign the tag "sigh", so that it won't get lost by doing "git gc"
or such. Now you can lean back!

Next step is committing "sigh" to the right branch. Depends on how you
arrived at that commit. Did you commit the complete tree you wanted, or
did you apply a change to geesh which you rather had wanted applied to
some other branch?

Michael

^ permalink raw reply

* Re: [PATCH] Do not scramble password read from .cvspass
From: Junio C Hamano @ 2009-09-04 16:21 UTC (permalink / raw)
  To: Dirk Hoerner; +Cc: git list, pascal
In-Reply-To: <4AA100E8.9030806@obry.net>

Pascal Obry <pascal@obry.net> writes:

> Passwords stored in .cvspass are already scrambled, we do not
> want to scramble them twice. Only passwords read from the
> command line are scrambled.
>
> This fixes a regression introduced by:
> b2139dbd72d16e40eddfd5b9ad1314703b39fe65
>
> Signed-off-by: Pascal Obry <pascal@obry.net>
> ---

Thanks, Pascal.  This bug is not in any tagged release yet, and I am happy
you caught it before -rc0 ;-)

Dirk, does the patch look Ok to you?

>  git-cvsimport.perl |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 593832d..c5cdcae 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -238,7 +238,10 @@ sub conn {
>                 }
>                 my $rr = ":pserver:$user\@$serv:$port$repo";
>
> -               unless ($pass) {
> +               if ($pass) {
> +                   $pass = $self->_scramble($pass);
> +               } else
> +               {
>                         open(H,$ENV{'HOME'}."/.cvspass") and do {
>                                 #
> :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah<Z
>                                 while (<H>) {
> @@ -253,8 +256,6 @@ sub conn {
>                         };
>                 }
>
> -               $pass = $self->_scramble($pass);
> -
>                 my ($s, $rep);
>                 if ($proxyhost) {
>
> --
> 1.6.4.2.253.g0b1fac

^ permalink raw reply

* Re: [PATCH 0/8] VCS helpers
From: Daniel Barkalow @ 2009-09-04 16:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johan Herland
In-Reply-To: <7v63bz198j.fsf@alter.siamese.dyndns.org>

On Fri, 4 Sep 2009, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > This is the next version of the db/vcs-helper series in pu.
> 
> Thanks.
> 
> > The first patch is new, a rework of the remote-curl build to produce
> > "remote-curl" and call it as a special case for the sorts of URLs that
> > we accept as indicating something that it now handled by this helper.
> >
> > The series is rebased onto current next, with some conflicts resolved.
> 
> Because the theme of the topic does not have anything to do with fixing
> the deepening of shallow clones nor giving an extended error message from
> non-fast-forward git-push, I queued the series after reverse-rebasing onto
> old db/vcs-helper~8, in order to keep the topic branch pure, instead of
> merging unrelated topics from maint, master or next into it.  The result
> merged in 'pu' obviously has to match what you expected by applying the
> patches on top of 'next', and I am reasonably sure it does.

I'd thought that topics in pu were carried as based on next, particularly 
once they depend on something (e.g., the beginning of the series) in next. 
I suppose there's better options, but what do you do to find them? (Feel 
free to refer me to the "note from the maintainer" if it's there, but I 
don't remember that detail)

FWIW, there was a semantic mismerge in the original basing of this series 
on 07a4a3b496, which I finally fixed in this version; the code to handle 
NULL urls in builtin-fetch was after a new conversion of the url.

In any case, I think both the reverse-rebase and merge are correct.

> > Two patches have been dropped: a memory leak fix for code that was
> > removed entirely by the first patch, and the "mark" helper capability,
> > which is not needed (I believe) due to the "option" fast-import command.
> 
> Johan's cvs-helper series were depending on the previous iteration of this
> series, but I thought it is being rerolled, so I'd drop it from pu for now.

You could probably stick the "mark" patch into the start of the cvs-helper 
series until it gets rerolled if you want to keep that series in pu 
meanwhile; it was at the end of the series, and doesn't have subtle 
interactions or interact with anything that's changed.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [JGIT PATCH v2 1/3] Allow RefUpdate.setExpectedOldObjectId to accept RevCommit
From: Shawn O. Pearce @ 2009-09-04 16:22 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, Shawn O. Pearce

RevCommit overrides .equals() such that it only implements a
reference equality test.  If the expected old ObjectId was set
by the application to a RevCommit instance, it would always fail,
resulting in LOCK_FAILURE.  Instead use AnyObject.equals() to compare
the value, ignoring the possibly overloaded equals in RevCommit.

Signed-off-by: Shawn O. Pearce <sop@google.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |   52 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |    2 +-
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 800c0a4..a8ccf43 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -45,6 +45,7 @@
 
 import org.spearce.jgit.lib.RefUpdate.Result;
 import org.spearce.jgit.revwalk.RevCommit;
+import org.spearce.jgit.revwalk.RevWalk;
 
 public class RefUpdateTest extends RepositoryTestCase {
 
@@ -397,6 +398,57 @@ public void testUpdateRefLockFailureWrongOldValue() throws IOException {
 	}
 
 	/**
+	 * Try modify a ref forward, fast forward, checking old value first
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefForwardWithCheck1() throws IOException {
+		ObjectId ppid = db.resolve("refs/heads/master^");
+		ObjectId pid = db.resolve("refs/heads/master");
+
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(ppid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update);
+		assertEquals(ppid, db.resolve("refs/heads/master"));
+
+		// real test
+		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
+		updateRef2.setExpectedOldObjectId(ppid);
+		updateRef2.setNewObjectId(pid);
+		Result update2 = updateRef2.update();
+		assertEquals(Result.FAST_FORWARD, update2);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
+	/**
+	 * Try modify a ref forward, fast forward, checking old commit first
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefForwardWithCheck2() throws IOException {
+		ObjectId ppid = db.resolve("refs/heads/master^");
+		ObjectId pid = db.resolve("refs/heads/master");
+
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(ppid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update);
+		assertEquals(ppid, db.resolve("refs/heads/master"));
+
+		// real test
+		RevCommit old = new RevWalk(db).parseCommit(ppid);
+		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
+		updateRef2.setExpectedOldObjectId(old);
+		updateRef2.setNewObjectId(pid);
+		Result update2 = updateRef2.update();
+		assertEquals(Result.FAST_FORWARD, update2);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
+	/**
 	 * Try modify a ref that is locked
 	 *
 	 * @throws IOException
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 69399ec..8dffed2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -466,7 +466,7 @@ private Result updateImpl(final RevWalk walk, final Store store)
 			if (expValue != null) {
 				final ObjectId o;
 				o = oldValue != null ? oldValue : ObjectId.zeroId();
-				if (!expValue.equals(o))
+				if (!AnyObjectId.equals(expValue, o))
 					return Result.LOCK_FAILURE;
 			}
 			if (oldValue == null)
-- 
1.6.4.2.395.ge3d52

^ permalink raw reply related

* [JGIT PATCH v2 3/3] Fix DirCache.findEntry to work on an empty cache
From: Shawn O. Pearce @ 2009-09-04 16:22 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, Shawn O. Pearce
In-Reply-To: <1252081365-2335-2-git-send-email-spearce@spearce.org>

If the cache has no entries, we want to return -1 rather than throw
ArrayIndexOutOfBoundsException.  This binary search loop was stolen
from some other code which contained a test before the loop to see if
the collection was empty or not, but we failed to include that here.

Flipping the loop around to a standard while loop ensures we test
the condition properly first.

Signed-off-by: Shawn O. Pearce <sop@google.com>
---
 .../spearce/jgit/dircache/DirCacheBasicTest.java   |    6 ++++++
 .../src/org/spearce/jgit/dircache/DirCache.java    |    6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
index b3097ac..4d737c0 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/dircache/DirCacheBasicTest.java
@@ -39,6 +39,7 @@
 
 import java.io.File;
 
+import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.RepositoryTestCase;
 
 public class DirCacheBasicTest extends RepositoryTestCase {
@@ -182,4 +183,9 @@ public void testBuildThenClear() throws Exception {
 		assertEquals(0, dc.getEntryCount());
 	}
 
+	public void testFindOnEmpty() throws Exception {
+		final DirCache dc = DirCache.newInCore();
+		final byte[] path = Constants.encode("a");
+		assertEquals(-1, dc.findEntry(path, path.length));
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
index bfb7925..9f0810a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
@@ -583,8 +583,6 @@ public void unlock() {
 	 *         information. If < 0 the entry does not exist in the index.
 	 */
 	public int findEntry(final String path) {
-		if (entryCnt == 0)
-			return -1;
 		final byte[] p = Constants.encode(path);
 		return findEntry(p, p.length);
 	}
@@ -592,7 +590,7 @@ public int findEntry(final String path) {
 	int findEntry(final byte[] p, final int pLen) {
 		int low = 0;
 		int high = entryCnt;
-		do {
+		while (low < high) {
 			int mid = (low + high) >>> 1;
 			final int cmp = cmp(p, pLen, sortedEntries[mid]);
 			if (cmp < 0)
@@ -603,7 +601,7 @@ else if (cmp == 0) {
 				return mid;
 			} else
 				low = mid + 1;
-		} while (low < high);
+		}
 		return -(low + 1);
 	}
 
-- 
1.6.4.2.395.ge3d52

^ permalink raw reply related

* [JGIT PATCH v2 2/3] Work around Sun javac compiler bug in RefUpdate
From: Shawn O. Pearce @ 2009-09-04 16:22 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, Shawn O. Pearce
In-Reply-To: <1252081365-2335-1-git-send-email-spearce@spearce.org>

Sun's javac, version 1.5.0_06 and also a Google internal fork of
OpenJDK 1.6.0 both apparently miscompile the for loop which is
looking for a conflicting ref name in the existing set of refs for
this repository.

Debugging this code showed the control flow to return LOCK_FAILURE
when startsWith returned false, which is highly illogical and the
exact opposite of what we have written here.

OpenJDK 1.6.0's javap tool was unable to disassemble the compiled
method.  Instead it simply failed to produce anything about
updateImpl.  So my remark about the code being compiled wrong
is only a guess based on how I observed the behavior, and not by
actually studying the resulting instructions.

Eclipse 3.4.2's JDT appears to have compiled the updateImpl method
correctly, and produces a working executable.  But this is a much
less common compiler to build Java libraries with.

Robin was able to successfully compile the ancestor version of this
code with both javac 1.5.0_19 and also 1.6.0_16, but not everyone
building JGit has these versions installed.

This refactoring to extract the name conflicting test out into its
own method appears to work around the Sun javac bug I observed above,
and the resulting class works correctly with any compiler tested.
The code is also more clear, so its a gain either way.

Signed-off-by: Shawn O. Pearce <sop@google.com>
---
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   26 +++++++++++++-------
 1 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 8dffed2..8226e10 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -449,15 +449,8 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		RevObject newObj;
 		RevObject oldObj;
 
-		int lastSlash = getName().lastIndexOf('/');
-		if (lastSlash > 0)
-			if (db.getRepository().getRef(getName().substring(0, lastSlash)) != null)
-				return Result.LOCK_FAILURE;
-		String rName = getName() + "/";
-		for (Ref r : db.getAllRefs().values()) {
-			if (r.getName().startsWith(rName))
-				return Result.LOCK_FAILURE;
-		}
+		if (isNameConflicting())
+			return Result.LOCK_FAILURE;
 		lock = new LockFile(looseFile);
 		if (!lock.lock())
 			return Result.LOCK_FAILURE;
@@ -490,6 +483,21 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		}
 	}
 
+	private boolean isNameConflicting() throws IOException {
+		final String myName = getName();
+		final int lastSlash = myName.lastIndexOf('/');
+		if (lastSlash > 0)
+			if (db.getRepository().getRef(myName.substring(0, lastSlash)) != null)
+				return true;
+
+		final String rName = myName + "/";
+		for (Ref r : db.getAllRefs().values()) {
+			if (r.getName().startsWith(rName))
+				return true;
+		}
+		return false;
+	}
+
 	private static RevObject safeParse(final RevWalk rw, final AnyObjectId id)
 			throws IOException {
 		try {
-- 
1.6.4.2.395.ge3d52

^ permalink raw reply related

* Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof   correctly
From: Junio C Hamano @ 2009-09-04 16:26 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4AA101BB.7010206@viscovery.net>

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

> After reading this explanation, I was worried that added blank lines that
> are at the end of a patch but apply in the middle of a file would be
> mis-attributed as blank lines at EOF.

The codepath this patch is about checks "does the hunk result in more
blank at the end of the place it applies to?".  There is a separate logic
that checks "does the hunk applies at the end of the file", which is the
topic of the codepath that is fixed by Patch #1.

^ permalink raw reply

* Re: [PATCH 4/8] Allow fetch to modify refs
From: Daniel Barkalow @ 2009-09-04 16:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0909041243420.4605@intel-tinevez-2-302>

On Fri, 4 Sep 2009, Johannes Schindelin wrote:

> Hi,
> 
> On Thu, 3 Sep 2009, Daniel Barkalow wrote:
> 
> > +	/**
> > +	 * Fetch the objects for the given refs. Note that this gets
> > +	 * an array, and should ignore the list structure.
> 
> This is not clear at all.  You should rather say "[...] and should not 
> look at, or set, the 'next' member of the refs".

That is a better wording, yes.

> > +	 *
> > +	 * If the transport did not get hashes for refs in
> > +	 * get_refs_list(), it should set the old_sha1 fields in the
> > +	 * provided refs now.
> 
> Not the "new_sha1"?

No, because get_refs_list() sets the old_sha1, and this isn't indicating 
anything different. The old/new thing is to indicate that the ref is 
changing value. What's happening here is that the ref isn't changing value 
but we didn't know what value it always (effectively) had until now.

> > +	 **/
> > +	int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
> > +
> > [...]
> > +	/** get_refs_list(), fetch(), and push_refs() can keep
> 
> The "/**" wants to have a line to itself.

Good point, thanks.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH 0/8] VCS helpers
From: Junio C Hamano @ 2009-09-04 17:03 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0909041141020.28290@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Fri, 4 Sep 2009, Junio C Hamano wrote:
>
>> Because the theme of the topic does not have anything to do with fixing
>> the deepening of shallow clones nor giving an extended error message from
>> non-fast-forward git-push, I queued the series after reverse-rebasing onto
>> old db/vcs-helper~8, in order to keep the topic branch pure, instead of
>> merging unrelated topics from maint, master or next into it.  The result
>> merged in 'pu' obviously has to match what you expected by applying the
>> patches on top of 'next', and I am reasonably sure it does.
>
> I'd thought that topics in pu were carried as based on next, particularly 
> once they depend on something (e.g., the beginning of the series) in next. 
> I suppose there's better options, but what do you do to find them? (Feel 
> free to refer me to the "note from the maintainer" if it's there, but I 
> don't remember that detail)

Oh, I was *not* complaining that you gave a patch based on 'next'.  I was
merely explaining what I did to your patch, in case somebody wonders why
the output of "log -p -8 db/vcs-helper" looks different from what you
posted on the list.  I also wanted you to verify the result.

> FWIW, there was a semantic mismerge in the original basing of this series 
> on 07a4a3b496, which I finally fixed in this version; the code to handle 
> NULL urls in builtin-fetch was after a new conversion of the url.
>
> In any case, I think both the reverse-rebase and merge are correct.

Thanks.  Actually, the fact your patch was based on 'next' did help me
verify the result of the rebase and the merge.

It is a good discipline to spend some extra effort to keep a topic pure if
and only if the other topics that the topic textually depends on were of
more dubious quality than the topioc itself.  In such a case, there is no
guarantee that they graduate ever, and the topic will be blocked without
major effort later.

It does not matter in practice when we are reasonably sure that other
topics that the topic depends on are likely to be already in when the
topic is completed.  In this particular case, many of the changes the
reverse rebase needed to remove are in fact already in master and some are
even in maint, and in retrospect, there was not much point doing this only
to risk mistakes.

In fact, I originally merged the commits whose changes overlapped and were
already in master (Nico's "deepening shallow", Matthieu's "give better
warning to non-fast-forward push", to name a few) on top of the part of
db/vcs-helper that was already in 'next', and then applied your patches on
top of the result, as a middle ground solution.  The topic would not have
been as pure as the result I pushed out, but it would still have been much
better than merging the whole master before applying the series.

The principle of keeping the topic branch pure in this case may fall into
my OCD ;-).

^ permalink raw reply

* Re: Commit to wrong branch. How to fix?
From: Erik Faye-Lund @ 2009-09-04 17:11 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Howard Miller, git@vger.kernel.org
In-Reply-To: <4AA13DF4.4050604@drmicha.warpmail.net>

On Fri, Sep 4, 2009 at 6:19 PM, Michael J
Gruber<git@drmicha.warpmail.net> wrote:
> Let's say "geesh" is the branch on which you committed by mistake, and
> which you have reset.
>
> git reflog geesh

That should be "git reflog show geesh", no?

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

^ permalink raw reply

* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special  case
From: Sverre Rabbelier @ 2009-09-04 17:14 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909041114440.28290@iabervon.org>

Heya,

On Fri, Sep 4, 2009 at 17:40, Daniel Barkalow<barkalow@iabervon.org> wrote:
> As of this version, the idea is that there will be three ways helpers get
> selected:

How does this interact with wanting to support
"hg://example.org/example" by adding 'git-remote-hg' to you path? Does
it make that harder, or is it just not part of this series? I really
do think we should support that, and only resort to "svn::" or such if
the url is ambiguous (e.g., with a 'https://' prefix, etc).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special  case
From: Junio C Hamano @ 2009-09-04 17:23 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Johannes Schindelin, Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909041114440.28290@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> It turns out that the method used to form URLs that use a helper doesn't 
> generalize well to other cases, because it interferes with the ssh-style 
> locations. Instead, some different mechanism needs to be made up to handle 
> arbitrary handlers that git doesn't know about. Since we want to keep 
> supporting "http://something", that'll have to be a special case anyway, 
> and so we might as well handle it by having git know what helpers to use 
> for things that we've always supported, and use a single descriptive name 
> for the helper that handles that collection of URLs.
>
> As of this version, the idea is that there will be three ways helpers get 
> selected:
>
>  - git selects a helper based on the URL being something traditionally 
>    supported internally; that is, git recognizes the URL and knows what to 
>    run, if possible, to handle it
>
>  - git uses the "vcs" option if it is set
>
>  - something with the URL that we don't understand well enough yet to 
>    design, but which doesn't seem to be possible to fit in as a single 
>    rule with the first item.

Thanks for a clear description.

I do not see that there is much difference between the above description
and what Dscho is advocating, and I do not see anything to get excited
about as Dscho seems to do.  In his world, hg:// or any URL that begins
with <unknown>:// wants to be a short-hand to name the helper, and the
third rule whose detail is unspecified in the above list could be
something like:

 - With an explicit <prefix-separator>, i.e.

        <helper-name> <prefix-separator> <any-string>

   tells the named helper git-remote-<helper-name> to interact with
   repository that it can find using <any-string>.  We do not interpret,
   nor guess from, what <any-string> is, in this case.

 - When all else fails, and the URL looks like <unknown>://<any-string>,
   we see if git-remote-<unknown> is available and give it the whole
   string (including the <unknown>::// part).

which means that what Dscho wants is already a subset of the future
direction planned for this series.

As to the "curl" indirection, if you consider the possiblity of someday
adding the transparently backward compatible cgi based server with updated
clients Gitney talked about, I am reasonably sure that we would want to
have a new helper, say http-cgi, and have interested people invoke it
using the "more explicit" escape hatch:

    $ git clone http-cgi::http://repo.or.cz/w/alt-git.git/

while others can continue using the walker via a plain http://repo.or.cz/
URL.  When http-cgi helper proves to be successful and everybody's server
upgrades, we might choose to swap the default, say in git 1.10.0 release,
while leaving the door open for people to choose the old helper via an
explicit curl::http://repo.or.cz/ URL.

In short, from where I sit, I do not see much disagreement in the
semantics and in the future direction between what Dscho is saying (unless
I again misunderstood what he said) and what this round wants to bring.

The only slight difference is that having an explicit excape hatch as the
foundation, that usually does not have to be spelled out but does allow
you to, keeps the concept cleaner, while keeping the usability of the end
result.

^ permalink raw reply

* Re: [JGIT] Request for help
From: Mark Struberg @ 2009-09-04 17:28 UTC (permalink / raw)
  To: Douglas Campos; +Cc: Jonas Fonseca, git, Gabe McArthur
In-Reply-To: <585278.66341.qm@web27802.mail.ukl.yahoo.com>

Hi!

Work has been done at 

http://github.com/sonatype/JGit/tree/mavenize

Please feel free to pull/fork and share your changes! I'd be happy to pull it in.

@Gabe: your patch seems to got filtered by the list, I think sharing such big things is easier by using github. Would be cool if you could help us!

LieGrue,
strub

--- On Fri, 9/4/09, Mark Struberg <struberg@yahoo.de> wrote:

> From: Mark Struberg <struberg@yahoo.de>
> Subject: Re: [JGIT] Request for help
> To: "Douglas Campos" <douglas@theros.info>
> Cc: "Jonas Fonseca" <jonas.fonseca@gmail.com>, git@vger.kernel.org, "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> Date: Friday, September 4, 2009, 4:49 PM
> Hi Douglas!
> 
> http://github.com/sonatype/JGit
> 
> The branch will be called mavenizing or so.
> 
> Will post this after I got the tests running.
> 
> LieGrue,
> strub
> 
> --- On Fri, 9/4/09, Douglas Campos <douglas@theros.info>
> wrote:
> 
> > From: Douglas Campos <douglas@theros.info>
> > Subject: Re: [JGIT] Request for help
> > To: "Mark Struberg" <struberg@yahoo.de>
> > Cc: "Jonas Fonseca" <jonas.fonseca@gmail.com>,
> git@vger.kernel.org,
> "Gabe McArthur" <gabriel.mcarthur@gmail.com>
> > Date: Friday, September 4, 2009, 4:44 PM
> > On Fri, Sep 4, 2009
> > at 9:47 AM, Mark Struberg <struberg@yahoo.de>
> > wrote:
> > 
> > 
> > as an old saying tells us: how to climb a mountain?
> step
> > after step! ;)
> > 
> > 
> > 
> > I suggest we create a fresh branch based on the
> Shawns
> > current version and add all the features
> incrementally.
> > 
> > 
> > 
> > please point out where this branch will happen, I want
> to
> > give some help too.
> > 
> > 
> 
> 
>       
> --
> 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
> 


      

^ permalink raw reply

* Re: [PATCH 1/8] Make the "traditionally-supported" URLs a special  case
From: Sverre Rabbelier @ 2009-09-04 17:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Daniel Barkalow, Johannes Schindelin, git
In-Reply-To: <7vy6ouk4io.fsf@alter.siamese.dyndns.org>

Heya,

On Fri, Sep 4, 2009 at 19:23, Junio C Hamano<gitster@pobox.com> wrote:
> In short, from where I sit, I do not see much disagreement in the
> semantics and in the future direction between what Dscho is saying (unless
> I again misunderstood what he said) and what this round wants to bring.

I think Dscho's main worry matches what I asked about earlier, will we
be able to say "hg://example.org" or not.

-- 
Cheers,

Sverre Rabbelier

^ 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