Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation
From: Antoine Pelisse @ 2013-01-09 17:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1357603821-8647-4-git-send-email-gitster@pobox.com>

> +static struct string_list_item *lookup_prefix(struct string_list *map,
> +                                             const char *string, size_t len)
> +{
> +       int i = string_list_find_insert_index(map, string, 1);
> +       if (i < 0) {
> +               /* exact match */
> +               i = -1 - i;
> +               /* does it match exactly? */
> +               if (!map->items[i].string[len])
> +                       return &map->items[i];

I'm not sure the condition above is necessary, as I don't see why an
exact match would not be an exact match.
We have to trust the cmp function (that mailmap sets itself) to not
return 0 when the lengths are different.

> +       }
> +
> +       /*
> +        * i is at the exact match to an overlong key, or
> +        * location the possibly overlong key would be inserted,
> +        * which must be after the real location of the key.
> +        */
> +       while (0 <= --i && i < map->nr) {
> +               int cmp = strncasecmp(map->items[i].string, string, len);
> +               if (cmp < 0)
> +                       /*
> +                        * "i" points at a key definitely below the prefix;
> +                        * the map does not have string[0:len] in it.
> +                        */
> +                       break;
> +               else if (!cmp && !map->items[i].string[len])
> +                       /* found it */
> +                       return &map->items[i];
> +               /*
> +                * otherwise, the string at "i" may be string[0:len]
> +                * followed by a string that sorts later than string[len:];
> +                * keep trying.
> +                */
> +       }
> +       return NULL;
> +}
> +

I've tried to think about nasty use cases but everything seems fine.

^ permalink raw reply

* Re: Enabling scissors by default?
From: Junio C Hamano @ 2013-01-09 17:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Phillip Susi, git
In-Reply-To: <20130109171011.GB5332@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:
>
>> You could introduce a new configuration variable "am.scissors" and
>> personally turn it on, though.  Setting that variable *does* count
>> as the user explicitly asking for it.
>
> I think we have mailinfo.scissors already.

Oh, spoiler.  I was hoping that I could entice new people into doing
a little digging on their own X-<.


>> > I often see patches being tweaked in response to feedback and
>> > resubmitted, usually with a description of what has changed since the
>> > previous version.  Such descriptions don't need to be in the change
>> > log when it is finally applied and seem a perfect use of scissors.
>> 
>> Putting such small logs under "---" line is the accepted practice.
>
> Maybe it is just me, but I find the scissors form more readable, because
> the "cover letter" material often serves to introduce and give context
> to the patch (e.g., "Thanks for your feedback. I've tried to do X, and
> it came out well. Here's the patch." serves as an introduction, and
> logically comes before the commit message itself).
>
> That does not say anything one way or another about how dangerous or not
> it might be to enable scissors by default. Just my two cents that I like
> the scissors style for patches that come as part of a discussion (and I
> prefer the "---" style when making comments on the contents of a patch;
> i.e., when the comments make more sense to be read after reading the
> commit message to understand what the patch does).

Yes, scissors have their uses, namely when presenting a patch in a
discussion context.  Otherwise we wouldn't have introduced it in the
first place.

But the "desription of what has changed since the previous version"
use case I was responding to is where the space below "---" is meant
to be used from very early days of Git (the convention established
on the kernel list).

^ permalink raw reply

* Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation
From: Junio C Hamano @ 2013-01-09 17:46 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git
In-Reply-To: <CALWbr2wp7rV7Vh0=gwmWaZE5hLHQNL+UciDsL+z-1GyhS9pTkQ@mail.gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

>> +static struct string_list_item *lookup_prefix(struct string_list *map,
>> +                                             const char *string, size_t len)
>> +{
>> +       int i = string_list_find_insert_index(map, string, 1);
>> +       if (i < 0) {
>> +               /* exact match */
>> +               i = -1 - i;
>> +               /* does it match exactly? */
>> +               if (!map->items[i].string[len])
>> +                       return &map->items[i];
>
> I'm not sure the condition above is necessary, as I don't see why an
> exact match would not be an exact match.

You have a overlong string "ABCDEFG", but you only want to look for
"ABCDEF", i.e. len=6.  The string_list happens to have an existing
string "ABCDEFG".  The insert-index function will report an exact
match, but that does not mean you found what you are looking for. 

For the particular case of "looking up e-mail from a string-list
used for the mailmap, using a string that potentially has an extra
'>' at the end", it may not be an issue (i.e. your overlong string
would be "ABCDEF>", and the string-list used for the mailmap will
not have an entry that ends with '>'), but it is likely that people
will try to mimic this function or try to generalize and move it to
strbuf.c and at that point, such a special case condition will no
longer hold and the bug will manifest itself.  Being defensive like
the above is a way to avoid that.

^ permalink raw reply

* Re: git branch case insensitivity (Possible bug)
From: Joshua Jensen @ 2013-01-09 17:47 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Alexander Gallego, git
In-Reply-To: <50ED925B.2060402@op5.se>

----- Original Message -----
From: Andreas Ericsson
Date: 1/9/2013 8:52 AM
>
> Are you using Mac OSX?
> Are you using the HFS+ filesystem shipped with it?
> Did you use the filesystem's default settings rather than reinstall your
> system with sensible settings?
>
> If you said "yes" to all of the above, this is a filesystem "feature",
> courtesy of (cr)Apple, and you're screwed.
>
> You can work around it by running "git pack-refs" every time you create
> a branch or a tag though.
There are two popular default file systems that are case preserving, 
case insensitive.  One is on Mac.  One is on Windows.

Since Git relies on file system behavior to store the equivalent of 
database entries like these refs, it cannot give a consistent user 
experience across platforms or even file systems within platforms.

That sounds like a bug in Git to me.

Perhaps pack-refs should be run automatically by any internal command 
that updates a ref to ensure a non-confusing, consistent user experience.

Further, if refs are no longer entries on the disk, then this nasty 
namespacing issue goes away.

User A:
$ git branch render
$ git push

User B:
$ git pull
$ git branch render/myfeature

render/myfeature can't be created, because Git assumes a filesystem 
structure.  The render namespace is locked out forever.

-Josh

^ permalink raw reply

* Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation
From: Junio C Hamano @ 2013-01-09 17:56 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git
In-Reply-To: <7vfw2a6yh5.fsf@alter.siamese.dyndns.org>

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

> Antoine Pelisse <apelisse@gmail.com> writes:
>
>>> +static struct string_list_item *lookup_prefix(struct string_list *map,
>>> +                                             const char *string, size_t len)
>>> +{
>>> +       int i = string_list_find_insert_index(map, string, 1);
>>> +       if (i < 0) {
>>> +               /* exact match */
>>> +               i = -1 - i;
>>> +               /* does it match exactly? */
>>> +               if (!map->items[i].string[len])
>>> +                       return &map->items[i];
>>
>> I'm not sure the condition above is necessary, as I don't see why an
>> exact match would not be an exact match.
>
> You have a overlong string "ABCDEFG", but you only want to look for
> "ABCDEF", i.e. len=6.  The string_list happens to have an existing
> string "ABCDEFG".  The insert-index function will report an exact
> match, but that does not mean you found what you are looking for. 

To put it another way, we can further clarify the situation by
rewording the comment "Does it match exactly?", perhaps like this

	if (i < 0) {
                /* exact match */
                i = -1 - i;
                if (!string[len])
                        return &map->items[i];
                /*
                 * It matched exactly even to the cruft at the end
                 * of the string, so it is not really a match.
                 */
	}

^ permalink raw reply

* Re: git branch case insensitivity (Possible bug)
From: Johannes Sixt @ 2013-01-09 18:34 UTC (permalink / raw)
  To: Alexander Gallego; +Cc: Andreas Ericsson, git
In-Reply-To: <CAL+iW2-eTtMnn65HN9zeFBFXubgkCA7RwVRZjkq7+tRpPk1TGg@mail.gmail.com>

Am 09.01.2013 18:03, schrieb Alexander Gallego:
> On Wed, Jan 9, 2013 at 10:52 AM, Andreas Ericsson <ae@op5.se> wrote:
>> [about case-insensitivity of HFS+ and branch names]
>> If you said "yes" to all of the above, this is a filesystem "feature",
>> courtesy of (cr)Apple, and you're screwed.
>>
>> You can work around it by running "git pack-refs" every time you create
>> a branch or a tag though.
> 
> Thanks for the tips. I'll be sure to use this.

Naah... that's unworkable. I'm sure the Andreas meant the suggestion
tongue-in-cheek. The important part of his reply is "you're screwed".

-- Hannes

^ permalink raw reply

* t7400 broken on pu (Mac OS X)
From: Torsten Bögershausen @ 2013-01-09 18:43 UTC (permalink / raw)
  To: git; +Cc: Torsten Bögershausen

The current pu fails on Mac OS, case insensitive FS.


Bisecting points out
commit 3f28e4fafc046284657945798d71c57608bee479
[snip]
Date:   Sun Jan 6 13:21:07 2013 +0700

    Convert add_files_to_cache to take struct pathspec

And I veryfied that the preceeding commit 05647d2d8a5dc456d1f4ef73
is OK.

It fails here:
not ok 38 - gracefully add submodule with a trailing slash

A run of a modified t7400 looks like this:
Is there anything more, that I can do to debug this?


[snip]
ok 37 - do not add files from a submodule

expecting success: 

	git reset --hard &&
	echo 1 >&2 &&
	git commit -m "commit subproject" init &&
	echo 2 >&2 &&
	(cd init &&
	echo 3 >&2 &&
	 echo b > a) &&
	echo 4 >&2 &&
	git add init/ &&
	echo 5 >&2 &&
	git diff --exit-code --cached init &&
	echo 6 >&2 &&
	commit=$(cd init &&
	echo 7 >&2 &&
	 git commit -m update a >/dev/null &&
	echo 8 >&2 &&
	 git rev-parse HEAD) &&
	echo 9 >&2 &&
	git add init/ &&
	echo 10 >&2 &&
	test_must_fail git diff --exit-code --cached init &&
	echo 11 >&2 &&
	test $commit = $(git ls-files --stage |
		sed -n "s/^160000 \([^ ]*\).*/\1/p")


HEAD is now at 57f2622 super commit 1
1
[second 1b8c63f] commit subproject
 Author: A U Thor <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
2
3
4
5
6
7
8
9
10
test_must_fail: command succeeded: git diff --exit-code --cached init
not ok 38 - gracefully add submodule with a trailing slash

^ permalink raw reply

* Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index
From: Martin von Zweigbergk @ 2013-01-09 18:43 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20130109170119.GA5332@sigill.intra.peff.net>

On Wed, Jan 9, 2013 at 9:01 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 09, 2013 at 12:16:13AM -0800, Martin von Zweigbergk wrote:
>
>> "git reset [--mixed]" without --quiet refreshes the index in order to
>> display the "Unstaged changes after reset". When --quiet is given,
>> that output is suppressed, removing the need to refresh the index.
>> Other porcelain commands that care about a refreshed index should
>> already be refreshing it, so running e.g. "git reset -q && git diff"
>> is still safe.
>
> Hmm. But "git reset -q && git diff-files" would not be?

Right. Actually, "git reset -q && git diff" was perhaps not a good
example, because its analogous plumbing command would be "git reset -q
&& git diff-files -p", which is also safe. But, as you say, "git reset
-q && git diff-files" (without -p) might list files for which only the
stat information has changed.

> We have never been very clear about which commands refresh the index.

Yes, git-reset's documentation doesn't mention it.

> Since "reset" is about manipulating the index, I'd expect it to be
> refreshed afterwards. On the other hand, since we have never guaranteed
> anything, perhaps a careful script should always use "git update-index
> --refresh".

Since "git diff-files" is a plumbing command, users of it to a
hopefully a bit more careful than regular users, but you never know.

> I would not be too surprised if some of our own scripts are
> not that careful, though.

I didn't find any, but I might have missed something.

Regardless, this patch was tangential. The goal of this series can be
achieved independently of this patch, so if it's too risky, we can
drop easily drop it.

Also, even though it does make "git reset -q" faster, I'm not sure how
important that is in practice. Most use cases would probably refresh
the index afterwards anyway. In such cases, the improvement on warm
cache would still be there, but the relative improvement in the cold
cache case would be pretty much gone (since the entire tree would be
stat'ed by the following refresh anyway).


Martin

^ permalink raw reply

* Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index
From: Junio C Hamano @ 2013-01-09 19:12 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Jeff King, git
In-Reply-To: <CANiSa6joBuAJVHkMfNbVMHFJ6BFOh7sGRw_txRO81CKudRwsfA@mail.gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

>> We have never been very clear about which commands refresh the index.
>
> Yes, git-reset's documentation doesn't mention it.
>
>> Since "reset" is about manipulating the index, I'd expect it to be
>> refreshed afterwards. On the other hand, since we have never guaranteed
>> anything, perhaps a careful script should always use "git update-index
>> --refresh".
>
> Since "git diff-files" is a plumbing command, users of it to a
> hopefully a bit more careful than regular users, but you never know.
>
>> I would not be too surprised if some of our own scripts are
>> not that careful, though.
>
> I didn't find any, but I might have missed something.

contrib/examples/ have some, but looking at it makes me realize that
we have been fairly careful to avoid using "git reset" which is a
Porcelain.

And as a Porcelain, I would rather expect it to leave the resulting
index refreshed.

^ permalink raw reply

* Re: t7400 broken on pu (Mac OS X)
From: Junio C Hamano @ 2013-01-09 19:16 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <50EDBA37.30205@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

> The current pu fails on Mac OS, case insensitive FS.
>
>
> Bisecting points out
> commit 3f28e4fafc046284657945798d71c57608bee479
> [snip]
> Date:   Sun Jan 6 13:21:07 2013 +0700

Next time do not [snip] but please find the author address there,
and Cc such a report.

I think this topic is planned to be rerolled anyway, and your report
would be a valuable input while doing so.

Thanks.

^ permalink raw reply

* git-completion.tcsh and git-completion.zsh are broken?
From: Manlio Perillo @ 2013-01-09 19:17 UTC (permalink / raw)
  To: git@vger.kernel.org

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi.

I have finally resolved all the problems with my path completion in
git-completion.bash and, in order to avoid regressions, I'm checking the
git-completion.zsh and git-completion.tcsh scripts, since they use the
bash completion support.

I have installed (Debian 6.0.6):
* zsh 4.3.10 (i686-pc-linux-gnu)
* tcsh 6.17.02 (Astron) 2010-05-12 (i586-intel-linux)
  options wide,nls,dl,al,kan,rh,nd,color,filec

Note that I'm using my modified git-completion.bash script.


zsh compatibility support in git-completion.bash seems to "work" (I just
get a segmentation fault ...), however I have problems with the .zsh and
.tcsh scripts.


$zsh
synapsis% source contrib/completion/git-completion.zsh
(anon):6: command not found: ___main
_git:11: command not found: _default

I have disabled compinit autoload (since, I don't know how, it is able
to find the git completion script)


$tcsh
synapsis:~/projects/git/contrib/git> source ~/.git-completion.tcsh
synapsis:~/projects/git/contrib/git> git show HEAD:<TAB>

does not show the file list for the tree object in the HEAD

another problem is that a space is added after a directory name.


Another problem with zsh:

$zsh
synapsis% git show HEAD:<TAB>569GPXZims

I don't know where that 569GPXZims came from.


Can someone else confirm these problems?


Thanks  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDtwjcACgkQscQJ24LbaURpuACfVQnoBC3tzvxB0JYxQ5aL3rmN
8GEAnA7OjVtPqz+aq/PGtNtTHWgFqhKK
=3UdZ
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
From: Junio C Hamano @ 2013-01-09 19:26 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357719376-16406-4-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> We use the path arguments in two places in reset.c: in
> interactive_reset() and read_from_tree(). Both of these call
> get_pathspec(), so we pass the (prefix, arv) pair to both
> functions. Move the call to get_pathspec() out of these methods, for
> two reasons: 1) One argument is simpler than two. 2) It lets us use
> the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
> ---
> If I understand correctly, this should be rebased on top of
> nd/parse-pathspec. Please let me know.

Yeah, this will conflict with the get_pathspec-to-parse_pathspec
conversion Duy has been working on.

Without the interactions with that topic, the conversion seems
straightforward to me, though.

>
>  builtin/reset.c | 27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 65413d0..045c960 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,
>  	}
>  }
>  
> -static int interactive_reset(const char *revision, const char **argv,
> -			     const char *prefix)
> -{
> -	const char **pathspec = NULL;
> -
> -	if (*argv)
> -		pathspec = get_pathspec(prefix, argv);
> -
> -	return run_add_interactive(revision, "--patch=reset", pathspec);
> -}
> -
> -static int read_from_tree(const char *prefix, const char **argv,
> -		unsigned char *tree_sha1, int refresh_flags)
> +static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
> +			  int refresh_flags)
>  {
>  	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
>  	int index_fd;
>  	struct diff_options opt;
>  
>  	memset(&opt, 0, sizeof(opt));
> -	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
> +	diff_tree_setup_paths(pathspec, &opt);
>  	opt.output_format = DIFF_FORMAT_CALLBACK;
>  	opt.format_callback = update_index_from_diff;
>  
> @@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	const char *rev = "HEAD";
>  	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
>  				*old_orig = NULL, sha1_old_orig[20];
> +	const char **pathspec = NULL;
>  	struct commit *commit;
>  	struct strbuf msg = STRBUF_INIT;
>  	const struct option options[] = {
> @@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  		die(_("Could not parse object '%s'."), rev);
>  	hashcpy(sha1, commit->object.sha1);
>  
> +	if (i < argc)
> +		pathspec = get_pathspec(prefix, argv + i);
> +
>  	if (patch_mode) {
>  		if (reset_type != NONE)
>  			die(_("--patch is incompatible with --{hard,mixed,soft}"));
> -		return interactive_reset(rev, argv + i, prefix);
> +		return run_add_interactive(rev, "--patch=reset", pathspec);
>  	}
>  
>  	/* git reset tree [--] paths... can be used to
>  	 * load chosen paths from the tree into the index without
>  	 * affecting the working tree nor HEAD. */
> -	if (i < argc) {
> +	if (pathspec) {
>  		if (reset_type == MIXED)
>  			warning(_("--mixed with paths is deprecated; use 'git reset -- <paths>' instead."));
>  		else if (reset_type != NONE)
>  			die(_("Cannot do %s reset with paths."),
>  					_(reset_type_names[reset_type]));
> -		return read_from_tree(prefix, argv + i, sha1,
> +		return read_from_tree(pathspec, sha1,
>  				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
>  	}
>  	if (reset_type == NONE)

^ permalink raw reply

* Re: [PATCH 04/19] reset: don't allow "git reset -- $pathspec" in bare repo
From: Junio C Hamano @ 2013-01-09 19:32 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-5-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> ---
>  builtin/reset.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

With the patch that does not have any explicit check for bareness
nor new error message to scold user with, it is rather hard to tell
what is going on, without any description on what (if anything) is
broken at the end user level and what remedy is done about that
breakage...

>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 045c960..664fad9 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  		else if (reset_type != NONE)
>  			die(_("Cannot do %s reset with paths."),
>  					_(reset_type_names[reset_type]));
> -		return read_from_tree(pathspec, sha1,
> -				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
>  	}
>  	if (reset_type == NONE)
>  		reset_type = MIXED; /* by default */
> @@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  		die(_("%s reset is not allowed in a bare repository"),
>  		    _(reset_type_names[reset_type]));
>  
> +	if (pathspec)
> +		return read_from_tree(pathspec, sha1,
> +				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
> +
>  	/* Soft reset does not touch the index file nor the working tree
>  	 * at all, but requires them in a good order.  Other resets reset
>  	 * the index file to the tree object we are switching to. */

^ permalink raw reply

* [PATCHv2] commit: make default of "cleanup" option configurable
From: Ralf Thielow @ 2013-01-09 19:36 UTC (permalink / raw)
  To: gitster, jrnieder; +Cc: git, Ralf Thielow
In-Reply-To: <1357676176-30019-1-git-send-email-ralf.thielow@gmail.com>

The default of the "cleanup" option in "git commit"
is not configurable. Users who don't want to use the
default have to pass this option on every commit since
there's no way to configure it. This commit introduces
a new config option "commit.cleanup" which can be used
to change the default of the "cleanup" option in
"git commit".

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---

Changes in v2:
- simplify implementation
- mention configuration variable in documentation of "git commit --cleanup"
- add an example usecase to documention of commit.cleanup configuration variable
- add tests

 Documentation/config.txt        |  7 ++++
 Documentation/git-commit.txt    |  4 +-
 builtin/commit.c                |  5 ++-
 t/t7500/add-content-and-comment |  4 ++
 t/t7502-commit.sh               | 84 +++++++++++++++++++++++++++++++++++++----
 5 files changed, 95 insertions(+), 9 deletions(-)
 create mode 100755 t/t7500/add-content-and-comment

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 53c4ca1..0452d56 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -917,6 +917,13 @@ column.tag::
 	Specify whether to output tag listing in `git tag` in columns.
 	See `column.ui` for details.
 
+commit.cleanup::
+	This setting overrides the default of the `--cleanup` option in
+	`git commit`. See linkgit:git-commit[1] for details. Changing the
+	default can be useful if you want to use the comment character (#)
+	consistently within your commit messages, in which case you would
+	like to change the default to 'whitespace'.
+
 commit.status::
 	A boolean to enable/disable inclusion of status information in the
 	commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 7bdb039..41b27da 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -179,7 +179,9 @@ OPTIONS
 	only if the message is to be edited. Otherwise only whitespace
 	removed. The 'verbatim' mode does not change message at all,
 	'whitespace' removes just leading/trailing whitespace lines
-	and 'strip' removes both whitespace and commentary.
+	and 'strip' removes both whitespace and commentary. The default
+	can be changed by the 'commit.cleanup' configuration variable
+	(see linkgit:git-config[1]).
 
 -e::
 --edit::
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..4d55f8f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -103,7 +103,7 @@ static enum {
 	CLEANUP_NONE,
 	CLEANUP_ALL
 } cleanup_mode;
-static char *cleanup_arg;
+static const char *cleanup_arg;
 
 static enum commit_whence whence;
 static int use_editor = 1, include_status = 1;
@@ -966,6 +966,7 @@ static const char *read_commit_message(const char *name)
 	return out;
 }
 
+
 static int parse_and_validate_options(int argc, const char *argv[],
 				      const struct option *options,
 				      const char * const usage[],
@@ -1320,6 +1321,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 		include_status = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "commit.cleanup"))
+		return git_config_string(&cleanup_arg, k, v);
 
 	status = git_gpg_config(k, v, NULL);
 	if (status)
diff --git a/t/t7500/add-content-and-comment b/t/t7500/add-content-and-comment
new file mode 100755
index 0000000..988f5e9
--- /dev/null
+++ b/t/t7500/add-content-and-comment
@@ -0,0 +1,4 @@
+#!/bin/sh
+echo "commit message" >> "$1"
+echo "# comment" >> "$1"
+exit 0
\ No newline at end of file
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 1a5cb69..b1c7648 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -4,6 +4,15 @@ test_description='git commit porcelain-ish'
 
 . ./test-lib.sh
 
+commit_msg_is () {
+	expect=commit_msg_is.expect
+	actual=commit_msg_is.actual
+
+	printf "%s" "$(git log --pretty=format:%s%b -1)" >$actual &&
+	printf "%s" "$1" >$expect &&
+	test_i18ncmp $expect $actual
+}
+
 # Arguments: [<prefix] [<commit message>] [<commit options>]
 check_summary_oneline() {
 	test_tick &&
@@ -168,7 +177,7 @@ test_expect_success 'verbose respects diff config' '
 	git config --unset color.diff
 '
 
-test_expect_success 'cleanup commit messages (verbatim,-t)' '
+test_expect_success 'cleanup commit messages (verbatim option,-t)' '
 
 	echo >>negative &&
 	{ echo;echo "# text";echo; } >expect &&
@@ -178,7 +187,7 @@ test_expect_success 'cleanup commit messages (verbatim,-t)' '
 
 '
 
-test_expect_success 'cleanup commit messages (verbatim,-F)' '
+test_expect_success 'cleanup commit messages (verbatim option,-F)' '
 
 	echo >>negative &&
 	git commit --cleanup=verbatim -F expect -a &&
@@ -187,7 +196,7 @@ test_expect_success 'cleanup commit messages (verbatim,-F)' '
 
 '
 
-test_expect_success 'cleanup commit messages (verbatim,-m)' '
+test_expect_success 'cleanup commit messages (verbatim option,-m)' '
 
 	echo >>negative &&
 	git commit --cleanup=verbatim -m "$(cat expect)" -a &&
@@ -196,7 +205,7 @@ test_expect_success 'cleanup commit messages (verbatim,-m)' '
 
 '
 
-test_expect_success 'cleanup commit messages (whitespace,-F)' '
+test_expect_success 'cleanup commit messages (whitespace option,-F)' '
 
 	echo >>negative &&
 	{ echo;echo "# text";echo; } >text &&
@@ -207,7 +216,7 @@ test_expect_success 'cleanup commit messages (whitespace,-F)' '
 
 '
 
-test_expect_success 'cleanup commit messages (strip,-F)' '
+test_expect_success 'cleanup commit messages (strip option,-F)' '
 
 	echo >>negative &&
 	{ echo;echo "# text";echo sample;echo; } >text &&
@@ -218,7 +227,7 @@ test_expect_success 'cleanup commit messages (strip,-F)' '
 
 '
 
-test_expect_success 'cleanup commit messages (strip,-F,-e)' '
+test_expect_success 'cleanup commit messages (strip option,-F,-e)' '
 
 	echo >>negative &&
 	{ echo;echo sample;echo; } >text &&
@@ -231,10 +240,71 @@ echo "sample
 # Please enter the commit message for your changes. Lines starting
 # with '#' will be ignored, and an empty message aborts the commit." >expect
 
-test_expect_success 'cleanup commit messages (strip,-F,-e): output' '
+test_expect_success 'cleanup commit messages (strip option,-F,-e): output' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'cleanup commit message (fail on invalid cleanup mode option)' '
+	test_must_fail git commit --cleanup=non-existent
+'
+
+test_expect_success 'cleanup commit message (fail on invalid cleanup mode configuration)' '
+	test_must_fail git -c commit.cleanup=non-existent commit
+'
+
+test_expect_success 'cleanup commit message (no config and no option uses default)' '
+	echo content >>file &&
+	git add file &&
+	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	git commit --no-status &&
+	commit_msg_is "commit message"
+'
+
+test_expect_success 'cleanup commit message (option overrides default)' '
+	echo content >>file &&
+	git add file &&
+	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	git commit --cleanup=whitespace --no-status &&
+	commit_msg_is "commit message # comment"
+'
+
+test_expect_success 'cleanup commit message (config overrides default)' '
+	echo content >>file &&
+	git add file &&
+	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	git -c commit.cleanup=whitespace commit --no-status &&
+	commit_msg_is "commit message # comment"
+'
+
+test_expect_success 'cleanup commit message (option overrides config)' '
+	echo content >>file &&
+	git add file &&
+	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	git -c commit.cleanup=whitespace commit --cleanup=default &&
+	commit_msg_is "commit message"
+'
+
+test_expect_success 'cleanup commit message (default, -m)' '
+	echo content >>file &&
+	git add file &&
+	git commit -m "message #comment " &&
+	commit_msg_is "message #comment"
+'
+
+test_expect_success 'cleanup commit message (whitespace option, -m)' '
+	echo content >>file &&
+	git add file &&
+	git commit --cleanup=whitespace --no-status -m "message #comment " &&
+	commit_msg_is "message #comment"
+'
+
+test_expect_success 'cleanup commit message (whitespace config, -m)' '
+	echo content >>file &&
+	git add file &&
+	git -c commit.cleanup=whitespace commit --no-status -m "message #comment " &&
+	commit_msg_is "message #comment"
+'
+
 test_expect_success 'message shows author when it is not equal to committer' '
 	echo >>negative &&
 	git commit -e -m "sample" -a &&
-- 
1.8.1.165.g0eb732d.dirty

^ permalink raw reply related

* Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index
From: Martin von Zweigbergk @ 2013-01-09 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v7gnm6uhm.fsf@alter.siamese.dyndns.org>

On Wed, Jan 9, 2013 at 11:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
> And as a Porcelain, I would rather expect it to leave the resulting
> index refreshed.

Yeah, I guess you're right. Regular users (those using only porcelain)
shouldn't notice, but it does make sense to think that the index would
be refreshed after running a porcelain. And the risk of breaking
people's scripts seems real too. I'll drop patch this from the re-roll
(which I'll also make sure I'll sign off)

(FYI, the reason I wrote this patch was because I was surprised that
"git reset" did anything with the worktree at all.)

^ permalink raw reply

* Re: [PATCH 06/19] reset.c: remove unnecessary variable 'i'
From: Junio C Hamano @ 2013-01-09 19:39 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-7-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Throughout most of parse_args(), the variable 'i' remains at 0. In the
> remaining few cases, we can do pointer arithmentic on argv itself
> instead.
> ---
> This is clearly mostly a matter of taste. The remainder of the series
> does not depend on it in any way.

I agree that it indeed is a matter of taste between

 (1) look at av[i], check with (i < ac) for the end, and increment i to
     iterate over the arguments; and

 (2) look at av[0], check with (0 < ac) for the end, and increment
     av and decrement ac at the same time to iterate over the
     arguments.

When (ac, av) appear as a pair, however, adjusting only av without
adjusting ac is asking for future trouble.  It violates a common
expectation that av[ac] points at the NULL at the end of the list.

If a code chooses to use !av[0] as the terminating condition and
never looks at ac, then incrementing only av is fine, but in such a
case, the function probably should lose ac altogether.

>  builtin/reset.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 9473725..68be05c 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -199,7 +199,6 @@ static void die_if_unmerged_cache(int reset_type)
>  }
>  
>  const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) {
> -	int i = 0;
>  	const char *rev = "HEAD";
>  	unsigned char unused[20];
>  	/*
> @@ -210,34 +209,34 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c
>  	 * git reset [-opts] -- <paths>...
>  	 * git reset [-opts] <paths>...
>  	 *
> -	 * At this point, argv[i] points immediately after [-opts].
> +	 * At this point, argv points immediately after [-opts].
>  	 */
>  
> -	if (i < argc) {
> -		if (!strcmp(argv[i], "--")) {
> -			i++; /* reset to HEAD, possibly with paths */
> -		} else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
> -			rev = argv[i];
> -			i += 2;
> +	if (argc) {
> +		if (!strcmp(argv[0], "--")) {
> +			argv++; /* reset to HEAD, possibly with paths */
> +		} else if (argc > 1 && !strcmp(argv[1], "--")) {
> +			rev = argv[0];
> +			argv += 2;
>  		}
>  		/*
> -		 * Otherwise, argv[i] could be either <rev> or <paths> and
> +		 * Otherwise, argv[0] could be either <rev> or <paths> and
>  		 * has to be unambiguous.
>  		 */
> -		else if (!get_sha1_committish(argv[i], unused)) {
> +		else if (!get_sha1_committish(argv[0], unused)) {
>  			/*
> -			 * Ok, argv[i] looks like a rev; it should not
> +			 * Ok, argv[0] looks like a rev; it should not
>  			 * be a filename.
>  			 */
> -			verify_non_filename(prefix, argv[i]);
> -			rev = argv[i++];
> +			verify_non_filename(prefix, argv[0]);
> +			rev = *argv++;
>  		} else {
>  			/* Otherwise we treat this as a filename */
> -			verify_filename(prefix, argv[i], 1);
> +			verify_filename(prefix, argv[0], 1);
>  		}
>  	}
>  	*rev_ret = rev;
> -	return i < argc ? get_pathspec(prefix, argv + i) : NULL;
> +	return *argv ? get_pathspec(prefix, argv) : NULL;
>  }
>  
>  int cmd_reset(int argc, const char **argv, const char *prefix)

^ permalink raw reply

* [PATCH] remote-hg: store converted URL
From: Max Horn @ 2013-01-09 19:43 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras, Max Horn

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

Mercurial might convert the URL to something more appropriate, like an
absolute path. Lets store that instead of the original URL, which won't
work from a different working directory if it's relative.

Suggested-by: Max Horn <max@quendi.de>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Max Horn <max@quendi.de>
---
For a discussion of the problem, see also
  http://article.gmane.org/gmane.comp.version-control.git/210250
While I am not quite happy with using "git config" to solve it, there
doesn't seem to be a better way right now.

 contrib/remote-helpers/git-remote-hg | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index c700600..7c74d8b 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -720,6 +720,14 @@ def do_export(parser):
     if peer:
         parser.repo.push(peer, force=False)
 
+def fix_path(alias, repo, orig_url):
+    repo_url = util.url(repo.url())
+    url = util.url(orig_url)
+    if str(url) == str(repo_url):
+        return
+    cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % repo_url]
+    subprocess.call(cmd)
+
 def main(args):
     global prefix, dirname, branches, bmarks
     global marks, blob_marks, parsed_refs
@@ -766,6 +774,9 @@ def main(args):
     repo = get_repo(url, alias)
     prefix = 'refs/hg/%s' % alias
 
+    if not is_tmp:
+        fix_path(alias, peer or repo, url)
+
     if not os.path.exists(dirname):
         os.makedirs(dirname)
 
-- 
1.8.0.1.525.gaaf5ad5

^ permalink raw reply related

* Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()
From: Junio C Hamano @ 2013-01-09 19:48 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-9-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Use a single condition to guard the call to die_if_unmerged_cache for
> both --soft and --keep. This avoids the small distraction of the
> precondition check from the logic following it.
>
> Also change an instance of
>
>   if (e)
>     err = err || f();
>
> to the almost as short, but clearer
>
>   if (e && !err)
>     err = f();
>
> (which is equivalent since we only care whether exit code is 0)

It is not just equivalent, but should give us identical result, even
if we cared the actual value.

And I tend to agree that the latter is more readable, especially
when f() can be longer, which is often the case in real life.

Happy to see this change.

> ---
>  builtin/reset.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 4d556e7..42d1563 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	/* Soft reset does not touch the index file nor the working tree
>  	 * at all, but requires them in a good order.  Other resets reset
>  	 * the index file to the tree object we are switching to. */
> -	if (reset_type == SOFT)
> +	if (reset_type == SOFT || reset_type == KEEP)
>  		die_if_unmerged_cache(reset_type);
> -	else {
> -		int err;
> -		if (reset_type == KEEP)
> -			die_if_unmerged_cache(reset_type);
> -		err = reset_index_file(sha1, reset_type, quiet);
> -		if (reset_type == KEEP)
> -			err = err || reset_index_file(sha1, MIXED, quiet);
> +
> +	if (reset_type != SOFT) {
> +		int err = reset_index_file(sha1, reset_type, quiet);
> +		if (reset_type == KEEP && !err)
> +			err = reset_index_file(sha1, MIXED, quiet);
>  		if (err)
>  			die(_("Could not reset index file to revision '%s'."), rev);
>  	}

^ permalink raw reply

* [PATCH v2 1/2] git-fast-import(1): combine documentation of --[no-]relative-marks
From: John Keeping @ 2013-01-09 19:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Eric S. Raymond, git, David Michael Barr,
	Pete Wyckoff, Thomas Rast
In-Reply-To: <cover.1357760256.git.john@keeping.me.uk>

The descriptions of '--relative-marks' and '--no-relative-marks' make
more sense when read together instead of as two independent options.
Combine them into a single description block.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 Documentation/git-fast-import.txt | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 3da5cc2..75ce808 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -83,19 +83,17 @@ OPTIONS
 	Like --import-marks but instead of erroring out, silently
 	skips the file if it does not exist.
 
---relative-marks::
+--[no-]relative-marks::
 	After specifying --relative-marks the paths specified
 	with --import-marks= and --export-marks= are relative
 	to an internal directory in the current repository.
 	In git-fast-import this means that the paths are relative
 	to the .git/info/fast-import directory. However, other
 	importers may use a different location.
++
+Relative and non-relative marks may be combined by interweaving
+--(no-)-relative-marks with the --(import|export)-marks= options.
 
---no-relative-marks::
-	Negates a previous --relative-marks. Allows for combining
-	relative and non-relative marks by interweaving
-	--(no-)-relative-marks with the --(import|export)-marks=
-	options.
 
 --cat-blob-fd=<fd>::
 	Write responses to `cat-blob` and `ls` queries to the
-- 
1.8.1.468.g3d9f9b6

^ permalink raw reply related

* [PATCH v2 0/2] Reorganise options in git-fast-import(1)
From: John Keeping @ 2013-01-09 19:43 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Eric S. Raymond, git, David Michael Barr,
	Pete Wyckoff, Thomas Rast
In-Reply-To: <7va9slnc07.fsf@alter.siamese.dyndns.org>

Here's a second attempt at this taking into account the feedback received so far.

Changes since v1:

 * Left dedup '--done' as a separate patch (now merged)
 * Split combining '--[no-]relative-marks' into a separate patch
 * '--force' moved to the top of the options, making the catchall
   section alphabetically sorted
 * Section headings changed:
    'Options related to the input stream' => 'Options for Frontends'
    'Options related to marks' => 'Locations of Marks Files'
    'Options for tuning' => 'Performance and Compression Tuning'
 * '--export-pack-edges' moves to 'Performance and Compression Tuning'
 * '--cat-blob-fd' moves to 'Options for Frontends'


John Keeping (2):
  git-fast-import(1): combine documentation of --[no-]relative-marks
  git-fast-import(1): reorganise options

 Documentation/git-fast-import.txt | 98 +++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 46 deletions(-)

-- 
1.8.1.468.g3d9f9b6

^ permalink raw reply

* Re: [PATCH 09/19] reset.c: replace switch by if-else
From: Junio C Hamano @ 2013-01-09 19:53 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-10-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> ---
>  builtin/reset.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 42d1563..05ccfd4 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	 * saving the previous head in ORIG_HEAD before. */
>  	update_ref_status = update_refs(rev, sha1);
>  
> -	switch (reset_type) {
> -	case HARD:
> -		if (!update_ref_status && !quiet)
> -			print_new_head_line(commit);
> -		break;
> -	case SOFT: /* Nothing else to do. */
> -		break;
> -	case MIXED: /* Report what has not been updated. */
> +	if (reset_type == HARD && !update_ref_status && !quiet)
> +		print_new_head_line(commit);
> +	else if (reset_type == MIXED) /* Report what has not been updated. */
>  		update_index_refresh(0, NULL,
>  				quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
> -		break;
> -	}

Justification?

It might be shorter, but I somehow find the original _much_ easier
to follow, and to possibly extend.  The case arms delineate the
major modes of operation, and when somebody is interested in what
happens in "reset --hard", the case labels allow eyes to immediately
spot and skip uninteresting case arms.  On the other hand, the
updated one forces you to read the if/else cascade through.

^ permalink raw reply

* git-archive fails against smart-http repos
From: Bruce Lysik @ 2013-01-09 18:52 UTC (permalink / raw)
  To: git

Hi,

Trying to run git-archive fails against smart-http based repos.  Example:

$ git archive --verbose --format=zip --remote=http://code.toofishes.net/git/dan/initscripts.git
fatal: Operation not supported by protocol.
Unexpected end of command stream

This problem was brought up against my internal repos as well.

^ permalink raw reply

* [PATCH v2 2/2] git-fast-import(1): reorganise options
From: John Keeping @ 2013-01-09 19:45 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Eric S. Raymond, git, David Michael Barr,
	Pete Wyckoff, Thomas Rast
In-Reply-To: <cover.1357760256.git.john@keeping.me.uk>

The options in git-fast-import(1) are not currently arranged in a
logical order, which has caused the '--done' options to be documented
twice (commit 3266de10).

Rearrange them into logical groups under subheadings.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: John Keeping <john@keeping.me.uk>

---
 Documentation/git-fast-import.txt | 88 +++++++++++++++++++++------------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 75ce808..bf1a02a 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -33,34 +33,46 @@ the frontend program in use.
 
 OPTIONS
 -------
---date-format=<fmt>::
-	Specify the type of dates the frontend will supply to
-	fast-import within `author`, `committer` and `tagger` commands.
-	See ``Date Formats'' below for details about which formats
-	are supported, and their syntax.
 
 --force::
 	Force updating modified existing branches, even if doing
 	so would cause commits to be lost (as the new commit does
 	not contain the old commit).
 
---max-pack-size=<n>::
-	Maximum size of each output packfile.
-	The default is unlimited.
+--quiet::
+	Disable all non-fatal output, making fast-import silent when it
+	is successful.  This option disables the output shown by
+	\--stats.
 
---big-file-threshold=<n>::
-	Maximum size of a blob that fast-import will attempt to
-	create a delta for, expressed in bytes.  The default is 512m
-	(512 MiB).  Some importers may wish to lower this on systems
-	with constrained memory.
+--stats::
+	Display some basic statistics about the objects fast-import has
+	created, the packfiles they were stored into, and the
+	memory used by fast-import during this run.  Showing this output
+	is currently the default, but can be disabled with \--quiet.
 
---depth=<n>::
-	Maximum delta depth, for blob and tree deltification.
-	Default is 10.
+Options for Frontends
+~~~~~~~~~~~~~~~~~~~~~
 
---active-branches=<n>::
-	Maximum number of branches to maintain active at once.
-	See ``Memory Utilization'' below for details.  Default is 5.
+--cat-blob-fd=<fd>::
+	Write responses to `cat-blob` and `ls` queries to the
+	file descriptor <fd> instead of `stdout`.  Allows `progress`
+	output intended for the end-user to be separated from other
+	output.
+
+--date-format=<fmt>::
+	Specify the type of dates the frontend will supply to
+	fast-import within `author`, `committer` and `tagger` commands.
+	See ``Date Formats'' below for details about which formats
+	are supported, and their syntax.
+
+--done::
+	Terminate with error if there is no `done` command at the end of
+	the stream.  This option might be useful for detecting errors
+	that cause the frontend to terminate before it has started to
+	write a stream.
+
+Locations of Marks Files
+~~~~~~~~~~~~~~~~~~~~~~~~
 
 --export-marks=<file>::
 	Dumps the internal marks table to <file> when complete.
@@ -94,19 +106,22 @@ OPTIONS
 Relative and non-relative marks may be combined by interweaving
 --(no-)-relative-marks with the --(import|export)-marks= options.
 
+Performance and Compression Tuning
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
---cat-blob-fd=<fd>::
-	Write responses to `cat-blob` and `ls` queries to the
-	file descriptor <fd> instead of `stdout`.  Allows `progress`
-	output intended for the end-user to be separated from other
-	output.
+--active-branches=<n>::
+	Maximum number of branches to maintain active at once.
+	See ``Memory Utilization'' below for details.  Default is 5.
 
---done::
-	Terminate with error if there is no `done` command at the
-	end of the stream.
-	This option might be useful for detecting errors that
-	cause the frontend to terminate before it has started to
-	write a stream.
+--big-file-threshold=<n>::
+	Maximum size of a blob that fast-import will attempt to
+	create a delta for, expressed in bytes.  The default is 512m
+	(512 MiB).  Some importers may wish to lower this on systems
+	with constrained memory.
+
+--depth=<n>::
+	Maximum delta depth, for blob and tree deltification.
+	Default is 10.
 
 --export-pack-edges=<file>::
 	After creating a packfile, print a line of data to
@@ -117,16 +132,9 @@ Relative and non-relative marks may be combined by interweaving
 	as these commits can be used as edge points during calls
 	to 'git pack-objects'.
 
---quiet::
-	Disable all non-fatal output, making fast-import silent when it
-	is successful.  This option disables the output shown by
-	\--stats.
-
---stats::
-	Display some basic statistics about the objects fast-import has
-	created, the packfiles they were stored into, and the
-	memory used by fast-import during this run.  Showing this output
-	is currently the default, but can be disabled with \--quiet.
+--max-pack-size=<n>::
+	Maximum size of each output packfile.
+	The default is unlimited.
 
 
 Performance
-- 
1.8.1.468.g3d9f9b6

^ permalink raw reply related

* Re: [PATCH 10/19] reset --keep: only write index file once
From: Junio C Hamano @ 2013-01-09 19:55 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-11-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> "git reset --keep" calls reset_index_file() twice, first doing a
> two-way merge to the target revision, updating the index and worktree,
> and then resetting the index. After each call, we write the index
> file.
>
> In the unlikely event that the second call to reset_index_file()
> fails, the index will have been merged to the target revision, but
> HEAD will not be updated, leaving the user with a dirty index.
>
> By moving the locking, writing and committing out of
> reset_index_file() and into the caller, we can avoid writing the index
> twice, thereby making the sure we don't end up in the half-way reset
> state.

Nice.

^ permalink raw reply

* Re: [PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
From: Junio C Hamano @ 2013-01-09 19:59 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1357719376-16406-16-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> By not returning from inside the "if (pathspec)" block, we can let the
> pathspec-aware and pathspec-less code share a bit more, making it
> easier to make future changes that should affect both cases. This also
> highlights the similarity between read_from_tree() and reset_index().
> ---
> Should error reporting be aligned too? Speaking of which,
> do_diff_cache() never returns anything by 0. Is the return value for
> future-proofing?

Perhaps, and yes.

>
>  builtin/reset.c | 42 ++++++++++++++++++------------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 254afa9..9bcad29 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -308,19 +308,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  		die(_("%s reset is not allowed in a bare repository"),
>  		    _(reset_type_names[reset_type]));
>  
> -	if (pathspec) {
> -		struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
> -		int index_fd = hold_locked_index(lock, 1);
> -		if (read_from_tree(pathspec, sha1))
> -			return 1;
> -		update_index_refresh(
> -			quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
> -		if (write_cache(index_fd, active_cache, active_nr) ||
> -		    commit_locked_index(lock))
> -			return error("Could not write new index file.");
> -		return 0;
> -	}
> -
>  	/* Soft reset does not touch the index file nor the working tree
>  	 * at all, but requires them in a good order.  Other resets reset
>  	 * the index file to the tree object we are switching to. */
> @@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	if (reset_type != SOFT) {
>  		struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
>  		int newfd = hold_locked_index(lock, 1);
> -		int err = reset_index(sha1, reset_type, quiet);
> -		if (reset_type == KEEP && !err)
> -			err = reset_index(sha1, MIXED, quiet);
> -		if (err)
> -			die(_("Could not reset index file to revision '%s'."), rev);
> +		if (pathspec) {
> +			if (read_from_tree(pathspec, sha1))
> +				return 1;
> +		} else {
> +			int err = reset_index(sha1, reset_type, quiet);
> +			if (reset_type == KEEP && !err)
> +				err = reset_index(sha1, MIXED, quiet);
> +			if (err)
> +				die(_("Could not reset index file to revision '%s'."), rev);
> +		}
>  
>  		if (reset_type == MIXED) /* Report what has not been updated. */
>  			update_index_refresh(
> @@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  			die(_("Could not write new index file."));
>  	}
>  
> -	/* Any resets update HEAD to the head being switched to,
> -	 * saving the previous head in ORIG_HEAD before. */
> -	update_ref_status = update_refs(rev, sha1);
> +	if (!pathspec) {
> +		/* Any resets without paths update HEAD to the head being
> +		 * switched to, saving the previous head in ORIG_HEAD before. */
> +		update_ref_status = update_refs(rev, sha1);
>  
> -	if (reset_type == HARD && !update_ref_status && !quiet)
> -		print_new_head_line(commit);
> +		if (reset_type == HARD && !update_ref_status && !quiet)
> +			print_new_head_line(commit);
>  
> -	remove_branch_state();
> +		remove_branch_state();
> +	}
>  
>  	return update_ref_status;
>  }

^ permalink raw reply


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