Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-gui: Allow users to set commit.signoff from options.
From: Shawn O. Pearce @ 2007-07-08 21:48 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: git
In-Reply-To: <20070706144654.12095.qmail@c8e1b1dc78de94.315fe32.mid.smarden.org>

Gerrit Pape <pape@smarden.org> wrote:
> Users may want to automatically sign-off any commit for a specific
> repository.  If they are mostly a git-gui user they should be able to
> view/set this option from within the git-gui environment, rather than
> needing to edit a raw text file on their local filesystem.

Sure.  But your patch to git-gui actually just lets the user set
the flag, but doesn't make git-gui honor it.  So the user can set
"Automatically Sign-Off" through git-gui but it will have no effect
within git-gui (git-gui doesn't use git-commit.sh, it has its own
pure-Tcl implementation).

I'm not applying this to git-gui, for the very same reason that
Junio already gave as to why he won't apply the git-commit.sh patch.

Within git-gui adding a signoff is either one mouse click (the
button on the toolbar), a single keystroke (Ctrl-S) or a menu action
(Commit->Signoff).  Three easy ways to insert the signoff line.
But it still needs to be a choice from the user, every time they
make a commit.
 
-- 
Shawn.

^ permalink raw reply

* Re: [RFC] Dynamic window size on repack?
From: Linus Torvalds @ 2007-07-08 21:39 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <alpine.LFD.0.999.0707081429500.31544@woody.linux-foundation.org>



On Sun, 8 Jul 2007, Linus Torvalds wrote:
>
> In other words, just look at how the variables "struct unpacked *array" 
> (the whole window array) and the "struct unpacked *n" (the "next entry" in 
> the array using a simple circular queue using "idx") are accessed.

Side note: a limit based on object sizes is likely a much better way to 
handle the window than just a "number of objects" thing ever was. Doing 
the size in number of objects was easier, and is fine for source code that 
tends to have a reasonably normal distribution of sized, but yeah, if you 
have a few really big objects with lots of history, then it's likely the 
wrong thing to do just because it can get really expensive.

		Linus

^ permalink raw reply

* Re: [PATCH] Enable "git rerere" by the config variable rerere.enabled
From: Shawn O. Pearce @ 2007-07-08 21:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0707061303450.4093@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Earlier, "git rerere" was enabled by creating the directory
> .git/rr-cache.  That is definitely not in line with most other
> features, which are enabled by a config variable.
> 
> So, check the config variable "rerere.enabled". If it is set
> to "false" explicitely, do not activate rerere, even if
> .git/rr-cache exists. This should help when you want to disable
> rerere temporarily.
...
> diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
> index f9791f6..578bef8 100644
> --- a/git-gui/lib/commit.tcl
> +++ b/git-gui/lib/commit.tcl
> @@ -331,9 +331,7 @@ A rescan will be automatically started now.
>  
>  	# -- Let rerere do its thing.
>  	#
> -	if {[file isdirectory [gitdir rr-cache]]} {
> -		catch {git rerere}
> -	}
> +	catch {git rerere}
>  
>  	# -- Run the post-commit hook.
>  	#

While I applaud the idea of someone else doing the dirty work for me,
the patch above to git-gui always runs rerere, even if the user has
disabled it with rerere.enabled=false.  Not very nice to do.

The master branch of git-gui will (very soon) have a change that
actually honors rerere.enabled, as described in the commit message
of git.git's b4372ef136b0a5a2c1dbd88a11dd72b478d0e0a5, aka the text
I quoted above.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC] Dynamic window size on repack?
From: Linus Torvalds @ 2007-07-08 21:35 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <20070708211606.GF4087@lavos.net>



On Sun, 8 Jul 2007, Brian Downing wrote:
> 
> I think what I'd like is an extra option to repack to limit window
> memory usage.  This would dynamically scale the window size down if it
> can't fit within the limit, then scale it back up once you're off of the
> nasty file.  This would let me repack my repository with --window=100
> and have it actually finish someday on the machines I have access to.
> The big file may not be as efficiently packed as possible, but I can
> live with that.
> 
> My question is, is this sane?  Does the repack algorithm depend on having
> a fixed window size to work?  I'd rather not look into implementing this
> if it's silly on the face of it.

It doesn't sound silly, and it should even be fairly easy. The window code 
is all in builtin-pack-objects.c (find_deltas()) and while it's currently 
coded for a constant-sized window, it shouldn't be too hard to free more 
old entries if you allocate one big one to make sure that the "array" 
thing doesn't grow to contain too much data.

In other words, just look at how the variables "struct unpacked *array" 
(the whole window array) and the "struct unpacked *n" (the "next entry" in 
the array using a simple circular queue using "idx") are accessed.

		Linus

^ permalink raw reply

* Re: [RFC] Dynamic window size on repack?
From: Dana How @ 2007-07-08 21:35 UTC (permalink / raw)
  To: Brian Downing; +Cc: git, danahow
In-Reply-To: <20070708211606.GF4087@lavos.net>

On 7/8/07, Brian Downing <bdowning@lavos.net> wrote:
> I have a CVS repository which is mostly sane, but has an approximately
> 20MB RTF file that has two hundred revisions or so.  (Thank you, Windows
> help.)
>
> Now, since this is old history, I want to make it as small as possible.
> The only problem is that when I use high --window values for repack,
> it goes along swimmingly until it gets to this file, at which point
> memory usage quickly rises to the point where I'm well into my swap file.
>
> I think what I'd like is an extra option to repack to limit window
> memory usage.  This would dynamically scale the window size down if it
> can't fit within the limit, then scale it back up once you're off of the
> nasty file.  This would let me repack my repository with --window=100
> and have it actually finish someday on the machines I have access to.
> The big file may not be as efficiently packed as possible, but I can
> live with that.
>
> My question is, is this sane?  Does the repack algorithm depend on having
> a fixed window size to work?  I'd rather not look into implementing this
> if it's silly on the face of it.

Sounds very sane to me.

It seems like you want something like this
(I've not referred to the code,  but there is a loop
 that could be modifed to include something like this):
  /* build list of delta candidates */
  tot = 0;
  for (i = 0; i < window; ++i ) {
    obj = objects_sorted_for_delta[here + i];
    tot += SIZE(obj);
    if ( tot > window_limit )
      break;
    /* insert obj in list of things to delta, or just try it here */
    ...
  }
  if ( i <= 1 )
    break/return;

window_limit could be set automatically like the variables
for the mmap windows are (no new options).
SIZE() should be defined to return the actual bytes consumed
by the object (I think for this it's uncompressed and undeltified,
but as I said I haven't looked at the code).

It would be better if the current list of objects in the window
were a FIFO.  Before each deltification attempt,  add objects
from the sort list until #objects > window or tot > window_limit.
After each attempt, drop off the object we were trying to delta.

I like your idea and think you should look into implementing it.
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

^ permalink raw reply

* [RFC] Dynamic window size on repack?
From: Brian Downing @ 2007-07-08 21:16 UTC (permalink / raw)
  To: git

I have a CVS repository which is mostly sane, but has an approximately
20MB RTF file that has two hundred revisions or so.  (Thank you, Windows
help.)

Now, since this is old history, I want to make it as small as possible.
The only problem is that when I use high --window values for repack,
it goes along swimmingly until it gets to this file, at which point
memory usage quickly rises to the point where I'm well into my swap file.

I think what I'd like is an extra option to repack to limit window
memory usage.  This would dynamically scale the window size down if it
can't fit within the limit, then scale it back up once you're off of the
nasty file.  This would let me repack my repository with --window=100
and have it actually finish someday on the machines I have access to.
The big file may not be as efficiently packed as possible, but I can
live with that.

My question is, is this sane?  Does the repack algorithm depend on having
a fixed window size to work?  I'd rather not look into implementing this
if it's silly on the face of it.

Thanks,
-bcd

^ permalink raw reply

* Re: [PATCH] Push code for transport library
From: Shawn O. Pearce @ 2007-07-08 21:14 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0707081608100.6977@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> wrote:
> This moves the code to call push backends into a library that can be
> extended to make matching fetch and push decisions based on the URL it
> gets, and which could be changed to have built-in implementations
> instead of calling external programs.
...
> +static const struct transport_ops git_transport = {
> +	.set_option = set_git_option,
> +	.push = git_transport_push
> +};

Interesting.

But can we please avoid that struct initialization syntax?
I build Git with a C compiler that doesn't understand it.  No, I
can't upgrade the compiler.  In the past we've largely been able
to avoid using this feature of C99.  If you look at the current
codebase this would again become the first usage of the feature,
as I have long since eliminated all of them.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 4/4] Add git-rewrite-commits
From: Sven Verdoolaege @ 2007-07-08 21:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Steven Grimm, git, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0707081920410.4248@racer.site>

On Sun, Jul 08, 2007 at 07:41:53PM +0100, Johannes Schindelin wrote:
> Hey, if skimo would be willing to work until it is a complete replacement 
> for filter-branch, and more, I will stop working on filter-branch at once.  

As I said, I'm willing to put some more time into this.
However, I didn't want to wait until I had all the bells and whistles
before sending something out.  If people don't like the direction
I'm going, then there is no point continuing and then I'll just
use it for myself.

I guess the major thing that is missing is --subdirectory-filter.
Anything else?

skimo

^ permalink raw reply

* Re: git cvsimport branches not consistent with CVS branches
From: Brian Downing @ 2007-07-08 21:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Robin Rosenberg, Gordon Heydon, git
In-Reply-To: <Pine.LNX.4.64.0707081246040.4248@racer.site>

On Sun, Jul 08, 2007 at 12:47:16PM +0100, Johannes Schindelin wrote:
> The only problem is that it is a misnomer: it is not fromcvs, but
> fromrcs, since you have to have access at the _files_. This is not
> always possible.

For the problem space I'm interested in (a /fast/ incremental CVS
importer) I think access to the RCS files is a must.  You can use inode
signatures to know which files you have to look at at all since the last
run, and you can abort your parse at the end of the delta section if no
new revisions appeared (rather than continuing to parse the deltatext
section, the largest part of the file).

For the repository I'm interested in (work), running a "real" cvs update
takes two or three minutes on a LAN.  Running a full rlog takes about
10 or so.  With the tricks above, I can import a single file change in
seconds instead.  (That includes the time to rsync the CVS repository
to my local mirror.)

Granted, though, not everyone can get access to the RCS files.

(The name's not really a misnomer though.  There's tons of CVS-specific
crap you have to deal with in the RCS file to successfully import it --
magic branch revision symbols, horrible vendor branch behavior, etc.)

I will try fromcvs; I hadn't looked at it, since from the documentation
it looked like it wasn't incremental.

-bcd

^ permalink raw reply

* [TICPATCH] rebase -i: put a nice warning into the todo list
From: Johannes Schindelin @ 2007-07-08 20:32 UTC (permalink / raw)
  To: git, VMiklos


It seems that not everybody expects a difference between keeping a "pick" 
line, and deleting it.  So be a bit more explicit about that, with all 
capitals to get the attention.

Noticed by vmiklos on IRC.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 git-rebase--interactive.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a9bb622..03ad8e6 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -400,6 +400,9 @@ do
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#
+# If you remove a line here THAT COMMIT WILL BE LOST.
+#
 EOF
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse $UPSTREAM..$HEAD | \

^ permalink raw reply related

* Re: [PATCH] Push code for transport library
From: Johannes Schindelin @ 2007-07-08 20:29 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0707081608100.6977@iabervon.org>

Hi,

On Sun, 8 Jul 2007, Daniel Barkalow wrote:

> This moves the code to call push backends into a library that can be 
> extended to make matching fetch and push decisions based on the URL it 
> gets, and which could be changed to have built-in implementations 
> instead of calling external programs.

I do not have time to look at the patch more closely, but I like the 
direction this takes very much.

Thanks,
Dscho

^ permalink raw reply

* Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add
From: Matthieu Moy @ 2007-07-08 20:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jan Hudec, Yann Dirson, Christian Jaeger
In-Reply-To: <Pine.LNX.4.64.0707081855300.4248@racer.site>

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

>> This patch proposes a saner behavior. When there are no difference at 
>> all between file, index and HEAD, the file is removed both from the 
>> index and the tree, as before.
>> 
>> Otherwise, if the index matches either the file on disk or the HEAD, the 
>> file is removed from the index, but the file is kept on disk, it may 
>> contain important data.
>
> However, if some of the files are of the first kind, and some are of the 
> second kind, you happily apply with mixed strategies.  IMO that is wrong.

I'm not sure whether this is really wrong. The things git should
really care about are the index and the repository itself, and the
proposed behavior is consistant regarding that (either remove all
files from the index, or remove none).

I'm not opposed to your proposal, but I'd like to have other
opinion(s) on that before changing the code.

>>  static struct {
>>  	int nr, alloc;
>> -	const char **name;
>> +	struct file_info * files;
>>  } list;
>>  
>>  static void add_list(const char *name)
>>  {
>>  	if (list.nr >= list.alloc) {
>>  		list.alloc = alloc_nr(list.alloc);
>> -		list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
>> +		list.files = xrealloc(list.files, list.alloc * sizeof(const char *));
>
> This is wrong, too.  Yes, it works.  But it really should be 
> "sizeof(struct file_info *)".  Remember, code is also documentation.

You don't need to argue, that was a typo. My code is definitely wrong,
but you're wrong too ;-). That's actually sizeof(struct file_info).

>> +		if (!quiet)
>> +			fprintf(stderr, 
>> +				"note: file '%s' not removed "
>> +				"(doesn't match %s).\n",
>> +				path,
>> +				fi.local_changes?"the index":"HEAD");
>> +		return 0;
>> +	}
>> +}
>
> I suspect that this case does never fail. 0 means success for 
> remove_file().  Not good.  You should at least have a way to ensure that 
> it removed the files from the working tree from a script.  Otherwise there 
> is not much point in returning a value to begin with.

I've changed it to have exit_status = 1 if git-rm aborted before
starting, and 2 if git-rm skiped some file removals (and of course, 0
if everything is done as expected).

>> @@ -224,13 +257,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
>>  	if (!index_only) {
>>  		int removed = 0;
>>  		for (i = 0; i < list.nr; i++) {
>> -			const char *path = list.name[i];
>> -			if (!remove_file(path)) {
>> +			if (!remove_file_maybe(list.files[i], quiet)) {
>>  				removed = 1;
>>  				continue;
>>  			}
>>  			if (!removed)
>> -				die("git-rm: %s: %s", path, strerror(errno));
>> +				die("git-rm: %s: %s", 
>> +				    list.files[i].name, strerror(errno));
>>  		}
>>  	}
>
> Style: the old code set and used "path" for readability.  You should do 
> the same (with "file", probably).

Done.

> Additionally, since this changes semantics, you better provide test cases 
> to show what is expected to work, and _ensure_ that it actually works.

Sure. I forgot to mention it in my message, but I wanted to have
feedback before getting into the testsuite stuff.

I'm posting the updated patch for info, but it should anyway not be
merged until

* We agree on the behavior when different files have different kinds
  of changes

* I add a testcase.



>From f39ae646049b95b055e34da378ea470ef3f3caef Mon Sep 17 00:00:00 2001
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Sun, 8 Jul 2007 19:27:44 +0200
Subject: [PATCH] Change the behavior of git-rm to let it obey in more circumstances without -f.

In the previous behavior of git-rm, git refused to do anything in case of
a difference between the file on disk, the index, and the HEAD. As a
result, the -f flag is forced even for simple senarios like:

$ git add foo
$ git rm -f [--cached] foo

This patch proposes a saner behavior. When there are no difference at all
between file, index and HEAD, the file is removed both from the index and
the tree, as before.

Otherwise, if the index matches either the file on disk or the HEAD, the
file is removed from the index, but the file is kept on disk, it may
contain important data.

Otherwise, that's an error, and git-rm aborts.

The above senario becomes

$ git add foo
$ git rm foo

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-rm.txt |    9 +++--
 builtin-rm.c             |   71 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 78f45dc..180671c 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -11,10 +11,11 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-Remove files from the working tree and from the index.  The
-files have to be identical to the tip of the branch, and no
-updates to its contents must have been placed in the staging
-area (aka index).
+Remove files from the working tree and from the index. The content
+placed in the staging area (aka index) must match either the content
+of the file on disk, or the tip of the branch. If it matches only one
+of them, the file is kept on disk for safety, but is still removed
+from the index.
 
 
 OPTIONS
diff --git a/builtin-rm.c b/builtin-rm.c
index 4a0bd93..08af5de 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -12,20 +12,30 @@
 static const char builtin_rm_usage[] =
 "git-rm [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...";
 
+struct file_info {
+	const char *name;
+	int local_changes;
+	int staged_changes;
+};
+
 static struct {
 	int nr, alloc;
-	const char **name;
+	struct file_info *files;
 } list;
 
 static void add_list(const char *name)
 {
 	if (list.nr >= list.alloc) {
 		list.alloc = alloc_nr(list.alloc);
-		list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
+		list.files = xrealloc(list.files, list.alloc * sizeof(struct file_info));
 	}
-	list.name[list.nr++] = name;
+	list.files[list.nr].name = name;
+	list.files[list.nr].local_changes  = 0;
+	list.files[list.nr].staged_changes = 0;
+	list.nr++;
 }
 
+/* Returns -1 on error, zero on success */
 static int remove_file(const char *name)
 {
 	int ret;
@@ -46,6 +56,30 @@ static int remove_file(const char *name)
 	return ret;
 }
 
+/* Returns 0 if the file was actually deleted, -1 if the file removal
+   was a failure, and 1 if remove_file wasn't actually called */
+static int remove_file_maybe(const struct file_info fi, int quiet)
+{
+	const char *path = fi.name;
+	if (!fi.local_changes && !fi.staged_changes)
+		/* The file matches either the index or the HEAD.
+		 * It's content exists somewhere else, it's safe to
+		 * delete it.
+		 */
+		return remove_file(path);
+	else {
+		if (!quiet)
+			fprintf(stderr, 
+				"note: file '%s' not removed "
+				"(%s).\n",
+				path,
+				fi.local_changes ? 
+				"the index doesn't match HEAD" :
+				"the file doesn't match the index");
+		return 1;
+	}
+}
+
 static int check_local_mod(unsigned char *head)
 {
 	/* items in list are already sorted in the cache order,
@@ -62,7 +96,7 @@ static int check_local_mod(unsigned char *head)
 		struct stat st;
 		int pos;
 		struct cache_entry *ce;
-		const char *name = list.name[i];
+		const char *name = list.files[i].name;
 		unsigned char sha1[20];
 		unsigned mode;
 
@@ -87,13 +121,17 @@ static int check_local_mod(unsigned char *head)
 			continue;
 		}
 		if (ce_match_stat(ce, &st, 0))
-			errs = error("'%s' has local modifications "
-				     "(hint: try -f)", ce->name);
+			list.files[i].local_changes = 1;
+
 		if (no_head
 		     || get_tree_entry(head, name, sha1, &mode)
 		     || ce->ce_mode != create_ce_mode(mode)
 		     || hashcmp(ce->sha1, sha1))
-			errs = error("'%s' has changes staged in the index "
+			list.files[i].staged_changes = 1;
+
+		if (list.files[i].local_changes && 
+		    list.files[i].staged_changes)
+			errs = error("'%s' doesn't match neither HEAD nor the index "
 				     "(hint: try -f)", name);
 	}
 	return errs;
@@ -108,6 +146,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	int ignore_unmatch = 0;
 	const char **pathspec;
 	char *seen;
+	int exit_status = 0;
 
 	git_config(git_default_config);
 
@@ -201,7 +240,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 * the index unless all of them succeed.
 	 */
 	for (i = 0; i < list.nr; i++) {
-		const char *path = list.name[i];
+		const char *path = list.files[i].name;
 		if (!quiet)
 			printf("rm '%s'\n", path);
 
@@ -224,13 +263,21 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (!index_only) {
 		int removed = 0;
 		for (i = 0; i < list.nr; i++) {
-			const char *path = list.name[i];
-			if (!remove_file(path)) {
+			struct file_info file = list.files[i];
+			int status = remove_file_maybe(file, quiet);
+			if (status == 0) {
 				removed = 1;
 				continue;
+			} else if (status == 1) {
+				/* Let the user know from a script
+				 * that a file was not deleted on disk
+				 */
+				exit_status = 2;
+				continue;
 			}
 			if (!removed)
-				die("git-rm: %s: %s", path, strerror(errno));
+				die("git-rm: %s: %s", 
+				    file.name, strerror(errno));
 		}
 	}
 
@@ -240,5 +287,5 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			die("Unable to write new index file");
 	}
 
-	return 0;
+	return exit_status;
 }
-- 
1.5.3.rc0.64.gf4f4a-dirty



-- 
Matthieu

^ permalink raw reply related

* Re: [PATCH] t7004: Skip tests for signed tags in an old version of gpg.
From: Sven Verdoolaege @ 2007-07-08 20:22 UTC (permalink / raw)
  To: Carlos Rica; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <46913CC2.4030101@gmail.com>

On Sun, Jul 08, 2007 at 09:36:34PM +0200, Carlos Rica wrote:
> As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
> the gpg version 1.0.6 didn't parse trust packets correctly, so for
> that version, creation of signed tags using the generated key fails.
> 
> Signed-off-by: Carlos Rica <jasampler@gmail.com>
Acked-by: Sven Verdoolaege <skimo@kotnet.org>

skimo

^ permalink raw reply

* [PATCH] Push code for transport library
From: Daniel Barkalow @ 2007-07-08 20:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This moves the code to call push backends into a library that can be
extended to make matching fetch and push decisions based on the URL it
gets, and which could be changed to have built-in implementations
instead of calling external programs.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile       |    3 +-
 builtin-push.c |   92 ++++++++++--------------------
 transport.c    |  177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |   55 +++++++++++++++++
 4 files changed, 264 insertions(+), 63 deletions(-)
 create mode 100644 transport.c
 create mode 100644 transport.h

diff --git a/Makefile b/Makefile
index 4ea5e45..b8f9af1 100644
--- a/Makefile
+++ b/Makefile
@@ -321,7 +321,8 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
+	transport.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index 2612f07..ac8cfa2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -6,10 +6,11 @@
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
+#include "transport.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
-static int all, force, thin = 1, verbose;
+static int all, thin = 1, verbose;
 static const char *receivepack;
 
 static const char **refspec;
@@ -43,80 +44,46 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static int do_push(const char *repo)
+static int do_push(const char *repo, int flags)
 {
 	int i, errs;
-	int common_argc;
-	const char **argv;
-	int argc;
 	struct remote *remote = remote_get(repo);
 
 	if (!remote)
 		die("bad repository '%s'", repo);
 
-	if (remote->receivepack) {
-		char *rp = xmalloc(strlen(remote->receivepack) + 16);
-		sprintf(rp, "--receive-pack=%s", remote->receivepack);
-		receivepack = rp;
-	}
 	if (!refspec && !all && remote->push_refspec_nr) {
 		refspec = remote->push_refspec;
 		refspec_nr = remote->push_refspec_nr;
 	}
-
-	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
-	argv[0] = "dummy-send-pack";
-	argc = 1;
-	if (all)
-		argv[argc++] = "--all";
-	if (force)
-		argv[argc++] = "--force";
-	if (receivepack)
-		argv[argc++] = receivepack;
-	common_argc = argc;
-
 	errs = 0;
 	for (i = 0; i < remote->uri_nr; i++) {
-		int err;
-		int dest_argc = common_argc;
-		int dest_refspec_nr = refspec_nr;
-		const char **dest_refspec = refspec;
-		const char *dest = remote->uri[i];
-		const char *sender = "send-pack";
-		if (!prefixcmp(dest, "http://") ||
-		    !prefixcmp(dest, "https://"))
-			sender = "http-push";
-		else {
-			char *rem = xmalloc(strlen(remote->name) + 10);
-			sprintf(rem, "--remote=%s", remote->name);
-			argv[dest_argc++] = rem;
-			if (thin)
-				argv[dest_argc++] = "--thin";
+		struct transport *transport =
+			transport_get(remote, remote->uri[i]);
+		int err = 1;
+		if (receivepack)
+			transport_set_option(transport,
+					     TRANS_OPT_RECEIVEPACK, receivepack);
+		if (thin)
+			transport_set_option(transport, TRANS_OPT_THIN, "yes");
+
+		if (transport->ops->push) {
+			err = 0;
+			if (transport->ops->connect_for_send)
+				err = transport->ops->connect_for_send(transport);
+			if (!err) {
+				if (verbose)
+					fprintf(stderr, "Pushing to %s\n", remote->uri[i]);
+				err = transport->ops->push(transport, refspec_nr, refspec, flags);
+			}
+			if (transport->ops->disconnect)
+				err |= transport->ops->disconnect(transport);
 		}
-		argv[0] = sender;
-		argv[dest_argc++] = dest;
-		while (dest_refspec_nr--)
-			argv[dest_argc++] = *dest_refspec++;
-		argv[dest_argc] = NULL;
-		if (verbose)
-			fprintf(stderr, "Pushing to %s\n", dest);
-		err = run_command_v_opt(argv, RUN_GIT_CMD);
+
 		if (!err)
 			continue;
 
 		error("failed to push to '%s'", remote->uri[i]);
-		switch (err) {
-		case -ERR_RUN_COMMAND_FORK:
-			error("unable to fork for %s", sender);
-		case -ERR_RUN_COMMAND_EXEC:
-			error("unable to exec %s", sender);
-			break;
-		case -ERR_RUN_COMMAND_WAITPID:
-		case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
-		case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-		case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
-			error("%s died with strange error", sender);
-		}
 		errs++;
 	}
 	return !!errs;
@@ -125,6 +92,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
+	int flags = 0;
 	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
@@ -144,7 +112,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--all")) {
-			all = 1;
+			flags |= TRANSPORT_PUSH_ALL;
 			continue;
 		}
 		if (!strcmp(arg, "--tags")) {
@@ -152,7 +120,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--force") || !strcmp(arg, "-f")) {
-			force = 1;
+			flags |= TRANSPORT_PUSH_FORCE;
 			continue;
 		}
 		if (!strcmp(arg, "--thin")) {
@@ -164,11 +132,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!prefixcmp(arg, "--receive-pack=")) {
-			receivepack = arg;
+			receivepack = arg + 15;
 			continue;
 		}
 		if (!prefixcmp(arg, "--exec=")) {
-			receivepack = arg;
+			receivepack = arg + 7;
 			continue;
 		}
 		usage(push_usage);
@@ -177,5 +145,5 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	if (all && refspec)
 		usage(push_usage);
 
-	return do_push(repo);
+	return do_push(repo, flags);
 }
diff --git a/transport.c b/transport.c
new file mode 100644
index 0000000..3b84983
--- /dev/null
+++ b/transport.c
@@ -0,0 +1,177 @@
+#include "cache.h"
+#include "transport.h"
+#include "run-command.h"
+
+static const struct transport_ops rsync_transport = {
+};
+
+static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+	const char **argv;
+	int argc;
+	int err;
+
+	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv[0] = "http-push";
+	argc = 1;
+	if (flags & TRANSPORT_PUSH_ALL)
+		argv[argc++] = "--all";
+	if (flags & TRANSPORT_PUSH_FORCE)
+		argv[argc++] = "--force";
+	argv[argc++] = transport->url;
+	while (refspec_nr--)
+		argv[argc++] = *refspec++;
+	argv[argc] = NULL;
+	err = run_command_v_opt(argv, RUN_GIT_CMD);
+	switch (err) {
+	case -ERR_RUN_COMMAND_FORK:
+		error("unable to fork for %s", argv[0]);
+	case -ERR_RUN_COMMAND_EXEC:
+		error("unable to exec %s", argv[0]);
+		break;
+	case -ERR_RUN_COMMAND_WAITPID:
+	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
+	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
+	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
+		error("%s died with strange error", argv[0]);
+	}
+	return !!err;
+}
+
+static const struct transport_ops curl_transport = {
+	.push = curl_transport_push,
+};
+
+static const struct transport_ops bundle_transport = {
+};
+
+struct git_transport_data {
+	unsigned thin : 1;
+
+	const char *receivepack;
+};
+
+static int set_git_option(struct transport *connection,
+			  const char *name, const char *value)
+{
+	struct git_transport_data *data = connection->data;
+	if (!strcmp(name, TRANS_OPT_RECEIVEPACK)) {
+		data->receivepack = value;
+		return 0;
+	} else if (!strcmp(name, TRANS_OPT_THIN)) {
+		data->thin = !!value;
+		return 0;
+	}
+	return 1;
+}
+
+static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+	struct git_transport_data *data = transport->data;
+	const char **argv;
+	char *rem;
+	int argc;
+	int err;
+
+	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv[0] = "send-pack";
+	argc = 1;
+	if (flags & TRANSPORT_PUSH_ALL)
+		argv[argc++] = "--all";
+	if (flags & TRANSPORT_PUSH_FORCE)
+		argv[argc++] = "--force";
+	if (data->receivepack) {
+		char *rp = xmalloc(strlen(data->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", data->receivepack);
+		argv[argc++] = rp;
+	}
+	if (data->thin)
+		argv[argc++] = "--thin";
+	rem = xmalloc(strlen(transport->remote->name) + 10);
+	sprintf(rem, "--remote=%s", transport->remote->name);
+	argv[argc++] = rem;
+	argv[argc++] = transport->url;
+	while (refspec_nr--)
+		argv[argc++] = *refspec++;
+	argv[argc] = NULL;
+	err = run_command_v_opt(argv, RUN_GIT_CMD);
+	switch (err) {
+	case -ERR_RUN_COMMAND_FORK:
+		error("unable to fork for %s", argv[0]);
+	case -ERR_RUN_COMMAND_EXEC:
+		error("unable to exec %s", argv[0]);
+		break;
+	case -ERR_RUN_COMMAND_WAITPID:
+	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
+	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
+	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
+		error("%s died with strange error", argv[0]);
+	}
+	return !!err;
+}
+
+static const struct transport_ops git_transport = {
+	.set_option = set_git_option,
+	.push = git_transport_push
+};
+
+static int is_local(const char *url)
+{
+	const char *colon = strchr(url, ':');
+	const char *slash = strchr(url, '/');
+	return !colon || (slash && slash < colon);
+}
+
+static int is_file(const char *url)
+{
+	struct stat buf;
+	if (stat(url, &buf))
+		return 0;
+	return S_ISREG(buf.st_mode);
+}
+
+struct transport *transport_get(struct remote *remote, const char *url)
+{
+	struct transport *ret = NULL;
+	if (!prefixcmp(url, "rsync://")) {
+		ret = xmalloc(sizeof(*ret));
+		ret->data = NULL;
+		ret->ops = &rsync_transport;
+	} else if (!prefixcmp(url, "http://") || !prefixcmp(url, "https://") ||
+		   !prefixcmp(url, "ftp://")) {
+		ret = xmalloc(sizeof(*ret));
+		ret->data = NULL;
+		ret->ops = &curl_transport;
+	} else if (is_local(url) && is_file(url)) {
+		ret = xmalloc(sizeof(*ret));
+		ret->data = NULL;
+		ret->ops = &bundle_transport;
+	} else {
+		struct git_transport_data *data = xcalloc(1, sizeof(*data));
+		ret = xcalloc(1, sizeof(*ret));
+		ret->data = data;
+		data->thin = 1;
+		data->receivepack = "git-receive-pack";
+		if (remote->receivepack)
+			data->receivepack = remote->receivepack;
+		ret->ops = &git_transport;
+	}
+	if (ret) {
+		ret->remote = remote;
+		ret->url = url;
+	}
+	return ret;
+}
+
+int transport_set_option(struct transport *transport,
+			 const char *name, const char *value)
+{
+	int ret = 1;
+	if (transport->ops->set_option)
+		ret = transport->ops->set_option(transport, name, value);
+	if (ret < 0)
+		fprintf(stderr, "For '%s' option %s cannot be set to '%s'\n",
+			transport->url, name, value);
+	if (ret > 0)
+		fprintf(stderr, "For '%s' option %s is ignored\n",
+			transport->url, name);
+	return ret;
+}
diff --git a/transport.h b/transport.h
new file mode 100644
index 0000000..bff265a
--- /dev/null
+++ b/transport.h
@@ -0,0 +1,55 @@
+#ifndef TRANSPORT_H
+#define TRANSPORT_H
+
+#include "cache.h"
+#include "remote.h"
+
+struct transport {
+	unsigned verbose : 1;
+	unsigned connected : 1;
+	struct remote *remote;
+	const char *url;
+
+	void *data;
+
+	const struct transport_ops *ops;
+};
+
+#define TRANSPORT_PUSH_ALL 1
+#define TRANSPORT_PUSH_FORCE 2
+
+struct transport_ops {
+	/**
+	 * Returns 0 if successful, positive if the option is not
+	 * recognized or is inapplicable, and negative if the option
+	 * is applicable but the value is invalid.
+	 **/
+	int (*set_option)(struct transport *connection, const char *name,
+			  const char *value);
+
+	int (*connect_for_send)(struct transport *connection);
+
+	int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
+
+	int (*disconnect)(struct transport *connection);
+};
+
+/* Returns a transport suitable for the url */
+struct transport *transport_get(struct remote *remote, const char *url);
+
+/* Transport options which apply to git:// and scp-style URLs */
+
+/* The program to use on the remote side to receive a pack */
+#define TRANS_OPT_RECEIVEPACK "receivepack"
+
+/* Transfer the data as a thin pack if not null */
+#define TRANS_OPT_THIN "thin"
+
+/**
+ * Returns 0 if the option was used, non-zero otherwise. Prints a
+ * message to stderr if the option is not used.
+ **/
+int transport_set_option(struct transport *transport, const char *name,
+			 const char *value);
+
+#endif
-- 
1.5.2.2.1399.g097d5-dirty

^ permalink raw reply related

* Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue
From: Junio C Hamano @ 2007-07-08 20:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Gerrit Pape, git, Rémi Vanicat
In-Reply-To: <Pine.LNX.4.64.0707081353560.4248@racer.site>

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

> Besides, IMHO there is a deeper issue. Since merge-recursive started out 
> as a Python script, and grew there until it was usable, and grew the 
> rename detection therein, too, until it was finally converted to C, it 
> accumulated a lot of features that would have been nice to have 
> independently.
> ...
> So there we are, with two really big and unwieldy chunks of code, each 
> deserving an own GSoC project to clean them up.  Or maybe not even a GSoC 
> project, but a longer project.

I would not disagree that tree merging part is a large piece of
code with nontrivial development history.  While I do agree that
merge-tree approach is attractive in the longer run, I do not
think the current "use read-tree for policy-neutral merge and
then higher level to decide what to do with conflicts" is beyond
repair.

I tried a variant of cherry-pick that does _not_ use
merge-recursive on the reproduction recipe from Gerrit & Rémi.
Essentially, a cherry-pick with a backend is:

	git-merge-$backend $commit^ -- HEAD $commit

It was made cumbersome to try different merge backend when
cherry-pick has become built-in, but the above essentially was
what we had in the shell script version.  

Interestingly, git-merge-resolve fails, but for an entirely
different reason; there is a bug in git-merge-one-file.  I am
kind of surprised that this has been left undetected for a long
time (this shows how everybody uses git-merge-recursive these
days and not git-merge-resolve).  commit ed93b449 adds tests for
only the "read-tree" part, even though it makes "matching"
change to merge-one-file, which was the breakage.  The bug is
that merge-one-file did not "resolve" an unmerged index entry
when it decided what the result should be --- it decided only in
its comments but not with what it does!  Embarrassing (a fix is
attached).

With that fix, the above "cherry-pick" lookalike with $backend
set to "resolve" and $commit set to "branch" in Gerrit's example
reproduction recipe seems to do the right thing.

> As it is, both unpack_trees() and merge-recursive have a certain degree of 
> not-quite duplicated yet wants-to-do-largely-the-same functionality.  
> Which of course leads to much finger pointing: "it's unpack_trees() fault. 
> no. it's merge-recursive's fault. no, vice versa."

I do not think there is any pointing-fingers involved in this
case.  The division of labor between "read-tree -m" and its
caller has been very clear from the beginning, and has not
changed.  The former does "uncontroversial parts of merge", and
the latter uses its own policy decision to resolve conflicts.

The "uncontroversial parts of merge" explicitly exclude "one
side removes (or adds), other side does not do anything" case.
This is cumbersome for rename-unaware merge-resolve, because its
policy is "we do not worry about the case that the apparent
removal is in fact it moves it to somewhere else -- if one side
removes, the result will not have it", and for that policy if
"read-tree -m" did so it would have less work to do.  But we
don't, exactly because other policy like merge-recursive may
want to look at the apparently removed path and figure out if
there is a rename involved.

The last time I looked at merge-recursive.c, I think the problem
I saw was the way it maintains and uses two lists that keeps
track of the set of directories and files; get_files_dirs() is
called for both head and merge at the beginning, and I did not
see a code that says "Oh, we recorded the path 'link' as being
potentially a directory at the beginning, but we have decided to
resolve 'link/file' by removing it, and 'link' does not have to
exist as a directory anymore. resolving 'link' as a symbolic
link is perfectly fine" --- and the reason Gerrit's test case
fails was something like that.

-- >8 --
[PATCH] Fix merge-one-file for our-side-added/our-side-removed cases

When commit ed93b449 changed the script so that it does not
touch untracked working tree file, we forgot that we still
needed to resolve the index entry (otherwise they are left
unmerged).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index ebbb575..1e7727d 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -27,8 +27,9 @@ case "${1:-.}${2:-.}${3:-.}" in
 		# read-tree checked that index matches HEAD already,
 		# so we know we do not have this path tracked.
 		# there may be an unrelated working tree file here,
-		# which we should just leave unmolested.
-		exit 0
+		# which we should just leave unmolested.  Make sure
+		# we do not have it in the index, though.
+		exec git update-index --remove -- "$4"
 	fi
 	if test -f "$4"; then
 		rm -f -- "$4" &&
@@ -42,7 +43,8 @@ case "${1:-.}${2:-.}${3:-.}" in
 #
 ".$2.")
 	# the other side did not add and we added so there is nothing
-	# to be done.
+	# to be done, except making the path merged.
+	exec git update-index --add --cacheinfo "$6" "$2" "$4"
 	;;
 "..$3")
 	echo "Adding $4"
@@ -50,7 +52,7 @@ case "${1:-.}${2:-.}${3:-.}" in
 		echo "ERROR: untracked $4 is overwritten by the merge."
 		exit 1
 	}
-	git update-index --add --cacheinfo "$6$7" "$2$3" "$4" &&
+	git update-index --add --cacheinfo "$7" "$3" "$4" &&
 		exec git checkout-index -u -f -- "$4"
 	;;
 

^ permalink raw reply related

* [PATCH] Make fetch-pack a builtin with an internal API
From: Daniel Barkalow @ 2007-07-08 19:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

In addition to making fetch-pack a builtin, this allows it to be called 
directly from other built-in code without generating and parsing argument 
lists, which will be useful for builtin-fetch.

Incidently, it makes git-fetch-pack not output lists of what it fetched 
when it fails.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile                             |    1 +
 fetch-pack.c => builtin-fetch-pack.c |   99 ++++++++++++++++++++++++++--------
 builtin.h                            |    1 +
 fetch-pack.h                         |   15 +++++
 git.c                                |    1 +
 5 files changed, 95 insertions(+), 22 deletions(-)
 rename fetch-pack.c => builtin-fetch-pack.c (92%)
 create mode 100644 fetch-pack.h

diff --git a/Makefile b/Makefile
index 4ea5e45..da750f8 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,7 @@ BUILTIN_OBJS = \
 	builtin-diff-files.o \
 	builtin-diff-index.o \
 	builtin-diff-tree.o \
+	builtin-fetch-pack.o \
 	builtin-fetch--tool.o \
 	builtin-fmt-merge-msg.o \
 	builtin-for-each-ref.o \
diff --git a/fetch-pack.c b/builtin-fetch-pack.c
similarity index 92%
rename from fetch-pack.c
rename to builtin-fetch-pack.c
index 9c81305..828ae24 100644
--- a/fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -6,6 +6,7 @@
 #include "exec_cmd.h"
 #include "pack.h"
 #include "sideband.h"
+#include "fetch-pack.h"
 
 static int keep_pack;
 static int transfer_unpack_limit = -1;
@@ -573,7 +574,7 @@ static int get_pack(int xd[2])
 	die("%s died of unnatural causes %d", argv[0], status);
 }
 
-static int fetch_pack(int fd[2], int nr_match, char **match)
+static struct ref *do_fetch_pack(int fd[2], int nr_match, char **match)
 {
 	struct ref *ref;
 	unsigned char sha1[20];
@@ -615,12 +616,7 @@ static int fetch_pack(int fd[2], int nr_match, char **match)
 		die("git-fetch-pack: fetch failed.");
 
  all_done:
-	while (ref) {
-		printf("%s %s\n",
-		       sha1_to_hex(ref->old_sha1), ref->name);
-		ref = ref->next;
-	}
-	return 0;
+	return ref;
 }
 
 static int remove_duplicates(int nr_heads, char **heads)
@@ -663,15 +659,42 @@ static int fetch_pack_config(const char *var, const char *value)
 
 static struct lock_file lock;
 
-int main(int argc, char **argv)
+void setup_fetch_pack(const char *_uploadpack,
+		      int _quiet,
+		      int _keep_pack,
+		      int _use_thin_pack,
+		      int _fetch_all,
+		      int _verbose,
+		      int _depth,
+		      int _no_progress)
+{
+	uploadpack = _uploadpack;
+	quiet = _quiet;
+	keep_pack = _keep_pack;
+	if (keep_pack)
+		unpack_limit = 0;
+	use_thin_pack = _use_thin_pack;
+	fetch_all = _fetch_all;
+	verbose = _verbose;
+	depth = _depth;
+	no_progress = _no_progress;
+
+#if 0
+	git_config(fetch_pack_config);
+
+	if (0 <= transfer_unpack_limit)
+		unpack_limit = transfer_unpack_limit;
+	else if (0 <= fetch_unpack_limit)
+		unpack_limit = fetch_unpack_limit;
+#endif
+}
+
+int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret, nr_heads;
+	struct ref *ref;
 	char *dest = NULL, **heads;
-	int fd[2];
-	pid_t pid;
-	struct stat st;
 
-	setup_git_directory();
 	git_config(fetch_pack_config);
 
 	if (0 <= transfer_unpack_limit)
@@ -682,7 +705,7 @@ int main(int argc, char **argv)
 	nr_heads = 0;
 	heads = NULL;
 	for (i = 1; i < argc; i++) {
-		char *arg = argv[i];
+		const char *arg = argv[i];
 
 		if (*arg == '-') {
 			if (!prefixcmp(arg, "--upload-pack=")) {
@@ -716,8 +739,6 @@ int main(int argc, char **argv)
 			}
 			if (!prefixcmp(arg, "--depth=")) {
 				depth = strtol(arg + 8, NULL, 0);
-				if (stat(git_path("shallow"), &st))
-					st.st_mtime = 0;
 				continue;
 			}
 			if (!strcmp("--no-progress", arg)) {
@@ -726,22 +747,53 @@ int main(int argc, char **argv)
 			}
 			usage(fetch_pack_usage);
 		}
-		dest = arg;
-		heads = argv + i + 1;
+		dest = (char *)arg;
+		heads = (char **)(argv + i + 1);
 		nr_heads = argc - i - 1;
 		break;
 	}
 	if (!dest)
 		usage(fetch_pack_usage);
-	pid = git_connect(fd, dest, uploadpack, verbose ? CONNECT_VERBOSE : 0);
+
+	ref = fetch_pack(dest, nr_heads, heads);
+
+	ret = !ref;
+
+	while (ref) {
+		printf("%s %s\n",
+		       sha1_to_hex(ref->old_sha1), ref->name);
+		ref = ref->next;
+	}
+
+	return ret;
+}
+
+struct ref *fetch_pack(const char *dest, int nr_heads, char **heads)
+{
+	int i, ret;
+	int fd[2];
+	pid_t pid;
+	struct ref *ref;
+	struct stat st;
+
+	if (depth > 0) {
+		if (stat(git_path("shallow"), &st))
+			st.st_mtime = 0;
+	}
+
+
+	printf("connect to %s\n", dest);
+
+	pid = git_connect(fd, (char *)dest, uploadpack,
+                          verbose ? CONNECT_VERBOSE : 0);
 	if (pid < 0)
-		return 1;
+		return NULL;
 	if (heads && nr_heads)
 		nr_heads = remove_duplicates(nr_heads, heads);
-	ret = fetch_pack(fd, nr_heads, heads);
+	ref = do_fetch_pack(fd, nr_heads, heads);
 	close(fd[0]);
 	close(fd[1]);
-	ret |= finish_connect(pid);
+	ret = finish_connect(pid);
 
 	if (!ret && nr_heads) {
 		/* If the heads to pull were given, we should have
@@ -785,5 +837,8 @@ int main(int argc, char **argv)
 		}
 	}
 
-	return !!ret;
+	if (ret)
+		ref = NULL;
+
+	return ref;
 }
diff --git a/builtin.h b/builtin.h
index 661a92f..8fa38d4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -31,6 +31,7 @@ extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
 extern int cmd_diff(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_fetch__tool(int argc, const char **argv, const char *prefix);
 extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
 extern int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
diff --git a/fetch-pack.h b/fetch-pack.h
new file mode 100644
index 0000000..a317b70
--- /dev/null
+++ b/fetch-pack.h
@@ -0,0 +1,15 @@
+#ifndef FETCH_PACK_API
+#define FETCH_PACK_API
+
+void setup_fetch_pack(const char *_uploadpack,
+		      int _quiet,
+		      int _keep_pack,
+		      int _use_thin_pack,
+		      int _fetch_all,
+		      int _verbose,
+		      int _depth,
+		      int _no_progress);
+
+struct ref *fetch_pack(const char *dest, int nr_heads, char **heads);
+
+#endif
diff --git a/git.c b/git.c
index b949cbb..df45161 100644
--- a/git.c
+++ b/git.c
@@ -307,6 +307,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "diff-files", cmd_diff_files },
 		{ "diff-index", cmd_diff_index, RUN_SETUP },
 		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
 		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
 		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
 		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-- 
1.5.2.2.1399.g097d5-dirty

^ permalink raw reply related

* [PATCH] t7004: Skip tests for signed tags in an old version of gpg.
From: Carlos Rica @ 2007-07-08 19:36 UTC (permalink / raw)
  To: git, Junio C Hamano, Sven Verdoolaege, Johannes Schindelin

As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
the gpg version 1.0.6 didn't parse trust packets correctly, so for
that version, creation of signed tags using the generated key fails.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
This was already discussed in a previous message from Sven,
who runs that old version of gpg:
http://thread.gmane.org/gmane.comp.version-control.git/51291

The page describing the bug was found by Johannes after some research:
http://colabti.de/irclogger/irclogger_log/git?date=2007-07-01,Sun&sel=482#l814

 t/t7004-tag.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index a845930..b785080 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -460,6 +460,17 @@ if [ $? -eq 127 ]; then
 	exit
 fi

+# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
+# the gpg version 1.0.6 didn't parse trust packets correctly, so for
+# that version, creation of signed tags using the generated key fails.
+case "$(gpg --version)" in
+'gpg (GnuPG) 1.0.6'*)
+	echo "Skipping signed tag tests, because a bug in 1.0.6 version"
+	test_done
+	exit
+	;;
+esac
+
 # key generation info: gpg --homedir t/t7004 --gen-key
 # Type DSA and Elgamal, size 2048 bits, no expiration date.
 # Name and email: C O Mitter <committer@example.com>
-- 
1.5.0

^ permalink raw reply related

* Re: [PATCH] branch.autosetupmerge: allow boolean values, or "all"
From: Johannes Schindelin @ 2007-07-08 19:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Paolo Bonzini
In-Reply-To: <7vzm2620wp.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sun, 8 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	> Eh?  I did not want this to get applied for my local branches.
> >
> > 	That is certainly unexpected and unwelcomed.  Alas, I think it is 
> > 	one of the consequences of rarely executed (and thus, tested) 
> > 	code.
> > ...
> > +test_expect_success 'autosetupmerge = all' '
> > +	git config branch.autosetupmerge true &&
> > +	git branch all1 master &&
> > +	test -z "$(git config branch.all1.merge)" &&
> > +	git config branch.autosetupmerge all &&
> > +	git branch all2 master &&
> > +	test $(git config branch.all2.merge) = refs/heads/master
> > +'
> 
> Thanks.
> 
> Having prepared the patch below, I do not think if the original
> patch even wanted to have 'all' semantics.  The surrounding text
> only talks about "off a remote branch" and I strongly suspect
> that nobody wanted to do this for a local branch case at all.

I remember that the comment was correct for the first few versions.  
Somehow I missed that change in semantics.  Paolo, what was the rationale?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 4/4] Add git-rewrite-commits
From: Sven Verdoolaege @ 2007-07-08 19:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0707081911160.4248@racer.site>

On Sun, Jul 08, 2007 at 07:17:16PM +0100, Johannes Schindelin wrote:
> On Sun, 8 Jul 2007, Sven Verdoolaege wrote:
> > On Sun, Jul 08, 2007 at 05:37:22PM +0100, Johannes Schindelin wrote:
> > > I am really unhappy that so much is talked about filtering out 
> > > commits.  That is amost certainly not what you want in most cases.  
> > > In particular, I suspect that most users would expect the _changes_ 
> > > filtered out by such a command, which is just not true.
> > 
> > I don't care about that either.  I'm just mentioning it because it's 
> > mentioned in the git-filter-branch documentation (which you added).
> 
> Which I copied.  And this is not the first, let alone the only example in 
> filter-branch's documentation.

All I'm saying is that you shouldn't blame me for doing something
you have done yourself.
And if you're not blaming me, but just making a general comment,
then all I can say is that I agree with your comment.

> However, this leaves things only in half-finished states.
> 
> - "git filter-branch" did not learn the useful features that you seem to 
>   need, and
> 
> - your builtin is at most a start of a builtin replacement for 
>   filter-branch, which changes the semantics, to be sure.
> 
> I have no doubts that it will stay that way for a while, since this 
> builtin seems to be good enough for what you want it to do.

If people find rewrite-commits useful, but think that something
is missing, then I'd be willing to look into that.
I'm personally not likely to work on fiter-branch, but maybe
someone else, possibly inspired by rewrite-commits, will.
But it is true that rewrite-commits does everything I want now.

skimo

^ permalink raw reply

* Re: [PATCH 4/4] Add git-rewrite-commits
From: Johannes Schindelin @ 2007-07-08 18:41 UTC (permalink / raw)
  To: Steven Grimm; +Cc: skimo, git, Junio C Hamano
In-Reply-To: <46912726.5080807@midwinter.com>

Hi,

On Sun, 8 Jul 2007, Steven Grimm wrote:

> Johannes Schindelin wrote:
>
> > I am really unhappy that so much is talked about filtering out 
> > commits. That is amost certainly not what you want in most cases.  In 
> > particular, I suspect that most users would expect the _changes_ 
> > filtered out by such a command, which is just not true.
> >   
> 
> I agree that unless it's named and documented very carefully, users 
> might expect this to tweak history such that the commits in question 
> never happened (unlike revert, which of course adds a new commit and 
> leaves the old ones alone.) The documentation for this command could 
> stand to be more explicit about that.

In the documentation of filter-branch, you have at least a couple of more 
useful examples, and the misunderstanding is cleared up.  At least a 
little.

> > Further, I do not see much value in making this operation faster.  It 
> > is meant to be a one-time operation, for example when you open-source 
> > a product and have to cull a lot of history that you must not show for 
> > legal reasons.  It is a one-shot operation.
> 
> Your recent changes to git-rebase (which, BTW, are great) include a 
> feature that's very similar to this: the "squash these commits together 
> in my history" feature. That'd be my use case for this, when I want to 
> publish my changes to other developers who don't care about all my 
> intermediate checkpoints of work in progress, and when the commits I'm 
> removing haven't been published anywhere else yet.
> 
> With this command, I could do something like:
> 
> git rewrite-commits --grep="!@@@checkpoint"
> git push

git reset --head HEAD~<n> &&
git rev-list --pretty=oneline ..HEAD@{1} | while read sha1 message
do
	git cherry-pick -n $sha1 || exit
	case "$message" in
	*@@@checkpoint*) ;;
	*) git commit -C $sha1;;
	esac
done

Yes, it is longer than the command line you gave.  But it does not need 
any fancy new stuff, so it works even on 1.4.4.4.  And you can make that a 
script, or if you really need to, an alias.  Since you want to avoid the 
interactivity, rebase -i is kind of the wrong place to look.

> and it would strip out all my intermediate checkpoint commits (assuming 
> I've marked them as such in my commit comments, which I always do) 
> before pushing to my project's shared repo. Right now that's a much more 
> cumbersome, and very manual, operation. Even with the new git-rebase 
> changes, I still have to pick out those commits by hand, and it assumes 
> that I otherwise want to do a rebase in the first place.

If you _have_ to use some fancy stuff, you can fake an editor by 
resetting VISUAL, like illustrated in t3404.

> > So there are two things I see here that filter-branch cannot do yet.  
> > The first is to rewrite _all_ branches, which should be easy to do, it 
> > only has to be done.
> 
> I wonder if it makes sense to go that direction, though, or to make this 
> command do the things that filter-branch can do, for the simple reason 
> that filter-branch is a shell script and this is already a nice 
> non-shell-dependent C program.

Exactly it is a C program.  And if you are fleshing out issues, it is 
always easier to work on a shell script.  That is why git-remote is still 
a perl script; the semantics are not yet flashed out, and it is orders of 
magnitudes slower to change them in C.  And to test them in C.

Besides, it is way easier to fsck up some obscure detail in C, since you 
have low-level access to everything.  And those bugs are harder to spot.  
In shell, you have a limited interface, but for prototyping, that is 
actually an advantage.

> Obviously you end up in the same place either way eventually once 
> filter-branch percolates to the top of the "port these scripts to C" 
> list, but it seems odd to me to port features from a C program back to a 
> shell script only to have to convert the shell script to C later on.

Hey, if skimo would be willing to work until it is a complete replacement 
for filter-branch, and more, I will stop working on filter-branch at once.  
Somehow I did not get this impression, however.

> Ironically, this app doesn't really speed up the one thing I find too 
> slow in filter-branch: the "remove a file from the tree in all 
> revisions" case. To do that you still have to launch a filter app for 
> every commit, which is especially bad when the file in question only 
> appears in a few revisions deep in the history of a repo.

Semantics.  That is the stage we are at.  Trying to find the best 
semantics.

Filtering out single files might well suffice for you.  However, I think 
that this is only a very special corner case of something much bigger and 
more versatile.  ATM I am quite certain that the eval'ed filters are the 
way to go.

Since even in your corner case, you usually do it _once_.

> This command points us in the direction of a "remove/rename this file in 
> history" feature that doesn't require forking tens of thousands of child 
> processes on a repo with lots of history. For that alone I think it's 
> worthwhile, even though it's not there yet; that will never happen with 
> a shell script. And yeah, that's not a frequent operation, but it's sure 
> nice when even the infrequent operations are lightning fast.

It is sure nice when the frequent operations get lightning fast _first_.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-commit: don't add multiple Signed-off-by: from the same identity
From: Junio C Hamano @ 2007-07-08 18:44 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: git
In-Reply-To: <20070708150055.7018.qmail@10e1f648410572.315fe32.mid.smarden.org>

Gerrit Pape <pape@smarden.org> writes:

> with the patch, iff the last line already was the signoff to be added,
> it won't be added again.

Ah, I completely misread the patch.  Thanks.

^ permalink raw reply

* [PATCH] git-stash: try reusing cached stat info as much as possible
From: Junio C Hamano @ 2007-07-08 18:43 UTC (permalink / raw)
  To: git

Earlier when we read a tree into a temporary index, we read it
from scratch.  Start from the current index and use read-tree -m
to preserve cached stat information as much as possible, in
order to speed up "git add -u".  This makes "git stash" usable
in a source tree of nontrivial size.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-stash.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index eac5551..de13dd1 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,11 +58,11 @@ save_stash () {
 
 	# state of the working tree
 	w_tree=$( (
+		rm -f "$TMP-index" &&
+		cp -p ${GIT_INDEX_FILE-"$GIT_DIR/index"} "$TMP-index" &&
 		GIT_INDEX_FILE="$TMP-index" &&
 		export GIT_INDEX_FILE &&
-
-		rm -f "$TMP-index" &&
-		git read-tree $i_tree &&
+		git read-tree -m $i_tree &&
 		git add -u &&
 		git write-tree &&
 		rm -f "$TMP-index"
-- 
1.5.3.rc0.858.gd08b

^ permalink raw reply related

* Re: [PATCH] branch.autosetupmerge: allow boolean values, or "all"
From: Junio C Hamano @ 2007-07-08 18:41 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Paolo Bonzini
In-Reply-To: <Pine.LNX.4.64.0707081336020.4248@racer.site>

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

> 	> Eh?  I did not want this to get applied for my local branches.
>
> 	That is certainly unexpected and unwelcomed.  Alas, I think it is 
> 	one of the consequences of rarely executed (and thus, tested) 
> 	code.
> ...
> +test_expect_success 'autosetupmerge = all' '
> +	git config branch.autosetupmerge true &&
> +	git branch all1 master &&
> +	test -z "$(git config branch.all1.merge)" &&
> +	git config branch.autosetupmerge all &&
> +	git branch all2 master &&
> +	test $(git config branch.all2.merge) = refs/heads/master
> +'

Thanks.

Having prepared the patch below, I do not think if the original
patch even wanted to have 'all' semantics.  The surrounding text
only talks about "off a remote branch" and I strongly suspect
that nobody wanted to do this for a local branch case at all.



diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4b67f0a..aeece84 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -309,7 +309,10 @@ branch.autosetupmerge::
 	so that gitlink:git-pull[1] will appropriately merge from that
 	remote branch.  Note that even if this option is not set,
 	this behavior can be chosen per-branch using the `--track`
-	and `--no-track` options.  This option defaults to false.
+	and `--no-track` options.  This option can have values
+	'false' (never touch the configuration), 'all' (do this
+	for all branches), or 'true' (do this only when
+	branching from a remote tracking branch), and defaults to 'true'.
 
 branch.<name>.remote::
 	When in branch <name>, it tells `git fetch` which remote to fetch.
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 818b720..8292952 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -52,8 +52,9 @@ OPTIONS
 	set up configuration so that git-pull will automatically
 	retrieve data from the remote branch.  Set the
 	branch.autosetupmerge configuration variable to true if you
-	want git-checkout and git-branch to always behave as if
-	'--track' were given.
+	want git-checkout and git-branch to behave as if
+	'--track' were given when you branch from a remote
+	tracking branch.
 
 --no-track::
 	When -b is given and a branch is created off a remote branch,

^ permalink raw reply related

* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-08 18:32 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Git Mailing List
In-Reply-To: <e5bfff550707080737j56c161bdrf0c079b61877085b@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> Without this patch in case of a merge, duplicated parents are
> omitted in first line output, but still listed in following
> parents information.
>
> Signed-off-by: Marco Costalba <mcostalba@gmail.com>
> ---
>
> Following the hint of someone I submit this patch.
>
> Please take it as a wish of not reverting the patch ;-)

Sorry, I am puzzled... which patch are you talking about
possibly getting reverted?

In any case, the caller asked "log" to:

 1. simplify the history, by giving pathspec,
 2. show the list of parents,
 3. show the diff with each of the parents.

If we omit dupilicated parents in the simplified view from 2.,
we should be consistent in the output from 3.; I think what your
patch does makes sense.

I'll look at it again later.

^ permalink raw reply

* [mingw port] Problem starting git subcommands
From: Henning Rogge @ 2007-07-08 18:26 UTC (permalink / raw)
  To: git

I have experimented with GIT on my linux system successfully but one
of my projects has to be done on Windows (neither VMware Workstation
nor Wine can help at the moment) so I got a copy of the mingw port of
GIT (http://lilypond.org/git/binaries/mingw/git-1.5.2.1-1.mingw.exe).

Installation succeeded without a single error but when I tried to run
a few GIT commands I noticed a "bug":

the git.exe command does not recognize any scripted subcommand
(git-clone in my case) !
> C:\Programme\Git>git clone
> git: 'clone' is not a git-command

Do I need another software package ? I tried to install the Mingw
package itself, but it did not help at all.

^ 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