Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] On compresing large index
From: Nguyen Thai Ngoc Duy @ 2012-02-06  1:35 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Joshua Redstone
In-Reply-To: <87ehu9ug9i.fsf@thomas.inf.ethz.ch>

2012/2/6 Thomas Rast <trast@inf.ethz.ch>:
>> We need to figure out what git uses 4s user time for.
>
> When I worked on the cache-tree stuff, my observation (based on
> profiling, so I had actual data :-) was that computing SHA1s absolutely
> dominates everything in such operations.  It does that when writing the
> index to write the trailing checksum, and also when loading it to verify
> that the index is valid.

You're right. This is on another machine but with same index (2M
files), without SHA1 checksum:

$ time ~/w/git/git ls-files --stage|head > /dev/null
real    0m1.533s
user    0m1.228s
sys     0m0.306s

and with SHA-1 checksum:

$ time git ls-files --stage|head > /dev/null
real     0m7.525s
user    0m7.257s
sys     0m0.268s

I guess we could fall back to cheaper digests for such a large index.
Still more than one second for doing nothing but reading index is too
slow to me.

> ls-files shouldn't be so slow though.  A quick run with callgrind in a
> linux-2.6.git tells me it spends about 45% of its time on SHA1s and a
> whopping 25% in quote_c_style().  I wonder what's so hard about
> quoting...

That's why I put "| head" there, to cut output processing overhead (hopefully).
-- 
Duy

^ permalink raw reply

* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Junio C Hamano @ 2012-02-06  1:48 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Thomas Rast, git, Brandon Casey, Uwe Kleine-König,
	Brian Gernhardt, Robin H. Johnson, Ævar Arnfjörð
In-Reply-To: <CAMP44s30VmJasMLJxs-JFwksvPEPpG1LB3Gr_pA2+hpE1AnwXg@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Anyway, it seems people don't care if 'git send-email' attempts to
> send random garbage regardless, so I'm not going to pursue this patch.

I actually think people _do_ care, and that is the _only_ reason you are
getting review comments.

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06  2:06 UTC (permalink / raw)
  To: git; +Cc: Michael Haggerty, Jeff King
In-Reply-To: <20120130215043.GB16149@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Is it really worth warning? After all, by definition you are not leaving
> any commits or useful work behind.

I actually do not know if this change itself is worth doing, but if we
were to do this, then I think the user benefits from the warning.

The patch is made on maint-1.7.6 track for no good reason, so it may have
some merge conflicts around "resolve_ref()" vs "resolve_refdup()" if we
were to apply it on a more modern codebase, but the resolution should be
trivial.

---
Subject: [PATCH] git checkout -b: allow switching out of an unborn branch

Running "git checkout -b another" immediately after "git init" when you do
not even have a commit on 'master' is forbidden, with a readable message:

    $ git checkout -b another
    fatal: You are on a branch yet to be born

It is readable but not easily understandable unless the user knows what
"yet to be born" really means.

So let's try allowing it and see what happens. I strongly suspect that
this may just shift the confusion one step further without adding much
value to the resulting system, because the next question that would come
to somebody who does not understand what "yet to be born" is is "why don't
I see 'master' in the output from 'git branch' command?", and the new
warning may not be descriptive enough to explain what the user is doing.

The early part of switch_branches() that computes old is probably be
better moved to the caller cmd_checkout() and used in the new code that
detects the "unborn" case, and passed as to switch_branches() as the third
parameter.  Such improvements and tests are left as an exercise for the
interested and motivated, as usual ;-)

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/checkout.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c20dae..5894f40 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -916,6 +916,19 @@ static int parse_branchname_arg(int argc, const char **argv,
 	return argcount;
 }
 
+static int switch_unborn_to_new_branch(struct checkout_opts *opts, const char *old_ref)
+{
+	int status;
+	struct strbuf branch_ref = STRBUF_INIT;
+
+	strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
+	warning(_("Leaving the unborn branch '%s' behind..."),
+		skip_prefix(old_ref, "refs/heads/"));
+	status = create_symref("HEAD", branch_ref.buf, "checkout -b");
+	strbuf_release(&branch_ref);
+	return status;
+}
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;
@@ -1089,5 +1102,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.writeout_stage)
 		die(_("--ours/--theirs is incompatible with switching branches."));
 
+	if (!new.commit) {
+		unsigned char rev[20];
+		int flag, status;
+		const char *old_ref = resolve_ref("HEAD", rev, 0, &flag);
+
+		if ((flag & REF_ISSYMREF) && is_null_sha1(rev)) {
+			status = switch_unborn_to_new_branch(&opts, old_ref);
+			free((char *)old_ref);
+			return status;
+		}
+	}
 	return switch_branches(&opts, &new);
 }
-- 
1.7.9.172.ge26ae

^ permalink raw reply related

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06  2:08 UTC (permalink / raw)
  To: git; +Cc: Michael Haggerty, Jeff King
In-Reply-To: <7vobtcbtqa.fsf@alter.siamese.dyndns.org>

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

> The patch is made on maint-1.7.6 track for no good reason, so it may have
> some merge conflicts around "resolve_ref()" vs "resolve_refdup()" if we
> were to apply it on a more modern codebase, but the resolution should be
> trivial.

The resolution should look like this, just in case.

diff --cc builtin/checkout.c
index 5bf96ba,5894f40..41b9b34
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@@ -1079,5 -1102,16 +1092,16 @@@ int cmd_checkout(int argc, const char *
  	if (opts.writeout_stage)
  		die(_("--ours/--theirs is incompatible with switching branches."));
  
+ 	if (!new.commit) {
+ 		unsigned char rev[20];
+ 		int flag, status;
 -		const char *old_ref = resolve_ref("HEAD", rev, 0, &flag);
++		char *old_ref = resolve_refdup("HEAD", rev, 0, &flag);
+ 
+ 		if ((flag & REF_ISSYMREF) && is_null_sha1(rev)) {
+ 			status = switch_unborn_to_new_branch(&opts, old_ref);
 -			free((char *)old_ref);
++			free(old_ref);
+ 			return status;
+ 		}
+ 	}
  	return switch_branches(&opts, &new);
  }

^ permalink raw reply

* What's cooking in git.git (Feb 2012, #02; Sun, 5)
From: Junio C Hamano @ 2012-02-06  2:44 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in
'next'.

Compared to the recent activity level on discussions of new features on
the list, some people may be wondering if the rate of advancement of the
'master' and 'next' branches is getting throttled.

That is because it is.

Now the obviously good bits that have been cooking during the feature
freeze are pushed out to 'master', I'd want to make sure we can have a
timely release of v1.7.9.1 so that people can start benefiting from the
features and fixes introduced in v1.7.9 more smoothly and sooner, and that
is where my focus lies at this moment. I've been picking up new topics and
adding them to 'pu' only "as time and attention permit" basis, and this
mode of operation probably will continue throughout the second week of the
post v1.7.9 cycle (cf. http://tinyurl.com/gitcal).

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo:

        git://git.kernel.org/pub/scm/git/git.git
        git://repo.or.cz/alt-git.git
        https://code.google.com/p/git-core/
        https://github.com/git/git

With only maint and master:

        git://git.sourceforge.jp/gitroot/git-core/git.git
        git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches:

        https://github.com/gitster/git

The preformatted documentation in HTML and man format are found in:

        git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
        git://repo.or.cz/git-{htmldocs,manpages}.git/
        https://code.google.com/p/git-{htmldocs,manpages}.git/
        https://github.com/gitster/git-{htmldocs,manpages}.git/

--------------------------------------------------
[New Topics]

* bw/inet-pton-ntop-compat (2012-02-05) 1 commit
 - Drop system includes from inet_pton/inet_ntop compatibility wrappers

The inclusion order of header files bites Solaris again and this fixes it.

Will merge to 'next'.

* jc/branch-desc-typoavoidance (2012-02-05) 2 commits
 - branch --edit-description: protect against mistyped branch name
 + tests: add write_script helper function
 (this branch is tangled with jk/tests-write-script.)

Typo in "git branch --edit-description my-tpoic" was not diagnosed.

Will merge to 'next'.

* jc/checkout-out-of-unborn (2012-02-05) 1 commit
 - git checkout -b: allow switching out of an unborn branch

I am fairly negative on this one, as I think it is just shifting the
problem around.

* jc/maint-mailmap-output (2012-02-05) 1 commit
 - mailmap: do not leave '>' in the output when answering "we did something"

map_user() was not rewriting its output correctly, which resulted in the
user visible symptom that "git blame -e" sometimes showed excess '>' at
the end of email addresses.

* jc/merge-ff-only-stronger-than-signed-merge (2012-02-05) 1 commit
 - merge: do not create a signed tag merge under --ff-only option

"git merge --ff-only $tag" failed because it cannot record the required
mergetag without creating a merge, but this is so common operation for
branch that is used _only_ to follow the upstream, so it is allowed to
fast-forward without recording the mergetag.

* tt/profile-build-fix (2012-02-05) 1 commit
 - Fix build problems related to profile-directed optimization

--------------------------------------------------
[Stalled]

* jc/advise-push-default (2011-12-18) 1 commit
 - push: hint to use push.default=upstream when appropriate

Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
 - perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.

* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
 - commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
 - cache-tree: update API to take abitrary flags

May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.

* jc/split-blob (2012-01-24) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - varint-in-pack: refactor varint encoding/decoding

Not ready.

I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.

--------------------------------------------------
[Cooking]

* nd/diffstat-gramnum (2012-02-03) 1 commit
  (merged to 'next' on 2012-02-05 at 7335ecc)
 + Use correct grammar in diffstat summary line

The commands in the "git diff" family and "git apply --stat" that count
the number of files changed and the number of lines inserted/deleted have
been updated to match the output from "diffstat".  This also opens the
door to i18n this line.

* jx/i18n-more-marking (2012-02-01) 2 commits
  (merged to 'next' on 2012-02-05 at 44e8cf6)
 + i18n: format_tracking_info "Your branch is behind" message
 + i18n: git-commit whence_s "merge/cherry-pick" message

Marks a few more messages we forgot to mark for i18n.

* jk/grep-binary-attribute (2012-02-02) 9 commits
  (merged to 'next' on 2012-02-05 at 9dffa7e)
 + grep: pre-load userdiff drivers when threaded
 + grep: load file data after checking binary-ness
 + grep: respect diff attributes for binary-ness
 + grep: cache userdiff_driver in grep_source
 + grep: drop grep_buffer's "name" parameter
 + convert git-grep to use grep_source interface
 + grep: refactor the concept of "grep source" into an object
 + grep: move sha1-reading mutex into low-level code
 + grep: make locking flag global

Fixes a longstanding bug that there was no way to tell "git grep" that a
path may look like text but it is not, which "git diff" can do using the
attributes system. Now "git grep" honors the same "binary" (or "-diff")
attribute.

* jc/parse-date-raw (2012-02-03) 2 commits
 - parse_date(): '@' prefix forces git-timestamp
 - parse_date(): allow ancient git-timestamp

"rebase" and "commit --amend" failed to work on commits with ancient
timestamps near year 1970.

Waiting for comments.

* jk/git-dir-lookup (2012-02-02) 1 commit
  (merged to 'next' on 2012-02-05 at 1856d74)
 + standardize and improve lookup rules for external local repos

When you have both .../foo and .../foo.git, "git clone .../foo" did not
favor the former but the latter.

* jk/prompt-fallback-to-tty (2012-02-03) 2 commits
 - prompt: fall back to terminal if askpass fails
 - prompt: clean up strbuf usage

The code to ask for password did not fall back to the terminal input when
GIT_ASKPASS is set but does not work (e.g. lack of X with GUI askpass
helper).

Will merge to 'next'.

* jk/tests-write-script (2012-02-03) 2 commits
  (merged to 'next' on 2012-02-05 at 4264ffa)
 + t0300: use write_script helper
 + tests: add write_script helper function
 (this branch is tangled with jc/branch-desc-typoavoidance.)

* jn/gitweb-search-utf-8 (2012-02-03) 1 commit
  (merged to 'next' on 2012-02-05 at 055e446)
 + gitweb: Allow UTF-8 encoded CGI query parameters and path_info

Search box in "gitweb" did not accept non-ASCII characters correctly.

* jn/rpm-spec (2012-02-03) 1 commit
  (merged to 'next' on 2012-02-05 at dba940b)
 + git.spec: Workaround localized messages not put in any RPM

Fix breakage in v1.7.9 Makefile; rpmbuild notices an unpackaged but
installed *.mo file and fails.

* fc/zsh-completion (2012-02-03) 3 commits
 - completion: simplify __gitcomp and __gitcomp_nl implementations
 - completion: use ls -1 instead of rolling a loop to do that ourselves
 - completion: work around zsh option propagation bug

Fix git subcommand completion for zsh (in contrib/completion).

Will merge to 'next'.

* jc/maint-request-pull-for-tag (2012-01-31) 1 commit
  (merged to 'next' on 2012-02-01 at 7649f18)
 + request-pull: explicitly ask tags/$name to be pulled

When asking for a tag to be pulled, "request-pull" shows the name of the
tag prefixed with "tags/"

* nd/find-pack-entry-recent-cache-invalidation (2012-02-01) 2 commits
  (merged to 'next' on 2012-02-01 at e26aed0)
 + find_pack_entry(): do not keep packed_git pointer locally
 + sha1_file.c: move the core logic of find_pack_entry() into fill_pack_entry()

* nd/pack-objects-parseopt (2012-02-01) 3 commits
  (merged to 'next' on 2012-02-05 at d0dc25d)
 + pack-objects: convert to use parse_options()
 + pack-objects: remove bogus comment
 + pack-objects: do not accept "--index-version=version,"

"pack-objects" learned use parse-options, losing custom command line
parsing code.

* bl/gitweb-project-filter (2012-02-01) 8 commits
  (merged to 'next' on 2012-02-01 at 2c96ce7)
 + gitweb: Make project search respect project_filter
 + gitweb: improve usability of projects search form
 + gitweb: place links to parent directories in page header
 + gitweb: show active project_filter in project_list page header
 + gitweb: limit links to alternate forms of project_list to active project_filter
 + gitweb: add project_filter to limit project list to a subdirectory
 + gitweb: prepare git_get_projects_list for use outside 'forks'.
 + gitweb: move hard coded .git suffix out of git_get_projects_list

"gitweb" allows intermediate entries in the directory hierarchy that leads
to a projects to be clicked, which in turn shows the list of projects
inside that directory.

* rt/completion-branch-edit-desc (2012-01-29) 1 commit
  (merged to 'next' on 2012-02-01 at 0627ebf)
 + completion: --edit-description option for git-branch

Originally merged to 'next' on 2012-01-31.
Will merge to 'master'.

* jn/svn-fe (2012-02-02) 47 commits
  (merged to 'next' on 2012-02-05 at e9d3917)
 + vcs-svn: suppress a -Wtype-limits warning
 + vcs-svn: allow import of > 4GiB files
 + vcs-svn: rename check_overflow arguments for clarity
  (merged to 'next' on 2012-02-01 at 9288c95)
 + vcs-svn/svndiff.c: squelch false "unused" warning from gcc
 + Merge branch 'svn-fe' of git://repo.or.cz/git/jrn into jn/svn-fe
 + vcs-svn: reset first_commit_done in fast_export_init
 + Merge branch 'db/text-delta' into svn-fe
 + vcs-svn: do not initialize report_buffer twice
 + Merge branch 'db/text-delta' into svn-fe
 + vcs-svn: avoid hangs from corrupt deltas
 + vcs-svn: guard against overflow when computing preimage length
 + Merge branch 'db/delta-applier' into db/text-delta
 + vcs-svn: implement text-delta handling
 + Merge branch 'db/delta-applier' into db/text-delta
 + Merge branch 'db/delta-applier' into svn-fe
 + vcs-svn: cap number of bytes read from sliding view
 + test-svn-fe: split off "test-svn-fe -d" into a separate function
 + vcs-svn: let deltas use data from preimage
 + vcs-svn: let deltas use data from postimage
 + vcs-svn: verify that deltas consume all inline data
 + vcs-svn: implement copyfrom_data delta instruction
 + vcs-svn: read instructions from deltas
 + vcs-svn: read inline data from deltas
 + vcs-svn: read the preimage when applying deltas
 + vcs-svn: parse svndiff0 window header
 + vcs-svn: skeleton of an svn delta parser
 + vcs-svn: make buffer_read_binary API more convenient
 + vcs-svn: learn to maintain a sliding view of a file
 + Makefile: list one vcs-svn/xdiff object or header per line
 + Merge branch 'db/svn-fe-code-purge' into svn-fe
 + vcs-svn: drop obj_pool
 + vcs-svn: drop treap
 + vcs-svn: drop string_pool
 + vcs-svn: pass paths through to fast-import
 + Merge branch 'db/strbufs-for-metadata' into db/svn-fe-code-purge
 + Merge branch 'db/length-as-hash' (early part) into db/svn-fe-code-purge
 + Merge branch 'db/vcs-svn-incremental' into svn-fe
 + vcs-svn: avoid using ls command twice
 + vcs-svn: use mark from previous import for parent commit
 + vcs-svn: handle filenames with dq correctly
 + vcs-svn: quote paths correctly for ls command
 + vcs-svn: eliminate repo_tree structure
 + vcs-svn: add a comment before each commit
 + vcs-svn: save marks for imported commits
 + vcs-svn: use higher mark numbers for blobs
 + vcs-svn: set up channel to read fast-import cat-blob response
 + Merge commit 'v1.7.5' into svn-fe

Originally merged to 'next' on 2012-01-29.

"vcs-svn"/"svn-fe" learned to read dumps with svn-deltas and support
incremental imports.

Will merge to 'master'.

^ permalink raw reply

* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-06  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vehu8dcc8.fsf@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 04:39:35PM -0800, Junio C Hamano wrote:

> > We could also go as far as saying that map_user would _always_ terminate
> > in this way (i.e., the caller gets a munged result, whether we found
> > anything or not). Then internally, map_user could be simplified to stop
> > worrying about making a temporary copy in mailbuf. And callers could
> > simply call map_user without worrying about branching on whether it
> > found anything or not.
> 
> I thought about it, but such a change needs to audit all the call sites
> that assumes the promise original map_user() used to make before it was
> broken. If we return 0 to the caller, the caller does not have to worry
> about map_user() munging the buffer it lent to it.
> 
> It might be a worthwhile thing to do. I dunno; I didn't look into it.

Ugh, yeah. I was thinking about how it would improve this call site, but
I don't want to get into auditing the others. Let's drop it and go with
your patch.

-Peff

^ permalink raw reply

* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06  3:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120206030339.GA29123@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> It might be a worthwhile thing to do. I dunno; I didn't look into it.
>
> Ugh, yeah. I was thinking about how it would improve this call site, but
> I don't want to get into auditing the others.

There aren't that many, though.  shortlog has one, pretty has another and
that is about it.

But both seems to care that map_user() is not a function that returns void,
so...

^ permalink raw reply

* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06  3:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120206030339.GA29123@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Ugh, yeah. I was thinking about how it would improve this call site, but
> I don't want to get into auditing the others. Let's drop it and go with
> your patch.

In any case, here is what I queued for tonight.

-- >8 --
Subject: [PATCH] mailmap: do not leave '>' in the output when answering "we did something"

The callers of map_user() give email and name to it, and expect to get an
up-to-date versions of email and/or name to be used in their output. The
function rewrites the given buffers in place. To optimize the majority of
cases, the function returns 0 when it did not do anything, and it returns
1 when the caller should use the updated contents.

The 'email' input to the function is terminated by '>' or a NUL (whichever
comes first) for historical reasons, but when a rewrite happens, the value
is replaced with the mailbox inside the <> pair.  However, it failed to
meet this expectation when it only rewrote the name part without rewriting
the email part, and the email in the input was terminated by '>'.

This causes an extra '>' to appear in the output of "blame -e", because the
caller does send in '>'-terminated email, and when the function returned 1
to tell it that rewriting happened, it appends '>' that is necessary when
the email part was rewritten.

The patch looks bigger than it actually is, because this change makes a
variable that points at the end of the email part in the input 'p' live
much longer than it used to, deserving a more descriptive name.

Noticed and diagnosed by Felipe Contreras and Jeff King.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 mailmap.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index 8c3196c..47aa419 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -190,27 +190,27 @@ void clear_mailmap(struct string_list *map)
 int map_user(struct string_list *map,
 	     char *email, int maxlen_email, char *name, int maxlen_name)
 {
-	char *p;
+	char *end_of_email;
 	struct string_list_item *item;
 	struct mailmap_entry *me;
 	char buf[1024], *mailbuf;
 	int i;
 
 	/* figure out space requirement for email */
-	p = strchr(email, '>');
-	if (!p) {
+	end_of_email = strchr(email, '>');
+	if (!end_of_email) {
 		/* email passed in might not be wrapped in <>, but end with a \0 */
-		p = memchr(email, '\0', maxlen_email);
-		if (!p)
+		end_of_email = memchr(email, '\0', maxlen_email);
+		if (!end_of_email)
 			return 0;
 	}
-	if (p - email + 1 < sizeof(buf))
+	if (end_of_email - email + 1 < sizeof(buf))
 		mailbuf = buf;
 	else
-		mailbuf = xmalloc(p - email + 1);
+		mailbuf = xmalloc(end_of_email - email + 1);
 
 	/* downcase the email address */
-	for (i = 0; i < p - email; i++)
+	for (i = 0; i < end_of_email - email; i++)
 		mailbuf[i] = tolower(email[i]);
 	mailbuf[i] = 0;
 
@@ -236,6 +236,8 @@ int map_user(struct string_list *map,
 		}
 		if (maxlen_email && mi->email)
 			strlcpy(email, mi->email, maxlen_email);
+		else
+			*end_of_email = '\0';
 		if (maxlen_name && mi->name)
 			strlcpy(name, mi->name, maxlen_name);
 		debug_mm("map_user:  to '%s' <%s>\n", name, mi->email ? mi->email : "");
-- 
1.7.9.204.gdf845

^ permalink raw reply related

* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-06  4:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vy5sgaby1.fsf@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 07:16:22PM -0800, Junio C Hamano wrote:

> In any case, here is what I queued for tonight.
> 
> -- >8 --
> Subject: [PATCH] mailmap: do not leave '>' in the output when answering "we did something"

Looks good to me.

> The callers of map_user() give email and name to it, and expect to get an
> up-to-date versions of email and/or name to be used in their output. The

Minor grammar error.

-Peff

^ permalink raw reply

* Re: [PATCH 1/3] blame: fix email output with mailmap
From: Jeff King @ 2012-02-06  4:09 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Felipe Contreras, git
In-Reply-To: <87liohvysi.fsf@thomas.inf.ethz.ch>

On Sun, Feb 05, 2012 at 08:57:17PM +0100, Thomas Rast wrote:

> (Granted, omitting *Peff* doesn't make that much of a difference, since
> for all I know he reads every email that crosses this list.  But my
> point still stands.)

It's not true at all. I didn't read this message, for instance.

-Peff

^ permalink raw reply

* Re: [PATCH] Fix build problems related to profile-directed optimization
From: Jeff King @ 2012-02-06  4:18 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: git, Andi Kleen
In-Reply-To: <1328489090-14178-1-git-send-email-tytso@mit.edu>

On Sun, Feb 05, 2012 at 07:44:50PM -0500, Theodore Ts'o wrote:

> diff --git a/INSTALL b/INSTALL
> index 6fa83fe..5b7eec1 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -28,16 +28,25 @@ set up install paths (via config.mak.autogen), so you can write instead
>  If you're willing to trade off (much) longer build time for a later
>  faster git you can also do a profile feedback build with
>  
> -	$ make profile-all
> -	# make prefix=... install
> +	$ make --prefix=/usr PROFILE=BUILD all
> +	# make --prefix=/usr PROFILE=BUILD install

Eh? --prefix?

> +As a caveat: a profile-optimized build takes a *lot* longer since it
> +is the sources have to be built twice, and in order for the profiling

s/it is//

> diff --git a/Makefile b/Makefile
> index c457c34..8cea247 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7
> [...]
> +ifeq "$(PROFILE)" "GEN"
> +	CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
> +	EXTLIBS += -lgcov
> +	export CCACHE_DISABLE=t
> +	V=1
> +else ifneq "$PROFILE" ""
> +	CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
> +	export CCACHE_DISABLE=t
> +	V=1
> +endif

Did you mean "$(PROFILE)" in the second conditional?

-Peff

^ permalink raw reply

* Re: [PATCH] branch --edit-description: protect against mistyped branch name
From: Jeff King @ 2012-02-06  4:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <7vaa4wda60.fsf_-_@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 05:26:31PM -0800, Junio C Hamano wrote:

> This incidentally also errors out --edit-description when the HEAD points
> at an unborn branch (immediately after "init", or "checkout --orphan"),
> because at that point, you do not even have any commit that is part of
> your history and there is no point in describing how this particular
> branch is different from the branch it forked off of, which is the useful
> bit of information the branch description is designed to capture.
> 
> We may want to special case the unborn case later, but that is outside the
> scope of this patch to prevent more common mistakes before 1.7.9 series
> gains too much widespread use.

That sounds OK to me. I'm not even sure people will want to use
"--edit-description" on an unborn pointed-to branch or not (I mentioned
it only as "this is a plausible use case to me that we might be
breaking"). I think people will still be figuring out workflows around
it. So it's not a big deal to wait and see.

-Peff

^ permalink raw reply

* Re: Specifying revisions in the future
From: Miles Bader @ 2012-02-06  4:28 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Philip Oakley, Jakub Narebski, Matthieu Moy, jpaugh, git
In-Reply-To: <m2obtcx4i2.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:
> The rule should be to follow the leftmost parent as far as possible.
> That means that X+2->D is B.

It might also be reasonable (and safer -- the user may not actually
realize when there's an ambiguating branch-point) to simply have it
abort with an error ("ambiguous future-ref specification") when
there's any doubt...  I suspect most uses would be very simple "+1"
etc., and not crossing branch points.

-miles

-- 
`There are more things in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.'

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jeff King @ 2012-02-06  4:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <7vobtcbtqa.fsf@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 06:06:53PM -0800, Junio C Hamano wrote:

> Subject: [PATCH] git checkout -b: allow switching out of an unborn branch
> 
> Running "git checkout -b another" immediately after "git init" when you do
> not even have a commit on 'master' is forbidden, with a readable message:
> 
>     $ git checkout -b another
>     fatal: You are on a branch yet to be born
> 
> It is readable but not easily understandable unless the user knows what
> "yet to be born" really means.
> 
> So let's try allowing it and see what happens. I strongly suspect that
> this may just shift the confusion one step further without adding much
> value to the resulting system, because the next question that would come
> to somebody who does not understand what "yet to be born" is is "why don't
> I see 'master' in the output from 'git branch' command?", and the new
> warning may not be descriptive enough to explain what the user is doing.

I thought the concern wasn't confusion at the error message, but rather
"how do I start a new repository with a branch named something besides
'master'?"

You would expect:

  git init
  git checkout -b foo

to work, but it doesn't. And there's no easy way to do what you want
(you have to resort to plumbing to put the value in HEAD). So the issue
is not a bad error message or a confusing situation, but that the user
wants to accomplish X, and we don't provide a reasonable way to do it.

I suspect it hasn't come up so far because the "X" in this case is not
something people generally want to do. I.e., they are almost always
cloning and making a new branch from old history. If they have a
brand-new repo, they almost certainly don't actually care what the
branch is called.

And perhaps in that case we should be discouraging them from calling it
something besides master (because while master is mostly convention,
there are a few magic spots in the code where it is treated specially,
and departing from the convention for no good reason should be
discouraged).

So I don't see "this is just shifting confusion" as a real argument. But
you could argue that it is enabling the user to do something stupid and
pointless, and for that reason it should be disallowed (and in that
case, it might be better for the error to say "what you are doing is
stupid and pointless").

-Peff

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Andrew Ardill @ 2012-02-06  4:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Michael Haggerty
In-Reply-To: <20120206043012.GD29365@sigill.intra.peff.net>

On 6 February 2012 15:30, Jeff King <peff@peff.net> wrote:

> And perhaps in that case we should be discouraging them from calling it
> something besides master (because while master is mostly convention,
> there are a few magic spots in the code where it is treated specially,
> and departing from the convention for no good reason should be
> discouraged).

What exactly are the areas where 'master' is treated specially? I
agree that people should be discouraged from needlessly abandoning
convention, however I think users should have the ability to name
their branches as they see fit.

If I am forced to abandon code targeted at the 'master' naming
convention in order to use my desired naming convention, we should fix
that. Additionally, if I have to either manually set a branch name
with plumbing commands, or delete existing branches that are generated
automatically with no option not to generate them, we should improve
the porcelain to cover these cracks.

In general, I think it plausible that in some use cases the term
'master' might be misleading or inappropriate and users should not be
punished for that.

Regards,

Andrew Ardill

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jeff King @ 2012-02-06  5:06 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Junio C Hamano, git, Michael Haggerty
In-Reply-To: <CAH5451ndjozo8-Cx3+Vc84TCjKJvCnSuOUsQS5cnqXsdc=8fMQ@mail.gmail.com>

On Mon, Feb 06, 2012 at 03:42:24PM +1100, Andrew Ardill wrote:

> On 6 February 2012 15:30, Jeff King <peff@peff.net> wrote:
> 
> > And perhaps in that case we should be discouraging them from calling it
> > something besides master (because while master is mostly convention,
> > there are a few magic spots in the code where it is treated specially,
> > and departing from the convention for no good reason should be
> > discouraged).
> 
> What exactly are the areas where 'master' is treated specially? I
> agree that people should be discouraged from needlessly abandoning
> convention, however I think users should have the ability to name
> their branches as they see fit.

Fairly minor stuff. Between the top of my head and a quick grep:

  1. Some transports (like git://) are incapable of communicating the
     destination of a remote's symbolic ref (they see only that HEAD
     points to some specific sha1). But things like "clone" want to know
     where the HEAD is pointing to set up the remotes/$origin/HEAD
     link. We can guess that if HEAD and some branch have the same sha1,
     that HEAD is pointing to that branch. But you might have two or
     more such branches pointing to the same spot. In this case, we
     prefer "master" over other branches.

     This code is in guess_remote_head.

  2. When merging, if the current branch is named "master", the default
     merge message says "Merge branch foo". Otherwise, it says "Merge
     branch foo into bar".

  3. It looks like the antique "branches" file format defaults to
     fetching "master" when no branch specifier is given. I doubt anyone
     is still using this file format these days.

I was actually surprised how infrequently the term "master" comes up in
a grep of the code. So while I wouldn't call my search exhaustive, I did
inspect every match in the C code.

> If I am forced to abandon code targeted at the 'master' naming
> convention in order to use my desired naming convention, we should fix
> that. Additionally, if I have to either manually set a branch name
> with plumbing commands, or delete existing branches that are generated
> automatically with no option not to generate them, we should improve
> the porcelain to cover these cracks.
> 
> In general, I think it plausible that in some use cases the term
> 'master' might be misleading or inappropriate and users should not be
> punished for that.

I kind of agree that we shouldn't be unnecessarily restrictive. On the
other hand, I am stretching to find the plausible reason that one would
want to throw away the normal convention. Code aside, it simply
introduces a slight communication barrier when talking with other git
users, and for that reason should be something you don't do lightly. I
don't recall seeing anybody complain seriously about it in the past six
years of git's existence.

-Peff

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06  5:15 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Michael Haggerty
In-Reply-To: <20120206043012.GD29365@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I thought the concern wasn't confusion at the error message, but rather
> "how do I start a new repository with a branch named something besides
> 'master'?"
>
> You would expect:
>
>   git init
>   git checkout -b foo
>
> to work, but it doesn't. And there's no easy way to do what you want
> (you have to resort to plumbing to put the value in HEAD). So the issue
> is not a bad error message or a confusing situation, but that the user
> wants to accomplish X, and we don't provide a reasonable way to do it.

I think the right interface for "I want to use 'foo' instead of 'master'
like everybody else" would be:

	$ git init --some-option foo

I wouldn't have any issue with that.

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Jeff King @ 2012-02-06  5:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <7vty34a6fd.fsf@alter.siamese.dyndns.org>

On Sun, Feb 05, 2012 at 09:15:34PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I thought the concern wasn't confusion at the error message, but rather
> > "how do I start a new repository with a branch named something besides
> > 'master'?"
> >
> > You would expect:
> >
> >   git init
> >   git checkout -b foo
> >
> > to work, but it doesn't. And there's no easy way to do what you want
> > (you have to resort to plumbing to put the value in HEAD). So the issue
> > is not a bad error message or a confusing situation, but that the user
> > wants to accomplish X, and we don't provide a reasonable way to do it.
> 
> I think the right interface for "I want to use 'foo' instead of 'master'
> like everybody else" would be:
> 
> 	$ git init --some-option foo
> 
> I wouldn't have any issue with that.

Sure, that's one way to do it. But I don't see any point in not allowing
"git checkout -b" to be another way of doing it. Is there some other use
case for "git checkout -b" from an unborn branch? Or is there some
harmful outcome that can come from doing so that we need to be
protecting against? Am I missing something?

-Peff

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06  5:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Michael Haggerty
In-Reply-To: <20120206051834.GA5062@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Sure, that's one way to do it. But I don't see any point in not allowing
> "git checkout -b" to be another way of doing it. Is there some other use
> case for "git checkout -b" from an unborn branch? Or is there some
> harmful outcome that can come from doing so that we need to be
> protecting against? Am I missing something?

Mostly because it is wrong at the conceptual level to do so.

	git checkout -b foo

is a short-hand for

	git checkout -b foo HEAD

which is a short-hand for

	git branch foo HEAD &&
        git checkout foo

But the last one has no chance of working if you think about it, because
"git branch foo $start" is a way to start a branch at $start and you need
to have something to point at with refs/heads/foo.

So we are breaking the equivalence between these three only when HEAD
points at an unborn branch.

^ permalink raw reply

* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06  5:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Michael Haggerty
In-Reply-To: <7vk440a5qw.fsf@alter.siamese.dyndns.org>

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

> But the last one has no chance of working if you think about it, because
> "git branch foo $start" is a way to start a branch at $start and you need
> to have something to point at with refs/heads/foo.

... which brings us back to your earlier point ...

>> I like your patch better than trying to pass around "0{40}", but:

which is why my conclusion was that "checkout -b" is shifting the
confusion around to different parts.

> So we are breaking the equivalence between these three only when HEAD
> points at an unborn branch.

^ permalink raw reply

* [PATCH 1/6] read-cache: use sha1file for sha1 calculation
From: Nguyễn Thái Ngọc Duy @ 2012-02-06  5:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Joshua Redstone,
	Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 read-cache.c |   90 +++++++++++++++-------------------------------------------
 1 files changed, 23 insertions(+), 67 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index a51bba1..e9a20b6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -12,6 +12,7 @@
 #include "commit.h"
 #include "blob.h"
 #include "resolve-undo.h"
+#include "csum-file.h"
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 
@@ -1395,73 +1396,28 @@ int unmerged_index(const struct index_state *istate)
 	return 0;
 }
 
-#define WRITE_BUFFER_SIZE 8192
-static unsigned char write_buffer[WRITE_BUFFER_SIZE];
-static unsigned long write_buffer_len;
-
-static int ce_write_flush(git_SHA_CTX *context, int fd)
-{
-	unsigned int buffered = write_buffer_len;
-	if (buffered) {
-		git_SHA1_Update(context, write_buffer, buffered);
-		if (write_in_full(fd, write_buffer, buffered) != buffered)
-			return -1;
-		write_buffer_len = 0;
-	}
-	return 0;
-}
-
-static int ce_write(git_SHA_CTX *context, int fd, void *data, unsigned int len)
+static int ce_write(struct sha1file *f, void *data, unsigned int len)
 {
-	while (len) {
-		unsigned int buffered = write_buffer_len;
-		unsigned int partial = WRITE_BUFFER_SIZE - buffered;
-		if (partial > len)
-			partial = len;
-		memcpy(write_buffer + buffered, data, partial);
-		buffered += partial;
-		if (buffered == WRITE_BUFFER_SIZE) {
-			write_buffer_len = buffered;
-			if (ce_write_flush(context, fd))
-				return -1;
-			buffered = 0;
-		}
-		write_buffer_len = buffered;
-		len -= partial;
-		data = (char *) data + partial;
-	}
-	return 0;
+	return sha1write(f, data, len);
 }
 
-static int write_index_ext_header(git_SHA_CTX *context, int fd,
+static int write_index_ext_header(struct sha1file *f,
 				  unsigned int ext, unsigned int sz)
 {
 	ext = htonl(ext);
 	sz = htonl(sz);
-	return ((ce_write(context, fd, &ext, 4) < 0) ||
-		(ce_write(context, fd, &sz, 4) < 0)) ? -1 : 0;
+	return ((ce_write(f, &ext, 4) < 0) ||
+		(ce_write(f, &sz, 4) < 0)) ? -1 : 0;
 }
 
-static int ce_flush(git_SHA_CTX *context, int fd)
+static int ce_flush(struct sha1file *f)
 {
-	unsigned int left = write_buffer_len;
-
-	if (left) {
-		write_buffer_len = 0;
-		git_SHA1_Update(context, write_buffer, left);
-	}
-
-	/* Flush first if not enough space for SHA1 signature */
-	if (left + 20 > WRITE_BUFFER_SIZE) {
-		if (write_in_full(fd, write_buffer, left) != left)
-			return -1;
-		left = 0;
-	}
+	unsigned char sha1[20];
+	int fd = sha1close(f, sha1, 0);
 
-	/* Append the SHA1 signature at the end */
-	git_SHA1_Final(write_buffer + left, context);
-	left += 20;
-	return (write_in_full(fd, write_buffer, left) != left) ? -1 : 0;
+	if (fd < 0)
+		return -1;
+	return (write_in_full(fd, sha1, 20) != 20) ? -1 : 0;
 }
 
 static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
@@ -1513,7 +1469,7 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
 	}
 }
 
-static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)
+static int ce_write_entry(struct sha1file *f, struct cache_entry *ce)
 {
 	int size = ondisk_ce_size(ce);
 	struct ondisk_cache_entry *ondisk = xcalloc(1, size);
@@ -1542,7 +1498,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)
 		name = ondisk->name;
 	memcpy(name, ce->name, ce_namelen(ce));
 
-	result = ce_write(c, fd, ondisk, size);
+	result = ce_write(f, ondisk, size);
 	free(ondisk);
 	return result;
 }
@@ -1574,7 +1530,7 @@ void update_index_if_able(struct index_state *istate, struct lock_file *lockfile
 
 int write_index(struct index_state *istate, int newfd)
 {
-	git_SHA_CTX c;
+	struct sha1file *f;
 	struct cache_header hdr;
 	int i, err, removed, extended;
 	struct cache_entry **cache = istate->cache;
@@ -1598,8 +1554,8 @@ int write_index(struct index_state *istate, int newfd)
 	hdr.hdr_version = htonl(extended ? 3 : 2);
 	hdr.hdr_entries = htonl(entries - removed);
 
-	git_SHA1_Init(&c);
-	if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
+	f = sha1fd(newfd, NULL);
+	if (ce_write(f, &hdr, sizeof(hdr)) < 0)
 		return -1;
 
 	for (i = 0; i < entries; i++) {
@@ -1608,7 +1564,7 @@ int write_index(struct index_state *istate, int newfd)
 			continue;
 		if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
 			ce_smudge_racily_clean_entry(ce);
-		if (ce_write_entry(&c, newfd, ce) < 0)
+		if (ce_write_entry(f, ce) < 0)
 			return -1;
 	}
 
@@ -1617,8 +1573,8 @@ int write_index(struct index_state *istate, int newfd)
 		struct strbuf sb = STRBUF_INIT;
 
 		cache_tree_write(&sb, istate->cache_tree);
-		err = write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sb.len) < 0
-			|| ce_write(&c, newfd, sb.buf, sb.len) < 0;
+		err = write_index_ext_header(f, CACHE_EXT_TREE, sb.len) < 0
+			|| ce_write(f, sb.buf, sb.len) < 0;
 		strbuf_release(&sb);
 		if (err)
 			return -1;
@@ -1627,15 +1583,15 @@ int write_index(struct index_state *istate, int newfd)
 		struct strbuf sb = STRBUF_INIT;
 
 		resolve_undo_write(&sb, istate->resolve_undo);
-		err = write_index_ext_header(&c, newfd, CACHE_EXT_RESOLVE_UNDO,
+		err = write_index_ext_header(f, CACHE_EXT_RESOLVE_UNDO,
 					     sb.len) < 0
-			|| ce_write(&c, newfd, sb.buf, sb.len) < 0;
+			|| ce_write(f, sb.buf, sb.len) < 0;
 		strbuf_release(&sb);
 		if (err)
 			return -1;
 	}
 
-	if (ce_flush(&c, newfd) || fstat(newfd, &st))
+	if (ce_flush(f) || fstat(newfd, &st))
 		return -1;
 	istate->timestamp.sec = (unsigned int)st.st_mtime;
 	istate->timestamp.nsec = ST_MTIME_NSEC(st);
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH 2/6] csum-file: make sha1 calculation optional
From: Nguyễn Thái Ngọc Duy @ 2012-02-06  5:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Joshua Redstone,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1328507319-24687-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 csum-file.c |   16 +++++++++++++---
 csum-file.h |    2 +-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/csum-file.c b/csum-file.c
index 53f5375..4c517d1 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -11,6 +11,12 @@
 #include "progress.h"
 #include "csum-file.h"
 
+static void sha1update(struct sha1file *f, const void *data, unsigned offset)
+{
+	if (f->do_sha1)
+		git_SHA1_Update(&f->ctx, data, offset);
+}
+
 static void flush(struct sha1file *f, void *buf, unsigned int count)
 {
 	if (0 <= f->check_fd && count)  {
@@ -47,7 +53,7 @@ void sha1flush(struct sha1file *f)
 	unsigned offset = f->offset;
 
 	if (offset) {
-		git_SHA1_Update(&f->ctx, f->buffer, offset);
+		sha1update(f, f->buffer, offset);
 		flush(f, f->buffer, offset);
 		f->offset = 0;
 	}
@@ -58,7 +64,10 @@ int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
 	int fd;
 
 	sha1flush(f);
-	git_SHA1_Final(f->buffer, &f->ctx);
+	if (f->do_sha1)
+		git_SHA1_Final(f->buffer, &f->ctx);
+	else
+		hashclr(f->buffer);
 	if (result)
 		hashcpy(result, f->buffer);
 	if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
@@ -110,7 +119,7 @@ int sha1write(struct sha1file *f, void *buf, unsigned int count)
 		buf = (char *) buf + nr;
 		left -= nr;
 		if (!left) {
-			git_SHA1_Update(&f->ctx, data, offset);
+			sha1update(f, data, offset);
 			flush(f, data, offset);
 			offset = 0;
 		}
@@ -154,6 +163,7 @@ struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp
 	f->tp = tp;
 	f->name = name;
 	f->do_crc = 0;
+	f->do_sha1 = 1;
 	git_SHA1_Init(&f->ctx);
 	return f;
 }
diff --git a/csum-file.h b/csum-file.h
index 3b540bd..c23ea62 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -12,7 +12,7 @@ struct sha1file {
 	off_t total;
 	struct progress *tp;
 	const char *name;
-	int do_crc;
+	int do_crc, do_sha1;
 	uint32_t crc32;
 	unsigned char buffer[8192];
 };
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH 3/6] Stop producing index version 2
From: Nguyễn Thái Ngọc Duy @ 2012-02-06  5:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Joshua Redstone,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1328507319-24687-1-git-send-email-pclouds@gmail.com>

read-cache.c learned to produce version 2 or 3 depending on whether
extended cache entries exist in 06aaaa0 (Extend index to save more flags
- 2008-10-01), first released in 1.6.1. The purpose is to keep
compatibility with older git. It's been more than three years since
then and git has reached 1.7.9. Drop support for older git.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 read-cache.c                          |    8 +++-----
 t/t2104-update-index-skip-worktree.sh |   12 ------------
 2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index e9a20b6..fe6b0e0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1532,26 +1532,24 @@ int write_index(struct index_state *istate, int newfd)
 {
 	struct sha1file *f;
 	struct cache_header hdr;
-	int i, err, removed, extended;
+	int i, err, removed;
 	struct cache_entry **cache = istate->cache;
 	int entries = istate->cache_nr;
 	struct stat st;
 
-	for (i = removed = extended = 0; i < entries; i++) {
+	for (i = removed = 0; i < entries; i++) {
 		if (cache[i]->ce_flags & CE_REMOVE)
 			removed++;
 
 		/* reduce extended entries if possible */
 		cache[i]->ce_flags &= ~CE_EXTENDED;
 		if (cache[i]->ce_flags & CE_EXTENDED_FLAGS) {
-			extended++;
 			cache[i]->ce_flags |= CE_EXTENDED;
 		}
 	}
 
 	hdr.hdr_signature = htonl(CACHE_SIGNATURE);
-	/* for extended format, increase version so older git won't try to read it */
-	hdr.hdr_version = htonl(extended ? 3 : 2);
+	hdr.hdr_version = htonl(3);
 	hdr.hdr_entries = htonl(entries - removed);
 
 	f = sha1fd(newfd, NULL);
diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index 1d0879b..8221ffa 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -28,19 +28,11 @@ test_expect_success 'setup' '
 	git ls-files -t | test_cmp expect.full -
 '
 
-test_expect_success 'index is at version 2' '
-	test "$(test-index-version < .git/index)" = 2
-'
-
 test_expect_success 'update-index --skip-worktree' '
 	git update-index --skip-worktree 1 sub/1 &&
 	git ls-files -t | test_cmp expect.skip -
 '
 
-test_expect_success 'index is at version 3 after having some skip-worktree entries' '
-	test "$(test-index-version < .git/index)" = 3
-'
-
 test_expect_success 'ls-files -t' '
 	git ls-files -t | test_cmp expect.skip -
 '
@@ -50,8 +42,4 @@ test_expect_success 'update-index --no-skip-worktree' '
 	git ls-files -t | test_cmp expect.full -
 '
 
-test_expect_success 'index version is back to 2 when there is no skip-worktree entry' '
-	test "$(test-index-version < .git/index)" = 2
-'
-
 test_done
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH 4/6] Introduce index version 4 with global flags
From: Nguyễn Thái Ngọc Duy @ 2012-02-06  5:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Joshua Redstone,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1328507319-24687-1-git-send-email-pclouds@gmail.com>

v4 adds 32-bit field to cache header after 32-bit number of entries.
If this field is zero, fall back to v3.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/technical/index-format.txt |    4 ++-
 cache.h                                  |    6 +++++
 read-cache.c                             |   31 ++++++++++++++++++++++-------
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt
index 8930b3f..2b6a38e 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -12,10 +12,12 @@ GIT index format
        The signature is { 'D', 'I', 'R', 'C' } (stands for "dircache")
 
      4-byte version number:
-       The current supported versions are 2 and 3.
+       The current supported versions are 2, 3 and 4.
 
      32-bit number of index entries.
 
+     32-bit flags (version 4 only).
+
    - A number of sorted index entries (see below).
 
    - Extensions
diff --git a/cache.h b/cache.h
index 9bd8c2d..c2e884a 100644
--- a/cache.h
+++ b/cache.h
@@ -105,6 +105,11 @@ struct cache_header {
 	unsigned int hdr_entries;
 };
 
+struct ext_cache_header {
+	struct cache_header h;
+	unsigned int hdr_flags;
+};
+
 /*
  * The "cache_time" is just the low 32 bits of the
  * time. It doesn't matter if it overflows - we only
@@ -314,6 +319,7 @@ static inline unsigned int canon_mode(unsigned int mode)
 struct index_state {
 	struct cache_entry **cache;
 	unsigned int cache_nr, cache_alloc, cache_changed;
+	unsigned int hdr_flags;
 	struct string_list *resolve_undo;
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
diff --git a/read-cache.c b/read-cache.c
index fe6b0e0..fd21af6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1190,7 +1190,9 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)
 
 	if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
 		return error("bad signature");
-	if (hdr->hdr_version != htonl(2) && hdr->hdr_version != htonl(3))
+	if (hdr->hdr_version != htonl(2) &&
+	    hdr->hdr_version != htonl(3) &&
+	    hdr->hdr_version != htonl(4))
 		return error("bad index version");
 	git_SHA1_Init(&c);
 	git_SHA1_Update(&c, hdr, size - 20);
@@ -1320,7 +1322,12 @@ int read_index_from(struct index_state *istate, const char *path)
 	istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));
 	istate->initialized = 1;
 
-	src_offset = sizeof(*hdr);
+	if (ntohl(hdr->hdr_version) >= 4) {
+		struct ext_cache_header *ehdr = mmap;
+		istate->hdr_flags = ntohl(ehdr->hdr_flags);
+		src_offset = sizeof(*ehdr);
+	} else
+		src_offset = sizeof(*hdr);
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct ondisk_cache_entry *disk_ce;
 		struct cache_entry *ce;
@@ -1375,6 +1382,7 @@ int discard_index(struct index_state *istate)
 	resolve_undo_clear_index(istate);
 	istate->cache_nr = 0;
 	istate->cache_changed = 0;
+	istate->hdr_flags = 0;
 	istate->timestamp.sec = 0;
 	istate->timestamp.nsec = 0;
 	istate->name_hash_initialized = 0;
@@ -1531,8 +1539,8 @@ void update_index_if_able(struct index_state *istate, struct lock_file *lockfile
 int write_index(struct index_state *istate, int newfd)
 {
 	struct sha1file *f;
-	struct cache_header hdr;
-	int i, err, removed;
+	struct ext_cache_header hdr;
+	int i, err, removed, hdr_size;
 	struct cache_entry **cache = istate->cache;
 	int entries = istate->cache_nr;
 	struct stat st;
@@ -1548,12 +1556,19 @@ int write_index(struct index_state *istate, int newfd)
 		}
 	}
 
-	hdr.hdr_signature = htonl(CACHE_SIGNATURE);
-	hdr.hdr_version = htonl(3);
-	hdr.hdr_entries = htonl(entries - removed);
+	hdr.h.hdr_signature = htonl(CACHE_SIGNATURE);
+	if (istate->hdr_flags) {
+		hdr.h.hdr_version = htonl(4);
+		hdr.hdr_flags = htonl(istate->hdr_flags);
+		hdr_size = sizeof(hdr);
+	} else {
+		hdr.h.hdr_version = htonl(3);
+		hdr_size = sizeof(hdr.h);
+	}
+	hdr.h.hdr_entries = htonl(entries - removed);
 
 	f = sha1fd(newfd, NULL);
-	if (ce_write(f, &hdr, sizeof(hdr)) < 0)
+	if (ce_write(f, &hdr, hdr_size) < 0)
 		return -1;
 
 	for (i = 0; i < entries; i++) {
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH 5/6] Allow to use crc32 as a lighter checksum on index
From: Nguyễn Thái Ngọc Duy @ 2012-02-06  5:48 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Joshua Redstone,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1328507319-24687-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-update-index.txt |   12 +++++++-
 builtin/update-index.c             |   11 +++++++
 cache.h                            |    2 +
 read-cache.c                       |   54 ++++++++++++++++++++++++++++--------
 4 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index a3081f4..2574a4e 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	     [--add] [--remove | --force-remove] [--replace]
 	     [--refresh] [-q] [--unmerged] [--ignore-missing]
 	     [(--cacheinfo <mode> <object> <file>)...]
-	     [--chmod=(+|-)x]
+	     [--chmod=(+|-)x] [--[no-]crc32]
 	     [--assume-unchanged | --no-assume-unchanged]
 	     [--skip-worktree | --no-skip-worktree]
 	     [--ignore-submodules]
@@ -109,6 +109,16 @@ you will need to handle the situation manually.
 	set and unset the "skip-worktree" bit for the paths. See
 	section "Skip-worktree bit" below for more information.
 
+--crc32::
+--no-crc32::
+	Normally SHA-1 is used to check for index integrity. When the
+	index is large, SHA-1 computation cost can be significant.
+	--crc32 will convert current index to use (cheaper) crc32
+	instead. Note that later writes to index by other commands can
+	convert the index back to SHA-1. Older git versions may not
+	understand crc32 index, --no-crc32 can be used to convert it
+	back to SHA-1.
+
 -g::
 --again::
 	Runs 'git update-index' itself on the paths whose index
diff --git a/builtin/update-index.c b/builtin/update-index.c
index a6a23fa..6913226 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -707,6 +707,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
 	int newfd, entries, has_errors = 0, line_termination = '\n';
 	int read_from_stdin = 0;
+	int do_crc = -1;
 	int prefix_length = prefix ? strlen(prefix) : 0;
 	char set_executable_bit = 0;
 	struct refresh_params refresh_args = {0, &has_errors};
@@ -791,6 +792,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			"(for porcelains) forget saved unresolved conflicts",
 			PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			resolve_undo_clear_callback},
+		OPT_BOOL(0, "crc32", &do_crc,
+			 "use crc32 as checksum instead of sha1"),
 		OPT_END()
 	};
 
@@ -852,6 +855,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	}
 	argc = parse_options_end(&ctx);
 
+	if (do_crc != -1) {
+		if (do_crc)
+			the_index.hdr_flags |= CACHE_F_CRC;
+		else
+			the_index.hdr_flags &= ~CACHE_F_CRC;
+		active_cache_changed = 1;
+	}
+
 	if (read_from_stdin) {
 		struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
 
diff --git a/cache.h b/cache.h
index c2e884a..7352402 100644
--- a/cache.h
+++ b/cache.h
@@ -105,6 +105,8 @@ struct cache_header {
 	unsigned int hdr_entries;
 };
 
+#define CACHE_F_CRC	1	/* use crc32 instead of sha1 for index checksum */
+
 struct ext_cache_header {
 	struct cache_header h;
 	unsigned int hdr_flags;
diff --git a/read-cache.c b/read-cache.c
index fd21af6..a34878e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1185,20 +1185,33 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
 
 static int verify_hdr(struct cache_header *hdr, unsigned long size)
 {
+	int do_crc;
 	git_SHA_CTX c;
 	unsigned char sha1[20];
 
 	if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
 		return error("bad signature");
-	if (hdr->hdr_version != htonl(2) &&
-	    hdr->hdr_version != htonl(3) &&
-	    hdr->hdr_version != htonl(4))
+	if (hdr->hdr_version == htonl(2) ||
+	    hdr->hdr_version == htonl(3))
+		do_crc = 0;
+	else if (hdr->hdr_version == htonl(4)) {
+		struct ext_cache_header *ehdr = (struct ext_cache_header *)hdr;
+		do_crc = ntohl(ehdr->hdr_flags) & CACHE_F_CRC;
+	}
+	else
 		return error("bad index version");
-	git_SHA1_Init(&c);
-	git_SHA1_Update(&c, hdr, size - 20);
-	git_SHA1_Final(sha1, &c);
-	if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
-		return error("bad index file sha1 signature");
+	if (do_crc) {
+		uint32_t crc = crc32(0, NULL, 0);
+		crc = crc32(crc,(void *) hdr, size - sizeof(uint32_t));
+		if (crc != *(uint32_t*)((unsigned char *)hdr + size - sizeof(uint32_t)))
+			return error("bad index file crc32 signature");
+	} else {
+		git_SHA1_Init(&c);
+		git_SHA1_Update(&c, hdr, size - 20);
+		git_SHA1_Final(sha1, &c);
+		if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
+			return error("bad index file sha1 signature");
+	}
 	return 0;
 }
 
@@ -1421,11 +1434,24 @@ static int write_index_ext_header(struct sha1file *f,
 static int ce_flush(struct sha1file *f)
 {
 	unsigned char sha1[20];
-	int fd = sha1close(f, sha1, 0);
+	int fd;
 
-	if (fd < 0)
-		return -1;
-	return (write_in_full(fd, sha1, 20) != 20) ? -1 : 0;
+	if (f->do_crc) {
+		uint32_t crc;
+
+		assert(f->do_sha1 == 0);
+		sha1flush(f);
+		crc = crc32_end(f);
+		fd = sha1close(f, sha1, 0);
+		if (fd < 0)
+			return -1;
+		return (write_in_full(fd, &crc, sizeof(crc)) != sizeof(crc)) ? -1 : 0;
+	} else {
+		fd = sha1close(f, sha1, 0);
+		if (fd < 0)
+			return -1;
+		return (write_in_full(fd, sha1, 20) != 20) ? -1 : 0;
+	}
 }
 
 static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
@@ -1568,6 +1594,10 @@ int write_index(struct index_state *istate, int newfd)
 	hdr.h.hdr_entries = htonl(entries - removed);
 
 	f = sha1fd(newfd, NULL);
+	if (istate->hdr_flags & CACHE_F_CRC) {
+		crc32_begin(f);
+		f->do_sha1 = 0;
+	}
 	if (ce_write(f, &hdr, hdr_size) < 0)
 		return -1;
 
-- 
1.7.8.36.g69ee2

^ 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