Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add --no-reuse-delta, --window, and --depth options to git-gc
From: Dana How @ 2007-05-08  3:38 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Nicolas Pitre, Junio C Hamano, git, danahow
In-Reply-To: <20070508032122.GA10940@thunk.org>

On 5/7/07, Theodore Tso <tytso@mit.edu> wrote:
> On Mon, May 07, 2007 at 11:13:58PM -0400, Nicolas Pitre wrote:
> > On Mon, 7 May 2007, Theodore Ts'o wrote:
> > > Sometimes users might want to use more aggressive packing options
> > > when doing a git-gc.  This allows them to do so without having
> > > to use the low-level plumbing commands.
> > In the context of "gc", having an option that reads "window" looks a bit
> > strange too.
> I suppose, but you either need to then know all of the other commands
> which git-gc runs, and do them manually, skipping git-gc altogether,
> or use git-gc, and end up rewriting the pack twice, ince using the
> git-repack in git-gc, and then once manually so you can give the
> options that you really want to give to git-repack.
>
> Maybe the right approach is to have a way to specify default --window
> and --depth as git configuration variables?  Looks like there is a
> pack.window already, but not a pack.depth.
>
> What if we add a pack.depth configuration option, and add only
> --no-reuse-delta to git-gc?   Would that be better?

I would use pack.depth .

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

^ permalink raw reply

* [PATCH] user-manual: fix clone and fetch typos
From: J. Bruce Fields @ 2007-05-08  3:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20070508030732.GB9471@fieldses.org>

More typo fixes from Santi Béjar, plus a couple other mistakes I noticed
along the way.

Cc: Santi Béjar <sbejar@gmail.com>
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
---
 Documentation/user-manual.txt |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 67f5b9b..13db969 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1836,12 +1836,12 @@ Now, assume your personal repository is in the directory ~/proj.  We
 first create a new clone of the repository:
 
 -------------------------------------------------
-$ git clone --bare proj.git
+$ git clone --bare ~/proj proj.git
 -------------------------------------------------
 
-The resulting directory proj.git will contains a "bare" git
-repository--it is just the contents of the ".git" directory, without
-a checked-out copy of a working directory.
+The resulting directory proj.git contains a "bare" git repository--it is
+just the contents of the ".git" directory, without a checked-out copy of
+a working directory.
 
 Next, copy proj.git to the server where you plan to host the
 public repository.  You can use scp, rsync, or whatever is most
@@ -2372,9 +2372,8 @@ $ git config remote.example.fetch master:refs/remotes/example/master
 then the following commands will all do the same thing:
 
 -------------------------------------------------
-$ git fetch git://example.com/proj.git master:ref/remotes/example/master
-$ git fetch example master:ref/remotes/example/master
-$ git fetch example example/master
+$ git fetch git://example.com/proj.git master:refs/remotes/example/master
+$ git fetch example master:refs/remotes/example/master
 $ git fetch example
 -------------------------------------------------
 
-- 
1.5.1.1.98.gedb4f

^ permalink raw reply related

* Re: [EGIT 00/10] This weeks Eclipse patches
From: Shawn O. Pearce @ 2007-05-08  3:24 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <20070507212636.21546.29280.stgit@lathund.dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> A random collection of improvements to the Eclipse
> plugin.

Thanks, applied, pushed to usual locations.
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Add --no-reuse-delta, --window, and --depth options to git-gc
From: Theodore Tso @ 2007-05-08  3:21 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.99.0705072305270.24220@xanadu.home>

On Mon, May 07, 2007 at 11:13:58PM -0400, Nicolas Pitre wrote:
> On Mon, 7 May 2007, Theodore Ts'o wrote:
> 
> > Sometimes users might want to use more aggressive packing options
> > when doing a git-gc.  This allows them to do so without having
> > to use the low-level plumbing commands.
> 
> The 'git repack' command isn't _that_ low level, is it?  
> git-pack-objects is plumbing for sure, but not git-repack?
> 
> Especially if you're aware and interested in those options, you won't be 
> afraid of 'git repack -a -f -d --window=...".
> 
> In the context of "gc", having an option that reads "window" looks a bit 
> strange too.

I suppose, but you either need to then know all of the other commands
which git-gc runs, and do them manually, skipping git-gc altogether,
or use git-gc, and end up rewriting the pack twice, ince using the
git-repack in git-gc, and then once manually so you can give the
options that you really want to give to git-repack.

Maybe the right approach is to have a way to specify default --window
and --depth as git configuration variables?  Looks like there is a
pack.window already, but not a pack.depth.

What if we add a pack.depth configuration option, and add only
--no-reuse-delta to git-gc?   Would that be better?

						- Ted

^ permalink raw reply

* [PATCH] remove unnecessary loop
From: Liu Yubao @ 2007-05-08  3:18 UTC (permalink / raw)
  To: git

Hi,
   Here is a minor optimization, the involved second "for" loop doesn't
need to start from beginning.

Signed-off-by: Liu Yubao <yubao.liu@gmail.com>
---
 builtin-add.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 5e6748f..9d10fdc 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -239,20 +239,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		die("index file corrupt");
 
 	if (!ignored_too) {
-		int has_ignored = 0;
 		for (i = 0; i < dir.nr; i++)
 			if (dir.entries[i]->ignored)
-				has_ignored = 1;
-		if (has_ignored) {
+				break;
+		if (i < dir.nr) {
 			fprintf(stderr, ignore_warning);
-			for (i = 0; i < dir.nr; i++) {
+			do {
 				if (!dir.entries[i]->ignored)
 					continue;
 				fprintf(stderr, "%s", dir.entries[i]->name);
 				if (dir.entries[i]->ignored_dir)
 					fprintf(stderr, " (directory)");
 				fputc('\n', stderr);
-			}
+			} while (++i < dir.nr);
 			fprintf(stderr,
 				"Use -f if you really want to add them.\n");
 			exit(1);
-- 
1.5.2.rc0.95.ga0715-dirty

^ permalink raw reply related

* Re: [PATCH] user-manual.txt: Some doc updates
From: J. Bruce Fields @ 2007-05-08  3:21 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Git Mailing List
In-Reply-To: <87hcrcjre3.fsf@gmail.com>

On Thu, Apr 19, 2007 at 05:49:40PM +0200, Santi Béjar wrote:
>  git branch::
> +	list all local branches
> +git branch -r::
> +	list all remote branches
> +git branch -a::
>  	list all branches

I think I dropped this just because "remote branches" haven't yet been
introduced at this point in the manual.  (That's not till "Examining
branches from a remote repository".)  Could be some sections should be
reordered--suggestions welcomed.

> +-------------------------------------------------
> +$ git diff HEAD
> +-------------------------------------------------
> +
> +shows the difference between the working tree and HEAD.
> +

I took the mention of this later on, but didn't think it was necessary
to mention here too.

> @@ -1872,10 +1880,7 @@ As with git-fetch, you may also set up configuration options to
>  save typing; so, for example, after
>  
>  -------------------------------------------------
> -$ cat >.git/config <<EOF
> -[remote "public-repo"]
> -	url = ssh://yourserver.com/~you/proj.git
> -EOF
> +$ git config remote."public-repo".url ssh://yourserver.com/~you/proj.git
>  -------------------------------------------------

I think this was adequately discussed.

> +You can also instantly browse your working repository in gitweb using:
> +
> +-------------------------------------------------
> +$ git instaweb
> +-------------------------------------------------
> +
> +See gitlink:git-instaweb[1] for details.
> +

That's nifty, but doesn't seem necessary at this point, and doesn't
quite fit in the section (which is about public repositories).

Others look good, thanks.

But if you want whole patches applied or rejected with explanation, I
think you may have to be prepared to revise and resubmit when there are
objections to parts.

--b.

^ permalink raw reply

* Re: [FAQ?] Rationale for git's way to manage the index
From: Martin Langhoff @ 2007-05-08  3:16 UTC (permalink / raw)
  To: git
In-Reply-To: <vpqbqgxak1i.fsf@bauges.imag.fr>

On 5/7/07, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
> > On Sun, 6 May 2007, Matthieu Moy wrote:
> >>
> >> But the fact that git actually remembers the _content_ of files in the
> >> index, and that the default behavior for "commit" is to commit only
> >> the content that is explicitely "git add"ed is something I've never
> >> seen outside git.
> >
> > Yeah. You'd better get used to it, because it's fundamental.
>
> Thanks a lot for the detailed explanations.

Heh. Making the index very visible makes sense when you are merging,
Linus and Junio are both integrators and spend a lot of time merging.
Hence the default is for git-commit to observe the index.

I agree with Linus' other points too, but at the end of the day, it
makes life easier and saner mainly when merging, at the expense of
having to pay a bit more attention in common commits. The tradeoff
makes sense _specially_ if you are the integrator.

So I do git-commit -a, and typing that '-a' is small price to pay for
the best SCM I've ever used ;-)

cheers,


martin

^ permalink raw reply

* Re: [PATCH] Add --no-reuse-delta, --window, and --depth options to git-gc
From: Nicolas Pitre @ 2007-05-08  3:13 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Junio C Hamano, git
In-Reply-To: <E1HlFqU-0002ir-GK@candygram.thunk.org>

On Mon, 7 May 2007, Theodore Ts'o wrote:

> Sometimes users might want to use more aggressive packing options
> when doing a git-gc.  This allows them to do so without having
> to use the low-level plumbing commands.

The 'git repack' command isn't _that_ low level, is it?  
git-pack-objects is plumbing for sure, but not git-repack?

Especially if you're aware and interested in those options, you won't be 
afraid of 'git repack -a -f -d --window=...".

In the context of "gc", having an option that reads "window" looks a bit 
strange too.

Maybe it's just me...


Nicolas

^ permalink raw reply

* Re: [PATCH 5/6] user-manual: miscellaneous editing
From: J. Bruce Fields @ 2007-05-08  3:07 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Junio C Hamano, git
In-Reply-To: <8aa486160705070027g2cb6797fy5acbe338e30429ff@mail.gmail.com>

On Mon, May 07, 2007 at 09:27:40AM +0200, Santi Béjar wrote:
> On 5/7/07, J. Bruce Fields <bfields@citi.umich.edu> wrote:
> >Cc: Santi Béjar <sbejar@gmail.com>
>
> You forgot to CC: the e-mail to me :-)

The version of git-send-email I was using is supposed to do that
automatically based on the inlined Cc: line.  Did it not do that?

> >@@ -1830,14 +1836,14 @@ Now, assume your personal repository is in the 
> >directory ~/proj.  We
> > first create a new clone of the repository:
> >
> > -------------------------------------------------
> >-$ git clone --bare proj-clone.git
> >+$ git clone --bare proj.git
> > -------------------------------------------------
> 
> You missed the ~/proj as in:
> 
> $ git clone --bare ~/proj proj.git

Whoops, thanks.

> What do you think about the rest of my changes, in particular:
> 
> @@ -2374,7 +2388,6 @@ then the following commands will all do the same 
> thing:
> -------------------------------------------------
> $ git fetch git://example.com/proj.git master:ref/remotes/example/master
> $ git fetch example master:ref/remotes/example/master
> -$ git fetch example example/master
> $ git fetch example
> -------------------------------------------------

Yes, you're correct, thanks for your persistence.

The others--I don't remember.  I'll take a quick look.  If you want to
be sure they'll all be considered, you could update the patch and
resend.

--b.

^ permalink raw reply

* [PATCH] Add --no-reuse-delta, --window, and --depth options to git-gc
From: Theodore Ts'o @ 2007-05-08  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Sometimes users might want to use more aggressive packing options
when doing a git-gc.  This allows them to do so without having
to use the low-level plumbing commands.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 Documentation/git-gc.txt           |    3 ++-
 Documentation/git-pack-objects.txt |   19 +------------------
 Documentation/pack-options.txt     |   18 ++++++++++++++++++
 builtin-gc.c                       |   30 ++++++++++++++++++++++++++++--
 4 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/pack-options.txt

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index bc16584..3c807c2 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -8,7 +8,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository
 
 SYNOPSIS
 --------
-'git-gc' [--prune]
+'git-gc' [--prune] [--no-reuse-delta] [--window=N] [--depth=N]
 
 DESCRIPTION
 -----------
@@ -35,6 +35,7 @@ OPTIONS
 	repository at the same time (e.g. never use this option
 	in a cron script).
 
+include::pack-options.txt[]
 
 Configuration
 -------------
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index d9e11c6..aac71f6 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -73,18 +73,6 @@ base-name::
 	as if all refs under `$GIT_DIR/refs` are specified to be
 	included.
 
---window=[N], --depth=[N]::
-	These two options affect how the objects contained in
-	the pack are stored using delta compression.  The
-	objects are first internally sorted by type, size and
-	optionally names and compared against the other objects
-	within --window to see if using delta compression saves
-	space.  --depth limits the maximum delta depth; making
-	it too deep affects the performance on the unpacker
-	side, because delta data needs to be applied that many
-	times to get to the necessary object.
-	The default value for both --window and --depth is 10.
-
 --incremental::
 	This flag causes an object already in a pack ignored
 	even if it appears in the standard input.
@@ -120,12 +108,7 @@ base-name::
 	This flag makes the command not to report its progress
 	on the standard error stream.
 
---no-reuse-delta::
-	When creating a packed archive in a repository that
-	has existing packs, the command reuses existing deltas.
-	This sometimes results in a slightly suboptimal pack.
-	This flag tells the command not to reuse existing deltas
-	but compute them from scratch.
+include::pack-options.txt[]
 
 --delta-base-offset::
 	A packed archive can express base object of a delta as
diff --git a/Documentation/pack-options.txt b/Documentation/pack-options.txt
new file mode 100644
index 0000000..7b0ae5f
--- /dev/null
+++ b/Documentation/pack-options.txt
@@ -0,0 +1,18 @@
+--window=[N], --depth=[N]::
+	These two options affect how the objects contained in
+	the pack are stored using delta compression.  The
+	objects are first internally sorted by type, size and
+	optionally names and compared against the other objects
+	within --window to see if using delta compression saves
+	space.  --depth limits the maximum delta depth; making
+	it too deep affects the performance on the unpacker
+	side, because delta data needs to be applied that many
+	times to get to the necessary object.
+	The default value for both --window and --depth is 10.
+
+--no-reuse-delta::
+	When creating a packed archive in a repository that
+	has existing packs, the command reuses existing deltas.
+	This sometimes results in a slightly suboptimal pack.
+	This flag tells the command not to reuse existing deltas
+	but compute them from scratch.
diff --git a/builtin-gc.c b/builtin-gc.c
index 3b1f8c2..7e7775d 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -15,13 +15,15 @@
 
 #define FAILED_RUN "failed to run %s"
 
-static const char builtin_gc_usage[] = "git-gc [--prune]";
+static const char builtin_gc_usage[] = "git-gc [--prune] [--no-reuse-delta] [--window=N] [--depth=N]";
 
 static int pack_refs = -1;
 
+#define MAX_ADD 10
+
 static const char *argv_pack_refs[] = {"pack-refs", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
-static const char *argv_repack[] = {"repack", "-a", "-d", "-l", NULL};
+static const char *argv_repack[MAX_ADD] = {"repack", "-a", "-d", "-l", NULL};
 static const char *argv_prune[] = {"prune", NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
@@ -37,6 +39,21 @@ static int gc_config(const char *var, const char *value)
 	return git_default_config(var, value);
 }
 
+static append_option(const char **cmd, const char *opt, int max_length)
+{
+	int	i;
+
+	for (i=0; cmd[i]; i++)
+		;
+
+	if (i+2 >= max_length) {
+		fprintf(stderr, "Too many options specified\n");
+		exit(1);
+	}
+	cmd[i++] = opt;
+	cmd[i] = 0;
+}
+
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -53,6 +70,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			prune = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--no-reuse-delta")) {
+			append_option(argv_repack, "-f", MAX_ADD);
+			continue;
+		}
+		if (!strncmp(arg, "--window", 8) ||
+		    !strncmp(arg, "--depth", 7)) {
+			append_option(argv_repack, arg, MAX_ADD);
+			continue;
+		}
 		/* perhaps other parameters later... */
 		break;
 	}
-- 
1.5.2.rc1.20.g86b9-dirty

^ permalink raw reply related

* Re: [PATCH 6/6] user-manual: stop deprecating the manual
From: J. Bruce Fields @ 2007-05-08  2:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vbqgw787n.fsf@assigned-by-dhcp.cox.net>

On Mon, May 07, 2007 at 12:19:08PM -0700, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > FWIW I am a fan of the user manual. It is much, much better than the 
> > tutorial/manpage combo, when you want to learn how to use Git.
> 
> Likewise.  Thanks, JB.

OK, thanks!--b.

^ permalink raw reply

* [PATCH] Change all git-status output to be "comments"
From: Shawn O. Pearce @ 2007-05-08  2:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

You wouldn't believe how many of my merge commits end with
the following message from git-status:

  "nothing to commit (working directory clean)"

This happens because of the way I sometimes create (and resolve)
some types of merges.  My commit message buffer gets prepped with
the output of git-status, which has this nice message way down at the
bottom of the file, below a large-ish block of lines that start with
"#".  Since git-commit strips those lines, I always fail to see the
trailing line that doesn't start with "#" and consequently fail to
see that "nothing to commit ..." will be part of my merge message.

So always comment out the output of git-status.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 wt-status.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index a055990..a9ba1ae 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -336,13 +336,13 @@ void wt_status_print(struct wt_status *s)
 		if (s->amend)
 			printf("# No changes\n");
 		else if (s->workdir_dirty)
-			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
+			printf("# no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
 		else if (s->workdir_untracked)
-			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
+			printf("# nothing added to commit but untracked files present (use \"git add\" to track)\n");
 		else if (s->is_initial)
-			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
+			printf("# nothing to commit (create/copy files and use \"git add\" to track)\n");
 		else
-			printf("nothing to commit (working directory clean)\n");
+			printf("# nothing to commit (working directory clean)\n");
 	}
 }
 
-- 
1.5.2.rc2.716.gba45c

^ permalink raw reply related

* Re: Eclipse support status (was: Re: [EGIT 00/10] This weeks Eclipse patches)
From: Shawn O. Pearce @ 2007-05-08  2:07 UTC (permalink / raw)
  To: Grzegorz Kulewski; +Cc: Robin Rosenberg, git
In-Reply-To: <Pine.LNX.4.63.0705072331260.20408@alpha.polcom.net>

Grzegorz Kulewski <kangur@polcom.net> wrote:
> I wanted to ask about current status of Git-Eclipse support. Is this 
> plugin able to actually do anything or not? What works and what does not? 
> Can it be used by developers using Git? Or maybe git-svn or git-cvsserver 
> are better?

There's no support for committing through Eclipse yet, let alone
doing things like creating new branches or switching branches.

But Robin has done a lot of work on the status and history viewing
tools, to the point that you can use the QuickDiff feature in
Eclipse to see which parts of a file you have recently modified
(but not committed), which files/directories are modified in the
Navigator, and view the "git shortlog" for the currently selected
file/directory.

> If it it not usable at the moment do you have any idea when it may be 
> usable? I know several developers really waiting for such support even 
> early or beta quality.

I would say its *very* early alpha.  It could cause your Eclipse
workbench to run out of memory, and crash, especially on large
projects.  It also doesn't have that many features yet.  ;-)

-- 
Shawn.

^ permalink raw reply

* Re: FFmpeg considering GIT
From: Shawn O. Pearce @ 2007-05-08  2:03 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Linus Torvalds, Karl Hasselstr?m, Junio C Hamano, Carl Worth,
	Michael Niedermayer, Git Mailing List
In-Reply-To: <17983.6136.147062.346626@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> wrote:
> I have thought about rewriting it in a different language, but I
> haven't found anything that really appeals.  I don't want to go to
> C/GTK or C/Qt since that would make it hard to port to Windows and
> MacOS AFAIK.  Python/Tk would be a possibility, but I have never
> learnt python and I'm actually not all that comfortable with having to
> do things the object-oriented way.
> 
> Any suggestions?

Funny that you mention this.  Lately I have been hacking on git-gui,
trying to improve it and clean up some of the code.

I've thought about wxWindows but didn't really dig into it to see
how usuable it would be - primary reason is not everyone has it
installed on their system.  The same for GTK and Qt.  Actually I
don't even have GTK installed on my Mac but I did install Qt3
(took half a day!)  so I could build qgit at one point in time.

But almost everyone already has a wish installed.

I've thought about writing git-gui in C, but linking to the Tk
library for the "portable UI".  But not everyone has the Tcl/Tk
development headers and libraries installed, but they probably do
have the wish executable installed.

I want to limit the barrier to entry for git, and that means limiting
the barrier of entry for git-gui.  Keeping our requirements to a
minimum helps.

So I think I've settled on sticking to Tcl and its Tk extensions,
but making more use of newer Tcl constructs like namespaces.  If you
look at my `pu` branch of git-gui I have actually split the program
down into many files, and have started to organize the code in each
into different namespaces, depending on function.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH/RFC] diff: Make numstat machine friendly also for renames
From: Junio C Hamano @ 2007-05-08  1:58 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200705080345.26817.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>>> The numstat format for rename is now
>>>
>>>   added deleted TAB path for "src" TAB path for "dst" LF
>>>
>>> or if -z option is used
>>>
>>>   added deleted TAB path for "src" NUL NUL path for "dst" NUL
>> 
>> Why two NULs?
>
> That was the only way I could think of to separate pre-image name
> from posi-image name for renames. Note that file name might look like
> (part of) diffstat line, and there is no 'status' field in the
> numstat to mark rename (as there is in "git diff-tree --raw" output).

The --stat format is for human consumption, and --numstat (be it
with -z or without) is for machines, so I am not opposed to a
format change that gives information that is already computed
but currently is hard to parse.  If the format change breaks
existing scripts, we might want to do --numstat-extended,
though...

For example, I do not see a reason not to add "R98" in there.
I.e.

	added deleted status TAB "src" (TAB "dst"){0,1} LF
	added deleted status NUL "src" (NUL "dst"){0,1} NUL

where the dst path is present only when status says it is a
rename/copy, just like the --raw format.

> Did you mean --stat here?

No, I did mean --summary.  But that was foolish of me.  I forgot
that it had the same { namepart => namepart } issue.

>>> @@ -949,11 +955,19 @@ static void show_numstat(struct diffstat_t* data,
>>>  			printf("-\t-\t");
>>>  		else
>>>  			printf("%d\t%d\t", file->added, file->deleted);
>>> -		if (options->line_termination && !file->is_renamed &&
>>> +		if (options->line_termination &&
>>>  		    quote_c_style(file->name, NULL, NULL, 0))
>>>  			quote_c_style(file->name, NULL, stdout, 0);
>>>  		else
>>>  			fputs(file->name, stdout);
>>> +		if (file->is_renamed) {
>>> +			printf("%s", options->line_termination ? "\t" : "\0\0");
>
> It passes "make test".

But you already grepped to make sure this codepath is never
exercised ;-).

What I was hoping you to notice was that printf("%s", "\0\0")
thing.  %s would not even notice that the const char[] literal
is 2 bytes long.

^ permalink raw reply

* Re: How to set git commit timestamp
From: Shawn O. Pearce @ 2007-05-08  1:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <f1ol0d$tcd$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> wrote:
> By the way, is there fast-import version of git-quiltimport?

No, and I don't think its easy.  Isn't a quilt patchstack stored
as a series of patch files?  So "importing" it into Git requires
applying the patch to the base tree, then writing that base tree
to the ODB.  fast-import doesn't know how to run git-apply, though
Junio and I did kick it around (generally) a few months ago on #git.

-- 
Shawn.

^ permalink raw reply

* [PATCH] Use .git/MERGE_MSG in cherry-pick/revert
From: Shawn O. Pearce @ 2007-05-08  1:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Rather than storing the temporary commit message data in .msg (in
the working tree) we now store the message data in .git/MERGE_MSG.

By storing the message in the .git/ directory we are sure we will
never have a collision with a user file, should a project actually
have a ".msg" file in their top level tree.  We also don't need to
worry about leaving this stale file behind during a `reset --hard`
and have it show up in the output of status.

We are using .git/MERGE_MSG here to store the temporary message as
it is an already established convention between git-merge, git-am
and git-rebase that git-commit will default the user's edit buffer
to the contents of .git/MERGE_MSG.  If the user is going to need
to resolve this commit or wants to edit the message on their own
prepping that file with the desired message "just works".

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Resend of earlier patch, but with proper commit message.  ;-)

 builtin-revert.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 4ba0ee6..67c13a3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -237,6 +237,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	int i;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
+	const char *defmsg = git_path("MERGE_MSG");
 
 	git_config(git_default_config);
 	me = action == REVERT ? "revert" : "cherry-pick";
@@ -280,7 +281,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 * reverse of it if we are revert.
 	 */
 
-	msg_fd = hold_lock_file_for_update(&msg_file, ".msg", 1);
+	msg_fd = hold_lock_file_for_update(&msg_file, defmsg, 1);
 
 	encoding = get_encoding(message);
 	if (!encoding)
@@ -330,7 +331,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 				sha1_to_hex(head), "HEAD",
 				sha1_to_hex(next->object.sha1), oneline) ||
 			write_tree(head, 0, NULL)) {
-		const char *target = git_path("MERGE_MSG");
 		add_to_msg("\nConflicts:\n\n");
 		read_cache();
 		for (i = 0; i < active_nr;) {
@@ -345,10 +345,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			}
 		}
 		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up .msg");
-		unlink(target);
-		if (rename(".msg", target))
-			die ("Could not move .msg to %s", target);
+			die ("Error wrapping up %s", defmsg);
 		fprintf(stderr, "Automatic %s failed.  "
 			"After resolving the conflicts,\n"
 			"mark the corrected paths with 'git-add <paths>'\n"
@@ -362,7 +359,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		exit(1);
 	}
 	if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up .msg");
+		die ("Error wrapping up %s", defmsg);
 	fprintf(stderr, "Finished one %s.\n", me);
 
 	/*
@@ -376,11 +373,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				"-e", NULL);
+			return execl_git_cmd("commit", "-n", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				NULL);
+			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
 	}
 	if (reencoded_message)
 		free(reencoded_message);
-- 
1.5.2.rc2.716.gba45c

^ permalink raw reply related

* Re: How to set git commit timestamp
From: Jakub Narebski @ 2007-05-08  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <20070508013833.GB11311@spearce.org>

Shawn O. Pearce wrote:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
>> söndag 06 maj 2007 skrev Guido Ostkamp:

>>> I am writing a tool to convert from another SCM to git and need to replay 
>>> all checkins. I know how to set the log message and the author, but there 
>>> appears to be no command option for the timestamp (Mercurial for example, 
>>> has a 'hg commit -d date <file>' syntax for this).
>> 
>> If you are writing a converter then you should really use the plumbing 
>> commands rather than porcelains like git-commit and in particular you should 
>> look at git-fast-import for a  really fast methord of importing (and packing) 
>> objects.
> 
> As the author of a few convert-to-git things, fast-import really
> is the easiest way to import stuff into git, even if it doesn't
> need to be "fast".  The input format is quite simple to create,
> and is documented pretty heavily in the git-fast-import manual page.
> 
> There's also contrib/fast-import/import-tars.pl which handles
> decompressing and importing one or more tar files into a Git
> repository.  Works pretty well...

By the way, is there fast-import version of git-quiltimport?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 0/6] gitweb: Add combined diff support
From: Jakub Narebski @ 2007-05-08  1:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvef45cev.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> I've minimally tried this on my private machine.  Looks pretty
> nice for simple merges, but I think we would want --cc not -c
> most of the time.

Please note that --cc would need some changes to both git_difftree_body 
and git_patchset_body; it is not just matter of replacing 
'-c' by '--cc'. Hunk simplification might mean that whole patch 
vanishes. So sometimes we have difftree (raw diff, whatchanged) line 
which does not have corresponding patch, and there should be no "patch" 
link (this is harder part). This mean also that in git_patchset_body we 
need sometimes to skip some difftree line / difftree info line (this is 
easier part).

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.
From: Junio C Hamano @ 2007-05-08  1:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Gerrit Pape, git, Alex Riesen
In-Reply-To: <20070508013532.GA11311@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Gerrit's patch to try and use COMMIT_MSG feels wrong to me, as
> git-commit overwrites that file with what it gets from its "input".

Yes.

> I agree my patch steps us further from a "git whatnow", but we're
> already in deep with MERGE_MSG.  We might as well keep that existing
> convention that it can be used to prep the commit message for the
> next git-commit invocation, and record other data somehow for the
> "git whatnow" case.

Fair enough.

^ permalink raw reply

* Re: gitweb css - pixels?!
From: Allan Wind @ 2007-05-08  1:23 UTC (permalink / raw)
  To: git
In-Reply-To: <6efbd9b70705071613p23017509qaf9af12c1d14f9cb@mail.gmail.com>

On 2007-05-07T17:13:42-0600, Chris Riddoch wrote:
> I noticed myself squinting, on my 1400x1050 laptop screen, at the 10
> and 12-*pixel* font settings gitweb's CSS specifies before thinking to
> myself that I should bring this to the list's attention.

ctrl-+ (ctrl plus possible a couple of times) fixes that if you are
using a browser from the Mozilla Foundation.  It was perfectly readable
for me, but I fortunate enough to have a large LCD.


/Allan

^ permalink raw reply

* Re: [FAQ?] Rationale for git's way to manage the index
From: Shawn O. Pearce @ 2007-05-08  1:41 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Julian Phillips, Matthieu Moy, git
In-Reply-To: <20070507063505.GA31269@diana.vm.bytemark.co.uk>

Karl Hasselstr??m <kha@treskal.com> wrote:
> I thought "git add -i" was the best thing since sliced bread -- until
> I found the same feature in git-gui, but with a _much_ better
> interface. Just right-click on a hunk in a diff, and you have the
> option of staging/unstaging that hunk. Pure magic.

"git add -i" has a hunk splitting feature that git-gui lacks.
I'm thinking of adding features to git-gui to let you select a
region of a hunk using the text selection, and then stage only
that selection.  I also want to let you revert hunks from the
working directory copy.

But after reading Junio's comments about "git add -i" being a
possibly bad idea and instead letting you park everything into
a shelf, reset --hard your working directory to HEAD and then
pull things back off the shelf to be staged, I might want to
do that differently in git-gui...  like use a shelf.  ;-)


But I'm glad someone else finds the hunk feature useful in
git-gui.  I use it far too often myself.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH/RFC] diff: Make numstat machine friendly also for renames
From: Jakub Narebski @ 2007-05-08  1:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzm4g5ddu.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Instead of saving human readable rename information in the 'name'
>> field when diffstat info is generated, do it when writing --stat
>> output. Change --numstat output to be machine friendly.
>>
>> This makes result of git-diff --numstat more suitable for machines
>> also when renames are involved, by using format similar to the one for
>> renames in the raw diff format, instead of the format more suited for
>> humans.
>>
>> The numstat format for rename is now
>>
>>   added deleted TAB path for "src" TAB path for "dst" LF
>>
>> or if -z option is used
>>
>>   added deleted TAB path for "src" NUL NUL path for "dst" NUL
> 
> Why two NULs?

That was the only way I could think of to separate pre-image name
from posi-image name for renames. Note that file name might look like
(part of) diffstat line, and there is no 'status' field in the
numstat to mark rename (as there is in "git diff-tree --raw" output).

But it doesn't mean that this is the only way, that is why it is RFC.
Well that and the fact that this patch increases slightly memory
footprint.

> There are already a handful in-tree users of --numstat, and also
> a few tests scripts.  I think you would need to adjust them.

Right, I forgot to run "make test" to check which tests scripts
would need adjusting.

But result of "git grep -e -M --and -e --numstat -- t/" is empty,
so I don't think that any script test --numstat with rename detection.

>> The goal of this change is to make it possible to generate HTML
>> diffstat against first parent for merge commits in gitweb. The current
>> notation for renames, which looks for example like below:
>>
>>   t/{t6030-bisect-run.sh => t6030-bisect-porcelain.sh}
> 
> I do not have much objection against teaching --numstat to show
> the preimage pathnames.  I do not disagree with "the goal" of
> showing "git diff --stat -M $commit^1 $commit" even for merge
> commit.
> 
> But I do not see the connection between the two.  Why aren't you
> parsing --summary?

Did you mean --stat here? Because

  --summary::
        Output a condensed summary of extended header information
        such as creations, renames and mode changes.

I'd like to have diffstat for merge, similar to what "git pull <repo>"
does when doing true merge, not what "git commit" does.

And --stat is meant for human consumption, not for machine consumption.
File name may contain " => " inside. And there is no way to differentiate
between " => " in file name, and " => " separating "src" path name from
"dst" path name.

> Have you actually _tested_ your patch?

Compiled, but forgot to run "make test". But I have checked that it passes
"make test", which probably mean that we don't have enough coverage ;-)

>> @@ -949,11 +955,19 @@ static void show_numstat(struct diffstat_t* data,
>>  			printf("-\t-\t");
>>  		else
>>  			printf("%d\t%d\t", file->added, file->deleted);
>> -		if (options->line_termination && !file->is_renamed &&
>> +		if (options->line_termination &&
>>  		    quote_c_style(file->name, NULL, NULL, 0))
>>  			quote_c_style(file->name, NULL, stdout, 0);
>>  		else
>>  			fputs(file->name, stdout);
>> +		if (file->is_renamed) {
>> +			printf("%s", options->line_termination ? "\t" : "\0\0");
> 
> I know you marked it as RFC; but it is impolite to request
> comments from other people on a patch that does not do what you
> intended to do, without marking "this is untested".  It would
> waste people's time.

It passes "make test". I should perhaps mark more strongly that some
of _ideas_ are untested...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: How to set git commit timestamp
From: Shawn O. Pearce @ 2007-05-08  1:38 UTC (permalink / raw)
  To: Guido Ostkamp; +Cc: Robin Rosenberg, git
In-Reply-To: <200705062151.40261.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> söndag 06 maj 2007 skrev Guido Ostkamp:
> > I am writing a tool to convert from another SCM to git and need to replay 
> > all checkins. I know how to set the log message and the author, but there 
> > appears to be no command option for the timestamp (Mercurial for example, 
> > has a 'hg commit -d date <file>' syntax for this).
> 
> If you are writing a converter then you should really use the plumbing 
> commands rather than porcelains like git-commit and in particular you should 
> look at git-fast-import for a  really fast methord of importing (and packing) 
> objects.

As the author of a few convert-to-git things, fast-import really
is the easiest way to import stuff into git, even if it doesn't
need to be "fast".  The input format is quite simple to create,
and is documented pretty heavily in the git-fast-import manual page.

There's also contrib/fast-import/import-tars.pl which handles
decompressing and importing one or more tar files into a Git
repository.  Works pretty well...

So I agree with Robin, give fast-import a try.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.
From: Shawn O. Pearce @ 2007-05-08  1:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Gerrit Pape, git, Alex Riesen
In-Reply-To: <7vps5efnv8.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > What about this change instead?  We make cherry-pick/revert
> > use the same temporary file as merge, which is under .git/
> > (something Alex mentioned he wanted).  I think the use of ".msg"
> > in cherry-pick/revert has always just been a bug, and not a feature,
> > so I'm really not against changing things around like this.
> 
> While I would not say this is not an improvement, this makes
> MERGE_MSG even less about merges and pushes us away from a
> sensible "git whatnow".
 
I think that ship has already sailed.  Look at builtin-revert.c
on:

    333         const char *target = git_path("MERGE_MSG");

We're already using MERGE_MSG to prep the message for a conflicted
cherry-pick or revert that the user needs to resolve by hand.  I
think we do the same thing in git-rebase, don't we?

Gerrit's patch to try and use COMMIT_MSG feels wrong to me, as
git-commit overwrites that file with what it gets from its "input".

I agree my patch steps us further from a "git whatnow", but we're
already in deep with MERGE_MSG.  We might as well keep that existing
convention that it can be used to prep the commit message for the
next git-commit invocation, and record other data somehow for the
"git whatnow" case.

-- 
Shawn.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox