Re: [PATCH 4/4] t5002: check if unzip supports symlinks

2013-01-09 Thread Jonathan Nieder
René Scharfe wrote:
> Am 07.01.2013 09:52, schrieb Jonathan Nieder:

>> Hm.  Do some implementations of "unzip" not support symlinks, or is
>> the problem that some systems build Info-ZIP without the SYMLINKS
>> option?
>
> The unzip supplied with NetBSD 6.0.1, which is based on libarchive, doesn't
> support symlinks.  It creates a file with the link target path as its only
> content for such entries.

Ok, that makes sense.  A quick search finds
, which if
I understand correctly was fixed in libarchive 3.0.2.  NetBSD 6 uses a
patched 2.8.4.

[...]
> For the test script there is no difference: If we don't have a tool to
> verify symlinks in archives, we better skip that part.

Yeah, I just wanted to see if there were other parts of the world that
needed fixing while at it.  Thanks for explaining.

Ciao,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


pandora bracelets but by the time it has charms

2013-01-09 Thread zhangqy1
Every now and then some new trend in fashion or jewelry comes along, makes an
enormous splash, [url=http://www.pandoraclub.org/]pandora bracelets[/url]
has carried out just that. It appears like everybody's talking about it now.
Purchasing, collecting, and adding on to their necklaces and bracelets from
the ever growing assortment of charms and beads that Pandora produces in
their Amsterdam design home.However though with each and every designer
achievement right now comes it's share of fraudsters and copycats.Of course
there is usually the knockoff artist who remain busy duplicating the most
recent hot designer jewelry and fashion accessories. But with Pandora it
gets more complicated and that. This implies the you've to be on the lookout
for more than just the cheaply created fakes, despite the fact that they're
available on the web.

As a prime example, one of the more all too typical scams that is being run
on-line takes advantage of men and women shopping for retired
[url=http://www.pandoraclub.org/]pandora canada[/url]. These are the designs
have been taken out of standard production at their Amsterdam facility and
due to their rarity, as well as the demand can fetch prices as high as 10
times their original cost. Retired Pandora beads are a really hot item right
now. So what's the scam then?It's straightforward yet very useful for
parting people with their hard-earned funds everyday. That's rip-off artists
who advertise retired beads on the web that are nonetheless being produced
from the house of Pandora in Amsterdam. Unsuspecting shoppers see them for
sale on-line advertised as retired with an inflated price, and just assume
that they are retired, in no way bothering to double-check to make certain
that they are not getting cheated.If you are thinking about retired Pandora
jewelry then here's one more factoid which you ought to know right before
you start browsing on-line.

That is that Pandora very often will retire a piece in one country in
particular, say the US, but still have it available as a full production
piece in other countries. So here once more it pays to complete the
research, to get to know your product before you shop.Then obviously you
will find always going to be the fakes, and also the knockoffs, and they're
not hard to discover on the
internet.[url=http://www.pandoraclub.org/]pandora charm bracelet[/url] that
look great in the pictures on your personal computer screen but give
themselves away as obviousness inferior product when you are holding them in
your hand. So how do you avoid being scammed here? Learn to recognize the
Pandora trademark and engraving, and if the cost just seems too excellent to
be true, it probably is..
http://www.pandoraclub.org/   



--
View this message in context: 
http://git.661346.n2.nabble.com/pandora-bracelets-but-by-the-time-it-has-charms-tp7574656.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


people seem to return pandora bracelets to those ancient times

2013-01-09 Thread qitade655
pandora bracelets brand launched several carefully crafted pendant. Most of
people inclined to judge people by their dressing, and measure their status
and ability, to decide the attitude. The researcher found that there were
only twelve people of the dressing lower class style get the profile, while
another dressing style got the profile of forty-two people.
[url=http://www.buycheappandorabracelets.co.uk/]pandora uk[/url] to fit your
clothes, or that some day make the colors match a special occasion. The
worst thing about this is to find good prices on beads and bracelets. 

Many places are quite expensive and will try to rip you off. Designers like
to use classical inspiration to create beautiful jewelry, people seem to
return [url=http://www.buycheappandorabracelets.co.uk/]pandora
bracelets[/url] to those ancient times. The brand with superb craftsmanship
and careful pandora bracelet sale uk consideration to create jewelry out no
exception. No man drag. but half will be pandora bracelet to the East China
Sea. Saying he has been able to so readily these days Shensha large array
are sent weekly poetry pray. Maybe you will be interested in learning why I
am so engaged with Pandora jewelry there is not a solitary jewelry of other
brand names. I suppose there needs to be something exceptional about Pandora
charms that overwhelm my center. The Imagine watches are the main ones that
have interchangeable bezels. The most simplest design, I think, is the Pure
Slimline watch. It is not terribly outstanding, being a fairly simple watch
and watch face with a nice, easy to read face. This line has a double
leather watchband, very small and thin to match the overall style of the
watch. Bands are available in several different classy colors. Sun Xuehan
who are alarmed and asked. and he and several pandora australia have been
pushed back vigorously shares a sudden.
http://www.buycheappandorabracelets.co.uk/ 



--
View this message in context: 
http://git.661346.n2.nabble.com/people-seem-to-return-pandora-bracelets-to-those-ancient-times-tp7574655.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t7400 broken on pu (Mac OS X)

2013-01-09 Thread Duy Nguyen
On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote:
> 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
> 

I can reproduce it by setting core.ignorecase to true. There is a bug
that I overlooked. Can you verify if this throw-away patch fixes it
for you? A proper fix will be in the reroll later.

-- 8< --
diff --git a/builtin/add.c b/builtin/add.c
index 641037f..61cb8bd 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, 
const char **pathspec, int
return seen;
 }
 
-static void treat_gitlinks(const char **pathspec)
+static int treat_gitlinks(const char **pathspec)
 {
int i;
+   int modified = 0;
 
if (!pathspec || !*pathspec)
-   return;
+   return modified;
 
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
@@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec)
if (len2 <= len || pathspec[j][len] != '/' ||
memcmp(ce->name, pathspec[j], len))
continue;
-   if (len2 == len + 1)
+   if (len2 == len + 1) {
/* strip trailing slash */
pathspec[j] = xstrndup(ce->name, len);
-   else
+   modified = 1;
+   } else
die (_("Path '%s' is in submodule 
'%.*s'"),
pathspec[j], len, ce->name);
}
}
}
+   return modified;
 }
 
 static void refresh(int verbose, const struct pathspec *pathspec)
@@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
 
if (read_cache() < 0)
die(_("index file corrupt"));
-   treat_gitlinks(pathspec.raw);
+   if (treat_gitlinks(pathspec.raw))
+   /*
+* HACK: treat_gitlinks strips the trailing slashes
+* out of submodule entries but it only affects
+* raw[]. Everything in pathspec.items is not touched.
+* Re-init it to propagate the change. Long term, this
+* function should be moved to pathspec.c and update
+* everything in a consistent way.
+*/
+   init_pathspec(&pathspec, pathspec.raw);
 
if (add_new_files) {
int baselen;
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


selection of links of london friendship bracelet is extraordinary

2013-01-09 Thread liseera521
Silver charm bracelet natural Spar chain in the bottom half, dark green pole
with dirt, very natural, unaffected. Interval adding green metallic beads
and silver flower belt also left-right asymmetry of the links of london sale
is hot in recent years, the irregular wind. You will be able to do
especially in the links of the delights of London as her dress for almost
any operation and that the mood at a specific day of work! Besides, these
bracelets is often only one talent to a very good chance any of this type of
home such as heating, birthday or university workday. Department of partial
bonds beige necklace set in London, innocent lady. Round, oval, irregular
gravel spar with each other, are two means more second stack, and finally
leaves and roses of old silver bracelet silver belt.

Whether in short sleeves or long sleeves, light clothing, snow Wan Hao is a
great show of good wrist, why not buy a new bracelet? The silver bracelet
certainly brings the beautiful to you. links of london friendship bracelet
willing to lock the arms and legs, to take a slightly different, necklaces,
brooches, bracelets of silver, black and white decor. When the letters, the
pearl, the cross and other items that appear on the silver earring,
necklaces previous aspect, the need super cute accessories to match!
Brightly colored clothing, with fashion accessories, and become a new
favorite trend this season. Beautiful color, fresh and stunning beaded
bracelet is a good choice for you. All varieties of jewelry brand new games
released each season decorations, so no worries about matching accessories,
there will be a perfect system of the direct effects of wear. Links Of
London Bracelets accessories that is best reflects the personality, you can
select the accessories a bit exaggerated, it shows more clearly the
personality.
http://www.cheaplinksoflondonbracelet.co.uk/  




--
View this message in context: 
http://git.661346.n2.nabble.com/selection-of-links-of-london-friendship-bracelet-is-extraordinary-tp7574652.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


thomas sabo bracelet technology which offers all the college

2013-01-09 Thread jiuzheyab
Thomas Sabo is a popular name in the field of sterling silver jewelry. Modern
youth have fallen in love with exclusive designs of  thomas sabo uk
  . Nowadays, one can
easily spot people wearing different products (like bracelets, earrings, and
necklaces) designed by Thomas Sabo. Charm collection by Thomas Sabo is
certainly among the most popular. Nowadays, it is a popular trend to
complement modern casual dressing with products from Thomas Sabo Charm
collection.

Another fast abounding varieties elements really are really all the argent
jewelry. The things incorporated, a top-notch-quality timber a fabulous
makeup come about swank specifically approximately inimitable. There is
completely different systems all the provided by all the Jones Sabo
agreeableness organization.  thomas sabo bracelet
   technology which
offers all the college for the completely different altitude is normally
important and various. There might be necklaces for argent contrast. All of
these Jones Sabo Necklaces really are for that reason commendable by means
of whatever jewelry, settled by means of thomas sabo uk, anklet bracelets
and additionally jewelry.
http://www.thomassabobraceletsshop.co.uk/



--
View this message in context: 
http://git.661346.n2.nabble.com/thomas-sabo-bracelet-technology-which-offers-all-the-college-tp7574651.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


links of london sale have always endeavoured to create

2013-01-09 Thread jillysee
For someone looking for a classic fashion look, you might consider a 
links of london sale  
 or gems to go with a formal suit. A row of small diamonds set in
white gold or silver really brings out the spark, and not clearly defined
and the outline of the stones, making them looks larger. The brightness,
however, is draw attention to the simplicity of the piece without removing
the entire image. With the development of residing standards, impartial
thought, existing day ladies by now no lengthier adhere to in road really
are a range of Yuan a reasonable adorn article, and more, the pursuit using
the personality. However, the common working profits allow them to that
higher priced high-grade one-way links of london sale. What generates them
several rebel psychology satisfied? What can satisfy the need of those
beautiful? Who will happen once again profits myth? individualized one-way
links of London diamond could be the style using the long-term and tide.
http://www.linksoflondonbraceletsweetie.co.uk/



--
View this message in context: 
http://git.661346.n2.nabble.com/links-of-london-sale-have-always-endeavoured-to-create-tp7574650.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] git-clean: Display more accurate delete messages

2013-01-09 Thread Junio C Hamano
Zoltan Klinger  writes:

> Consider the output of the improved version:
>
>   $ git clean -fd
>   Removing tracked_dir/some_untracked_file
>   Removing untracked_file
>   warning: ignoring untracked git repository untracked_foo/frotz.git
>   Removing untracked_foo/bar
>   Removing untracked_foo/emptydir
>   warning: ignoring untracked git repository untracked_some.git/
>
> Now it displays only the file and directory names that got actually
> deleted and shows warnings about ignored untracked git repositories.
>
> Reported-by: Soren Brinkmann 
>
> Signed-off-by: Zoltan Klinger 
> ---

I think the code before this patch used to say "Would not remove"
and "Not removing" in certain cases to report the paths that the
command decided not to remove, but after this patch these two
messages no longer appear in the patch.

Is it expected, are we losing information, or...?

>  builtin/clean.c |  158 
> +--
>  1 file changed, 129 insertions(+), 29 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 69c1cda..1714546 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -10,6 +10,7 @@
>  #include "cache.h"
>  #include "dir.h"
>  #include "parse-options.h"
> +#include "refs.h"
>  #include "string-list.h"
>  #include "quote.h"
>  
> @@ -20,6 +21,12 @@ static const char *const builtin_clean_usage[] = {
>   NULL
>  };
>  
> +static const char *msg_remove = N_("Removing %s\n");
> +static const char *msg_would_remove = N_("Would remove %s\n");
> +static const char *msg_would_ignore_git_dir = N_("Would ignore untracked git 
> repository %s\n");
> +static const char *msg_warn_ignore_git_dir = N_("ignoring untracked git 
> repository %s");
> +static const char *msg_warn_remove_failed = N_("failed to remove %s");
> +
>  static int git_clean_config(const char *var, const char *value, void *cb)
>  {
>   if (!strcmp(var, "clean.requireforce"))
> @@ -34,11 +41,116 @@ static int exclude_cb(const struct option *opt, const 
> char *arg, int unset)
>   return 0;
>  }
>  
> +static int remove_dirs(struct strbuf *path, const char *prefix, int 
> force_flag,
> + int dry_run, int quiet, int *dir_gone)
> +{
> + DIR *dir;
> + struct strbuf quoted = STRBUF_INIT;
> + struct dirent *e;
> + int res = 0, ret = 0, gone = 1, original_len = path->len, len, i;
> + unsigned char submodule_head[20];
> + struct string_list dels = STRING_LIST_INIT_DUP;
> +
> + *dir_gone = 1;
> +
> + if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> + !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
> + if (dry_run && !quiet) {
> + quote_path_relative(path->buf, strlen(path->buf), 
> "ed, prefix);
> + printf(_(msg_would_ignore_git_dir), quoted.buf);
> + } else if (!dry_run) {
> + quote_path_relative(path->buf, strlen(path->buf), 
> "ed, prefix);
> + warning(_(msg_warn_ignore_git_dir), quoted.buf);
> + }
> +
> + *dir_gone = 0;
> + return 0;
> + }
> +
> + dir = opendir(path->buf);
> + if (!dir) {
> + /* an empty dir could be removed even if it is unreadble */
> + res = dry_run ? 0 : rmdir(path->buf);
> + if (res) {
> + quote_path_relative(path->buf, strlen(path->buf), 
> "ed, prefix);
> + warning(_(msg_warn_remove_failed), quoted.buf);
> + *dir_gone = 0;
> + }
> + return res;
> + }
> +
> + if (path->buf[original_len - 1] != '/')
> + strbuf_addch(path, '/');
> +
> + len = path->len;
> + while ((e = readdir(dir)) != NULL) {
> + struct stat st;
> + if (is_dot_or_dotdot(e->d_name))
> + continue;
> +
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, e->d_name);
> + if (lstat(path->buf, &st))
> + ; /* fall thru */
> + else if (S_ISDIR(st.st_mode)) {
> + if (remove_dirs(path, prefix, force_flag, dry_run, 
> quiet, &gone))
> + ret = 1;
> + if (gone) {
> + quote_path_relative(path->buf, 
> strlen(path->buf), "ed, prefix);
> + string_list_append(&dels, quoted.buf);
> + }
> + else
> + *dir_gone = 0;
> + continue;
> + } else {
> + res = dry_run ? 0 : unlink(path->buf);
> + if (!res) {
> + quote_path_relative(path->buf, 
> strlen(path->buf), "ed, prefix);
> + string_list_append(&dels, quoted.buf);
> + }
> + else {
> + quote_path_relative(path->buf, 

Re: [PATCH v4] git-clean: Display more accurate delete messages

2013-01-09 Thread Zoltan Klinger
> I wonder whether it's possible to make the output more consistent,
> as in:
>
> Removing tracked_dir/some_untracked_file
> Removing untracked_file
> Skipping repository untracked_foo/frotz.git
> Removing untracked_foo/bar
> Removing untracked_foo/emptydir
> Skipping repository untracked_some.git
>
> or similar.  What do you think?

Agree, the output looks much neater printed like that. I am going to
update the patch unless someone feels strongly against the proposed
output.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

2013-01-09 Thread Junio C Hamano
Jonathan Nieder  writes:

> I'm not sure whether there are SIGPIPE instances we really don't want
> to be silent about, though.  I suspect not. ;-)
>
> Compare ,
> .

Yeah, thanks for the pointer to 48665.  Quoting from there:

So EPIPE really _is_ special: because when you write to a pipe,
there's no guarantee that you'll get it at all, so whenever you get
an EPIPE you should ask yourself:

 - what would I have done if the data had fit in the 64kB kernel
   buffer?

 - should I really return a different error message or complain just 
   because I just happened to notice that the reader went away
   _this_ 
   time, even if I might not notice it next time?

In other words, the "exit(0)" is actually _more_ consistent than
"exit(1)", because exiting with an error message or with an error
return is going to depend on luck and timing.

and I think I still agree with the analysis and conclusion:

So what _should_ you do for EPIPE?

Here's what EPIPE _really_ means:

 - you might as well consider the write a success, but the
   reader isn't actually interested, so rather than go on, you
   might as well stop early.

Notice how I very carefull avoided the word "error" anywhere.
Because it's really not an error. The reader already got
everything it wanted. So EPIPE should generally be seen as an
"early success" rather than as a "failure".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t0008: avoid brace expansion

2013-01-09 Thread Adam Spiers
On Thu, Jan 10, 2013 at 12:18 AM, Junio C Hamano  wrote:
> Adam Spiers  writes:
>
>> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
>>  wrote:
>>> Brace expansion is not required by POSIX and not supported by dash nor
>>> NetBSD's sh.  Explicitly list all combinations instead.
>>
>> Good catch, thanks!
>
> Yeah; thanks.
>
> It would also be nice to avoid touch while we are at it, by the way.

Noted.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

2013-01-09 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jeff King  writes:

>> But we still say "error: ... died of signal 13", because that comes from
>> inside wait_or_whine. So it is a separate issue whether or not
>> wait_or_whine should be silent on SIGPIPE (we already are on SIGINT and
>> SIGQUIT, as of some recent patches).
>>
>> The upside is that it is noise in this case that we would no longer see.
>> The downside is that we may be losing a clue when debugging server
>> problems, which do not expect to die from SIGPIPE.  Should it be an
>> optional run-command flag?
>
> Do we know if we are upstream of a pager that reads from us through
> a pipe (I think we should, especially in a case where we are the one
> who processed the "git -p $alias" option)?  Is there any other case
> where we would want to ignore child's death by SIGPIPE?

When we die early by SIGPIPE because output was piped to "head", I
still think the early end of output is not notable enough to complain
about.

I'm not sure whether there are SIGPIPE instances we really don't want
to be silent about, though.  I suspect not. ;-)

Compare ,
.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t0008: avoid brace expansion

2013-01-09 Thread Junio C Hamano
Adam Spiers  writes:

> On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
>  wrote:
>> Brace expansion is not required by POSIX and not supported by dash nor
>> NetBSD's sh.  Explicitly list all combinations instead.
>
> Good catch, thanks!

Yeah; thanks.

It would also be nice to avoid touch while we are at it, by the way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2] commit: make default of "cleanup" option configurable

2013-01-09 Thread Junio C Hamano
Ralf Thielow  writes:

> 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 
> ---

Thanks.

> 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'.

When the documentation suggests to use 'whitespace', it would be
helpful to warn the readers that hints Git produces in '#'-commented
section need to be removed, if they are not ment to be kept (which
is 99.99% of the case).  Perhaps:

This setting overrides the default of the `--cleanup` option
in `git commit`. Changing the default can be useful when you
always want to keep lines that begin with comment character
`#` in your log message, in which case you would do `git
config commit.cleanup whitespace` (note that you will have
to remove the help lines that begin with '#' in the commit
log template yourself, if you do this).

or something?

> 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]).

Nicely written.

> --- 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[],

Don't add an extra blank line, please.

> @@ -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);

Nice.

> diff --git a/t/t7500/add-content-and-comment b/t/t7500/add-content-and-comment
> new file mode 100755
> index 000..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

Have newline at end of file, please.

> 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'
> +...
> +'
> +

Nicely done.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t0008: avoid brace expansion

2013-01-09 Thread Adam Spiers
On Wed, Jan 9, 2013 at 11:49 PM, René Scharfe
 wrote:
> Brace expansion is not required by POSIX and not supported by dash nor
> NetBSD's sh.  Explicitly list all combinations instead.

Good catch, thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t0008: avoid brace expansion

2013-01-09 Thread René Scharfe
Brace expansion is not required by POSIX and not supported by dash nor
NetBSD's sh.  Explicitly list all combinations instead.

Signed-off-by: Rene Scharfe 
---
 t/t0008-ignores.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9b0fcd6..0273680 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -129,8 +129,9 @@ test_expect_success 'setup' '
one
ignored-*
EOF
-   touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} &&
-   git add -f {,a/}ignored-but-in-index
+   touch not-ignored ignored-and-untracked ignored-but-in-index &&
+   touch a/not-ignored a/ignored-and-untracked a/ignored-but-in-index &&
+   git add -f ignored-but-in-index a/ignored-but-in-index &&
cat <<-\EOF >a/.gitignore &&
two*
*three
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] git-fast-import(1): reorganise options

2013-01-09 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-shortlog(1): document behaviour of zero-width wrap

2013-01-09 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

2013-01-09 Thread Junio C Hamano
Jeff King  writes:

> But we still say "error: ... died of signal 13", because that comes from
> inside wait_or_whine. So it is a separate issue whether or not
> wait_or_whine should be silent on SIGPIPE (we already are on SIGINT and
> SIGQUIT, as of some recent patches).
>
> The upside is that it is noise in this case that we would no longer see.
> The downside is that we may be losing a clue when debugging server
> problems, which do not expect to die from SIGPIPE.  Should it be an
> optional run-command flag?

Do we know if we are upstream of a pager that reads from us through
a pipe (I think we should, especially in a case where we are the one
who processed the "git -p $alias" option)?  Is there any other case
where we would want to ignore child's death by SIGPIPE?  If the
answers are yes and no, then perhaps we can ask pager_in_use() to
decide this?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] git-fast-import(1): combine documentation of --[no-]relative-marks

2013-01-09 Thread Jonathan Nieder
John Keeping wrote:

> 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.

Yep, this is easier to read.  Thanks.

Reviewed-by: Jonathan Nieder 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: On --depth=funny value

2013-01-09 Thread Stefan Beller
On 01/09/2013 03:53 AM, Junio C Hamano wrote:
> Can people sanity check the reasoning outlined here?  Anything I
> missed?
> 
> The above outline identifies three concrete tasks that different
> people can tackle more or less independently, each with updated
> code, documentation and test:
> 
>  1. "git fetch --unshallow" that gives a pretty surface on Duy's
> "--depth=inf";
> 
>  2. Making "git fetch" and "git clone" die on "--depth=0" or
> "--depth=-4";
> 
>  3 Updating "upload-pack" to count correctly.
> 
> I'll refrain from saying "Any takers?" for now.

Sorry for answering with delay, I am just contributing to git in my
spare time.
So if I understood Duy correctly, he is going to solve 1. and 3 by his
patches.
I'll try to come up with a solution for 2. within the next days.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: GIT get corrupted on lustre

2013-01-09 Thread Eric Chamberland

Hi Brian,

On 01/08/2013 11:11 AM, Eric Chamberland wrote:

On 12/24/2012 10:11 AM, Brian J. Murrell wrote:

Have you tried adding a "-q" to the git command line to quiet down git's
"feedback" messages?





I moved to git 1.8.1 and added the "-q" to the command "git gc" but it 
occured to return an error, so the "-q" option is not avoiding the 
problem here... :-/


command in crontab:

cd /rap/jsf-051-aa/ericc/tests_git_clones/GIREF && for i in seq 10; do 
/software/apps/git/1.8.1/bin/git gc -q || true;done


results:
error: index file 
.git/objects/pack/pack-1f09879c88cd71a15dcc891713cf038d249830ad.idx is 
too small
error: refs/remotes/origin/BIB_Branche_1_4_x does not point to a valid 
object!


and this clone was a "clean" clone in which only "git qc -q" has been 
run on


I still have a doubt on threads

Eric

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Jan 2013, #04; Wed, 9)

2013-01-09 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

So far, about 60 topics, most of which have been cooking since the
previous cycle, have been graduated to the 'master' branch in
preparation for the next release, which tentatively is called 1.8.2.
Many of these early topics are bugfixes and expected to later land
in the 'maint' branch for 1.8.1.1 release as well.

As usual, this cycle is expected to last for 8 to 10 weeks, with a
preview -rc0 sometime in the middle of next month.

You can find the changes described here in the integration branches of the
repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* nz/send-email-headers-are-case-insensitive (2013-01-06) 1 commit
 - git-send-email: treat field names as case-insensitively

 When user spells "cc:" in lowercase in the fake "header" in the
 trailer part, send-email failed to pick up the addresses from
 there. As e-mail headers field names are case insensitive, this
 script should follow suit and treat "cc:" and "Cc:" the same way.

 Will merge to 'next'.


* mk/complete-tcsh (2013-01-07) 1 commit
 - Prevent space after directories in tcsh completion

 Update tcsh command line completion so that an unwanted space is
 not added to a single directory name.

 Will merge to 'next'.


* dg/subtree-fixes (2013-01-08) 7 commits
 - contrib/subtree: mkdir the manual directory if needed
 - contrib/subtree: honor $(DESTDIR)
 - contrib/subtree: fix synopsis and command help
 - contrib/subtree: better error handling for "add"
 - contrib/subtree: add --unannotate option
 - contrib/subtree: use %B for split Subject/Body
 - t7900: remove test number comments

 contrib/subtree updates.

 Will merge to 'next'.


* ap/log-mailmap (2013-01-08) 11 commits
 - log --use-mailmap: optimize for cases without --author/--committer search
 - log: add log.mailmap configuration option
 - log: grep author/committer using mailmap
 - test: add test for --use-mailmap option
 - log: add --use-mailmap option
 - pretty: use mailmap to display username and email
 - mailmap: add mailmap structure to rev_info and pp
 - mailmap: simplify map_user() interface
 - mailmap: remove email copy and length limitation
 - Use split_ident_line to parse author and committer
 - string-list: allow case-insensitive string list

 Teach commands in the "log" family to optionally pay attention to
 the mailmap.

 Will merge to 'next'.


* nd/upload-pack-shallow-must-be-commit (2013-01-08) 1 commit
 - upload-pack: only accept commits from "shallow" line

 A minor consistency check patch that does not have much relevance
 to the real world.

 Will merge to 'next'.

--
[Graduated to "master"]

* ap/merge-stop-at-prepare-commit-msg-failure (2013-01-03) 1 commit
  (merged to 'next' on 2013-01-07 at 6790566)
 + merge: Honor prepare-commit-msg return code

 Originally merged to 'next' on 2013-01-04

 "git merge" started calling prepare-commit-msg hook like "git
 commit" does some time ago, but forgot to pay attention to the exit
 status of the hook.  t7505 may want a general clean-up but that is
 a different topic.


* as/test-name-alias-uniquely (2012-12-28) 1 commit
  (merged to 'next' on 2013-01-07 at 3b11c25)
 + Use longer alias names in subdirectory tests

 Originally merged to 'next' on 2013-01-02

 A few short-and-bland aliases used in the tests were interfering
 with git-custom command in user's $PATH.


* cc/no-gitk-build-dependency (2012-12-18) 3 commits
 + Makefile: replace "echo 1>..." with "echo >..."
 + Makefile: detect when PYTHON_PATH changes
 + Makefile: remove tracking of TCLTK_PATH

 Remove leftover bits from an earlier change to move gitk in its own
 subdirectory.  Reimplementing the dependency tracking rules needs
 to be done in gitk history separately.


* er/python-version-requirements (2012-12-28) 1 commit
  (merged to 'next' on 2013-01-07 at 4954e27)
 + Add checks to Python scripts for version dependencies.

 Originally merged to 'next' on 2013-01-02

 Some python scripts we ship cannot be run with old versions of the
 interpreter.


* er/stop-recommending-parsecvs (2012-12-28) 1 commit
  (merged to 'next' on 2013-01-07 at 689f28f)
 + Remove the suggestion to use parsecvs, which is currently broken.

 Originally merged to 'next' on 2013-01-02

 Stop recommending a defunct third-party software.


* fc/remote-bzr (2013-01-02) 9 commits
  (merged to 'next' on 2013-01-07 at f8c0b76)
 + remote-bzr: detect local repositories
 + remote-bzr: add support for older versions of bzr
 + remote-bzr: add support to push special modes
 + remote-bzr: add support for fecthing special modes
 + remote-bzr: add simple tests
 + remote-bzr: update working tree upon pushing
 + remote-bzr: add support for remote repositories
 + remote-bzr: add support for pushing
 + Ad

Re: git-completion.tcsh and git-completion.zsh are broken?

2013-01-09 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 09/01/2013 21:21, Marc Khouzam ha scritto:
> [...]
> 
> $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)
> 

The attached patch seems to fix it.
I'm still getting segmentation faults, but only when I try to complete
git rm contrib/ (in the git repository).

Sorry if this is a plain patch.
The code is simply copied from the one found in git-completion.bash.


I also noted that zsh on my system have preinstalled git completion
support (enabled with autoload).
The code is not the one available in the git source tree.
I don't know if the code is from Debian or zsh.

> 
> $tcsh
> synapsis:~/projects/git/contrib/git> source ~/.git-completion.tcsh
> synapsis:~/projects/git/contrib/git> git show HEAD:
> 
> does not show the file list for the tree object in the HEAD
> 
>> Hm.  That doesn't work for me either.  I'll look into it.
>> It is not caused by your changes.
> 
> another problem is that a space is added after a directory name.
> 
>> The lastest version of git-completion.tcsh in the pu branch should
>> fix that problem.  It was committed yesterday so you may not have it.
> 

Ok, thanks.



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

iEYEARECAAYFAlDt2ZoACgkQscQJ24LbaUR/ggCfYNbRrM1HzHWYDwkejNP/hD9k
ShkAnjv3JapVXPlj59CakY4kwaE/4z5J
=qYP5
-END PGP SIGNATURE-
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 4577502..4aeda2a 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -75,4 +75,5 @@ _git ()
 	return _ret
 }
 
-_git
+autoload -U +X compinit && compinit
+compdef _git git gitk


Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

2013-01-09 Thread Jeff King
On Wed, Jan 09, 2013 at 12:48:20PM -0800, Junio C Hamano wrote:

> >   $ git lg -p
> >   [user hits 'q' to exit pager]
> >   error: git lgbase --more-options died of signal 13
> >   fatal: While expanding alias 'lg': 'git lgbase --more-options': Success
> >
> > Many users won't see this, because we execute the external
> > command with the shell, and a POSIX shell will silently
> > rewrite the signal-death exit code into 128+signal, and we
> > will treat it like a normal exit code. However, this does
> > not always happen:
> 
> So... with the "flip the sign of the exit code when caught a signal"
> patch applied to 'next', do people still see this issue?

They see half. The patch you've applied clears up the "While
expanding...: Success" message.

But we still say "error: ... died of signal 13", because that comes from
inside wait_or_whine. So it is a separate issue whether or not
wait_or_whine should be silent on SIGPIPE (we already are on SIGINT and
SIGQUIT, as of some recent patches).

The upside is that it is noise in this case that we would no longer see.
The downside is that we may be losing a clue when debugging server
problems, which do not expect to die from SIGPIPE.  Should it be an
optional run-command flag?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases

2013-01-09 Thread Junio C Hamano
Jeff King  writes:

> When git executes an alias that specifies an external
> command, it will complain if the alias dies due to a signal.
> This is usually a good thing, as signal deaths are
> unexpected. However, SIGPIPE is not unexpected for many
> commands which produce a lot of output; it is intended that
> the user closing the pager would kill them them via SIGPIPE.
>
> As a result, the user might see annoying messages in a
> scenario like this:
>
>   $ cat ~/.gitconfig
>   [alias]
>   lgbase = log --some-options
>   lg = !git lgbase --more-options
>   lg2 = !git lgbase --other-options
>
>   $ git lg -p
>   [user hits 'q' to exit pager]
>   error: git lgbase --more-options died of signal 13
>   fatal: While expanding alias 'lg': 'git lgbase --more-options': Success
>
> Many users won't see this, because we execute the external
> command with the shell, and a POSIX shell will silently
> rewrite the signal-death exit code into 128+signal, and we
> will treat it like a normal exit code. However, this does
> not always happen:

So... with the "flip the sign of the exit code when caught a signal"
patch applied to 'next', do people still see this issue?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  writes:

> Thanks to b65982b (Optimize "diff-index --cached" using cache-tree,
> 2009-05-20), resetting with paths is much faster than resetting
> without paths. Some timings for the linux-2.6 repo to illustrate this
> (best of five, warm cache):
>
> reset   reset .
> real0m0.219s0m0.080s
> user0m0.140s0m0.040s
> sys 0m0.070s0m0.030s

Nice.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  writes:

> Resetting with paths does not update HEAD and there is nothing else
> that a commit should be needed for. Relax the argument parsing so only
> a tree is required.
>
> The sha1 is only passed to read_from_tree(), which already only
> requires a tree.
>
> The "rev" variable we pass to run_add_interactive() will resolve to a
> tree. This is fine since interactive_reset only needs the parameter to
> be a treeish and doesn't use it for display purposes.
> ---
> Is it correct that interactive_reset does not use the revision
> specifier for display purposes? Or, worse, that it requires it to be a
> commit in some cases? I tried it and didn't see any problem.

As far as I know, it is only given to git-diff-index as the tree-ish,
and resulting patch text is used for application via git-apply just
like any patch coming from any origin, so I think it should be fine.

> Can the two blocks of code that look up commit or tree be made to
> share more? I'm not very familiar with what functions are available. I
> think I tried keeping a separate "struct object *object" to be able to
> put the last three lines outside the blocks, but didn't like the
> result.

I think the patch looks fine from the sharing perspective, but it
may be even nicer to have a separate variable to hold a commit
object limited to the scope of if (!pathspec) block to make them
more symmetric.  The commit is only needed later to show "we are now
at this commit", but that code can find the commit itself given the
object name in sha1[].

>  builtin/reset.c  | 46 ++
>  t/t7102-reset.sh |  8 
>  2 files changed, 34 insertions(+), 20 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index a2e69eb..4c223bd 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, 
> const char *prefix, const c
>   /*
>* Possible arguments are:
>*
> -  * git reset [-opts]  ...
> -  * git reset [-opts]  -- ...
> -  * git reset [-opts] -- ...
> +  * git reset [-opts] []
> +  * git reset [-opts]  [...]
> +  * git reset [-opts]  -- [...]
> +  * git reset [-opts] -- [...]
>* git reset [-opts] ...
>*
>* At this point, argv points immediately after [-opts].
> @@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv, 
> const char *prefix, const c
>   }
>   /*
>* Otherwise, argv[0] could be either  or  and
> -  * has to be unambiguous.
> +  * has to be unambiguous. If there is a single argument, it
> +  * can not be a tree
>*/
> - else if (!get_sha1_committish(argv[0], unused)) {
> + else if ((argc == 1 && !get_sha1_committish(argv[0], unused)) ||
> +  (argc > 1 && !get_sha1_treeish(argv[0], unused))) {
>   /*
> -  * Ok, argv[0] looks like a rev; it should not
> +  * Ok, argv[0] looks like a commit/tree; it should not
>* be a filename.
>*/
>   verify_non_filename(prefix, argv[0]);
> @@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char 
> *prefix)
>   const char *rev;
>   unsigned char sha1[20];
>   const char **pathspec = NULL;
> - struct commit *commit;
> + struct commit *commit = NULL;
>   const struct option options[] = {
>   OPT__QUIET(&quiet, N_("be quiet, only report errors")),
>   OPT_SET_INT(0, "mixed", &reset_type,
> @@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char 
> *prefix)
>   PARSE_OPT_KEEP_DASHDASH);
>   pathspec = parse_args(argc, argv, prefix, &rev);
>  
> - if (get_sha1_committish(rev, sha1))
> - die(_("Failed to resolve '%s' as a valid ref."), rev);
> -
> - /*
> -  * NOTE: As "git reset $treeish -- $path" should be usable on
> -  * any tree-ish, this is not strictly correct. We are not
> -  * moving the HEAD to any commit; we are merely resetting the
> -  * entries in the index to that of a treeish.
> -  */
> - commit = lookup_commit_reference(sha1);
> - if (!commit)
> - die(_("Could not parse object '%s'."), rev);
> - hashcpy(sha1, commit->object.sha1);
> + if (!pathspec) {
> + if (get_sha1_committish(rev, sha1))
> + die(_("Failed to resolve '%s' as a valid revision."), 
> rev);
> + commit = lookup_commit_reference(sha1);
> + if (!commit)
> + die(_("Could not parse object '%s'."), rev);
> + hashcpy(sha1, commit->object.sha1);
> + } else {
> + struct tree *tree;
> + if (get_sha1_treeish(rev, sha1))
> + di

RE: git-completion.tcsh and git-completion.zsh are broken?

2013-01-09 Thread Marc Khouzam

> -Original Message-
> From: git-ow...@vger.kernel.org 
> [mailto:git-ow...@vger.kernel.org] On Behalf Of Manlio Perillo
> Sent: Wednesday, January 09, 2013 2:17 PM
> To: git@vger.kernel.org
> Subject: git-completion.tcsh and git-completion.zsh are broken?
> 
> -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:
> 
> does not show the file list for the tree object in the HEAD

Hm.  That doesn't work for me either.  I'll look into it.
It is not caused by your changes.

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

The lastest version of git-completion.tcsh in the pu branch should
fix that problem.  It was committed yesterday so you may not have it.

> 
> 
> Another problem with zsh:
> 
> $zsh
> synapsis% git show HEAD: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-
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-shortlog(1): document behaviour of zero-width wrap

2013-01-09 Thread John Keeping
Commit 00d3947 (Teach --wrap to only indent without wrapping) added
special behaviour for a width of zero in the '-w' argument to
'git-shortlog' but this was not documented.  Fix this.

Signed-off-by: John Keeping 
---
 Documentation/git-shortlog.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index afeb4cd..c308e91 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -56,6 +56,9 @@ OPTIONS
line of each entry is indented by `indent1` spaces, and the second
and subsequent lines are indented by `indent2` spaces. `width`,
`indent1`, and `indent2` default to 76, 6 and 9 respectively.
++
+If width is `0` (zero) then indent the lines of the output without wrapping
+them.
 
 
 MAPPING AUTHORS
-- 
1.8.0.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  writes:

> There is a test case in t7102 called '--mixed refreshes the index',
> but it only checks that right output it printed.

I think that comes from 620a6cd (builtin-reset: avoid forking
"update-index --refresh", 2007-11-03).  Before that commit, we
refreshed the index with --mixed, and the test tries to make sure we
continue to do so after the change.  Even though it is not testing
if the index has stat only changes (which is rather cumbersome to
write---you need to futz with timestamp or something) and using the
output from refresh machinery as a substitute, I think the intent of
that commit is fairly clear.

>  builtin/reset.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 9bcad29..a2e69eb 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
>   printf("\n");
>  }
>  
> -static void update_index_refresh(int flags)
> -{
> - refresh_index(&the_index, (flags), NULL, NULL,
> -   _("Unstaged changes after reset:"));
> -}
> -
>  static void update_index_from_diff(struct diff_queue_struct *q,
>   struct diff_options *opt, void *data)
>  {
> @@ -328,9 +322,9 @@ int cmd_reset(int argc, const char **argv, const char 
> *prefix)
>   die(_("Could not reset index file to revision 
> '%s'."), rev);
>   }
>  
> - if (reset_type == MIXED) /* Report what has not been updated. */
> - update_index_refresh(
> - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
> + if (reset_type == MIXED && !quiet) /* Report what has not been 
> updated. */
> + refresh_index(&the_index, REFRESH_IN_PORCELAIN, NULL, 
> NULL,
> +   _("Unstaged changes after reset:"));
>  
>   if (write_cache(newfd, active_cache, active_nr) ||
>   commit_locked_index(lock))
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 10/19] reset --keep: only write index file once

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] git-fast-import(1): reorganise options

2013-01-09 Thread John Keeping
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 
Signed-off-by: John Keeping 

---
 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=::
-   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=::
-   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=::
-   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=::
-   Maximum delta depth, for blob and tree deltification.
-   Default is 10.
+Options for Frontends
+~
 
---active-branches=::
-   Maximum number of branches to maintain active at once.
-   See ``Memory Utilization'' below for details.  Default is 5.
+--cat-blob-fd=::
+   Write responses to `cat-blob` and `ls` queries to the
+   file descriptor  instead of `stdout`.  Allows `progress`
+   output intended for the end-user to be separated from other
+   output.
+
+--date-format=::
+   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=::
Dumps the internal marks table to  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=::
-   Write responses to `cat-blob` and `ls` queries to the
-   file descriptor  instead of `stdout`.  Allows `progress`
-   output intended for the end-user to be separated from other
-   output.
+--active-branches=::
+   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=::
+   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=::
+   Maximum delta depth, for blob and tree deltification.
+   Default is 10.
 
 --export-pack-edges=::
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

git-archive fails against smart-http repos

2013-01-09 Thread Bruce Lysik
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/19] reset.c: replace switch by if-else

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] Reorganise options in git-fast-import(1)

2013-01-09 Thread John Keeping
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] git-fast-import(1): combine documentation of --[no-]relative-marks

2013-01-09 Thread John Keeping
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 
---
 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=::
Write responses to `cat-blob` and `ls` queries to the
-- 
1.8.1.468.g3d9f9b6

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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);
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] remote-hg: store converted URL

2013-01-09 Thread Max Horn
From: Felipe Contreras 

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 
Signed-off-by: Felipe Contreras 
Signed-off-by: Max Horn 
---
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/19] reset.c: remove unnecessary variable 'i'

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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] -- ...
>* git reset [-opts] ...
>*
> -  * 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  or  and
> +  * Otherwise, argv[0] could be either  or  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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Martin von Zweigbergk
On Wed, Jan 9, 2013 at 11:12 AM, Junio C Hamano  wrote:
> Martin von Zweigbergk  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.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2] commit: make default of "cleanup" option configurable

2013-01-09 Thread Ralf Thielow
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 
---

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 000..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: [] []
 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

Re: [PATCH 04/19] reset: don't allow "git reset -- $pathspec" in bare repo

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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. */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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 -- ' 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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git-completion.tcsh and git-completion.zsh are broken?

2013-01-09 Thread Manlio Perillo
-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:

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: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-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t7400 broken on pu (Mac OS X)

2013-01-09 Thread Junio C Hamano
Torsten Bögershausen  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Martin von Zweigbergk
On Wed, Jan 9, 2013 at 9:01 AM, Jeff King  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


t7400 broken on pu (Mac OS X)

2013-01-09 Thread 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/^16 \([^ ]*\).*/\1/p")


HEAD is now at 57f2622 super commit 1
1
[second 1b8c63f] commit subproject
 Author: A U Thor 
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git branch case insensitivity (Possible bug)

2013-01-09 Thread Johannes Sixt
Am 09.01.2013 18:03, schrieb Alexander Gallego:
> On Wed, Jan 9, 2013 at 10:52 AM, Andreas Ericsson  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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation

2013-01-09 Thread Junio C Hamano
Junio C Hamano  writes:

> Antoine Pelisse  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.
 */
}

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git branch case insensitivity (Possible bug)

2013-01-09 Thread Joshua Jensen

- 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation

2013-01-09 Thread Junio C Hamano
Antoine Pelisse  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-09 Thread Junio C Hamano
Jeff King  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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation

2013-01-09 Thread Antoine Pelisse
> +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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Enabling scissors by default?

2013-01-09 Thread Jeff King
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.

> > 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).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git branch case insensitivity (Possible bug)

2013-01-09 Thread Alexander Gallego
On Wed, Jan 9, 2013 at 10:52 AM, Andreas Ericsson  wrote:
> On 01/09/2013 04:46 PM, Alexander Gallego wrote:
>> Hello,
>>
>> Here is a pastebin where I've reproduced the steps on a clean git repo.
>>
>> http://pastebin.com/0vQZEat0
>>
>>
>>
>> Brief description of the problem:
>>
>>
>>
>> 1.Basically one creates a local branch call it 'imp_fix' (branch off
>> master --> this doesn't matter)
>> 2.One does work, commit, etc
>> 3.One rebases imp_fix with master via: (inside imp_fix) git rebase master
>> 4.One checks out master via: git checkout master
>> 5.One merges an incorrect name "imp_Fix" (notice the capital F)
>> 6.The expected output is that git would say, silly you --> that branch
>> does not exist.
>> 7. Instead it merges (what I think is incorrectly) imp_fix.
>>
>>
>> Kindly let me know if I can provide more details.
>>
>
> Are you using Mac OSX?

Yes

> Are you using the HFS+ filesystem shipped with it?

Likely. I have not touched the fs settings.

> Did you use the filesystem's default settings rather than reinstall your
> system with sensible settings?

Yup. I use whatever was shipped with it.

> 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.


> --
> Andreas Ericsson   andreas.erics...@op5.se
> OP5 AB www.op5.se
> Tel: +46 8-230225  Fax: +46 8-230231
>
> Considering the successes of the wars on alcohol, poverty, drugs and
> terror, I think we should give some serious thought to declaring war
> on peace.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Jeff King
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?

We have never been very clear about which commands refresh the index.
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". I would not be too surprised if some of our own scripts are
not that careful, though.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Makefile: track TCLTK_PATH as it used to be tracked

2013-01-09 Thread Junio C Hamano
From: Christian Couder 

gitk, when bound into the git.git project tree, used to live at the
root level, but in 62ba514 (Move gitk to its own subdirectory,
2007-11-17) it was moved to a subdirectory.  The code used to track
changes to TCLTK_PATH (which should cause gitk to be rebuilt to
point at the new interpreter) was left in the main Makefile instead
of being moved to the new Makefile that was created for the gitk
project.

Also add .gitignore file to list build artifacts for the gitk
project.

Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---

 Paul, this is relative to the tip of your tree, 386befb (gitk:
 Display important heads even when there are many, 2013-01-02).
 Could you consider applying it?

 Also I notice that you have many patches I still do not have
 there, and I'd appreciate a "Go ahead and pull from me!".

 Thanks.

 .gitignore |  2 ++
 Makefile   | 16 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)
 create mode 100644 .gitignore

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..d7ebcaf
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+/GIT-TCLTK-VARS
+/gitk-wish
diff --git a/Makefile b/Makefile
index e1b6045..5acdc90 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,16 @@ DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 bindir_SQ = $(subst ','\'',$(bindir))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
+### Detect Tck/Tk interpreter path changes
+TRACK_TCLTK = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
+
+GIT-TCLTK-VARS: FORCE
+   @VARS='$(TRACK_TCLTK)'; \
+   if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+   echo 1>&2 "* new Tcl/Tk interpreter location"; \
+   echo "$$VARS" >$@; \
+   fi
+
 ## po-file creation rules
 XGETTEXT   ?= xgettext
 ifdef NO_MSGFMT
@@ -49,9 +59,9 @@ uninstall::
$(RM) '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
 
 clean::
-   $(RM) gitk-wish po/*.msg
+   $(RM) gitk-wish po/*.msg GIT-TCLTK-VARS
 
-gitk-wish: gitk
+gitk-wish: gitk GIT-TCLTK-VARS
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) 
"$$0"|' $@+ && \
chmod +x $@+ && \
@@ -65,3 +75,5 @@ $(ALL_MSGFILES): %.msg : %.po
@echo Generating catalog $@
$(MSGFMT) --statistics --tcl $< -l $(basename $(notdir $<)) -d $(dir $@)
 
+.PHONY: all install uninstall clean update-po
+.PHONY: FORCE
-- 
1.8.1.336.g866ceff

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Please pull l10n updates

2013-01-09 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git interaction between push and auto-gc

2013-01-09 Thread Bob Lavey

Greetings,

We are seeing some unexpected behavior with git that we'd like to better 
understand.  We are running git 1.7.11.1 on the remote repo server 
(CentOS 6.3-x86) and the clients (mostly Windows 7), and we are running 
gitolite 3.04 on the server.


At times, our repos can get many unreachable loose objects, and if we 
don't clean them up, we will see instances where a push to the remote 
repo does not work correctly.  This almost always occurs when we try to 
delete a remote branch with a "git push origin :branch" call, but this 
week we saw two occurrences where a push failed to create a branch (as 
well as about a dozen cases where a push failed to delete the branch). 
This behavior does not occur for every push, however: I'd guess 10% of 
pushes fail.


Here is output from a user's machine:

d:\git\jedi\jedifw>git push origin 
configMgrTestBuilder:refs/23s/iq/qbar/user.n...@hp.com/jit20073

Counting objects: 59, done.
Delta compression using up to 16 threads.
Compressing objects: 100% (29/29), done.
Writing objects: 100% (30/30), 129.69 KiB, done.
Total 30 (delta 24), reused 1 (delta 0)
Auto packing the repository for optimum performance.
warning: There are too many unreachable loose objects; run 'git prune' 
to remove them.

To g...@git.boi.hp.com:/jedi/jedifw.git


After a couple of hours, that user was concerned that their branch 
hadn't started processing, so they sent an email to support and the 
re-pushed.  Here are the lines from the gitolite log showing both of 
those pushes:


gitolite-2013-01-07.log:2013-01-07.15:57:58 23285   update 
jedi/jedifw user.n...@hp.comW 
refs/23s/iq/qbar/user.n...@hp.com/jit20073 
 
2ab0e6626c7a51799179993ea0fdbb829a1ea852


gitolite-2013-01-07.log:2013-01-07.19:57:16 30374   update 
jedi/jedifw user.n...@hp.comW 
refs/23s/iq/qbar/user.n...@hp.com/jit20073 
 
2ab0e6626c7a51799179993ea0fdbb829a1ea852



There are no log entries for that branch or that SHA in between those 
two lines.  My understanding is that both of those lines show a new 
branch being created on the remote repo.  I'm not sure what happened to 
the first push, though: I expected that the branch would have been 
created at that time, and subsequent fetches from the remote repo would 
show the branch.  However, fetches from the remote repo did not show the 
branch until after the second push.


We've been running git for about 3 years, and this happened occasionally 
when we first started with git, but we always found it was related to a 
huge number of unreachable loose objects, which triggered an auto-gc on 
the remote repo.  When we performed manual gc --aggressive and prune 
operations on the remote repo, the problem stopped.  It happened this 
week because we'd deleted over 4,000 refs/topics branches on 
Friday/Saturday, which left a huge number of unreachable loose objects. 
 Our cleanup script was only pruning objects older than 2.weeks.ago, 
and when I pruned objects older than 2.days.ago the problem again stopped.


Is this expected behavior for the interaction between pushes and auto-gc 
on the remote repo?


Also, I went through the release notes for the latest versions of git, 
and I found this for 1.7.11.6:


* When "git push" triggered the automatic gc on the receiving end, a
   message from "git prune" that said it was removing cruft leaked to
   the standard output, breaking the communication protocol.

Could that be related to what we're seeing?

Thanks,
Bob Lavey


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: make default of "cleanup" option configurable

2013-01-09 Thread Junio C Hamano
Ralf Thielow  writes:

> It's actually my own usecase :). The bugtracker I'm using is able
> to create relationships between issues and related commits. It
> expects that a part of the commit message contains the issue number
> in format "#". So I need to use a cleanup mode different
> from "default" to keep the commentary.

This is orthogonal to the patch in question, but in your particular
case, I wonder if it is a workable solution to indent # in
your message, which won't be stripped away.

I also wonder, as a longer term alternative (which would require a
lot of code auditing and some refactoring), if it is useful to have
an option and/or configuration that lets you configure the "comment
in log message editor" character from the default "#" to something
else.  "git -c log.commentchar=% commit" may start the log message
editor with something like this in it:

% Please enter the commit message for your changes. Lines starting
% with '%' will be ignored, and an empty message aborts the commit.

Naturally, setting log.commentchar to "none" would disable stripping
of any commented lines if such a feature existed, and stop issuing
these helpful comments altogether, but still strip excess blank
lines and trailing whitespaces.

I wouldn't seriously suggest it as I am not sure if the usefulness
of such a feature would outweigh the cost of coding it, though; at
least not at this point yet.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git branch case insensitivity (Possible bug)

2013-01-09 Thread Andreas Ericsson
On 01/09/2013 04:46 PM, Alexander Gallego wrote:
> Hello,
> 
> Here is a pastebin where I've reproduced the steps on a clean git repo.
> 
> http://pastebin.com/0vQZEat0
> 
> 
> 
> Brief description of the problem:
> 
> 
> 
> 1.Basically one creates a local branch call it 'imp_fix' (branch off
> master --> this doesn't matter)
> 2.One does work, commit, etc
> 3.One rebases imp_fix with master via: (inside imp_fix) git rebase master
> 4.One checks out master via: git checkout master
> 5.One merges an incorrect name "imp_Fix" (notice the capital F)
> 6.The expected output is that git would say, silly you --> that branch
> does not exist.
> 7. Instead it merges (what I think is incorrectly) imp_fix.
> 
> 
> Kindly let me know if I can provide more details.
> 

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.

-- 
Andreas Ericsson   andreas.erics...@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: make default of "cleanup" option configurable

2013-01-09 Thread Junio C Hamano
Ralf Thielow  writes:

> When a user uses a script/importer which expects that the "default" option
> is used without setting it explicitly, and then the user changes the default,
> isn't it the users fault if that would break things?

Not necessarily.  There are many people who use scripts written by
others, without knowing how they work.  You could blame that the
script is broken, but not the poor user of that script.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: git-gui / Warning: "No newline at end of file”

2013-01-09 Thread Pat Thoyts
Junio C Hamano  writes:

>Tobias Preuss  writes:
>
>> Hello. I never got a response. Did my email pass the distribution
>> list? Best, Tobias
>
>Pat?
>

I did have a brief look at this but I don't have a solution at the
moment. The "\ No newline at end of file" is emitted by git at we don't
appear to handle it well in the lib/diff.tcl apply_range_or_line
function.

>> -- Forwarded message --
>> From: Tobias Preuss 
>> Date: Tue, Nov 13, 2012 at 9:26 PM
>> Subject: git-gui / Warning: "No newline at end of file”
>> To: git 
>>
>>
>> Hello.
>> I noticed a problem when working with git-gui which might be a bug.
>> The issue only affects you when you are visually trying to stage
>> changes line by line. Here are the steps to reproduce the problem:
>>
>> 1. Initialize a new repository.
>> 2. Create a file with three lines of content each with the word
>> "Hello". Do not put a new line at the end of the file.
>> 3. Add and commit the file.
>> 4. Edit the same file putting words inbetween the three lines.
>> 5. Open git-gui and try to stage the changes line by line.
>>
>> The editor will append the warning "No newline at end of file” to the
>> end of the diff. When you are trying to stage a line an error occurs.
>> The problem is also illustrated in a question on Stackoverflow [1].
>>
>> Please let me know if you need more information or if I should send
>> this problem to some other mailing list.
>> Thank you, Tobias
>>
>> 
>> [1] 
>> http://stackoverflow.com/questions/13223868/how-to-stage-line-by-line-in-git-gui-although-no-newline-at-end-of-file-warnin
>

-- 
Pat Thoytshttp://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: On --depth=funny value

2013-01-09 Thread Duy Nguyen
On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano  wrote:
>  * Make "git fetch" and "git clone" die() when zero or negative
>number is given with --depth=$N, for the following reasons:
>...

For Stefan when you update the patch. If "git fetch --depth=0" is
considered invalid too as Junio outlined, then the proper place for
the check is transport.c:set_git_option(), not clone.c. It already
catches --depth=random-string. Adding "depth < 1" check should be
trivial. You may want to update builtin/fetch-pack.c too because it
does not share this code.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: On --depth=funny value

2013-01-09 Thread Duy Nguyen
On Wed, Jan 9, 2013 at 12:19 PM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano  wrote:
>> ...
>>>  * We would like to update "clone --depth=1" to end up with a tip
>>>only repository, but let's not to touch "git fetch" (and "git
>>>clone") and make them send 0 over the wire when the command line
>>>tells them to use "--depth=1" (i.e. let's not do the "off-by-one"
>>>thing).
>>
>> You can't anyway. Depth 0 on the wire is considered invalid by upload-pack.
>
> Yes, that is a good point that we say "if (0 < opt->depth) do the
> shallow thing" everywhere, so 0 is spcial in that sense.
>
> Which suggests that if we wanted to, we could update the fetch side
> to do the off-by-one thing against the current upload-pack when the
> given depth is two or more, and still send 1 when depth=1.  When
> talking with an updated upload-pack that advertises exact-shallow
> protocol extension, it can disable that off-by-one for all values of
> depth.  That way, the updated client gets history of wrong depth
> only for --depth=1 when talking with the current upload-pack; all
> other cases, it will get history of correct depth.
>
> Hmm?

I haven't checked because frankly I have never run JGit, but are we
sure this off-by-one thing applies to JGit server as well? So far I'm
only aware of three sever implementations: C Git, JGit and Dulwich.
The last one does not support shallow extension so it's out of
question.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Failure and unhelpful error message from 'rebase --preserve-merges'

2013-01-09 Thread Phil Hord
Since 90e1818f9a  (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails in a case where it used to
succeed, and it does so with an unhelpful error message.

   $ git rebase --preserve-merges master
   error: Commit 452524... is a merge but no -m option was given.
   fatal: cherry-pick failed
   Could not pick 452524f925aecd0439ae5728fca3887292114dd7

I also tried rebase with '-m'
   $ git rebase --preserve-merges -m master
but that also failed.

The same commands worked fine for these same commits in v1.7.9

>From 90e1818f9a I figured out that the rebase-interactive
machinery had dropped one of my merges. I normally would not
notice this when using 'git rebase -p' since it does not invoke $EDITOR
by default; but I can see it if I use this:

   git -c sequence.editor=cat rebase -p master

With that I see my list of commits, including these:

  ...
  pick 184ec4d WIP: DHCP datastore reporting
  # pick 16ca56c Merge ptss into sock-threads
  pick 06aea55 WIP: More work normalizing config handlers
  ...
  pick 452524f Merge branch 'ptss' into sock-threads
  ...
  #
  # Note that empty commits are commented out

The failure points to the 2nd merge commit, but it is not the merge
commit which was commented out. It is a later merge between the same two
branches. I'm not sure how this is related, yet.

But I now know I can work around the problem with this:

git rebase --keep-empty -p master

I see three problems here, but I don't have any time to go fix them
myself right now.

 1. 'rebase -p' should default to --keep-empty since the user will not
be given the opportunity to edit the list to uncomment the missing
commits.

 2. 'rebase --interactive -p' should not drop empty merge commits.

 3. rebase should not die with a cryptic cherry-pick error message,
although I am not sure what useful thing it could say in this
particular case. Maybe there are other conditions which will cause
this same failure even if 1 and 2 are fixed.

Phil
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD

2013-01-09 Thread Matt Kraai
In the summary, {ORIG,} should be {ORIG_,}.

-- 
Matt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair

2013-01-09 Thread Matt Kraai
On Wed, Jan 09, 2013 at 12:16:00AM -0800, Martin von Zweigbergk wrote:
> 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
  ^^^
argv is misspelled.

-- 
Matt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Deutsch-Händler Online-Thomas Sabo

2013-01-09 Thread flexma
Thomas Sabo Charms    spielt eine wichtige
Rolle in der Definition von Stil und brilliant. Kein Mädchen oder eine Frau
auf Erden, die nicht bewegt wird mit dem Stil der Schmuck fasziniert .
Obwohl die Mädchen sind verrückt tragen spannende und verschiedene Methoden
Thomas Sabo Schmuck zu jeder Jahreszeit, eine weitere Sache, und weiterhin
den hohen Preis eines jeden Monats zu machen, um Schmuck zu kaufen zögern.
Modeschmuck direkt nach einigen Monaten eine sehr feine, aber bedauerlich,
ist schwierig für sie eine Vielzahl von Schmuck zu schaffen wie.  Thomas
Sabo Charms Anhänger    Modeschmuck, wie der
Name schon sagt, ist für Sie vorbereitet. Das bedeutet, dass eine gemeinsame
Mittelklasse Dame, mag sie Schmuck Objekte können eine gute Auswahl haben.
Die verschiedenen Arten von Dekorationen, Schmuck Großhändler in der
klassischen auf die neueste Mode-Design, mit dem niedrigen Preis gekoppelt.
Thomas Sabo Anhänger    nimmt Top-Qualität 925
Silber und von Hand geschnitten Inlay Zirkon. Ihre komplexe Handarbeiten
verleihen jeder Abschnitt Thomas Sabo Schmuck eine wirklich erhebliche
optischen Vorteil. Der Artikel verweist auf deinen Geist-Set mit Lebensdauer
und jeden einzelnen gesundheitliche Vorteile mit neuen Ringen assoziiert ist
definitiv interessant und Bolders die ganze Ringe Sektor und Weise Sektor.
Thomas Sabo ebenfalls zur Verringerung der traditionellen Wahrnehmung der
Männer und einzigartige Ringe gewidmet Unterschiede. Unisex Jeans und Ketten
in neuen Serie haben gezeigt, dass es auffällige Mode.
Sie können entscheiden,  Thomas Sabo Schmuck  
, die das Leben besondere Momente darstellen. Sie können die Geschichte
Ihres Lebens spielt sich in einer schönen Anordnung der Reize. Niemand
schafft diese Schmuckstücke in bessere Detailauflösung als Thomas Sabo
Charms. Alle Ketten und Armbänder sind aus Sterling Silber von höchster
Qualität. Abhängig von der Länge der Thomas Sabo Halskette, hat es viele
unterschiedliche Gewichte. können Sie rund dhs80 im Preis beginnen und bis
zu dhs250. Nachdem Sie eine Halskette wählen, gibt es silber Anschlüsse, die
Sie aus wieder auf weight.Due seiner einzigartigen Design und modische Thema
basiert, zog das Design der modernen Jugend Aufmerksamkeit und wurde sofort
beliebt. Eine beliebte Wahl des Designs und Aussehen frische Marke von
Silberschmuck Mode-Fans. Sie entsprechen dem immer beliebter globalen
Promi-Publikum anzupassen.
Thomas Sabo Schmuck Online Shop    Geschenk
wäre perfekt für jeden Anlass. Viele Leute denken, dass das perfekte
Geschenk für ihre Lieben zu kaufen. Auch in der letzten Minute Sie Shop für
Ihren besonderen Anlass, sind Sie sicher, ein paar gute Thomas Sabo
Kollektion. Thomas Sabo Geschenke sind nicht auf Ihrem Geliebten beschränkt,
können Sie auch ein perfektes Geschenk Thomas Sabo Charm Club fast jeder,
einschließlich Ihrer Familie und Freunden. Of Natürlich, werden Sie
innerhalb von Saab und Ihre Familie und Freunde lieben



-
nike air max 90 
nike air max 1 
nike air max classic 
nike air max online bestellen 
nike air max online shop 
--
View this message in context: 
http://git.661346.n2.nabble.com/Deutsch-Handler-Online-Thomas-Sabo-tp7574583.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Proposal for git stash rename

2013-01-09 Thread Michael Haggerty
On 01/04/2013 10:40 PM, Junio C Hamano wrote:
> Micheil Smith  writes:
> 
>>> This patch implements a "git stash rename" using a new
>>> "git reflog update" command that updates the message associated
>>> with a reflog entry.
>> ...
>> I note that this proposal is now two years old. A work in progress patch was 
>> requested, however, after one was given this thread ended. I'm also finding 
>> a need for this feature;
> 
> The whole point of reflog is that it is a mechanism to let users to
> go safely back to the previous state, by using a file that is pretty
> much append-only.  It feels that a mechanism to "rewrite" one goes
> completely against that principle, at least to me.

The implementation of "git stash" itself seems to violate your
principle, by storing its branches-that-are-not-branches within a
mutable reflog.

Just an observation...

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: make default of "cleanup" option configurable

2013-01-09 Thread Jonathan Nieder
Ralf Thielow wrote:

> It's actually my own usecase :). The bugtracker I'm using is able
> to create relationships between issues and related commits. It
> expects that a part of the commit message contains the issue number
> in format "#". So I need to use a cleanup mode different
> from "default" to keep the commentary. The mode I'd use is "whitespace",
> "verbatim" is also possible.

Hm, so "whitespace-when-editing" would be the ideal setting.

Would it be confusing if the '[commit] cleanup' setting only took
effect when launching an editor (and not with -F, -C, or -m)?  My
first impression is that I'd like that behavior better, even though
it's harder to explain.

[...]
> When a user uses a script/importer which expects that the "default" option
> is used without setting it explicitly, and then the user changes the default,
> isn't it the users fault if that would break things?

Consider the following series of events.

 1. My friend writes an importer that uses the "git commit" command.
I like it and start using it.

 2. Another friend writes a blog post about the '[commit] cleanup'
setting.  I like it and start using it.

 3. I try to use the importer again.

 4. Years later, I notice the commit messages are corrupted in the
imported history.

It's hard to assign blame.  I guess it's my fault. ;)

[...]
> I'll add a sentence of my bugtracker example to it. I think many developers
> are using such a tool, so it'd makes sense.

Thanks, sounds good.

Regards,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 12/19] reset.c: move update_index_refresh() call out of read_from_tree()

2013-01-09 Thread Martin von Zweigbergk
The final part of cmd_reset() essentially looks like:

  if (pathspec) {
...
read_from_tree(...);
  } else {
...
reset_index(...);
update_index_refresh(...);
...
  }

where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
---
 builtin/reset.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 54e3c5b..a21ba31 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct 
diff_queue_struct *q,
}
 }
 
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
 {
-   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
-   int index_fd;
struct diff_options opt;
 
memset(&opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned 
char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
 
-   index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned 
char *tree_sha1,
diff_flush(&opt);
diff_tree_release_paths(&opt);
 
-   return update_index_refresh(index_fd, lock, refresh_flags);
+   return 0;
 }
 
 static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -321,9 +317,13 @@ 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);
+   if (pathspec) {
+   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+   int index_fd = hold_locked_index(lock, 1);
+   return read_from_tree(pathspec, sha1) ||
+   update_index_refresh(index_fd, lock,
+   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
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 18/19] reset: allow reset on unborn branch

2013-01-09 Thread Martin von Zweigbergk
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, "git reset" currently fails with:

  fatal: Failed to resolve 'HEAD' as a valid ref.

Instead of making users figure out that they should run

 git rm --cached -r .

, let's teach "git reset" without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in "git reset HEAD".
---
 builtin/reset.c| 17 --
 t/t7106-reset-unborn-branch.sh | 52 ++
 2 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

diff --git a/builtin/reset.c b/builtin/reset.c
index 4c223bd..5cd48ac 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,7 +239,7 @@ static int update_refs(const char *rev, const unsigned char 
*sha1) {
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
int reset_type = NONE, update_ref_status = 0, quiet = 0;
-   int patch_mode = 0;
+   int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -264,8 +264,11 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argc, argv, prefix, &rev);
-
-   if (!pathspec) {
+   unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
+   if (unborn) {
+   /* reset on unborn branch: treat as reset to empty tree */
+   hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+   } else if (!pathspec) {
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid revision."), 
rev);
commit = lookup_commit_reference(sha1);
@@ -285,7 +288,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with 
--{hard,mixed,soft}"));
-   return run_add_interactive(rev, "--patch=reset", pathspec);
+   return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", 
pathspec);
}
 
/* git reset tree [--] paths... can be used to
@@ -337,16 +340,16 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_("Could not write new index file."));
}
 
-   if (!pathspec) {
+   if (!pathspec && !unborn) {
/* 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);
-
-   remove_branch_state();
}
+   if (!pathspec)
+   remove_branch_state();
 
return update_ref_status;
 }
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 000..4ff6af4
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   echo a >a &&
+   echo b >b
+'
+
+test_expect_success 'reset' '
+   git add a b &&
+   git reset &&
+   test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset HEAD' '
+   rm .git/index &&
+   git add a b &&
+   test_must_fail git reset HEAD
+'
+
+test_expect_success 'reset $file' '
+   rm .git/index &&
+   git add a b &&
+   git reset a &&
+   test "$(git ls-files)" == "b"
+'
+
+test_expect_success 'reset -p' '
+   rm .git/index &&
+   git add a &&
+   echo y | git reset -p &&
+   test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset --soft is a no-op' '
+   rm .git/index &&
+   git add a &&
+   git reset --soft
+   test "$(git ls-files)" == "a"
+'
+
+test_expect_success 'reset --hard' '
+   rm .git/index &&
+   git add a &&
+   git reset --hard &&
+   test "$(git ls-files)" == "" &&
+   test_path_is_missing a
+'
+
+test_done
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair

2013-01-09 Thread Martin von Zweigbergk
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.

 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 -- ' 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)
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 11/19] reset: avoid redundant error message

2013-01-09 Thread Martin von Zweigbergk
If writing or committing the new index file fails, we print "Could not
write new index file." followed by "Could not reset index file to
revision $rev.". The first message seems to imply the second, so print
only the first message.
---
 builtin/reset.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 8e5d097..54e3c5b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,13 +337,11 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
err = reset_index(sha1, MIXED, quiet);
-   if (!err &&
-   (write_cache(newfd, active_cache, active_nr) ||
-commit_locked_index(lock))) {
-   err = error(_("Could not write new index file."));
-   }
if (err)
die(_("Could not reset index file to revision '%s'."), 
rev);
+   if (write_cache(newfd, active_cache, active_nr) ||
+   commit_locked_index(lock))
+   die(_("Could not write new index file."));
}
 
/* Any resets update HEAD to the head being switched to,
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/19] reset.c: remove unnecessary variable 'i'

2013-01-09 Thread Martin von Zweigbergk
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.

 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] -- ...
 * git reset [-opts] ...
 *
-* 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  or  and
+* Otherwise, argv[0] could be either  or  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)
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD

2013-01-09 Thread Martin von Zweigbergk
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
---
 builtin/reset.c | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 68be05c..4d556e7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,16 +239,35 @@ const char **parse_args(int argc, const char **argv, 
const char *prefix, const c
return *argv ? get_pathspec(prefix, argv) : NULL;
 }
 
+static int update_refs(const char *rev, const unsigned char *sha1) {
+   int update_ref_status;
+   struct strbuf msg = STRBUF_INIT;
+   unsigned char *orig = NULL, sha1_orig[20],
+   *old_orig = NULL, sha1_old_orig[20];
+
+   if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+   old_orig = sha1_old_orig;
+   if (!get_sha1("HEAD", sha1_orig)) {
+   orig = sha1_orig;
+   set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+   update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+   }
+   else if (old_orig)
+   delete_ref("ORIG_HEAD", old_orig, 0);
+   set_reflog_message(&msg, "updating HEAD", rev);
+   update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, 
MSG_ON_ERR);
+   strbuf_release(&msg);
+   return update_ref_status;
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
-   unsigned char sha1[20], *orig = NULL, sha1_orig[20],
-   *old_orig = NULL, sha1_old_orig[20];
+   unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
-   struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -332,17 +351,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
/* Any resets update HEAD to the head being switched to,
 * saving the previous head in ORIG_HEAD before. */
-   if (!get_sha1("ORIG_HEAD", sha1_old_orig))
-   old_orig = sha1_old_orig;
-   if (!get_sha1("HEAD", sha1_orig)) {
-   orig = sha1_orig;
-   set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
-   update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
-   }
-   else if (old_orig)
-   delete_ref("ORIG_HEAD", old_orig, 0);
-   set_reflog_message(&msg, "updating HEAD", rev);
-   update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, 
MSG_ON_ERR);
+   update_ref_status = update_refs(rev, sha1);
 
switch (reset_type) {
case HARD:
@@ -359,7 +368,5 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
remove_branch_state();
 
-   strbuf_release(&msg);
-
return update_ref_status;
 }
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 16/19] reset [--mixed] --quiet: don't refresh index

2013-01-09 Thread Martin von Zweigbergk
"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.

This commit together with 686b2de (oneway_merge(): only lstat() when
told to update worktree, 2012-12-20) removes all calls to lstat() the
worktree from the command.

This speeds up "git reset -q" a little on the linux-2.6 repo (best
of five, warm cache):

Before  After
real0m0.215s0m0.176s
user0m0.150s0m0.130s
sys 0m0.060s0m0.040s

And with cold cache (best of five):

Before  After
real0m11.351s   0m8.420s
user0m0.230s0m0.220s
sys 0m0.270s0m0.060s
---
There is a test case in t7102 called '--mixed refreshes the index',
but it only checks that right output it printed. Is the test case not
testing right or not named right? As you can see, I suspect it's the
name/description that should change.

 builtin/reset.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 9bcad29..a2e69eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
 }
 
-static void update_index_refresh(int flags)
-{
-   refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:"));
-}
-
 static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
 {
@@ -328,9 +322,9 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_("Could not reset index file to revision 
'%s'."), rev);
}
 
-   if (reset_type == MIXED) /* Report what has not been updated. */
-   update_index_refresh(
-   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+   if (reset_type == MIXED && !quiet) /* Report what has not been 
updated. */
+   refresh_index(&the_index, REFRESH_IN_PORCELAIN, NULL, 
NULL,
+ _("Unstaged changes after reset:"));
 
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/19] reset.c: share call to die_if_unmerged_cache()

2013-01-09 Thread Martin von Zweigbergk
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)
---
 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);
}
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 00/19] reset improvements

2013-01-09 Thread Martin von Zweigbergk
This is kind of a re-roll of [1] (wow, apparently it took me almost
two months to get done). The goal was, then and now, to teach "git
reset" to work on an unborn branch and to not require a commit when a
tree would do. This time, I also made some tangential improvements
along the way, mostly related to readability and performance.

As usual, the risker patches are towards the end. In particular, I
find it hard to evaluate how risky the last patch is. That last patch
is responsible for much of the improvements in the timing table below,
so it would be nice if it doesn't break things too badly (test pass,
of course). The timings are best-of-five, wall time.

Command  Before After
reset (warm) 0.230.07
reset -q (warm)  0.230.03
reset . (warm)   0.090.07
reset -q . (warm)0.090.03
reset --keep (warm)  0.310.29
reset --keep -q (warm)   0.310.29
reset (cold) 9.742.60
reset -q (cold)  9.850.37
reset . (cold)   2.662.51
reset -q . (cold)2.590.33
reset --keep (cold)  7.587.52
reset --keep -q (cold)   7.377.21



  [1] http://thread.gmane.org/gmane.comp.version-control.git/210568/focus=210855

Martin von Zweigbergk (19):
  reset $pathspec: no need to discard index
  reset $pathspec: exit with code 0 if successful
  reset.c: pass pathspec around instead of (prefix, argv) pair
  reset: don't allow "git reset -- $pathspec" in bare repo
  reset.c: extract function for parsing arguments
  reset.c: remove unnecessary variable 'i'
  reset.c: extract function for updating {ORIG,}HEAD
  reset.c: share call to die_if_unmerged_cache()
  reset.c: replace switch by if-else
  reset --keep: only write index file once
  reset: avoid redundant error message
  reset.c: move update_index_refresh() call out of read_from_tree()
  reset.c: move lock, write and commit out of update_index_refresh()
  reset [--mixed]: don't write index file twice
  reset.c: finish entire cmd_reset() whether or not pathspec is given
  reset [--mixed] --quiet: don't refresh index
  reset $sha1 $pathspec: require $sha1 only to be treeish
  reset: allow reset on unborn branch
  reset [--mixed]: use diff-based reset whether or not pathspec was
given

 builtin/reset.c| 281 +++--
 t/t2013-checkout-submodule.sh  |   2 +-
 t/t7102-reset.sh   |  26 +++-
 t/t7106-reset-unborn-branch.sh |  52 
 4 files changed, 200 insertions(+), 161 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish

2013-01-09 Thread Martin von Zweigbergk
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.

The sha1 is only passed to read_from_tree(), which already only
requires a tree.

The "rev" variable we pass to run_add_interactive() will resolve to a
tree. This is fine since interactive_reset only needs the parameter to
be a treeish and doesn't use it for display purposes.
---
Is it correct that interactive_reset does not use the revision
specifier for display purposes? Or, worse, that it requires it to be a
commit in some cases? I tried it and didn't see any problem.

Can the two blocks of code that look up commit or tree be made to
share more? I'm not very familiar with what functions are available. I
think I tried keeping a separate "struct object *object" to be able to
put the last three lines outside the blocks, but didn't like the
result.

 builtin/reset.c  | 46 ++
 t/t7102-reset.sh |  8 
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index a2e69eb..4c223bd 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, const 
char *prefix, const c
/*
 * Possible arguments are:
 *
-* git reset [-opts]  ...
-* git reset [-opts]  -- ...
-* git reset [-opts] -- ...
+* git reset [-opts] []
+* git reset [-opts]  [...]
+* git reset [-opts]  -- [...]
+* git reset [-opts] -- [...]
 * git reset [-opts] ...
 *
 * At this point, argv points immediately after [-opts].
@@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv, 
const char *prefix, const c
}
/*
 * Otherwise, argv[0] could be either  or  and
-* has to be unambiguous.
+* has to be unambiguous. If there is a single argument, it
+* can not be a tree
 */
-   else if (!get_sha1_committish(argv[0], unused)) {
+   else if ((argc == 1 && !get_sha1_committish(argv[0], unused)) ||
+(argc > 1 && !get_sha1_treeish(argv[0], unused))) {
/*
-* Ok, argv[0] looks like a rev; it should not
+* Ok, argv[0] looks like a commit/tree; it should not
 * be a filename.
 */
verify_non_filename(prefix, argv[0]);
@@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
-   struct commit *commit;
+   struct commit *commit = NULL;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argc, argv, prefix, &rev);
 
-   if (get_sha1_committish(rev, sha1))
-   die(_("Failed to resolve '%s' as a valid ref."), rev);
-
-   /*
-* NOTE: As "git reset $treeish -- $path" should be usable on
-* any tree-ish, this is not strictly correct. We are not
-* moving the HEAD to any commit; we are merely resetting the
-* entries in the index to that of a treeish.
-*/
-   commit = lookup_commit_reference(sha1);
-   if (!commit)
-   die(_("Could not parse object '%s'."), rev);
-   hashcpy(sha1, commit->object.sha1);
+   if (!pathspec) {
+   if (get_sha1_committish(rev, sha1))
+   die(_("Failed to resolve '%s' as a valid revision."), 
rev);
+   commit = lookup_commit_reference(sha1);
+   if (!commit)
+   die(_("Could not parse object '%s'."), rev);
+   hashcpy(sha1, commit->object.sha1);
+   } else {
+   struct tree *tree;
+   if (get_sha1_treeish(rev, sha1))
+   die(_("Failed to resolve '%s' as a valid tree."), rev);
+   tree = parse_tree_indirect(sha1);
+   if (!tree)
+   die(_("Could not parse object '%s'."), rev);
+   hashcpy(sha1, tree->object.sha1);
+   }
 
if (patch_mode) {
if (reset_type != NONE)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '
test ! -f secondfile
 '
 
+test_expect_success 'reset with paths accepts tree' '
+   # for simpler tests, drop last commit containing added

[PATCH 09/19] reset.c: replace switch by if-else

2013-01-09 Thread Martin von Zweigbergk
---
 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;
-   }
 
remove_branch_state();
 
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/19] reset $pathspec: no need to discard index

2013-01-09 Thread Martin von Zweigbergk
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.

There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).

This speeds up "git reset -- ." a little on the linux-2.6 repo (best
of five, warm cache):

Before  After
real0m0.093s0m0.080s
user0m0.040s0m0.020s
sys 0m0.050s0m0.050s
---
 builtin/reset.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file 
*index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
 
-   if (read_cache() < 0)
-   return error(_("Could not read index"));
-
result = refresh_index(&the_index, (flags), NULL, NULL,
   _("Unstaged changes after reset:")) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct 
diff_queue_struct *q,
struct diff_options *opt, void *data)
 {
int i;
-   int *discard_flag = data;
-
-   /* do_diff_cache() mangled the index */
-   discard_cache();
-   *discard_flag = 1;
-   read_cache();
 
for (i = 0; i < q->nr; i++) {
struct diff_filespec *one = q->queue[i]->one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char 
**argv,
unsigned char *tree_sha1, int refresh_flags)
 {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
-   int index_fd, index_was_discarded = 0;
+   int index_fd;
struct diff_options opt;
 
memset(&opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
-   opt.format_callback_data = &index_was_discarded;
 
index_fd = hold_locked_index(lock, 1);
-   index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char 
**argv,
diff_flush(&opt);
diff_tree_release_paths(&opt);
 
-   if (!index_was_discarded)
-   /* The index is still clobbered from do_diff_cache() */
-   discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
 }
 
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/19] reset.c: move lock, write and commit out of update_index_refresh()

2013-01-09 Thread Martin von Zweigbergk
In preparation for the/a following patch, move the locking, writing
and committing of the index file out of update_index_refresh(). The
code duplication caused will soon be taken care of. What remains of
update_index_refresh() is just one line, but it is still called from
two places, so let's leave it for now.

In the process, we expose and fix the minor UI bug that makes us print
"Could not refresh index" when we fail to write the index file when
invoked with a pathspec. Copy the error message from the pathspec-less
codepath ("Could not write new index file.").
---
 builtin/reset.c | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index a21ba31..2243b95 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
 }
 
-static int update_index_refresh(int fd, struct lock_file *index_lock, int 
flags)
+static void update_index_refresh(int flags)
 {
-   if (!index_lock) {
-   index_lock = xcalloc(1, sizeof(struct lock_file));
-   fd = hold_locked_index(index_lock, 1);
-   }
-
refresh_index(&the_index, (flags), NULL, NULL,
  _("Unstaged changes after reset:"));
-   if (write_cache(fd, active_cache, active_nr) ||
-   commit_locked_index(index_lock))
-   return error ("Could not refresh index");
-   return 0;
 }
 
 static void update_index_from_diff(struct diff_queue_struct *q,
@@ -320,9 +311,14 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
if (pathspec) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd = hold_locked_index(lock, 1);
-   return read_from_tree(pathspec, sha1) ||
-   update_index_refresh(index_fd, lock,
-   quiet ? REFRESH_QUIET : 
REFRESH_IN_PORCELAIN);
+   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
@@ -350,9 +346,15 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
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);
+   else if (reset_type == MIXED) { /* Report what has not been updated. */
+   struct lock_file *index_lock = xcalloc(1, sizeof(struct 
lock_file));
+   int fd = hold_locked_index(index_lock, 1);
+   update_index_refresh(
+   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+   if (write_cache(fd, active_cache, active_nr) ||
+   commit_locked_index(index_lock))
+   error("Could not refresh index");
+   }
 
remove_branch_state();
 
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/19] reset --keep: only write index file once

2013-01-09 Thread Martin von Zweigbergk
"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. As a bonus, we speed up "git reset --keep" a little on the
linux-2.6 repo (best of five, warm cache):

Before  After
real0m0.315s0m0.296s
user0m0.290s0m0.280s
sys 0m0.020s0m0.010s
---
 builtin/reset.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 05ccfd4..8e5d097 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -38,14 +38,12 @@ static inline int is_merge(void)
return !access(git_path("MERGE_HEAD"), F_OK);
 }
 
-static int reset_index_file(const unsigned char *sha1, int reset_type, int 
quiet)
+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
 {
int nr = 1;
-   int newfd;
struct tree_desc desc[2];
struct tree *tree;
struct unpack_trees_options opts;
-   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 
memset(&opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int 
reset_type, int quiet
opts.reset = 1;
}
 
-   newfd = hold_locked_index(lock, 1);
-
read_cache_unmerged();
 
if (reset_type == KEEP) {
@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int 
reset_type, int quiet
prime_cache_tree(&active_cache_tree, tree);
}
 
-   if (write_cache(newfd, active_cache, active_nr) ||
-   commit_locked_index(lock))
-   return error(_("Could not write new index file."));
-
return 0;
 }
 
@@ -340,9 +332,16 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die_if_unmerged_cache(reset_type);
 
if (reset_type != SOFT) {
-   int err = reset_index_file(sha1, reset_type, quiet);
+   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_file(sha1, MIXED, quiet);
+   err = reset_index(sha1, MIXED, quiet);
+   if (!err &&
+   (write_cache(newfd, active_cache, active_nr) ||
+commit_locked_index(lock))) {
+   err = error(_("Could not write new index file."));
+   }
if (err)
die(_("Could not reset index file to revision '%s'."), 
rev);
}
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given

2013-01-09 Thread Martin von Zweigbergk
Thanks to b65982b (Optimize "diff-index --cached" using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):

reset   reset .
real0m0.219s0m0.080s
user0m0.140s0m0.040s
sys 0m0.070s0m0.030s

These two commands should do the same thing, so instead of having the
user type the trailing " ." to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so "git reset $rev" would also be faster).

Comparing before and after (best of five):

   Before After
reset(warm cache):   0.21  0.07
reset -q (warm cache)0.17  0.03
reset(cold cache):  10.31  2.72
reset -q (cold cache)7.64  0.38
---
Are unmerged entries handled the same? read_from_tree() calls
read_cache(), while reset_index() calls read_cache_unmerged(). I
haven't figured out if/why they should be different.

Are there other differences, or could unpack_trees() learn the same
optimization as do_diff_cache()? Actually, the commit mentioned above
does say

  Tweak unpack_trees() logic that is used to read in the tree object
  to catch the case where the tree entry we are looking at matches the
  index as a whole by looking at the cache-tree.

If there are differences, we are clearly missing tests for them. And
it seems like any difference between them should be fixed, so "git
reset" and "git reset ." (from root of tree) do the same thing even
before this patch.

 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 5cd48ac..6db0a10 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -320,7 +320,7 @@ 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);
-   if (pathspec) {
+   if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given

2013-01-09 Thread Martin von Zweigbergk
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?

 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;
 }
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 05/19] reset.c: extract function for parsing arguments

2013-01-09 Thread Martin von Zweigbergk
Declutter cmd_reset() a bit by moving out the argument parsing to its
own function.
---
 builtin/reset.c | 71 ++---
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 664fad9..9473725 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,36 +198,10 @@ static void die_if_unmerged_cache(int reset_type)
 
 }
 
-int cmd_reset(int argc, const char **argv, const char *prefix)
-{
-   int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
-   int patch_mode = 0;
+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 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[] = {
-   OPT__QUIET(&quiet, N_("be quiet, only report errors")),
-   OPT_SET_INT(0, "mixed", &reset_type,
-   N_("reset HEAD and index"), 
MIXED),
-   OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), 
SOFT),
-   OPT_SET_INT(0, "hard", &reset_type,
-   N_("reset HEAD, index and working tree"), HARD),
-   OPT_SET_INT(0, "merge", &reset_type,
-   N_("reset HEAD, index and working tree"), 
MERGE),
-   OPT_SET_INT(0, "keep", &reset_type,
-   N_("reset HEAD but keep local changes"), KEEP),
-   OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks 
interactively")),
-   OPT_END()
-   };
-
-   git_config(git_default_config, NULL);
-
-   argc = parse_options(argc, argv, prefix, options, git_reset_usage,
-   PARSE_OPT_KEEP_DASHDASH);
-
+   unsigned char unused[20];
/*
 * Possible arguments are:
 *
@@ -250,7 +224,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 * Otherwise, argv[i] could be either  or  and
 * has to be unambiguous.
 */
-   else if (!get_sha1_committish(argv[i], sha1)) {
+   else if (!get_sha1_committish(argv[i], unused)) {
/*
 * Ok, argv[i] looks like a rev; it should not
 * be a filename.
@@ -262,6 +236,40 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
verify_filename(prefix, argv[i], 1);
}
}
+   *rev_ret = rev;
+   return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+   int reset_type = NONE, update_ref_status = 0, quiet = 0;
+   int patch_mode = 0;
+   const char *rev;
+   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[] = {
+   OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+   OPT_SET_INT(0, "mixed", &reset_type,
+   N_("reset HEAD and index"), 
MIXED),
+   OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), 
SOFT),
+   OPT_SET_INT(0, "hard", &reset_type,
+   N_("reset HEAD, index and working tree"), HARD),
+   OPT_SET_INT(0, "merge", &reset_type,
+   N_("reset HEAD, index and working tree"), 
MERGE),
+   OPT_SET_INT(0, "keep", &reset_type,
+   N_("reset HEAD but keep local changes"), KEEP),
+   OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks 
interactively")),
+   OPT_END()
+   };
+
+   git_config(git_default_config, NULL);
+
+   argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+   PARSE_OPT_KEEP_DASHDASH);
+   pathspec = parse_args(argc, argv, prefix, &rev);
 
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
@@ -277,9 +285,6 @@ 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}"));
-- 
1.8.1.rc3.331.g1ef2165

--

[PATCH 02/19] reset $pathspec: exit with code 0 if successful

2013-01-09 Thread Martin von Zweigbergk
"git reset $pathspec" currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.

This makes the 4 "disambiguation" test cases in t7102 clearer since
they all used to "fail", 3 of which "failed" due to changes in the
work tree. Now only the ambiguous one fails.
---
I suppose this makes documenting the exit code unnecessary, since
"return zero iff successful" is probably understood to be the default.

The variable in refresh_index() containing the value to be returned is
called has_errors. I'm guessing I shouldn't take the name too
seriously.

 builtin/reset.c   |  8 +++-
 t/t2013-checkout-submodule.sh |  2 +-
 t/t7102-reset.sh  | 18 --
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
 
 static int update_index_refresh(int fd, struct lock_file *index_lock, int 
flags)
 {
-   int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
 
-   result = refresh_index(&the_index, (flags), NULL, NULL,
-  _("Unstaged changes after reset:")) ? 1 : 0;
+   refresh_index(&the_index, (flags), NULL, NULL,
+ _("Unstaged changes after reset:"));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error ("Could not refresh index");
-   return result;
+   return 0;
 }
 
 static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success '"reset " updates the index' '
git update-index --refresh &&
git diff-files --quiet &&
git diff-index --quiet --cached HEAD &&
-   test_must_fail git reset HEAD^ submodule &&
+   git reset HEAD^ submodule &&
test_must_fail git diff-files --quiet &&
git reset submodule &&
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed ' '
echo 4 > file4 &&
echo 5 > file1 &&
git add file1 file3 file4 &&
-   test_must_fail git reset HEAD -- file1 file2 file3 &&
+   git reset HEAD -- file1 file2 file3 &&
+   test_must_fail git diff --quiet &&
git diff > output &&
test_cmp output expect &&
git diff --cached > output &&
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give 
paths' '
>sub/file2 &&
git update-index --add sub/file1 sub/file2 &&
T=$(git write-tree) &&
-   test_must_fail git reset HEAD sub/file2 &&
+   git reset HEAD sub/file2 &&
+   test_must_fail git diff --quiet &&
U=$(git write-tree) &&
echo "$T" &&
echo "$U" &&
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is 
unmerged' '
echo "100644 $F3 3  file2"
} | git update-index --index-info &&
git ls-files -u &&
-   test_must_fail git reset HEAD file2 &&
+   git reset HEAD file2 &&
+   test_must_fail git diff --quiet &&
git diff-index --exit-code --cached HEAD
 '
 
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard &&
>secondfile &&
git add secondfile &&
-   test_must_fail git reset secondfile &&
+   git reset secondfile &&
+   test_must_fail git diff --quiet -- secondfile &&
test -z "$(git diff --cached --name-only)" &&
test -f secondfile &&
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
-   test_must_fail git reset HEAD secondfile &&
+   git reset HEAD secondfile &&
+   test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
 
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
-   test_must_fail git reset -- secondfile &&
+   git reset -- secondfile &&
+   test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
 '
-- 
1.8.1.rc3.331.g1ef

[PATCH 14/19] reset [--mixed]: don't write index file twice

2013-01-09 Thread Martin von Zweigbergk
When doing a mixed reset without paths, the index is locked, read,
reset, and written back as part of the actual reset operation (in
reset_index()). Then, when showing the list of worktree modifications,
we lock the index again, refresh it, and write it.

Change this so we only write the index once, making "git reset" a
little faster. It does mean that the index lock will be held a little
longer, but the difference is small compared to the time spent
refreshing the index.

There is one minor functional difference: We used to say "Could not
write new index file." if the first write failed, and "Could not
refresh index" if the second write failed. Now, we will only use the
first message.

This speeds up "git reset" a little on the linux-2.6 repo (best of
five, warm cache):

Before  After
real0m0.239s0m0.214s
user0m0.160s0m0.130s
sys 0m0.070s0m0.080s
---
 builtin/reset.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 2243b95..254afa9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -335,6 +335,11 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
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(
+   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_("Could not write new index file."));
@@ -346,15 +351,6 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
-   else if (reset_type == MIXED) { /* Report what has not been updated. */
-   struct lock_file *index_lock = xcalloc(1, sizeof(struct 
lock_file));
-   int fd = hold_locked_index(index_lock, 1);
-   update_index_refresh(
-   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
-   if (write_cache(fd, active_cache, active_nr) ||
-   commit_locked_index(index_lock))
-   error("Could not refresh index");
-   }
 
remove_branch_state();
 
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/19] reset: don't allow "git reset -- $pathspec" in bare repo

2013-01-09 Thread Martin von Zweigbergk
---
 builtin/reset.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

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. */
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: make default of "cleanup" option configurable

2013-01-09 Thread Ralf Thielow
Hi,

2013/1/9 Jonathan Nieder :
> Hi,
>
> Ralf Thielow wrote:
>
>> 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.
>
> Could you give an example?  I'm trying to get a sense of whether these
> habitual --cleanup= passers would use --cleanup=verbatim or
> --cleanup=strip.
>

It's actually my own usecase :). The bugtracker I'm using is able
to create relationships between issues and related commits. It
expects that a part of the commit message contains the issue number
in format "#". So I need to use a cleanup mode different
from "default" to keep the commentary. The mode I'd use is "whitespace",
"verbatim" is also possible.

> I'm a bit worried that a setting like 'commit.cleanup = strip' would
> be likely to break scripts.  Yes, scripts using "git commit" instead
> of commit-tree while caring about detailed semantics are asking for
> trouble, but I'm worried about importers, for example, which are
> sometimes written by new git users.  Some scripts might assume that
> "git commit" preserves the entire change description from their source
> data, even when some lines happen to start by listing the ways the
> author is #1.
>

When a user uses a script/importer which expects that the "default" option
is used without setting it explicitly, and then the user changes the default,
isn't it the users fault if that would break things?

> I don't think that necessarily rules out this change, but:
>
>  1. We need more of an explanation of the purpose than "this lets
> people type less".  What workflow does setting this option fit
> into?
>
>  2. I would prefer to see a little thought about whether it's possible
> to avoid silently impacting scripts.  E.g., depending on the
> answer to (1), maybe supporting "verbatim" but not "strip" would be
> ok?  Or alternatively, maybe a search of public code repositories
> would reveal that new git users tend to write their importers in a
> way that this doesn't break.
>
> As a side effect, the information gathered to answer (1) and (2) could
> have the potential to make the user-level documentation more useful,
> by adding context to the description of the configuration item.
>

I'll add a sentence of my bugtracker example to it. I think many developers
are using such a tool, so it'd makes sense.

> [...]
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -103,7 +103,7 @@ static enum {
>>   CLEANUP_NONE,
>>   CLEANUP_ALL
>>  } cleanup_mode;
>> -static char *cleanup_arg;
>> +const static char *cleanup_arg;
>
> Style nit: storage class comes first, then the type.  Otherwise the
> typename "const char *" is split up.
>

Thanks.
I'll send a new version of the patch including changes of your and
Junios comments.

Ralf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html