Git development
 help / color / mirror / Atom feed
* Re: [PATCH] bundle, fast-import: detect write failure
From: Jakub Narebski @ 2008-01-11  9:14 UTC (permalink / raw)
  To: git
In-Reply-To: <20080110162526.GB27808@artemis.madism.org>

Pierre Habouzit wrote:
> On Thu, Jan 10, 2008 at 01:00:15PM +0000, Jim Meyering wrote:

>> However, even if it's not technically required to fail at that point,
>> if it were my choice, I'd prefer to know when a .keep file whose
>> contents are unimportant just happens to reside on a bad spot on my
>> disk.  I/O errors should never be ignored.
> 
>   Actually I think .keep files are empty, so the write() should not be
> there in the first place, and we should only check for close() right ?
> not that it matters that much.

In theory the .keep file should contain description _why_ the pack
is made kept. In practice git creates IIRC empty .kep files.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] - Updated usage and simplified sub-command action invocation
From: Imran M Yousuf @ 2008-01-11  9:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8x2y8ahw.fsf@gitster.siamese.dyndns.org>

On Jan 10, 2008 12:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> imyousuf@gmail.com writes:
>
> > From: Imran M Yousuf <imyousuf@smartitengineering.com>
> >
> > - manual page of git-submodule and usage mentioned in git-subcommand.sh
> > were not same, thus synchronized them. In doing so also had to change the
> > way the subcommands were parsed.
> >
> > - Previous version did not allow commands such as "git-submodule add init
> > update". Thus not satisfying the following case -
> >
> > mkdir g; mkdir f; cd g/
> > touch g.txt; echo "sample text for g.txt" >> ./g.txt; git-init;
> > git-add g.txt; git-commit -a -m "First commit on g"
> > cd ../f/; ln -s ../g/ init
> > git-init; git-submodule add init update;
> > git-commit -a -m "With module update"
> > mkdir ../test; cd ../test
> > git-clone ../f/; cd f
> > git-submodule init update; git-submodule update update
> > cd ../..; rm -rf ./f/ ./test/ ./g/
>
> I find this too verbose with too little information.
>
> If I am reading you correctly, what you meant was that the way
> command parser was structured made subcommand names such as
> "init" and "update" reserved words, and it was impossible to use
> them as arguments to commands.
>
> You could have said something like this instead:
>
>         The command parser incorrectly made subcommand names to
>         git-submodule reserved, refusing them to be used as
>         parameters to subcommands.  For example,
>
>                 $ git submodule add init update
>
>         to add a submodule whose (symbolic) name is "init" and
>         that resides at path "update" was refused.
>
> That would have been much cleaner and easier on the reader than
> having to decipher what the 20+ command shell script sequence
> was doing.
>
> I do agree that the breakage is worth fixing, though.
>
> > +# Synopsis of this commands are as follows
> > +# git-submodule [--quiet] [-b branch] add <repository> [<path>]
> > +# git-submodule [--quiet] [--cached] [status] [--] [<path>...]
> > +# git-submodule [--quiet] init [--] [<path>...]
> > +# git-submodule [--quiet] update [--] [<path>...]
>
> I somehow feel that syntactically the original implementation
> that allowed subcommand specific options to come before the
> subcommand name was a mistake.  It may be easier for users that
> both "-b branch add" and "add -b branch" are accepted, but I
> have to wonder if it would really hurt if we made "-b branch
> add" a syntax error.

Just a point in this regard, if we allow to have both "-b branch add"
and "add -b branch" in that case there will be code redundancy as
there will have to parsing of "-b branch" in the subcommand parsing
and in the command module (module_add). I will be implementing now, to
support both; with the exception for --quiet.

>
> So how about reorganizing the top-level option parser like this:
>
>         while :
>         do
>                 case $# in 0) break ;; esac
>                 case "$1" in
>                 add | status | init | update)
>                         # we have found subcommand.
>                         command="$1"
>                         shift
>                         break ;;
>                 --)
>                         # end of parameters
>                         shift
>                         break ;;
>                 --quiet)
>                         quiet=1
>                         ;;
>                 -*)
>                         die "unknown option $1"
>                 esac
>                 shift
>         done
>         test -n "$command" || command=$default_command
>         module_$command "$@"
>
> And then make individual command implementations responsible for
> parsing their own options (and perhaps the common ones, to allow
> "git submodule add --quiet", but that is optional), like:
>
>         module_add () {
>                 while :
>                 do
>                         case $# in 0) break ;; esac
>                         case "$1" in
>                         --cached)
>                                 cached=1
>                                 ;;
>                         -b | --branch)
>                                 shift
>                                 branch="$1"
>                                 test -n "$branch" ||
>                                 die "no branch name after -b?"
>                                 ;;
>                         --)
>                                 shift
>                                 break
>                                 ;;
>                         --quiet)
>                                 quiet=1
>                                 ;;
>                         -*)
>                                 die "unknown option $1"
>                         esac
>                         shift
>                 done
>                 repo=$1
>                 path=$2
>                 ...
>         }
>
> In the above illustration I did not bother eliminating cut&paste
> duplication, but there may be a better way to share the piece to
> parse common options across subcommands option parsers and the
> toplevel one.
>
>



-- 
Imran M Yousuf

^ permalink raw reply

* Re: [PATCH] gitk: Update and fix Makefile
From: Junio C Hamano @ 2008-01-11  9:06 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Christian Stimming, git
In-Reply-To: <7vk5mg3fjf.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I applied three patches since last pull from you and pushed the
> result (pure gitk part) out in gitk branch:
>
> 	master.kernel.org:/pub/scm/git/git.git/ gitk
>
> Could you please pull?

Sorry, this was a very ill-behaved pull request.  The patches
picked up from the list I applied are these three:

Charles Bailey (1):
      gitk: Fix the Makefile to cope with systems lacking msgfmt

Christian Stimming (2):
      gitk: Fix typo in user message.
      gitk: Update German translation.

^ permalink raw reply

* Re: [PATCH] gitk: Update and fix Makefile
From: Junio C Hamano @ 2008-01-11  8:57 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Christian Stimming, git
In-Reply-To: <18308.17099.334609.80415@cargo.ozlabs.ibm.com>

I applied three patches since last pull from you and pushed the
result (pure gitk part) out in gitk branch:

	master.kernel.org:/pub/scm/git/git.git/ gitk

Could you please pull?

^ permalink raw reply

* Re: Decompression speed: zip vs lzo
From: Pierre Habouzit @ 2008-01-11  8:57 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linus Torvalds, Sam Vilain, Git Mailing List, Johannes Schindelin,
	Marco Costalba, Junio C Hamano
In-Reply-To: <alpine.LFD.1.00.0801101613050.3054@xanadu.home>

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

On Thu, Jan 10, 2008 at 09:30:59PM +0000, Nicolas Pitre wrote:
> On Thu, 10 Jan 2008, Linus Torvalds wrote:
> 
> > 
> > 
> > On Thu, 10 Jan 2008, Nicolas Pitre wrote:
> > > 
> > > Here's my rather surprising results:
> > > 
> > > My kernel repo pack size without the patch:	184275401 bytes
> > > Same repo with the above patch applied:		205204930 bytes
> > > 
> > > So it is only 11% larger.  I was expecting much more.
> > 
> > It's probably worth doing those statistics on some other projects.
> > 
> > Maybe the difference to other repositories isn't huge, and maybe the 
> > kernel *is* a good test-case, but I just wouldn't take that for granted. 
> 
> Obviously.
> 
> This was a really crud test, and my initial goal was to quickly dismiss 
> Pierre's assertion.  Turns out that he wasn't that wrong after all,

  Well that wasn't a random assertion, I made it, because I assumed that
a delta is usually less than a few hundred bytes, and as compression is
applied only to the delta without context, you end up packing 500 bytes
per 500 bytes which will seldomly have excellent compression ratios.

> and 
> if a significant increase in access speed by avoiding zlib for 82% of 
> object accesses can also be demonstrated for the kernel, then we have an 
> opportunity for some optimization tradeoff with no backward 
> compatibility concerns.

  Well, one could use the fact that deltas are not packed to avoid
copying them around, and that will _necessarily_ become a gain (you can
read them where they have been mmapped for instance). The number that
were given for git annotate use a compression of `0' which doesn't use
that fact, and I wouldn't be surprised to see a noticeable gain if one
does that.

  And actually, maybe that it's not the deltas we should not pack, but
objects under a certain size (say 512 bytes e.g. ?), whichever type they
have, and to have the code exploit that fact for real, and avoid copies.
With this criterion, I expect the repository to not grow a lot larger
(I'd say quite less than the 10% you had, as even in the kernel, there
_are_ some larger deltas, and we definitely loose space for them, I'd
expect less than a 5% size variation), and I _think_ it's worth
investigating. At least I expect visible results on commands (like blame
of even log[0]) that go through a lot of small objects to see 10 to 20%
increase speed (backed up by some experience I have in avoiding copies
in not-so-similar cases though, so it may be less, and I'll stand
corrected -- and disappointed, a bit).

  [0] If I'm correct commit messages are "objects" on their own, and I
      don't expect them to be very often over 512 octets.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [PATCH] additional help when editing during interactive rebase
From: Junio C Hamano @ 2008-01-11  8:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, William Morgan
In-Reply-To: <alpine.LSU.1.00.0801091120150.31053@racer.site>

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

> Hi,
>
> On Tue, 8 Jan 2008, Junio C Hamano wrote:
>
>> I would have removed those empty lines around the instruction if I were 
>> patching this, though.  Losing 5 lines out of 25-line terminal was 
>> marginally Ok.  Losing 9 lines 4 lines too many and is unacceptable.
>> 
>> Thoughts?
>
> I wonder if it would not make even more sense to record the current HEAD 
> name, and call "commit --amend" if it is the same upon "--continue".

My understanding of the original issue is that "git-rebase -i"
stops at 'edit' and gives the user a chance to muck with the
commit, saying "do whatever you want now and then record the
result with git commit --amend".  The user can follow that but
then needs to say "git rebase --continue" after that.  The insn
does not talk about it, so after running "git commit --amend" as
told, a clueless user is left wondering "huh, and then now
what?".

Do you mean you would instead suggest "git rebase --continue" in
the insn, and make the workflow like this:

	$ git rebase -i ...
        Now do whatever you want and say "rebase --continue"
	$ edit foo.c
        $ git add foo.c
        $ git rebase --continue

and have "rebase --continue" to continue with the modified
contents recorded in the index, invoking "git commit --amend",
but doing so only if the user hasn't run "git commit" with or
without --amend yet?

It feels like a better automation than what we currently have,
but I somewhat worry how that would change the user experience
for using 'edit' to split a commit into two or more.

^ permalink raw reply

* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Junio C Hamano @ 2008-01-11  8:00 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: git
In-Reply-To: <1200022189-2400-2-git-send-email-mlevedahl@gmail.com>

Mark Levedahl <mlevedahl@gmail.com> writes:

> This introduces a new configuration variable, remotes.default, that
> defines the name of the default remote to be used.

Does this mean "default" is now a new reserved word that cannot
be used as "git remote update default"?

> ... Traditionally, this
> is "origin", and could be overridden for a given branch. This change
> introduces a way to redefine the default as desired and have that honored
> regardless of the currently checked out head (e.g., remotes.default is
> used when on a detached head or any other non-tracking branch).

I'd 100% agree that being able to use anything not just
hardcoded 'origin' is much better than not being able to.  I do
not have much against that goal.

However, it is a bit hard to judge how much of inconvenience it
really is in your real life that the current behaviour does not
allow you to.

In your cover letter, you said:

>> As my project is distributed across multiple domains with
>> many firewalls and airgaps such that no single server is
>> available to all, we really need to use nicknames to refer
>> to different servers,...

If you need to access different repositories on different
machines from your submodules, you would of course need to
access different domains from your submodule repositories.  But
that does not mean each of them cannot be named 'origin'.  That
name is local to each of the submodule (and the toplevel) and
can point at different domains over different transfer channels.

> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index 695a409..1b235e0 100755
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,9 @@ get_remote_url () {
>  
>  get_default_remote () {
>  	curr_branch=$(git symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')
> -	origin=$(git config --get "branch.$curr_branch.remote")
> -	echo ${origin:-origin}
> +	git config --get "branch.$curr_branch.remote" ||
> +	git config --get "remotes.default" ||
> +	echo origin

This sequence cascaded with || is much nicer than the original,
even if it did not change the behaviour.

> @@ -300,6 +305,10 @@ static void read_config(void)
>  			make_branch(head_ref + strlen("refs/heads/"), 0);
>  	}
>  	git_config(handle_config);
> +	if (!default_remote_name) {
> +			default_remote_name = remotes_default_name ?
> +				remotes_default_name : xstrdup("origin");
> +	}

Is this a bit too deep indentation?

^ permalink raw reply

* [PATCH] Use commit template when cherry picking
From: Deepak Saxena @ 2008-01-11  7:45 UTC (permalink / raw)
  To: git; +Cc: Perry Wagle

When using a workflow with a default commit message template/header,
git-cherry-pick should also pick that up. Users can manually read
the header into the commit message, but this is simpler.

Signed-off-by: Deepak Saxena <dsaxena@mvista.com>

---

We are using git to manage our kernel tree and we often cherry-pick
patches from upstream and while we want to save the original commit
message, we also want to apply our default commit header to the log.
If this patch is not the ideal solution, please let me know how
I should go about this.

(I'm not on the mailing list, so please cc: me on replies)

diff --git a/builtin-commit.c b/builtin-commit.c
index 73f1e35..6bd937a 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -43,7 +43,8 @@ static enum {
 	COMMIT_PARTIAL,
 } commit_style;
 
-static char *logfile, *force_author, *template_file;
+char *template_file;
+static char *logfile, *force_author;
 static char *edit_message, *use_message;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
@@ -89,7 +90,7 @@ static struct option builtin_commit_options[] = {
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by: header"),
-	OPT_STRING('t', "template", &template_file, "FILE", "use specified template file"),
+	OPT_STRING('t', "template", &template_file, "FILE", "use specified commit message template file"),
 	OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
 
 	OPT_GROUP("Commit contents options"),
diff --git a/builtin-revert.c b/builtin-revert.c
index 4bf8eb2..cbb57c6 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -8,6 +8,7 @@
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "strbuf.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -49,6 +50,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
 		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
+		OPT_STRING('t', "template", &template_file, "FILE", "use specified commit message template file"),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_END(),
@@ -249,16 +251,24 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
+	struct strbuf sb;
 	int i;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
 	const char *defmsg = xstrdup(git_path("MERGE_MSG"));
 
-	git_config(git_default_config);
+	git_config(git_commit_config);	
 	me = action == REVERT ? "revert" : "cherry-pick";
 	setenv(GIT_REFLOG_ACTION, me, 0);
 	parse_args(argc, argv);
 
+	strbuf_init(&sb, 0);
+	if (template_file) {
+		if (strbuf_read_file(&sb, template_file, 0) < 0)
+			 die("could not read %s: %s", 
+				template_file, strerror(errno));
+	}
+
 	/* this is copied from the shell script, but it's never triggered... */
 	if (action == REVERT && !no_replay)
 		die("revert is incompatible with replay");
@@ -332,6 +342,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	oneline = get_oneline(message);
 
+	add_to_msg(sb.buf);
+
 	if (action == REVERT) {
 		char *oneline_body = strchr(oneline, ' ');
 
diff --git a/commit.h b/commit.h
index 10e2b5d..7746ec6 100644
--- a/commit.h
+++ b/commit.h
@@ -23,6 +23,7 @@ struct commit {
 
 extern int save_commit_buffer;
 extern const char *commit_type;
+extern char *template_file;
 
 /* While we can decorate any object with a name, it's only used for commits.. */
 extern struct decoration name_decoration;
@@ -116,6 +117,8 @@ int in_merge_bases(struct commit *, struct commit **, int);
 extern int interactive_add(int argc, const char **argv, const char *prefix);
 extern int rerere(void);
 
+extern int git_commit_config(const char *k, const char *v);
+
 static inline int single_parent(struct commit *commit)
 {
 	return commit->parents && !commit->parents->next;

-- 
   _____   __o  Living Deepak Saxena - CarFree and CareFree            (o>
------    -\<,  Towards Carfree Cities 2008 - www.carfreeportland.org  //\
 ----- ( )/ ( ) Linux Plumber's Conference - www.linuxplumbersconf.org V_/_
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

^ permalink raw reply related

* [PATCH] Use new compress helpers in sha1_file.c
From: Marco Costalba @ 2008-01-11  7:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

A multistep compress is required here, so
we need the full arsenal of compress helpers.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 sha1_file.c |   41 ++++++++++++-----------------------------
 1 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 6583797..6c94bd5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -7,6 +7,7 @@
  * creation etc.
  */
 #include "cache.h"
+#include "compress.h"
 #include "delta.h"
 #include "pack.h"
 #include "blob.h"
@@ -2086,33 +2087,23 @@ int write_sha1_file(void *buf, unsigned long
 	}

 	/* Set it up */
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	size = 8 + deflateBound(&stream, len+hdrlen);
+	size = 8 + compress_alloc(&stream, zlib_compression_level, len+hdrlen);
 	compressed = xmalloc(size);

 	/* Compress it */
-	stream.next_out = compressed;
-	stream.avail_out = size;
+	compress_start(&stream, (unsigned char *)hdr, hdrlen, compressed, size);

 	/* First header.. */
-	stream.next_in = (unsigned char *)hdr;
-	stream.avail_in = hdrlen;
-	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */;
+	compress_next(&stream, Z_NO_FLUSH);

 	/* Then the data itself.. */
 	stream.next_in = buf;
 	stream.avail_in = len;
-	ret = deflate(&stream, Z_FINISH);
+	ret = compress_next(&stream, Z_FINISH);
 	if (ret != Z_STREAM_END)
 		die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), ret);

-	ret = deflateEnd(&stream);
-	if (ret != Z_OK)
-		die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), ret);
-
-	size = stream.total_out;
+	size = compress_free(&stream);

 	if (write_buffer(fd, compressed, size) < 0)
 		die("unable to write sha1 file");
@@ -2147,30 +2138,22 @@ static void *repack_object(const unsigned char *sha1,
 	hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;

 	/* Set it up */
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	size = deflateBound(&stream, len + hdrlen);
+	size = compress_alloc(&stream, zlib_compression_level, len + hdrlen);
 	buf = xmalloc(size);

 	/* Compress it */
-	stream.next_out = buf;
-	stream.avail_out = size;
+	compress_start(&stream, (unsigned char *)hdr, hdrlen, buf, size);

 	/* First header.. */
-	stream.next_in = (void *)hdr;
-	stream.avail_in = hdrlen;
-	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */;
+	compress_next(&stream, Z_NO_FLUSH);

 	/* Then the data itself.. */
 	stream.next_in = unpacked;
 	stream.avail_in = len;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&stream);
-	free(unpacked);
+	compress_next(&stream, Z_FINISH);

-	*objsize = stream.total_out;
+	*objsize = compress_free(&stream);
+	free(unpacked);
 	return buf;
 }

-- 
1.5.4.rc2.89.g1b3f-dirty

^ permalink raw reply related

* [PATCH 4/5] Use new compress helpers in http-push.c
From: Marco Costalba @ 2008-01-11  7:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

A multistep compress is required here, so
we need the full arsenal of compress helpers.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 http-push.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/http-push.c b/http-push.c
index 55d0c94..b7fe57f 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "commit.h"
+#include "compress.h"
 #include "pack.h"
 #include "tag.h"
 #include "blob.h"
@@ -491,31 +492,24 @@ static void start_put(struct transfer_request
 	hdrlen = sprintf(hdr, "%s %lu", typename(type), len) + 1;

 	/* Set it up */
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	size = deflateBound(&stream, len + hdrlen);
+	size = compress_alloc(&stream, zlib_compression_level, len + hdrlen);
 	strbuf_init(&request->buffer.buf, size);
 	request->buffer.posn = 0;

 	/* Compress it */
-	stream.next_out = (unsigned char *)request->buffer.buf.buf;
-	stream.avail_out = size;
+	compress_start(&stream, (void *)hdr, hdrlen,
+                      (unsigned char *)request->buffer.buf.buf, size);

 	/* First header.. */
-	stream.next_in = (void *)hdr;
-	stream.avail_in = hdrlen;
-	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */;
+	compress_next(&stream, Z_NO_FLUSH);

 	/* Then the data itself.. */
 	stream.next_in = unpacked;
 	stream.avail_in = len;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&stream);
-	free(unpacked);
+	compress_next(&stream, Z_FINISH);

-	request->buffer.buf.len = stream.total_out;
+	request->buffer.buf.len = compress_free(&stream);
+	free(unpacked);

 	request->url = xmalloc(strlen(remote->url) +
 			       strlen(request->lock->token) + 51);
-- 
1.5.4.rc2.89.g1b3f-dirty

^ permalink raw reply related

* [PATCH 3/5] Use new compress helpers in fast-import
From: Marco Costalba @ 2008-01-11  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

Here is slightly more difficult, in particular
a xrealloc() has been substituted with a
free() + xmalloc() to keep the code simple.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 fast-import.c |   44 +++++++++++++++------------------------
 1 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 74597c9..6166d4a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -141,6 +141,7 @@ Format of STDIN stream:

 #include "builtin.h"
 #include "cache.h"
+#include "compress.h"
 #include "object.h"
 #include "blob.h"
 #include "tree.h"
@@ -994,13 +995,13 @@ static int store_object(
 	unsigned char *sha1out,
 	uintmax_t mark)
 {
-	void *out, *delta;
+	unsigned char *out, *delta;
 	struct object_entry *e;
 	unsigned char hdr[96];
 	unsigned char sha1[20];
 	unsigned long hdrlen, deltalen;
 	SHA_CTX c;
-	z_stream s;
+	int out_size;

 	hdrlen = sprintf((char*)hdr,"%s %lu", typename(type),
 		(unsigned long)dat->len) + 1;
@@ -1036,24 +1037,15 @@ static int store_object(
 	} else
 		delta = NULL;

-	memset(&s, 0, sizeof(s));
-	deflateInit(&s, zlib_compression_level);
-	if (delta) {
-		s.next_in = delta;
-		s.avail_in = deltalen;
-	} else {
-		s.next_in = (void *)dat->buf;
-		s.avail_in = dat->len;
-	}
-	s.avail_out = deflateBound(&s, s.avail_in);
-	s.next_out = out = xmalloc(s.avail_out);
-	while (deflate(&s, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&s);
+	if (delta)
+		out_size = compress_all(zlib_compression_level, delta, deltalen, &out);
+	else
+		out_size = compress_all(zlib_compression_level,
+                                       (unsigned char *)dat->buf,
dat->len, &out);

 	/* Determine if we should auto-checkpoint. */
-	if ((pack_size + 60 + s.total_out) > max_packsize
-		|| (pack_size + 60 + s.total_out) < pack_size) {
+	if ((pack_size + 60 + out_size) > max_packsize
+		|| (pack_size + 60 + out_size) < pack_size) {

 		/* This new object needs to *not* have the current pack_id. */
 		e->pack_id = pack_id + 1;
@@ -1064,15 +1056,9 @@ static int store_object(
 			free(delta);
 			delta = NULL;

-			memset(&s, 0, sizeof(s));
-			deflateInit(&s, zlib_compression_level);
-			s.next_in = (void *)dat->buf;
-			s.avail_in = dat->len;
-			s.avail_out = deflateBound(&s, s.avail_in);
-			s.next_out = out = xrealloc(out, s.avail_out);
-			while (deflate(&s, Z_FINISH) == Z_OK)
-				/* nothing */;
-			deflateEnd(&s);
+			free(out);
+			out_size = compress_all(zlib_compression_level,
+                                               (unsigned char
*)dat->buf, dat->len, &out);
 		}
 	}

@@ -1105,8 +1091,8 @@ static int store_object(
 		pack_size += hdrlen;
 	}

-	write_or_die(pack_data->pack_fd, out, s.total_out);
-	pack_size += s.total_out;
+	write_or_die(pack_data->pack_fd, out, out_size);
+	pack_size += out_size;

 	free(out);
 	free(delta);
-- 
1.5.4.rc2.89.g1b3f-dirty

^ permalink raw reply related

* [PATCH 2/5] Use new compress helpers in git files
From: Marco Costalba @ 2008-01-11  7:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

These are the 'easy' ones, where a signgle step
compression is requested so that we can use only
one call to compress_all()

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
 archive-zip.c          |   28 +++-------------------------
 builtin-pack-objects.c |   21 ++++-----------------
 diff.c                 |   22 +++++-----------------
 index-pack.c           |   20 +++-----------------
 4 files changed, 15 insertions(+), 76 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 74e30f6..9071b86 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "commit.h"
+#include "compress.h"
 #include "blob.h"
 #include "tree.h"
 #include "quote.h"
@@ -97,33 +98,10 @@ static void copy_le32(unsigned char *dest,
 static void *zlib_deflate(void *data, unsigned long size,
                           unsigned long *compressed_size)
 {
-	z_stream stream;
-	unsigned long maxsize;
-	void *buffer;
-	int result;
-
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	maxsize = deflateBound(&stream, size);
-	buffer = xmalloc(maxsize);
-
-	stream.next_in = data;
-	stream.avail_in = size;
-	stream.next_out = buffer;
-	stream.avail_out = maxsize;
-
-	do {
-		result = deflate(&stream, Z_FINISH);
-	} while (result == Z_OK);
-
-	if (result != Z_STREAM_END) {
-		free(buffer);
-		return NULL;
-	}

-	deflateEnd(&stream);
-	*compressed_size = stream.total_out;
+	unsigned char *buffer = NULL;

+	*compressed_size = compress_all(zlib_compression_level, data, size, &buffer);
 	return buffer;
 }

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index a39cb82..66dedf9 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "compress.h"
 #include "attr.h"
 #include "object.h"
 #include "blob.h"
@@ -409,9 +410,7 @@ static unsigned long write_object(struct sha1file *f,
 				 */

 	if (!to_reuse) {
-		z_stream stream;
-		unsigned long maxsize;
-		void *out;
+		unsigned char *out = NULL;
 		if (!usable_delta) {
 			buf = read_sha1_file(entry->idx.sha1, &obj_type, &size);
 			if (!buf)
@@ -432,20 +431,8 @@ static unsigned long write_object(struct sha1file *f,
 				OBJ_OFS_DELTA : OBJ_REF_DELTA;
 		}
 		/* compress the data to store and put compressed length in datalen */
-		memset(&stream, 0, sizeof(stream));
-		deflateInit(&stream, pack_compression_level);
-		maxsize = deflateBound(&stream, size);
-		out = xmalloc(maxsize);
-		/* Compress it */
-		stream.next_in = buf;
-		stream.avail_in = size;
-		stream.next_out = out;
-		stream.avail_out = maxsize;
-		while (deflate(&stream, Z_FINISH) == Z_OK)
-			/* nothing */;
-		deflateEnd(&stream);
-		datalen = stream.total_out;
-		deflateEnd(&stream);
+		datalen = compress_all(pack_compression_level, buf, size, &out);
+
 		/*
 		 * The object header is a byte of 'type' followed by zero or
 		 * more bytes of length.
diff --git a/diff.c b/diff.c
index b18c140..43f537c 100644
--- a/diff.c
+++ b/diff.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2005 Junio C Hamano
  */
 #include "cache.h"
+#include "compress.h"
 #include "quote.h"
 #include "diff.h"
 #include "diffcore.h"
@@ -1037,23 +1038,10 @@ static unsigned char *deflate_it(char *data,
 				 unsigned long size,
 				 unsigned long *result_size)
 {
-	int bound;
-	unsigned char *deflated;
-	z_stream stream;
-
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	bound = deflateBound(&stream, size);
-	deflated = xmalloc(bound);
-	stream.next_out = deflated;
-	stream.avail_out = bound;
-
-	stream.next_in = (unsigned char *)data;
-	stream.avail_in = size;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		; /* nothing */
-	deflateEnd(&stream);
-	*result_size = stream.total_out;
+	unsigned char *deflated = NULL;
+
+	*result_size = compress_all(zlib_compression_level,
+                                   (unsigned char *)data, size, &deflated);
 	return deflated;
 }

diff --git a/index-pack.c b/index-pack.c
index 9fd6982..880088e 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "compress.h"
 #include "delta.h"
 #include "pack.h"
 #include "csum-file.h"
@@ -494,24 +495,9 @@ static void parse_pack_objects(unsigned char

 static int write_compressed(int fd, void *in, unsigned int size,
uint32_t *obj_crc)
 {
-	z_stream stream;
-	unsigned long maxsize;
-	void *out;
+	unsigned char *out = NULL;

-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, zlib_compression_level);
-	maxsize = deflateBound(&stream, size);
-	out = xmalloc(maxsize);
-
-	/* Compress it */
-	stream.next_in = in;
-	stream.avail_in = size;
-	stream.next_out = out;
-	stream.avail_out = maxsize;
-	while (deflate(&stream, Z_FINISH) == Z_OK);
-	deflateEnd(&stream);
-
-	size = stream.total_out;
+	size = compress_all(zlib_compression_level, in, size, &out);
 	write_or_die(fd, out, size);
 	*obj_crc = crc32(*obj_crc, out, size);
 	free(out);
-- 
1.5.4.rc2.89.g1b3f-dirty

^ permalink raw reply related

* Re: [PATCH] bundle, fast-import: detect write failure
From: Junio C Hamano @ 2008-01-11  7:36 UTC (permalink / raw)
  To: Jim Meyering; +Cc: Johannes Schindelin, git list
In-Reply-To: <87hchlhm3k.fsf@rho.meyering.net>

Jim Meyering <jim@meyering.net> writes:

> On the other hand, if that write failure is truly ignorable,
> a mindless minimalist :-) might argue that it's best just to
> omit the syscall.

Usually the contents of .keep file is a small one-line comment
that describes who decided that the pack needs to be kept and
why, so the answer is no.

In this case, a failure while closing that small .keep file is
highly unlikely, and if we ever mange to trigger such a highly
unlikely failure, I think we would rather want to *know* about
it, as it is likely there is something more seriously wrong
going on.

So let's keep that check on close().

^ permalink raw reply

* [PATCH 1/5] Introduce stream compress helpers (v2 and complete)
From: Marco Costalba @ 2008-01-11  7:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

When decompressing a zlib stream use this
helpers instead of calling low level zlib
function.

This patch introduces the necessary framework,
still no code change.

This is the first step in generalizing compress and
decompress functions avoiding zlib directly calls.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---

This is the full compress cleanup series, no more open coded
zlib compress calls are around anymore

New in this version is:

- Renaming according to Linus suggestions
- Added of a compress_free() that seems to suit nicely
- git is zlib compress functions free now


Next step will be decompress helpers


 Makefile   |    4 ++--
 compress.c |   58 ++++++++++++++++++++++++++++++++++++
 compress.h |   12 ++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 compress.c
 create mode 100644 compress.h

diff --git a/Makefile b/Makefile
index 21c80e6..89bd99d 100644
--- a/Makefile
+++ b/Makefile
@@ -288,7 +288,7 @@ LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a

 LIB_H = \
-	archive.h blob.h cache.h cache-tree.h commit.h csum-file.h delta.h grep.h \
+	archive.h blob.h cache.h cache-tree.h commit.h compress.h
csum-file.h delta.h grep.h \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
@@ -301,7 +301,7 @@ DIFF_OBJS = \
 	diffcore-delta.o log-tree.o

 LIB_OBJS = \
-	blob.o commit.o connect.o csum-file.o cache-tree.o base85.o \
+	blob.o commit.o compress.o connect.o csum-file.o cache-tree.o base85.o \
 	date.o diff-delta.o entry.o exec_cmd.o ident.o \
 	pretty.o interpolate.o hash.o \
 	lockfile.o \
diff --git a/compress.c b/compress.c
new file mode 100644
index 0000000..be771a9
--- /dev/null
+++ b/compress.c
@@ -0,0 +1,58 @@
+#include "cache.h"
+#include "compress.h"
+
+unsigned long compress_alloc(z_stream *stream, int level, unsigned long size)
+{
+	memset(stream, 0, sizeof(*stream));
+	deflateInit(stream, level);
+	return deflateBound(stream, size);
+}
+
+int compress_start(z_stream *stream,
+                   unsigned char *in, unsigned long in_size,
+                   unsigned char *out, unsigned long out_size)
+{
+	stream->next_out = (out ? out : xmalloc(out_size));
+	stream->avail_out = out_size;
+	stream->next_in = in;
+	stream->avail_in = in_size;
+	return Z_OK;
+}
+
+int compress_next(z_stream *stream, int flush)
+{
+	int result;
+
+	do {
+		result = deflate(stream, flush);
+	} while (result == Z_OK);
+
+	return result;
+}
+
+unsigned long compress_free(z_stream *stream)
+{
+	deflateEnd(stream);
+	return stream->total_out;
+}
+
+unsigned long compress_all(int level, unsigned char *data,
+                           unsigned long size, unsigned char **out)
+{
+	int bound, result;
+	z_stream stream;
+
+	bound = compress_alloc(&stream, level, size);
+	compress_start(&stream, data, size, NULL, bound);
+
+	*out = stream.next_out;
+	result = compress_next(&stream, Z_FINISH);
+
+	if (result != Z_STREAM_END) {
+		compress_free(&stream);
+		free(*out);
+		*out = NULL;
+		return 0;
+	}
+	return compress_free(&stream);
+}
diff --git a/compress.h b/compress.h
new file mode 100644
index 0000000..16b0147
--- /dev/null
+++ b/compress.h
@@ -0,0 +1,12 @@
+#ifndef COMPRESS_H
+#define COMPRESS_H
+
+extern unsigned long compress_alloc(z_stream *stream, int level,
unsigned long size);
+extern int compress_start(z_stream *stream, unsigned char *in,
unsigned long in_size,
+                           unsigned char *out, unsigned long out_size);
+extern int compress_next(z_stream *stream, int flush);
+extern unsigned long compress_free(z_stream *stream);
+extern unsigned long compress_all(int level, unsigned char *data,
unsigned long size,
+                                  unsigned char **out);
+
+#endif
-- 
1.5.4.rc2.89.g1b3f-dirty

^ permalink raw reply related

* Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c
From: Junio C Hamano @ 2008-01-11  7:26 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Git Mailing List, Nicolas Pitre
In-Reply-To: <e5bfff550801102210g181a091s28725e24922b9b12@mail.gmail.com>

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

> On Jan 10, 2008 10:34 PM, Marco Costalba <mcostalba@gmail.com> wrote:
>>
>> -               while (deflate(&stream, Z_FINISH) == Z_OK)
>> -                       /* nothing */;
>> -               deflateEnd(&stream);
>> -               datalen = stream.total_out;
>> -               deflateEnd(&stream);
>
>
> One thing I would like someone to clarify is why deflateEnd(&stream)
> is called two times ?

I'd call it an insufficient reviewing while accepting

    http://thread.gmane.org/gmane.comp.version-control.git/45996

In fact, the second deflateEnd() has always been returning
Z_STREAM_ERROR.  We just never checked the error return from
that particular deflateEnd().

The first one returns 0 for success.  We might want to tighten
the check even more to check that.

-- >8 --
pack-objects: remove redundant call to deflateEnd()

We somehow called deflateEnd() on a stream that we have called
deflateEnd() on already.

Noticed by Marco.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-pack-objects.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index a39cb82..ec10238 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -445,7 +445,7 @@ static unsigned long write_object(struct sha1file *f,
 			/* nothing */;
 		deflateEnd(&stream);
 		datalen = stream.total_out;
-		deflateEnd(&stream);
+
 		/*
 		 * The object header is a byte of 'type' followed by zero or
 		 * more bytes of length.

^ permalink raw reply related

* Re: CRLF problems with Git on Win32
From: Steffen Prohaska @ 2008-01-11  7:10 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Gregory Jefferis, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801101556380.3148@woody.linux-foundation.org>


On Jan 11, 2008, at 1:02 AM, Linus Torvalds wrote:

>
>
> On Thu, 10 Jan 2008, Gregory Jefferis wrote:
>>
>> So this is what has to be accommodated.  But instead of having  
>> autocrlf
>> always set on Windows and always converting to LF in the  
>> repository, why not
>> do nothing by default [ .. ]
>
> Why? You can screw yourself more, and much more easily (and much more
> subtly), by leaving CRLF alone on Windows.
>
> The thing is, 99.9% of all people will be *much* better off with
> autocrlf=true on Windows than with it defaulting to off (or even  
> fail).
>
> Isn't *that* the whole point of having a default? Pick the thing  
> that is
> the right thing for almost everybody?

Are you also for "autocrlf=input" as the default on Unix?  This
is the second half of the solution to the cross-platform problem ...


> And no, "but think of the children.." is not a valid argument.  
> Sure, you
> *can* corrupt binary imags with CRLF conversion. But it's really quite
> hard, since the git heuristics for guessing are rather good. You  
> really
> have to work at it, and if you do, you're pretty damn likely to  
> know about
> the issue, so that 0.1% that really needs to not convert (and it's  
> usually
> one specific file type!) would probably not even turn off CRLF, but  
> rather
> add a .gitattributes entry for that one filetype!

... and then Windows and Unix users would have the same chance of
data corruption.

Which is very low, yes, but unfortunately it already hit me once
and I didn't immediately recognized what happend.  I guess that
less experienced git used would have a harder time to understand.
However, I don't have a test case at hand.  I should probably
better go and find one.  So for now, you may just want to ignore
this comment.

Yet, I'm a bit paranoid about the potential data corruption.  The
way data would be corrupted during commit can't be easily fixed.
You only have a chance for fixing this if you recognize the
problem before you delete the file in your work tree.  But
because git is extremely good at preserving your data once you
committed a file, I tend to feel _very_ safe after I committed and
I am teaching all people that once they committed data to git
they'll not loose it until the reflog expires (well and obviously
they must not delete .git).


> (Side note: if there are known filetype extensions that have  
> problems with
> the git guessing, we sure as heck could take the filename into account
> when guessing! There's absolutely nothing that says that we only  
> have to
> look at the contents when guessing about the text/binary thing!)

Looking on the content seems the right thing to do.  The filetype
extension could be misleading.

Maybe a mechanism similar to the file command would be more
valuable.  I guess a stripped down variant should be sufficient.

	Steffen

^ permalink raw reply

* Re: Decompression speed: zip vs lzo
From: Sam Vilain @ 2008-01-11  7:05 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Linus Torvalds, Nicolas Pitre, Pierre Habouzit, Git Mailing List,
	Johannes Schindelin, Marco Costalba, Junio C Hamano
In-Reply-To: <47870CDF.4010606@vilain.net>

Sam Vilain wrote:
> sv.c has about 1500 revisions, though the oldest line is 

only about 900 revisions old.

^ permalink raw reply

* Re: [PATCH] gitweb: Change feeds from commit to commitdiff output.
From: Florian La Roche @ 2008-01-11  6:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzll5t4u.fsf@gitster.siamese.dyndns.org>

On Thu, Jan 10, 2008 at 12:21:21PM -0800, Junio C Hamano wrote:
> Florian La Roche <laroche@redhat.com> writes:
> 
> > [PATCH] gitweb: Change feeds from commit to commitdiff output.
> >
> > Change feeds from displaying the commit to displaying the commitdiff
> > output. This way the patches are shown directly and code review is
> > done more easily via watching feeds.
> >
> > Signed-off-by: Florian La Roche <laroche@redhat.com>
> 
> I can see that easier access to commitdiff output is sometimes
> desirable.
> 
> If you are making this change unconditional, however, I think
> there needs a list discussion between you and the silent
> majority of people that have been perfectly happy with the
> current "log only" behaviour.


Hello Junio,

Right, this is a change in behaviour. Maybe a config option
for this would be good, so that users can configure their
wanted output style.


> And to have that discussion, you first have to wake them up,
> which this patch would serve well as a wake-up call.  But if
> that was the purpose of the posting, please (1) mark the patch
> as such (commonly done by saying [RFC/PATCH] instead), and (2)
> keep me out of the "To:" list, if the patch is not for inclusion
> but for discussion ("cc:" is fine but it's redundant as long as
> you are sending to the list).


Ok.


> After such a discussion, we may end up finding out that
> everybody have been silently unhappy and wanted to have
> commitdiff there, and agree on doing this unconditionally.
> 
> But we do not know that yet.


>From asking people, most don't use feeds to follow development
checkins, but prefer to keep with mailinglists and looking only
at the repos for their own interesting parts.

Hope more people will speak up here on the git@ list.

Thanks a lot,

Florian La Roche

^ permalink raw reply

* Re: git-write-error with cherry-pick -n usages
From: Junio C Hamano @ 2008-01-11  6:49 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: git
In-Reply-To: <20080111054811.GA7476@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> when you cherry-pick -n a commit that has changes to a file missing in
> the current tree, that file will be added as "Unmerged". A subsequent
> cherry-pick that tries to pick a commit that has changes to that file
> will then fail with:
>
> fatal: git-write-tree: error building trees

Yeah, this was because the original "rewrite in C" did somewhat
a sloppy job while stealing code from git-write-tree.

The caller pretends as if the write_tree() function would return
an error code and being able to issue a sensible error message
itself, but write_tree() function just calls die() and never
returns an error.  Worse yet, the function claims that it was
running git-write-tree (which is no longer true after
cherry-pick stole it).

I think you would need to do something like this patch.  I will
not apply it during the -rc period, but testing and resending
with "Tested-by:" would be helpful after post 1.5.4 cycle opens.

---
 builtin-revert.c     |    5 ++-
 builtin-write-tree.c |   74 +++++++++++---------------------------------------
 builtin.h            |    1 -
 cache-tree.c         |   59 +++++++++++++++++++++++++++++++++++++++
 cache-tree.h         |    5 +++
 5 files changed, 83 insertions(+), 61 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 4bf8eb2..d71f414 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -8,6 +8,7 @@
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "cache-tree.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -270,7 +271,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		 * that represents the "current" state for merge-recursive
 		 * to work on.
 		 */
-		if (write_tree(head, 0, NULL))
+		if (write_cache_as_tree(head, 0, NULL))
 			die ("Your index file is unmerged.");
 	} else {
 		struct wt_status s;
@@ -357,7 +358,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	if (merge_recursive(sha1_to_hex(base->object.sha1),
 				sha1_to_hex(head), "HEAD",
 				sha1_to_hex(next->object.sha1), oneline) ||
-			write_tree(head, 0, NULL)) {
+			write_cache_as_tree(head, 0, NULL)) {
 		add_to_msg("\nConflicts:\n\n");
 		read_cache();
 		for (i = 0; i < active_nr;) {
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index b89d02e..e838d01 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -11,66 +11,12 @@
 static const char write_tree_usage[] =
 "git-write-tree [--missing-ok] [--prefix=<prefix>/]";
 
-int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
-{
-	int entries, was_valid, newfd;
-
-	/* We can't free this memory, it becomes part of a linked list parsed atexit() */
-	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
-
-	newfd = hold_locked_index(lock_file, 1);
-
-	entries = read_cache();
-	if (entries < 0)
-		die("git-write-tree: error reading cache");
-
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
-
-	was_valid = cache_tree_fully_valid(active_cache_tree);
-
-	if (!was_valid) {
-		if (cache_tree_update(active_cache_tree,
-				      active_cache, active_nr,
-				      missing_ok, 0) < 0)
-			die("git-write-tree: error building trees");
-		if (0 <= newfd) {
-			if (!write_cache(newfd, active_cache, active_nr)
-					&& !close(newfd)) {
-				commit_lock_file(lock_file);
-				newfd = -1;
-			}
-		}
-		/* Not being able to write is fine -- we are only interested
-		 * in updating the cache-tree part, and if the next caller
-		 * ends up using the old index with unupdated cache-tree part
-		 * it misses the work we did here, but that is just a
-		 * performance penalty and not a big deal.
-		 */
-	}
-
-	if (prefix) {
-		struct cache_tree *subtree =
-			cache_tree_find(active_cache_tree, prefix);
-		if (!subtree)
-			die("git-write-tree: prefix %s not found", prefix);
-		hashcpy(sha1, subtree->sha1);
-	}
-	else
-		hashcpy(sha1, active_cache_tree->sha1);
-
-	if (0 <= newfd)
-		close(newfd);
-	rollback_lock_file(lock_file);
-
-	return 0;
-}
-
 int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int missing_ok = 0, ret;
 	const char *prefix = NULL;
 	unsigned char sha1[20];
+	const char *me = "git-write-tree";
 
 	git_config(git_default_config);
 	while (1 < argc) {
@@ -87,8 +33,20 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 	if (argc > 2)
 		die("too many options");
 
-	ret = write_tree(sha1, missing_ok, prefix);
-	printf("%s\n", sha1_to_hex(sha1));
-
+	ret = write_cache_as_tree(sha1, missing_ok, prefix);
+	switch (ret) {
+	case 0:
+		printf("%s\n", sha1_to_hex(sha1));
+		break;
+	case WRITE_TREE_UNREADABLE_INDEX:
+		die("%s: error reading the index", me);
+		break;
+	case WRITE_TREE_UNMERGED_INDEX:
+		die("%s: error building trees; the index is unmerged?", me);
+		break;
+	case WRITE_TREE_PREFIX_ERROR:
+		die("%s: prefix %s not found", me, prefix);
+		break;
+	}
 	return ret;
 }
diff --git a/builtin.h b/builtin.h
index cb675c4..3d1628c 100644
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,6 @@ extern const char git_usage_string[];
 
 extern void list_common_cmds_help(void);
 extern void help_unknown_cmd(const char *cmd);
-extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
diff --git a/cache-tree.c b/cache-tree.c
index 50b3526..69d0b46 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -529,3 +529,62 @@ struct cache_tree *cache_tree_find(struct cache_tree *it, const char *path)
 	}
 	return it;
 }
+
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix)
+{
+	int entries, was_valid, newfd;
+
+	/*
+	 * We can't free this memory, it becomes part of a linked list
+	 * parsed atexit()
+	 */
+	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+
+	newfd = hold_locked_index(lock_file, 1);
+
+	entries = read_cache();
+	if (entries < 0)
+		return WRITE_TREE_UNREADABLE_INDEX;
+
+	if (!active_cache_tree)
+		active_cache_tree = cache_tree();
+
+	was_valid = cache_tree_fully_valid(active_cache_tree);
+
+	if (!was_valid) {
+		if (cache_tree_update(active_cache_tree,
+				      active_cache, active_nr,
+				      missing_ok, 0) < 0)
+			return WRITE_TREE_UNMERGED_INDEX;
+		if (0 <= newfd) {
+			if (!write_cache(newfd, active_cache, active_nr)
+					&& !close(newfd)) {
+				commit_lock_file(lock_file);
+				newfd = -1;
+			}
+		}
+		/* Not being able to write is fine -- we are only interested
+		 * in updating the cache-tree part, and if the next caller
+		 * ends up using the old index with unupdated cache-tree part
+		 * it misses the work we did here, but that is just a
+		 * performance penalty and not a big deal.
+		 */
+	}
+
+	if (prefix) {
+		struct cache_tree *subtree =
+			cache_tree_find(active_cache_tree, prefix);
+		if (!subtree)
+			return WRITE_TREE_PREFIX_ERROR;
+		hashcpy(sha1, subtree->sha1);
+	}
+	else
+		hashcpy(sha1, active_cache_tree->sha1);
+
+	if (0 <= newfd)
+		close(newfd);
+	rollback_lock_file(lock_file);
+
+	return 0;
+}
+
diff --git a/cache-tree.h b/cache-tree.h
index 8243228..44aad42 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -30,4 +30,9 @@ int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int)
 
 struct cache_tree *cache_tree_find(struct cache_tree *, const char *);
 
+#define WRITE_TREE_UNREADABLE_INDEX (-1)
+#define WRITE_TREE_UNMERGED_INDEX (-2)
+#define WRITE_TREE_PREFIX_ERROR (-3)
+
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 #endif

^ permalink raw reply related

* Re: Decompression speed: zip vs lzo
From: Sam Vilain @ 2008-01-11  6:29 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Nicolas Pitre, Pierre Habouzit, Git Mailing List,
	Johannes Schindelin, Marco Costalba, Junio C Hamano
In-Reply-To: <alpine.LFD.1.00.0801101805540.3148@woody.linux-foundation.org>

Linus Torvalds wrote:
> 
> On Fri, 11 Jan 2008, Sam Vilain wrote:
>> Without compression of deltas:
>>
>> wilber:~/src/perl-preview$ du -sk .git/objects/pack/
>> 86781 .git/objects/pack/
>>
>> With compression of deltas:
>>
>> wilber:~/src/perl-preview$ du -sk .git/objects/pack/
>> 72907 .git/objects/pack/
> 
> Ok, so non-compressed deltas are 20% bigger.
> 
> That may well be a perfectly acceptable trade-off if the end result is 
> then a lot faster. Has somebody done performance numbers? I may have 
> missed them.. The best test is probably something like "git blame" on a 
> file that takes an appreciable amount of time.

The difference seems only barely measurable;

wilber:~/src/perl-preview$ time git annotate sv.c >/dev/null

real    0m8.130s
user    0m6.712s
sys     0m1.412s

wilber:~/src/perl-preview-loose$ time git annotate sv.c >/dev/null

real    0m7.930s
user    0m6.480s
sys     0m1.408s

(each one is last of three runs - dual-core x86_64 @ 2.1GHz w/512KB cache)

sv.c has about 1500 revisions, though the oldest line is    I also tried
annotate and log on the YACC generated parser which only has about 165
revisions, with similar results - a very minor difference or no difference.

Sam

^ permalink raw reply

* Re: [PATCH 6/6] Use new compress helpers in builtin-pack-objects.c
From: Marco Costalba @ 2008-01-11  6:10 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Nicolas Pitre
In-Reply-To: <e5bfff550801101334p5df5adaerf0eeae02ddf28334@mail.gmail.com>

On Jan 10, 2008 10:34 PM, Marco Costalba <mcostalba@gmail.com> wrote:
>
> -               while (deflate(&stream, Z_FINISH) == Z_OK)
> -                       /* nothing */;
> -               deflateEnd(&stream);
> -               datalen = stream.total_out;
> -               deflateEnd(&stream);


One thing I would like someone to clarify is why deflateEnd(&stream)
is called two times ?

With my patch is called only once as I have seen in all the others places.

Thanks
Marco

^ permalink raw reply

* git-write-error with cherry-pick -n usages
From: Björn Steinbrink @ 2008-01-11  5:48 UTC (permalink / raw)
  To: git

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

Hi,

when you cherry-pick -n a commit that has changes to a file missing in
the current tree, that file will be added as "Unmerged". A subsequent
cherry-pick that tries to pick a commit that has changes to that file
will then fail with:

fatal: git-write-tree: error building trees

I've attached a small shell script to easily reproduce that.

git version 1.5.4.rc2.84.gf85fd

That specific use of cherry-pick might be an user error, but, if
possible, git should give a less cryptic error message.

Björn

[-- Attachment #2: test.sh --]
[-- Type: application/x-sh, Size: 317 bytes --]

^ permalink raw reply

* [PATCH] git-clone - Set remotes.default config variable
From: Mark Levedahl @ 2008-01-11  3:29 UTC (permalink / raw)
  To: gitster; +Cc: git, Mark Levedahl
In-Reply-To: <1200022189-2400-3-git-send-email-mlevedahl@gmail.com>

This records the users choice of default remote name (by default "origin")
as given by the -o option.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/git-clone.txt |    3 ++-
 git-clone.sh                |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index fdccbd4..7fd3ea1 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -95,7 +95,8 @@ OPTIONS
 --origin <name>::
 -o <name>::
 	Instead of using the remote name 'origin' to keep track
-	of the upstream repository, use <name> instead.
+	of the upstream repository, use <name> instead. The name
+        is recorded in the  remotes.default config variable.
 
 --upload-pack <upload-pack>::
 -u <upload-pack>::
diff --git a/git-clone.sh b/git-clone.sh
index b4e858c..efbcee2 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -242,6 +242,7 @@ fi &&
 export GIT_DIR &&
 GIT_CONFIG="$GIT_DIR/config" git-init $quiet ${template+"$template"} || usage
 
+git config remotes.default $origin
 if test -n "$bare"
 then
 	GIT_CONFIG="$GIT_DIR/config" git config core.bare true
-- 
1.5.4.rc2.99.g3ef7-dirty

^ permalink raw reply related

* [PATCH] git-submodule - Possibly inherit parent's default remote on init/clone
From: Mark Levedahl @ 2008-01-11  3:29 UTC (permalink / raw)
  To: gitster; +Cc: git, Mark Levedahl
In-Reply-To: <1200022189-2400-4-git-send-email-mlevedahl@gmail.com>

For submodules defined relative to their parent, it is likely that the
parent's defined default remote is correct for the child as well. This
allows use of remote names other than "origin", important as managed
submodules are typically checked out on a detached head and therefore
submodule-update invokes git-fetch using the default remote. Without this
change, submodules effectively had to have a default remote of "origin."

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/git-submodule.txt |    8 +++++---
 git-submodule.sh                |   15 ++++++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index cffc6d4..440e234 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -36,9 +36,11 @@ status::
 
 init::
 	Initialize the submodules, i.e. register in .git/config each submodule
-	name and url found in .gitmodules. The key used in .git/config is
-	`submodule.$name.url`. This command does not alter existing information
-	in .git/config.
+	name and url found in .gitmodules, along with the default remote origin.
+	For submodules using a relative url, the default remote is inherited
+	from the parent project, for absolute urls the default "origin" is used.
+	The key used in .git/config is submodule.$name.url`. This command does
+	not alter existing information in .git/config.
 
 update::
 	Update the registered submodules, i.e. clone missing submodules and
diff --git a/git-submodule.sh b/git-submodule.sh
index ad9fe62..30e0270 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -7,6 +7,7 @@
 USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
 OPTIONS_SPEC=
 . git-sh-setup
+. git-parse-remote
 require_work_tree
 
 add=
@@ -95,6 +96,7 @@ module_clone()
 {
 	path=$1
 	url=$2
+	origin=${3:-origin}
 
 	# If there already is a directory at the submodule path,
 	# expect it to be empty (since that is the default checkout
@@ -110,7 +112,7 @@ module_clone()
 	test -e "$path" &&
 	die "A file already exist at path '$path'"
 
-	git-clone -n "$url" "$path" ||
+	git-clone -n -o "$origin" "$url" "$path" ||
 	die "Clone of '$url' into submodule path '$path' failed"
 }
 
@@ -130,9 +132,11 @@ module_add()
 		usage
 	fi
 
+	origin=origin
 	case "$repo" in
 	./*|../*)
 		# dereference source url relative to parent's url
+		origin=$(get_default_remote)
 		realrepo="$(resolve_relative_url $repo)" ;;
 	*)
 		# Turn the source into an absolute path if
@@ -157,7 +161,7 @@ module_add()
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"
 
-	module_clone "$path" "$realrepo" || exit
+	module_clone "$path" "$realrepo" "$origin" || exit
 	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
 	die "Unable to checkout submodule '$path'"
 	git add "$path" ||
@@ -189,12 +193,15 @@ modules_init()
 		die "No url found for submodule path '$path' in .gitmodules"
 
 		# Possibly a url relative to parent
+		origin=origin
 		case "$url" in
 		./*|../*)
 			url="$(resolve_relative_url "$url")"
+			origin=$(get_default_remote)
 			;;
 		esac
 
+		git config submodule."$name".origin "$origin" &&
 		git config submodule."$name".url "$url" ||
 		die "Failed to register url for submodule path '$path'"
 
@@ -222,10 +229,12 @@ modules_update()
 			say "Submodule path '$path' not initialized"
 			continue
 		fi
+		origin=$(git config submodule."$name".origin)
+		origin=${origin:-origin}
 
 		if ! test -d "$path"/.git
 		then
-			module_clone "$path" "$url" || exit
+			module_clone "$path" "$url" "$origin" || exit
 			subsha1=
 		else
 			subsha1=$(unset GIT_DIR; cd "$path" &&
-- 
1.5.4.rc2.99.g3ef7-dirty

^ permalink raw reply related

* [PATCH] git-remote - Unset remotes.default when deleting the default remote
From: Mark Levedahl @ 2008-01-11  3:29 UTC (permalink / raw)
  To: gitster; +Cc: git, Mark Levedahl
In-Reply-To: <1200022189-2400-2-git-send-email-mlevedahl@gmail.com>

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-remote.perl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index d13e4c1..2469b59 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -328,6 +328,11 @@ sub rm_remote {
 
 	$git->command('config', '--remove-section', "remote.$name");
 
+	my $defremote = $git->config("remotes.default");
+	if (defined $defremote && $defremote eq $name) {
+	       $git->command("config", "--unset", "remotes.default");
+	}
+
 	eval {
 	    my @trackers = $git->command('config', '--get-regexp',
 			'branch.*.remote', $name);
-- 
1.5.4.rc2.99.g3ef7-dirty

^ permalink raw reply related


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