Re: [PATCH v4 2/2] Rename suffixcmp() to ends_with() and invert its result

2013-11-20 Thread Christian Couder
From: Junio C Hamano 
>
> Antoine Pelisse  writes:
> 
>> I'm not exactly sure I understand the point of not squashing all those
>> patches together ?
>> It's not like one is going without the others, or that the commit
>> message provides some new information (except for the name of the
>> file, but that is not very relevant either). The downside is that it's
>> _many_ messages to bypass when reading mails from small-screen devices
>> :-)
> 
> The only plausible reason I could think of is to avoid clashing with
> topics in-flight, but then the approach to produce per-file patch is
> not perfect for that purpose, either, when more than one topic in
> flight touch the same file at different places.
> 
> I'd say probably the best organization would be something like:
> 
>  * A set of clean-up patches to normalize oddball usages of existing
>functions (e.g. normalize 'prefixcmp(a,b) != 0' in some file(s)
>to 'prefixcmp(a,b)');
> 
>  * A single patch to introduce the new function(s), to be applied on
>top of 1.8.5;
> 
>  * A large patch to convert all uses of prefixcmp to starts_with and
>suffixcmp to ends_with in the 1.8.5 codebase;
> 
>  * A patch for each topic in flight to convert newly introduced
>prefixcmp/suffixcmp to starts_with/ends_with, to be applied after
>the topic graduates to 'master' after 1.8.5; and then finally
> 
>  * A separate patch to remove prefixcmp and suffixcmp, to be applied
>after _all_ in-flight topic has graduated to 'master'.

Ok, I will wait for 1.8.5 and then send a patch series like what you
suggest.

Thanks,
Christian.

--
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 2/2] glossary-content.txt: fix documentation of "**" patterns

2013-11-20 Thread Nguyễn Thái Ngọc Duy
"**" means bold in ASCIIDOC, so we need to escape it. This is similar
to 8447dc8 (gitignore.txt: fix documentation of "**" patterns -
2013-11-07)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 There is another instance of '**' in RelNotes/1.8.5.txt. Junio you
 may want to fix it too.

 Documentation/glossary-content.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index e22b524..1b56ba4 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -362,12 +362,12 @@ full pathname may have special meaning:
 
  - A leading "`**`" followed by a slash means match in all
directories. For example, "`**/foo`" matches file or directory
-   "`foo`" anywhere, the same as pattern "`foo`". "**/foo/bar"
+   "`foo`" anywhere, the same as pattern "`foo`". "`**/foo/bar`"
matches file or directory "`bar`" anywhere that is directly
under directory "`foo`".
 
- - A trailing "/**" matches everything inside. For example,
-   "abc/**" matches all files inside directory "abc", relative
+ - A trailing "`/**`" matches everything inside. For example,
+   "`abc/**`" matches all files inside directory "abc", relative
to the location of the `.gitignore` file, with infinite depth.
 
  - A slash followed by two consecutive asterisks then a slash
-- 
1.8.2.82.gc24b958

--
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 1/2] glossary-content.txt: remove an obsolete paragrah

2013-11-20 Thread Nguyễn Thái Ngọc Duy
With the introduction of :(literal), :(glob) and :(icase), :(top) is
no longer the only recognized magic signature.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 .. on top of nd/magic-pathspec..

 Documentation/glossary-content.txt | 4 
 1 file changed, 4 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index e470661..e22b524 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -379,10 +379,6 @@ full pathname may have special meaning:
 Glob magic is incompatible with literal magic.
 --
 +
-Currently only the slash `/` is recognized as the "magic signature",
-but it is envisioned that we will support more types of magic in later
-versions of Git.
-+
 A pathspec with only a colon means "there is no pathspec". This form
 should not be combined with other pathspec.
 
-- 
1.8.2.82.gc24b958

--
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: Bizarre git merge behaviour

2013-11-20 Thread Matthew Cengia
On 2013-11-20 13:29, Johannes Sixt wrote:
[...]
> UU ppcadm/modules/quarantine.py
> 
> That's strange. I can't tell what is going on. Perhaps you have some
> criss-cross merges in your history that merge-recursive trips over?
> 
> Sorry, I don't know how to help you further.

Hah! I worked it out!

$coworker erroneously did a "git merge -s ours" into the branch I was
merging in, and it undid more changes than he intended, which is what
resulted in *everything* breaking for me.

I've appropriately chastised him and most importantly understand why the
problem occured, which puts my mind at ease.

Thanks again for all your help.

-- 
Regards,
Matthew Cengia


signature.asc
Description: Digital signature


Re: [PATCH] Support pathspec magic :(exclude) and its short form :-

2013-11-20 Thread Duy Nguyen
On Thu, Nov 21, 2013 at 6:48 AM, Junio C Hamano  wrote:
>>  We don't have many options that say "negative" in short form.
>>  Either '!', '-' or '~'. '!' is already used for bash history expansion.
>>  ~ looks more like $HOME expansion. Which left me '-'.
>
> I agree with your decision to reject ~, but "!not-this-pattern" is
> very much consistent with the patterns used in .gitignore (and the
> "--exclude " option), so avoiding "!" and introducing an
> inconsistent "-" only to appease bash leaves somewhat a funny taste
> in my mouth.

The thing about '!'  is it's history expansion in bash and I suspect
not many people are aware of it. So "git log -- :!something" may
recall the last command that has "something" in it, which is confusing
for those new people and may potentially be dangerous (multiple
command in one line, separated by semicolon). Compared to ":git log --
(exclude)somethign" the worst that could happen is a syntax error
message from bash.

Other than that I'm fine with '!' being the shortcut.

Btw I'm thinking of extending pathspec magic syntax a bit to allow
path completion. Right now the user has to write

git log -- :-Documentation

which does not play well with path completion. I'm thinking of accepting

git log -- :- Documentation

In other words, if there's no path (or pattern) component after the
magic, then the next argument must contain the path. This enables path
completion and I haven't seen any drawbacks yet..

>> @@ -427,6 +430,10 @@ void parse_pathspec(struct pathspec *pathspec,
>>   pathspec->magic |= item[i].magic;
>>   }
>>
>> + if (nr_exclude == n)
>> + die(_("There is nothing to exclude from by :(exclude) 
>> patterns.\n"
>> +   "Perhaps you forgot to add either ':/' or '.' ?"));
>
> ;-).

Hey it was originally not there, then I made a mistake of typing "git
log -- :-po" and wondered why it shows nothing. Intuitively, if "git
log" shows every path, then "git log -- :-po" should show every path
except 'po' and the user should not be required to type "git log -- :/
:-po". parse_pathspec() can do that, but it's more work and I'm lazy
so I push that back to the user until they scream :)

>> +enum interesting tree_entry_interesting(const struct name_entry *entry,
>> + struct strbuf *base, int base_offset,
>> + const struct pathspec *ps)
>> +{
>> + enum interesting positive, negative;
>> + positive = tree_entry_interesting_1(entry, base, base_offset, ps, 0);
>> +
>> + /*
>> +  *   #  | positive | negative | result
>> +  * -+--+--+---
>> +  * 1..4 |   -1 |* |  -1
>> +  * 5..8 |0 |* |   0
>> +  *   9  |1 |   -1 |   1
>> +  *  10  |1 |0 |   1
>> +  *  11  |1 |1 |   0
>> +  *  12  |1 |2 |   0
>> +  *  13  |2 |   -1 |   2
>> +  *  14  |2 |0 |   2
>> +  *  15  |2 |1 |   0
>> +  *  16  |2 |2 |  -1
>> +  */
>
> Not sure what this case-table means...

Sorry, because tree_entry_interesting_1() returns more than "match or
not", we need to combine the result from positive pathspec with the
negative one to correctly handle all_not_interesting and
all_interesting. This table sums it up. I'll add more explanation in
the next patch.
-- 
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: [PATCH] Support pathspec magic :(exclude) and its short form :-

2013-11-20 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  This is yet another stab at the negative pathspec thing. It's not
>  ready yet (there are a few XXXs) but I could use some feedback
>  regarding the interface, or the behavior. It looks better this time
>  now that pathspec magic is supported (or maybe I'm just biased).
>
>  For :(glob) or :(icase) you're more likely to enable it for all
>  pathspec, i.e. --glob-pathspecs. But I expect :(exclude) to be typed
>  more often (it does not make sense to add --exclude-pathspecs to
>  exclude everything), which is why I add the short form for it.
>
>  We don't have many options that say "negative" in short form.
>  Either '!', '-' or '~'. '!' is already used for bash history expansion.
>  ~ looks more like $HOME expansion. Which left me '-'.

I agree with your decision to reject ~, but "!not-this-pattern" is
very much consistent with the patterns used in .gitignore (and the
"--exclude " option), so avoiding "!" and introducing an
inconsistent "-" only to appease bash leaves somewhat a funny taste
in my mouth.

>  Documentation/glossary-content.txt |  5 
>  builtin/add.c  |  5 +++-
>  dir.c  | 50 +++-
>  pathspec.c |  9 ++-
>  pathspec.h |  4 ++-
>  tree-walk.c| 52 
> +++---
>  6 files changed, 112 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/glossary-content.txt 
> b/Documentation/glossary-content.txt
> index e470661..f7d7d8c 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -377,6 +377,11 @@ full pathname may have special meaning:
>   - Other consecutive asterisks are considered invalid.
>  +
>  Glob magic is incompatible with literal magic.
> +
> +exclude `-`;;
> + After a path matches any non-exclude pathspec, it will be run
> + through all exclude pathspec. If it matches, the path is
> + ignored.
>  --
>  +
>  Currently only the slash `/` is recognized as the "magic signature",

No longer, no?  "magic signature" is a non-alphanumeric that follows
the ':' introducer, as opposed to "magic words" that are in ":(...)".

> diff --git a/builtin/add.c b/builtin/add.c
> index 226f758..0df73ae 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -540,10 +540,13 @@ int cmd_add(int argc, const char **argv, const char 
> *prefix)
>  PATHSPEC_FROMTOP |
>  PATHSPEC_LITERAL |
>  PATHSPEC_GLOB |
> -PATHSPEC_ICASE);
> +PATHSPEC_ICASE |
> +PATHSPEC_EXCLUDE);
>  
>   for (i = 0; i < pathspec.nr; i++) {
>   const char *path = pathspec.items[i].match;
> + if (pathspec.items[i].magic & PATHSPEC_EXCLUDE)
> + continue;
>   if (!seen[i] &&
>   ((pathspec.items[i].magic &
> (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||

So "git add ':(exclude)junk/' '*.c'" to add all .c files except for
the ones in the 'junk/' directory may find that ':(exclude)junk/'
matched nothing (because there is no .c file in there), and that is
not an error.  It makes sense to me.

> diff --git a/dir.c b/dir.c
> index 23b6de4..e2df82f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -126,10 +126,13 @@ static size_t common_prefix_len(const struct pathspec 
> *pathspec)
>  PATHSPEC_MAXDEPTH |
>  PATHSPEC_LITERAL |
>  PATHSPEC_GLOB |
> -PATHSPEC_ICASE);
> +PATHSPEC_ICASE |
> +PATHSPEC_EXCLUDE);
>  
>   for (n = 0; n < pathspec->nr; n++) {
>   size_t i = 0, len = 0, item_len;
> + if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)
> + continue;
>   if (pathspec->items[n].magic & PATHSPEC_ICASE)
>   item_len = pathspec->items[n].prefix;
>   else

Likewise.  Exclusion does not participate in the early culling with
the common prefix.

> @@ -1375,11 +1407,17 @@ int read_directory(struct dir_struct *dir, const char 
> *path, int len, const stru
>  PATHSPEC_MAXDEPTH |
>  PATHSPEC_LITERAL |
>  PATHSPEC_GLOB |
> -PATHSPEC_ICASE);
> +PATHSPEC_ICASE |
> +PATHSPEC_EXCLUDE);
>  
>   if (has_symlink_leading_path(path, len))
>   return dir->nr;
>  
> + /*
> +  * XXX: exclude patterns are treated like positive ones in
> +  * create_simplify! This is not wrong, but may make path
> +  * filtering less efficient.
> +  */

True, but "git

Re: [PATCH] Documentation/gitcli.txt: fix double quotes

2013-11-20 Thread Junio C Hamano
Thanks; will queue.
--
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 v3] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-20 Thread Junio C Hamano
Jens Lehmann  writes:

> Changes since v2:
>
> - Honor the core.commentChar setting and add a test for that.
>
> - Fix the submodule test to set the editor in a portable way.
>
> - Only print scissor and description lines when not going to stdout
>   (otherwise a "git status -v" prints that on stdout too, which
>   doesn't make much sense). Now we definitely do not have to care
>   about coloring these lines either.

Hmph.  It makes me suspect that we were drunk when we decided to let
"git status -v" to keep showing diff when we declared that "git
status" is different from "git commit --dry-run", but it is too late
to fix it now, I think.

> - Nicer formatting of the commit message.

Certainly a lot easier to read, at least to me.

> @@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char 
> *prefix)
>   }
>
>   /* Truncate the message just before the diff, if any. */

I wonder if this comment still is valid, but it probably is OK.

> - if (verbose) {
> - p = strstr(sb.buf, "\ndiff --git ");
> - if (p != NULL)
> - strbuf_setlen(&sb, p - sb.buf + 1);
> - }
> + if (verbose)
> + wt_status_truncate_message_at_cut_line(&sb);

> diff --git a/wt-status.c b/wt-status.c
> index b4e44ba..734f94b 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -16,6 +16,9 @@
>  #include "column.h"
>  #include "strbuf.h"
>
> +static char wt_status_cut_line[] = /* 'X' is replaced with comment_line_char 
> */
> +"X  >8 \n";
> +
>  static char default_wt_status_colors[][COLOR_MAXLEN] = {
>   GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
>   GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
> @@ -767,6 +770,15 @@ conclude:
>   status_printf_ln(s, GIT_COLOR_NORMAL, "");
>  }
>
> +void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
> +{
> + const char *p;
> +
> + p = strstr(buf->buf, wt_status_cut_line);
> + if (p && (p == buf->buf || p[-1] == '\n'))
> + strbuf_setlen(buf, p - buf->buf);
> +}

Perhaps it may happen that all the current callers have called
wt_status_print_verbose() to cause wt_status_cut_line[0] to hold
comment_line_char, but relying on that calling sequence somehow
makes me feel uneasy.

Perhaps cut_line[] should only have "--- >8 ---" part and both
printing side (below) and finding side (this one) should check these
separately?  That is:

p = buf->buf;
while (p && *p) {
p = strchr(p, comment_line_char);
if (!p)
break;
if (strstr(p + 1, cut_line) == p + 1)
break;
p++;
continue;
}
if (p && *p && (p == buf->buf || p[-1] == '\n'))
strbuf_setlen(buf, p - buf->buf);

or something (the above is deliberately less-efficient-than-ideal,
because I want to keep the code structure in such a way that we can
later turn comment_line_char to a string[] that can hold "//" to
allow a multi-char comment introducer more easily)?

>  static void wt_status_print_verbose(struct wt_status *s)
>  {
>   struct rev_info rev;
> @@ -787,10 +799,17 @@ static void wt_status_print_verbose(struct wt_status *s)
>* If we're not going to stdout, then we definitely don't
>* want color, since we are going to the commit message
>* file (and even the "auto" setting won't work, since it
> -  * will have checked isatty on stdout).
> +  * will have checked isatty on stdout). But we then do want
> +  * to insert the scissor line here to reliably remove the
> +  * diff before committing.
>*/
> - if (s->fp != stdout)
> + if (s->fp != stdout) {
>   rev.diffopt.use_color = 0;
> + wt_status_cut_line[0] = comment_line_char;
> + fprintf(s->fp, wt_status_cut_line);
> + fprintf(s->fp, _("%c Do not touch the line above.\n"), 
> comment_line_char);
> + fprintf(s->fp, _("%c Everything below will be removed.\n"), 
> comment_line_char);
> + }

I didn't bother with my "how about this" version, but we may want to
use strbuf_add_commented_lines() to help i18n/l10n folks.  Depending
on the l10n, this message may want to become more or less than 2
lines.

>   run_diff_index(&rev, 1);
>  }
>
> diff --git a/wt-status.h b/wt-status.h
> index 6c29e6f..30a4812 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -91,6 +91,7 @@ struct wt_status_state {
>   unsigned char cherry_pick_head_sha1[20];
>  };
>
> +void wt_status_truncate_message_at_cut_line(struct strbuf *);
>  void wt_status_prepare(struct wt_status *s);
>  void wt_status_print(struct wt_status *s);
>  void wt_status_collect(struct wt_status *s);
--
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] diff: restrict pathspec limitations to diff b/f case only

2013-11-20 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> builtin_diff_b_f() needs a path, not pathspec. Other modes in diff
> can deal with pathspec just fine. But because of the current
> GUARD_PATHSPEC() location, other modes also reject :(glob) and
> :(icase).
>
> Move GUARD_PATHSPEC(), and the "path" assignment statement, which is
> the reason of this GUARD_PATHSPEC(), inside builtin_diff_b_f().
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

Makes sense.

Let me squash this in.


diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh
index 8d4a7fc..a7c7ff5 100755
--- a/t/t6131-pathspec-icase.sh
+++ b/t/t6131-pathspec-icase.sh
@@ -100,4 +100,10 @@ test_expect_success 'match_pathspec_depth matches 
:(icase)bar with empty prefix'
test_cmp expect actual
 '
 
+test_expect_success '"git diff" can take magic :(icase) pathspec' '
+   echo FOO/BAR >expect &&
+   git diff --name-only HEAD^ HEAD -- ":(icase)foo/bar" >actual &&
+   test_cmp expect actual
+'
+
 test_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: Bug: Pathspec parsing on Windows fails when branch specified

2013-11-20 Thread Johannes Sixt
Am 20.11.2013 22:09, schrieb Eris Belew:
> System: Windows Server 2008 R2
> Git: git version 1.8.4.msysgit.0
> Shell: Powershell V3 (No third-party modules loaded)
> 
> Summary:
>   When specifying a pathspec including a branch/commit, path separator
> characters are not translated. Since tab-completion in windows shells (ex:
> CMD, PowerShell, not unix-style shells running on windows) uses the windows
> path separator, and other git commands work fine with the windows path
> separator, the expected behavior would be to translate for me.
> 
> Reproduction:
> git diff BRANCH:path\to\file path\to\file
> 
> Result:
> fatal: Path 'path\to\file' does not exist in 'BRANCH'
> 
> Expected:
> Normal diff operation
> 
> Workaround:
> Manually convert pathspec. Examples of working command:
> git diff BRANCH:path/to/file path\to\otherfile
> git diff BRANCH:path/to/file path/to/otherfile

That's not a bug, it is expected behavior. "BRANCH:path/to/file" is not
"file on disk" syntax, but Git's syntax to reference a particular object
in the database. For this reason, forward-slashes are mandated; there is
no option to use backslashes in this case.

Backslashes could actually be part of directory and file names in the
database. It would be impossible to check out a tree with such names on
Windows, obviously.

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


[PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message

2013-11-20 Thread Jens Lehmann
When using the '-v' option of "git commit" the diff added to the commit
message temporarily for editing is stripped off after the user exited the
editor by searching for "\ndiff --git " and truncating the commmit message
there if it is found.

But this approach has two problems:

- when the commit message itself contains a line starting with
  "diff --git" it will be truncated there prematurely; and

- when the "diff.submodule" setting is set to "log", the diff may
  start with "Submodule ..", which will be left in
  the commit message while it shouldn't.

Fix that by introducing a special scissor separator line starting with the
comment character ('#' or the core.commentChar config if set) followed by
two lines describing what it is for. The scissor line - which will not be
translated - is used to reliably detect the start of the diff so it can be
chopped off from the commit message, no matter what the user enters there.

Turn a known test failure fixed by this change into a successful test;
also add one for a diff starting with a submodule log and another one for
proper handling of the comment char.

Reported-by: Ari Pollak 
Signed-off-by: Jens Lehmann 
---


Changes since v2:

- Honor the core.commentChar setting and add a test for that.

- Fix the submodule test to set the editor in a portable way.

- Only print scissor and description lines when not going to stdout
  (otherwise a "git status -v" prints that on stdout too, which
  doesn't make much sense). Now we definitely do not have to care
  about coloring these lines either.

- Nicer formatting of the commit message.


 builtin/commit.c  |  9 +++--
 t/t7507-commit-verbose.sh | 28 +++-
 wt-status.c   | 23 +--
 wt-status.h   |  1 +
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..fedb45a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct strbuf sb = STRBUF_INIT;
struct strbuf author_ident = STRBUF_INIT;
const char *index_file, *reflog_msg;
-   char *nl, *p;
+   char *nl;
unsigned char sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = &parents;
@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}

/* Truncate the message just before the diff, if any. */
-   if (verbose) {
-   p = strstr(sb.buf, "\ndiff --git ");
-   if (p != NULL)
-   strbuf_setlen(&sb, p - sb.buf + 1);
-   }
+   if (verbose)
+   wt_status_truncate_message_at_cut_line(&sb);

if (cleanup_mode != CLEANUP_NONE)
stripspace(&sb, cleanup_mode == CLEANUP_ALL);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b..2ddf28c 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' 
'
check_message diff
 '

-test_expect_failure 'diff in message is retained with -v' '
+test_expect_success 'diff in message is retained with -v' '
git commit --amend -F diff -v &&
check_message diff
 '

+test_expect_success 'submodule log is stripped out too with -v' '
+   git config diff.submodule log &&
+   git submodule add ./. sub &&
+   git commit -m "sub added" &&
+   (
+   cd sub &&
+   echo "more" >>file &&
+   git commit -a -m "submodule commit"
+   ) &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit -a -v 2>err
+   ) &&
+   test_i18ngrep "Aborting commit due to empty commit message." err
+'
+
+test_expect_success 'verbose diff is stripped out with set core.commentChar' '
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git -c core.commentchar=";" commit -a -v 2>err
+   ) &&
+   test_i18ngrep "Aborting commit due to empty commit message." err
+'
+
 test_done
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..734f94b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,9 @@
 #include "column.h"
 #include "strbuf.h"

+static char wt_status_cut_line[] = /* 'X' is replaced with comment_line_char */
+"X  >8 \n";
+
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
@@ -767,6 +770,15 @@ conclude:
status_printf_ln(s, GIT_COLOR_NORMAL, "");
 }

+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
+{
+   const char *p;
+
+   p = strstr(buf->buf, wt_status_cut_line);
+   if (p && (p == buf->buf || p[-1] == '\n'))

Re: corrupt object memory allocation error

2013-11-20 Thread Joey Hess
Jeff King wrote:
> As for your specific corruption, I can't make heads or tails of it. It
> is not a single-bit error.

Oh, I should have mentioned that I am generating corrupt git
repositories mechanically for testing. I think in this case it prepended
some garbage to an object.

-- 
see shy jo


signature.asc
Description: Digital signature


Re: Fwd: Error with git-svn pushing a rename

2013-11-20 Thread Benjamin Pabst
Hi,

is it possible to debug git-svn or get a more verbose / debug output
from it? I already tried with the "GIT_TRACE" variable, but it does
not include any further output on the svn methods.

Regards, Benjamin

2013/11/17 Andreas Stricker :
> Hi Jonathan
>
>> Can you give an exact sequence of steps (including "Upgrade Subversion
>> at this step") to reproduce the problem?  That would help immensely
>> --- if at all possible, I would very much like to keep existing
>> git-svn repos working on upgrade.
>
> Of course. I've attached a text file with the commands required to
> reproduce this error.
>
> From my experiments it looks like after the subversion is upgraded
> to 1.8 the problem only occurs if "git svn fetch" fetches new changes
> from the subversion repository. Without changes in upstream subversion
> repository I couldn't reproduce the error. And a rename is required too.
>
> Hope this helps.
>
> Regards, Andy
--
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


[ANNOUNCE] Git v1.8.5-rc3

2013-11-20 Thread Junio C Hamano
A release candidate Git v1.8.5-rc3 is now available for testing
at the usual places.

Regression in the previous rc's on "git blame --literal-pathspecs"
etc. has been fixed.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

df1dc514640dd38d5c98f8c7f5bb674e2abec74e  git-1.8.5.rc3.tar.gz
43ff8c2c7831a01633b10ae5a7aba6d620e13ab0  git-htmldocs-1.8.5.rc3.tar.gz
f155021b30ffb2a0be10d86806f5a8b92cc3c3e7  git-manpages-1.8.5.rc3.tar.gz

The following public repositories all have a copy of the v1.8.5-rc3
tag and the master branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.8.5 Release Notes (draft)


Backward compatibility notes (for Git 2.0)
--

When "git push [$there]" does not say what to push, we have used the
traditional "matching" semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there).  In Git 2.0, the default will change to the "simple"
semantics, which pushes:

 - only the current branch to the branch with the same name, and only
   when the current branch is set to integrate with that remote
   branch, if you are pushing to the same remote as you fetch from; or

 - only the current branch to the branch with the same name, if you
   are pushing to a remote that is not where you usually fetch from.

Use the user preference configuration variable "push.default" to
change this.  If you are an old-timer who is used to the "matching"
semantics, you can set the variable to "matching" to keep the
traditional behaviour.  If you want to live in the future early, you
can set it to "simple" today without waiting for Git 2.0.

When "git add -u" (and "git add -A") is run inside a subdirectory and
does not specify which paths to add on the command line, it
will operate on the entire tree in Git 2.0 for consistency
with "git commit -a" and other commands.  There will be no
mechanism to make plain "git add -u" behave like "git add -u .".
Current users of "git add -u" (without a pathspec) should start
training their fingers to explicitly say "git add -u ."
before Git 2.0 comes.  A warning is issued when these commands are
run without a pathspec and when you have local changes outside the
current directory, because the behaviour in Git 2.0 will be different
from today's version in such a situation.

In Git 2.0, "git add " will behave as "git add -A ", so
that "git add dir/" will notice paths you removed from the directory
and record the removal.  Versions before Git 2.0, including this
release, will keep ignoring removals, but the users who rely on this
behaviour are encouraged to start using "git add --ignore-removal "
now before 2.0 is released.

The default prefix for "git svn" will change in Git 2.0.  For a long
time, "git svn" created its remote-tracking branches directly under
refs/remotes, but it will place them under refs/remotes/origin/ unless
it is told otherwise with its --prefix option.


Updates since v1.8.4


Foreign interfaces, subsystems and ports.

 * "git-svn" has been taught to use the serf library, which is the
   only option SVN 1.8.0 offers us when talking the HTTP protocol.

 * "git-svn" talking over an https:// connection using the serf library
   dumped core due to a bug in the serf library that SVN uses.  Work
   around it on our side, even though the SVN side is being fixed.

 * On MacOS X, we detected if the filesystem needs the "pre-composed
   unicode strings" workaround, but did not automatically enable it.
   Now we do.

 * remote-hg remote helper misbehaved when interacting with a local Hg
   repository relative to the home directory, e.g. "clone hg::~/there".

 * imap-send ported to OS X uses Apple's security framework instead of
   OpenSSL's.

 * "git fast-import" treats an empty path given to "ls" as the root of
   the tree.


UI, Workflows & Features

 * xdg-open can be used as a browser backend for "git web-browse"
   (hence to show "git help -w" output), when available.

 * "git grep" and "git show" pay attention to the "--textconv" option
   when these commands are told to operate on blob objects (e.g. "git
   grep -e pattern --textconv HEAD:Makefile").

 * "git replace" helper no longer allows an object to be replaced with
   another object of a different type to avoid confusion (you can
   still manually craft such a replacement using "git update-ref", as an
   escape hatch).

 * "git status" no longer prints the dirty status information of
   submodules for which submodule.$name.ignore is set to "all".

 * "git rebase -i" honours core.abbrev when preparing the insn sheet
 

[ANNOUNCE] Git v1.8.4.4

2013-11-20 Thread Junio C Hamano
The latest maintenance release Git v1.8.4.4 is now available at
the usual places.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

1aaa1a51b599f19125e06fa6e839c9ff2e5ac941  git-1.8.4.4.tar.gz
c2ee47c2bbf8ede70eef2a1ba936a30aa0d78b2a  git-htmldocs-1.8.4.4.tar.gz
4637e22fd2fe59cf00b4a105f5104af9bfea8c2c  git-manpages-1.8.4.4.tar.gz

The following public repositories all have a copy of the v1.8.4.4
tag and the maint branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Also, http://www.kernel.org/pub/software/scm/git/ has copies of the
release tarballs.

Git v1.8.4.4 Release Notes


Fixes since v1.8.4.3


 * The fix in v1.8.4.3 to the pack transfer protocol to propagate
   the target of symbolic refs broke "git clone/git fetch" from a
   repository with too many symbolic refs. As a hotfix/workaround,
   we transfer only the information on HEAD.



Changes since v1.8.4.3 are as follows:

Junio C Hamano (2):
  Revert "upload-pack: send non-HEAD symbolic refs"
  Git 1.8.4.4

--
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: corrupt object memory allocation error

2013-11-20 Thread Jeff King
On Wed, Nov 20, 2013 at 04:33:50PM -0400, Joey Hess wrote:

> I've got a git repository of < 2 mb, where git wants to
> allocate a rather insane amount of memory:
> 
> >git fsck
> Checking object directories: 100% (256/256), done.
> fatal: Out of memory, malloc failed (tried to allocate 124865231165 bytes)
> 
> > git show 11644b5a075dc1425e01fbba51c045cea2d0c408
> fatal: Out of memory, malloc failed (tried to allocate 124865231165 bytes)
> 
> The problem seems to be the attached object file, which has gotten
> corrupted, presumably in the header that git reads to see how large it
> is. Thought I'd report this in case there is some easy way to
> add a sanity check.

Definitely a corrupt object. The start is not a valid zlib header, so we
guess that it is an "experimental loose object". This is a format that
git wrote for very short period as a performance experiment; it didn't
pan out and we no longer write it.

The loose object format contains the (purported) object size outside of
the checksum'd zlib data (whereas the normal format has a human-readable
header that gets zlib'd). Your corrupted bytes end up specifying a
ridiculously large size.

I wonder if it is time to drop reading support for the experimental
objects. It was never widely used, and was deprecated in v1.5.2 by
726f852 (deprecate the new loose object header format, 2007-05-09). That
would improve the case when the initial bytes of a loose object are
corrupted, because we would complain about the bogus zlib data before
trying to allocate the buffer.

The problem would still remain for packfiles, which use a similar
encoding, but I suspect it is less common there. For a single-byte
corruption, it is unlikely to be right in the length header. But for
absolute junk that is not git data at all, the first bytes are very
likely to be corrupted. In the pack case, we would notice early that it
does not look like a packfile; for the loose object, we have no such
header and proceed with the allocation.

As for your specific corruption, I can't make heads or tails of it. It
is not a single-bit error. The first two bytes of a loose object should
always be <0x78, 0x01>, which is the standard zlib deflate header. Your
bytes aren't even close, and decoding the rest with a corrupted zlib
header seems fruitless.

You don't happen to have another copy of the object (or of the data
contained in the object, such as the working tree file), do you? It
might be interesting to see a comparison of the bytes of the correct
data and your corruption.

-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: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Jens Lehmann
Am 20.11.2013 20:35, schrieb Ramsay Jones:
> On 20/11/13 17:22, Junio C Hamano wrote:
>> Ramsay Jones  writes:
>>
>>> Signed-off-by: Ramsay Jones 
>>> ---
>>>
>>> Hi Jens,
>>>
>>> commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
>>> from the commit message", 19-11-2013) in 'pu' fails the new test
>>> it added to t7507.
>>>
>>> I didn't spend too long looking at the problem, so take this patch
>>> as nothing more than a quick suggestion for a possible solution! :-P
>>> [The err file contained something like: "There was a problem with the
>>> editor '"$FAKE_EDITOR"'"].
>>>
>>> Having said that, this fixes it for me ...
>>
>> Well spotted.  test_must_fail being a shell function, not a command,
>> we shouldn't have used the "VAR=val cmd" one-shot environment
>> assignment for portability.
>>
>> Even though this happens to be the last test in the script, using
>> test_set_editor to permanently affect the choice of editor for tests
>> that follow is not generally a good idea.  It would be safer to do
>> this, I would have to say:
>>
>>  git commit -a -m "submodule commit"
>>  ) &&
>> (
>>  GIT_EDITOR=cat &&
>> export GIT_EDITOR &&
>> test_must_fail git commit -a -v 2>err
>>  ) &&
>> test_i18ngrep "Aborting ..."
> 
> Yes, this works great (and I very nearly wrote exactly this, but since
> the test was using test_set_editor anyway ...) :-D

Thanks all, will use that in the next iteration.
--
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


Bug: Pathspec parsing on Windows fails when branch specified

2013-11-20 Thread Eris Belew
System: Windows Server 2008 R2
Git: git version 1.8.4.msysgit.0
Shell: Powershell V3 (No third-party modules loaded)

Summary:
  When specifying a pathspec including a branch/commit, path separator
characters are not translated. Since tab-completion in windows shells (ex:
CMD, PowerShell, not unix-style shells running on windows) uses the windows
path separator, and other git commands work fine with the windows path
separator, the expected behavior would be to translate for me.

Reproduction:
git diff BRANCH:path\to\file path\to\file

Result:
fatal: Path 'path\to\file' does not exist in 'BRANCH'

Expected:
Normal diff operation

Workaround:
Manually convert pathspec. Examples of working command:
git diff BRANCH:path/to/file path\to\otherfile
git diff BRANCH:path/to/file path/to/otherfile


Thanks,

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


corrupt object memory allocation error

2013-11-20 Thread Joey Hess
I've got a git repository of < 2 mb, where git wants to
allocate a rather insane amount of memory:

>git fsck
Checking object directories: 100% (256/256), done.
fatal: Out of memory, malloc failed (tried to allocate 124865231165 bytes)

> git show 11644b5a075dc1425e01fbba51c045cea2d0c408
fatal: Out of memory, malloc failed (tried to allocate 124865231165 bytes)

The problem seems to be the attached object file, which has gotten
corrupted, presumably in the header that git reads to see how large it
is. Thought I'd report this in case there is some easy way to
add a sanity check.

-- 
see shy jo


644b5a075dc1425e01fbba51c045cea2d0c408
Description: Binary data


signature.asc
Description: Digital signature


Re: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Ramsay Jones
On 20/11/13 17:22, Junio C Hamano wrote:
> Ramsay Jones  writes:
> 
>> Signed-off-by: Ramsay Jones 
>> ---
>>
>> Hi Jens,
>>
>> commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
>> from the commit message", 19-11-2013) in 'pu' fails the new test
>> it added to t7507.
>>
>> I didn't spend too long looking at the problem, so take this patch
>> as nothing more than a quick suggestion for a possible solution! :-P
>> [The err file contained something like: "There was a problem with the
>> editor '"$FAKE_EDITOR"'"].
>>
>> Having said that, this fixes it for me ...
> 
> Well spotted.  test_must_fail being a shell function, not a command,
> we shouldn't have used the "VAR=val cmd" one-shot environment
> assignment for portability.
> 
> Even though this happens to be the last test in the script, using
> test_set_editor to permanently affect the choice of editor for tests
> that follow is not generally a good idea.  It would be safer to do
> this, I would have to say:
> 
>   git commit -a -m "submodule commit"
>   ) &&
> (
>   GIT_EDITOR=cat &&
> export GIT_EDITOR &&
> test_must_fail git commit -a -v 2>err
>   ) &&
> test_i18ngrep "Aborting ..."

Yes, this works great (and I very nearly wrote exactly this, but since
the test was using test_set_editor anyway ...) :-D

Thanks.

ATB,
Ramsay Jones




--
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] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Jeff King
On Wed, Nov 20, 2013 at 01:54:20PM -0500, Jeff King wrote:

> I'm not sure why the old $FAKE_EDITOR doesn't work there, though (not
> that it would make the test pass anyway, as it does something different
> than what the test wants, but I would not expect the shell to complain
> of failure).

Oh, I see. The original FAKE_EDITOR is:

  exec grep '^diff --git' "\$1"

But the whole point of the test is that we do not have such a "diff"
header, being only a submodule update, and so grep exits non-zero, and
git thinks the editor has failed.

So mystery solved, and the sole problem is the proper setting of the
variable.

-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: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Jeff King
On Wed, Nov 20, 2013 at 10:33:28AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Am I misremembering the issues with one-shot variables and functions?
> 
> I think there are two problems involved.

OK, I was misremembering. I recalled the "does not unset afterwards"
part, but not the "does not export" part. I think because:

> test_must_fail () {
>   (
>   env | sed -n -e '/EDITOR/s/^/>> /p'
>   )
> }

...here we _do_ have GIT_EDITOR set properly in the function itself, but
not in the subprocess.

Previous discussion and links to POSIX are here:

  http://article.gmane.org/gmane.comp.version-control.git/137095

Not that they matter compared to the code you demonstrated, but I was
digging them up when you responded. :)

> Another is that EDITOR="$FAKE_EDITOR" that is set up earlier in the
> is having trouble launching (I have a feeling that it never was
> actually used because everybody uses "commit -F ").

I think it is used, as there are several "git commit --amend -v"
invocations. Which makes sense, as you should not be able to test "-v"
with "-F", I would think.

I'm not sure why the old $FAKE_EDITOR doesn't work there, though (not
that it would make the test pass anyway, as it does something different
than what the test wants, but I would not expect the shell to complain
of failure).

-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: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Junio C Hamano
Jeff King  writes:

> The test_set_editor helper does some magic to help with quoting, but
> that should not be an issue in this case (since we are using "cat"). We
> are using test_set_editor elsewhere in the script, which would have set
> EDITOR previously. But I would think that GIT_EDITOR, which we are using
> here, would supersede that. However, the error message he shows
> indicates that git is using EDITOR (as FAKE_EDITOR is part of that quote
> magic).
>
> Am I misremembering the issues with one-shot variables and functions?

I think there are two problems involved.

The first is that we are not really using GIT_EDITOR under some
shells; to wit:

 >8 

$ cat >/var/tmp/dashtest.sh <<\EOF
#!/bin/sh

test_must_fail () {
(
env | sed -n -e '/EDITOR/s/^/>> /p'
)
}

EDITOR=dog
export EDITOR

GIT_EDITOR=cat test_must_fail foo
EOF

$ dash /var/tmp/dashtest.sh
>> EDITOR=dog
$ bash /var/tmp/dashtest.sh
>> GIT_EDITOR=cat
>> EDITOR=dog

 8< 

So it appears that GIT_EDITOR that was never exported in the script
fails to get exported with the "VAR=VAL cmd" syntax under dash, when
cmd is not a command but is a shell function.  The "git commit" in
question ends up using $EDITOR.

Another is that EDITOR="$FAKE_EDITOR" that is set up earlier in the
is having trouble launching (I have a feeling that it never was
actually used because everybody uses "commit -F ").
--
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] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Jeff King
On Wed, Nov 20, 2013 at 09:22:31AM -0800, Junio C Hamano wrote:

> > commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
> > from the commit message", 19-11-2013) in 'pu' fails the new test
> > it added to t7507.
> >
> > I didn't spend too long looking at the problem, so take this patch
> > as nothing more than a quick suggestion for a possible solution! :-P
> > [The err file contained something like: "There was a problem with the
> > editor '"$FAKE_EDITOR"'"].
> >
> > Having said that, this fixes it for me ...
> 
> Well spotted.  test_must_fail being a shell function, not a command,
> we shouldn't have used the "VAR=val cmd" one-shot environment
> assignment for portability.

Yeah, I noticed that, too upon reading Ramsay's patch. But I thought the
usual symptom there was that the variable is not properly unset after
the function returns? In other words, it might affect later tests, but
the failure that Ramsay sees is in _this_ test, so it must be a separate
issue.

The test_set_editor helper does some magic to help with quoting, but
that should not be an issue in this case (since we are using "cat"). We
are using test_set_editor elsewhere in the script, which would have set
EDITOR previously. But I would think that GIT_EDITOR, which we are using
here, would supersede that. However, the error message he shows
indicates that git is using EDITOR (as FAKE_EDITOR is part of that quote
magic).

Am I misremembering the issues with one-shot variables and functions?

Puzzled...

-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: [PATCH] pager: turn on "cat" optimization for DEFAULT_PAGER

2013-11-20 Thread Erik Faye-Lund
On Wed, Nov 20, 2013 at 6:33 PM, Junio C Hamano  wrote:
> Erik Faye-Lund  writes:
>
>>> ...
>>> is set to empty., 2006-04-16). At that time, the line
>>> directly above used:
>>>
>>>if (!pager)
>>>pager = "less";
>>>
>>> as a fallback, meaning that it could not possibly trigger
>>> the optimization. Later, a3d023d (Provide a build time
>>> default-pager setting, 2009-10-30) turned that constant into
>>> a build-time setting which could be anything, but didn't
>>> loosen the "else" to let DEFAULT_PAGER use the optimization.
>>>
>>> Noticed-by: Dale R. Worley 
>>> Suggested-by: Matthieu Moy 
>>> Signed-off-by: Jeff King 
>>> ---
>>>  pager.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/pager.c b/pager.c
>>> index c1ecf65..fa19765 100644
>>> --- a/pager.c
>>> +++ b/pager.c
>>> @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
>>> pager = getenv("PAGER");
>>> if (!pager)
>>> pager = DEFAULT_PAGER;
>>> -   else if (!*pager || !strcmp(pager, "cat"))
>>> +   if (!*pager || !strcmp(pager, "cat"))
>>
>> Hmmpf. It's sometimes useful to actually pipe through cat rather than
>> disabling the pager, as this changes the return-code from isatty. I
>> sometimes use this for debugging-purposes. Does this patch break that?
>
> If you have been running "GIT_PAGER=cat git whatever" and the like,
> we did not pipe the output through "cat" and this has been the case
> for a long time.  The only thing the patch in question changed is
> for those who build with
>
> make DEFAULT_PAGER=cat
>
> and I doubt that you have been debugging git by rebuilding it with
> such a setting, so
>

Yep. This was me simply not thinking things 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


Re: [PATCH] pager: turn on "cat" optimization for DEFAULT_PAGER

2013-11-20 Thread Erik Faye-Lund
On Wed, Nov 20, 2013 at 6:30 PM, Jeff King  wrote:
> On Wed, Nov 20, 2013 at 06:24:45PM +0100, Erik Faye-Lund wrote:
>
>> > diff --git a/pager.c b/pager.c
>> > index c1ecf65..fa19765 100644
>> > --- a/pager.c
>> > +++ b/pager.c
>> > @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
>> > pager = getenv("PAGER");
>> > if (!pager)
>> > pager = DEFAULT_PAGER;
>> > -   else if (!*pager || !strcmp(pager, "cat"))
>> > +   if (!*pager || !strcmp(pager, "cat"))
>>
>> Hmmpf. It's sometimes useful to actually pipe through cat rather than
>> disabling the pager, as this changes the return-code from isatty. I
>> sometimes use this for debugging-purposes. Does this patch break that?
>
> My patch should not change the behavior of PAGER=cat, GIT_PAGER=cat,
> core.pager, etc. It should _only_ impact the case where DEFAULT_PAGER is
> set to "cat" (or NULL), and bring it in line with the other cases.
>
> I am not clear on how you are using "cat", so I can't say whether it is
> broken. But if you are doing:
>
>   PAGER=cat git log
>
> that already is a no-op, and that is not changed by my patch. If you
> want to make stdout not a tty, I'd think:
>
>   git log | cat
>
> is the right way to do it (and anyway, when the pager is in effect git
> will pretend that stdout is a tty, since you would still want things
> like auto-color to go to the pager).

You are of course right. Explicitly piping through cat is plenty fine
for my purposes, sorry for disturbing you.
--
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] pager: turn on "cat" optimization for DEFAULT_PAGER

2013-11-20 Thread Junio C Hamano
Erik Faye-Lund  writes:

>> ...
>> is set to empty., 2006-04-16). At that time, the line
>> directly above used:
>>
>>if (!pager)
>>pager = "less";
>>
>> as a fallback, meaning that it could not possibly trigger
>> the optimization. Later, a3d023d (Provide a build time
>> default-pager setting, 2009-10-30) turned that constant into
>> a build-time setting which could be anything, but didn't
>> loosen the "else" to let DEFAULT_PAGER use the optimization.
>>
>> Noticed-by: Dale R. Worley 
>> Suggested-by: Matthieu Moy 
>> Signed-off-by: Jeff King 
>> ---
>>  pager.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/pager.c b/pager.c
>> index c1ecf65..fa19765 100644
>> --- a/pager.c
>> +++ b/pager.c
>> @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
>> pager = getenv("PAGER");
>> if (!pager)
>> pager = DEFAULT_PAGER;
>> -   else if (!*pager || !strcmp(pager, "cat"))
>> +   if (!*pager || !strcmp(pager, "cat"))
>
> Hmmpf. It's sometimes useful to actually pipe through cat rather than
> disabling the pager, as this changes the return-code from isatty. I
> sometimes use this for debugging-purposes. Does this patch break that?

If you have been running "GIT_PAGER=cat git whatever" and the like,
we did not pipe the output through "cat" and this has been the case
for a long time.  The only thing the patch in question changed is
for those who build with

make DEFAULT_PAGER=cat

and I doubt that you have been debugging git by rebuilding it with
such a setting, so


--
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] pager: turn on "cat" optimization for DEFAULT_PAGER

2013-11-20 Thread Jeff King
On Wed, Nov 20, 2013 at 06:24:45PM +0100, Erik Faye-Lund wrote:

> > diff --git a/pager.c b/pager.c
> > index c1ecf65..fa19765 100644
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
> > pager = getenv("PAGER");
> > if (!pager)
> > pager = DEFAULT_PAGER;
> > -   else if (!*pager || !strcmp(pager, "cat"))
> > +   if (!*pager || !strcmp(pager, "cat"))
> 
> Hmmpf. It's sometimes useful to actually pipe through cat rather than
> disabling the pager, as this changes the return-code from isatty. I
> sometimes use this for debugging-purposes. Does this patch break that?

My patch should not change the behavior of PAGER=cat, GIT_PAGER=cat,
core.pager, etc. It should _only_ impact the case where DEFAULT_PAGER is
set to "cat" (or NULL), and bring it in line with the other cases.

I am not clear on how you are using "cat", so I can't say whether it is
broken. But if you are doing:

  PAGER=cat git log

that already is a no-op, and that is not changed by my patch. If you
want to make stdout not a tty, I'd think:

  git log | cat

is the right way to do it (and anyway, when the pager is in effect git
will pretend that stdout is a tty, since you would still want things
like auto-color to go to the pager).

-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: [PATCH] pager: turn on "cat" optimization for DEFAULT_PAGER

2013-11-20 Thread Erik Faye-Lund
On Tue, Sep 3, 2013 at 9:41 AM, Jeff King  wrote:
> On Mon, Sep 02, 2013 at 10:27:48PM -0400, Dale R. Worley wrote:
>
>> > I guess the "else" could and should be dropped. If you do so (and
>> > possibly insert a blank line between the DEFAULT_PAGER case and the
>> > "pager = NULL" case), you get a nice pattern
>> >
>> > if (!pager)
>> > try_something();
>> > if (!pager)
>> > try_next_option();
>>
>> That's true, but it would change the effect of using "cat" as a value:
>> "cat" as a value of DEFAULT_PAGER would cause git_pager() to return
>> NULL, whereas now it causes git_pager() to return "cat".  (All other
>> places where "cat" can be a value are translated to NULL already.)
>>
>> This is why I want to know what the *intended* behavior is, because we
>> might be changing Git's behavior, and I want to know that if we do
>> that, we're changing it to what it should be.  And I haven't seen
>> anyone venture an opinion on what the intended behavior is.
>
> I'll venture my opinion. We should do this:
>
> -- >8 --
> Subject: pager: turn on "cat" optimization for DEFAULT_PAGER
>
> If the user specifies a pager of "cat" (or the empty
> string), whether it is in the environment or from config, we
> automagically optimize it out to mean "no pager" and avoid
> forking at all. We treat an empty pager variable similary.
>
> However, we did not apply this optimization when
> DEFAULT_PAGER was set to "cat" (or the empty string). There
> is no reason to treat DEFAULT_PAGER any differently. The
> optimization should not be user-visible (unless the user has
> a bizarre "cat" in their PATH). And even if it is, we are
> better off behaving consistently between the compile-time
> default and the environment and config settings.
>
> The stray "else" we are removing from this code was
> introduced by 402461a (pager: do not fork a pager if PAGER
> is set to empty., 2006-04-16). At that time, the line
> directly above used:
>
>if (!pager)
>pager = "less";
>
> as a fallback, meaning that it could not possibly trigger
> the optimization. Later, a3d023d (Provide a build time
> default-pager setting, 2009-10-30) turned that constant into
> a build-time setting which could be anything, but didn't
> loosen the "else" to let DEFAULT_PAGER use the optimization.
>
> Noticed-by: Dale R. Worley 
> Suggested-by: Matthieu Moy 
> Signed-off-by: Jeff King 
> ---
>  pager.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pager.c b/pager.c
> index c1ecf65..fa19765 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
> pager = getenv("PAGER");
> if (!pager)
> pager = DEFAULT_PAGER;
> -   else if (!*pager || !strcmp(pager, "cat"))
> +   if (!*pager || !strcmp(pager, "cat"))

Hmmpf. It's sometimes useful to actually pipe through cat rather than
disabling the pager, as this changes the return-code from isatty. I
sometimes use this for debugging-purposes. Does this patch break 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: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Junio C Hamano
Ramsay Jones  writes:

> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Jens,
>
> commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
> from the commit message", 19-11-2013) in 'pu' fails the new test
> it added to t7507.
>
> I didn't spend too long looking at the problem, so take this patch
> as nothing more than a quick suggestion for a possible solution! :-P
> [The err file contained something like: "There was a problem with the
> editor '"$FAKE_EDITOR"'"].
>
> Having said that, this fixes it for me ...

Well spotted.  test_must_fail being a shell function, not a command,
we shouldn't have used the "VAR=val cmd" one-shot environment
assignment for portability.

Even though this happens to be the last test in the script, using
test_set_editor to permanently affect the choice of editor for tests
that follow is not generally a good idea.  It would be safer to do
this, I would have to say:

git commit -a -m "submodule commit"
) &&
(
GIT_EDITOR=cat &&
export GIT_EDITOR &&
test_must_fail git commit -a -v 2>err
) &&
test_i18ngrep "Aborting ..."

Thanks.

> ATB,
> Ramsay Jones
>
>  t/t7507-commit-verbose.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 09c1150..49cfb3c 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -79,7 +79,8 @@ test_expect_success 'submodule log is stripped out too with 
> -v' '
>   echo "more" >>file &&
>   git commit -a -m "submodule commit"
>   ) &&
> - GIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&
> + test_set_editor cat &&
> + test_must_fail git commit -a -v 2>err &&
>   test_i18ngrep "Aborting commit due to empty commit message." err
>  '
--
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] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

2013-11-20 Thread Ramsay Jones

Signed-off-by: Ramsay Jones 
---

Hi Jens,

commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.

I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].

Having said that, this fixes it for me ...

ATB,
Ramsay Jones

 t/t7507-commit-verbose.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 09c1150..49cfb3c 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -79,7 +79,8 @@ test_expect_success 'submodule log is stripped out too with 
-v' '
echo "more" >>file &&
git commit -a -m "submodule commit"
) &&
-   GIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&
+   test_set_editor cat &&
+   test_must_fail git commit -a -v 2>err &&
test_i18ngrep "Aborting commit due to empty commit message." err
 '
 
-- 
1.8.4
--
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: Bizarre git merge behaviour

2013-11-20 Thread Jakub Narębski

W dniu 2013-11-20 13:03, Matthew Cengia pisze:

On 2013-11-20 08:20, Johannes Sixt wrote:
[...]

Not really. It's impossible to tell what's wrong if you


Hi Hannes,

Thanks for your response, and sorry for providing insufficient
information; this is a company Git repo (it's also about 200MB), so I've
got be careful what I post, but I can certainly give more than I've
shown already.


I think there is some anonymizing tool for git, which replaces data
(blobs contents and file names) with artificial names, while preserving
history.  It was intended to help debug repos with proprietary data...
but unfortunately I have not bookmarked it (or lost bookmark).

[...]

I'm truly stumped. It's also worth noting that I've gone through and
manually resolved this merge one file at a time, and I'm about 90% sure
I ended up with the correct result, but it'd be nice to have had the
merge do the right thing in the first place, and I obviously want to
avoid having to do this again in a few months' time.


Well, there is git-rerere (reuse recorded resolutions) which records 
file-level (textual) merge resolutions and tries to reapply if 
appropriate when merging.


Perhaps git-imerge tool would be of help in complicated merges?

--
Jakub Narębski



--
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 01/10] test-chmtime: Fix exit code on Windows

2013-11-20 Thread Erik Faye-Lund
On Fri, Jun 7, 2013 at 10:53 PM, Johannes Sixt  wrote:
> MinGW's bash does not recognize an exit code -1 as failure. See also
> 47e3de0e (MinGW: truncate exit()'s argument to lowest 8 bits) and 2488df84
> (builtin run_command: do not exit with -1). Exit code 1 is good enough.
>
> Signed-off-by: Johannes Sixt 
> ---
>  test-chmtime.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/test-chmtime.c b/test-chmtime.c
> index 02b42ba..2e601a8 100644
> --- a/test-chmtime.c
> +++ b/test-chmtime.c
> @@ -84,7 +84,7 @@ int main(int argc, const char *argv[])
> if (stat(argv[i], &sb) < 0) {
> fprintf(stderr, "Failed to stat %s: %s\n",
> argv[i], strerror(errno));
> -   return -1;
> +   return 1;
> }
>
>  #ifdef WIN32
> @@ -92,7 +92,7 @@ int main(int argc, const char *argv[])
> chmod(argv[i], sb.st_mode | S_IWUSR)) {
> fprintf(stderr, "Could not make user-writable %s: %s",
> argv[i], strerror(errno));
> -   return -1;
> +   return 1;
> }
>  #endif
>
> @@ -107,7 +107,7 @@ int main(int argc, const char *argv[])
> if (utb.modtime != sb.st_mtime && utime(argv[i], &utb) < 0) {
> fprintf(stderr, "Failed to modify time on %s: %s\n",
> argv[i], strerror(errno));
> -   return -1;
> +   return 1;
> }
> }
>
> @@ -115,5 +115,5 @@ int main(int argc, const char *argv[])
>
>  usage:
> fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
> -   return -1;
> +   return 1;
>  }
> --
> 1.8.3.rc1.32.g8b61cbb
>
> --
> 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

Hmmm. Perhaps we should do something like this?

diff --git a/compat/mingw.h b/compat/mingw.h
index 1f9ab5f..cbee8bd 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -462,7 +462,7 @@ static int mingw_main(c,v); \
 int main(c,v) \
 { \
  mingw_startup(); \
- return mingw_main(__argc, __argv); \
+ return mingw_main(__argc, __argv) & 0xff; \
 } \
 static int mingw_main(c,v)
--
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: write() _will_ fail on Mac OS X/XNU if nbytes > INT_MAX

2013-11-20 Thread Torsten Bögershausen
Hej,
I think the patch went in and out in git.git, please see below.

(I coudn't the following  in msysgit,
 but if it was there, the clipped_write() for Windows could go away.

/Torsten



commit 0b6806b9e45c659d25b87fb5713c920a3081eac8
Author: Steffen Prohaska 
Date:   Tue Aug 20 08:43:54 2013 +0200

xread, xwrite: limit size of IO to 8MB

Checking out 2GB or more through an external filter (see test) fails
on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed

The reason is that read() immediately returns with EINVAL when asked
to read more than 2GB.  According to POSIX [1], if the value of
nbyte passed to read() is greater than SSIZE_MAX, the result is
implementation-defined.  The write function has the same restriction
[2].  Since OS X still supports running 32-bit executables, the
32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also
imposed on 64-bit executables under certain conditions.  For write,
the problem has been addressed earlier [6c642a].

Address the problem for read() and write() differently, by limiting
size of IO chunks unconditionally on all platforms in xread() and
xwrite().  Large chunks only cause problems, like causing latencies
when killing the process, even if OS X was not buggy.  Doing IO in
reasonably sized smaller chunks should have no negative impact on
performance.

The compat wrapper clipped_write() introduced earlier [6c642a] is
not needed anymore.  It will be reverted in a separate commit.  The
new test catches read and write problems.

Note that 'git add' exits with 0 even if it prints filtering errors
to stderr.  The test, therefore, checks stderr.  'git add' should
probably be changed (sometime in another commit) to exit with
nonzero if filtering fails.  The test could then be changed to use
test_must_fail.


On 2013-11-20 11.15, Erik Faye-Lund wrote:
> I know I'm extremely late to the party, and this patch has already
> landed, but...
> 
> On Sat, May 11, 2013 at 1:05 AM, Junio C Hamano  wrote:
>> Filipe Cabecinhas  writes:
>>
>>> Due to a bug in the Darwin kernel, write() calls have a maximum size of
>>> INT_MAX bytes.
>>>
>>> This patch introduces a new compat function: clipped_write
>>> This function behaves the same as write() but will write, at most, INT_MAX
>>> characters.
>>> It may be necessary to include this function on Windows, too.
> 
> We are already doing something similar for Windows in mingw_write (see
> compat/mingw.c), but with a much smaller size.
> 
> It feels a bit pointless to duplicate this logic.
> 
>> diff --git a/compat/clipped-write.c b/compat/clipped-write.c
>> new file mode 100644
>> index 000..9183698
>> --- /dev/null
>> +++ b/compat/clipped-write.c
>> @@ -0,0 +1,13 @@
>> +#include 
>> +#include 
>> +
>> +/*
>> + * Version of write that will write at most INT_MAX bytes.
>> + * Workaround a xnu bug on Mac OS X
>> + */
>> +ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)
>> +{
>> +   if (nbyte > INT_MAX)
>> +   nbyte = INT_MAX;
>> +   return write(fildes, buf, nbyte);
>> +}
> 
> If we were to reuse this logic with Windows, we'd need to have some
> way of overriding the max-size of the write.
> --
> 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


Re: Check out doesn't set x-flag on CIFS

2013-11-20 Thread Erik Faye-Lund
Sigh, it seems replying from Gmail on my phone culled the CC-list.
Sorry about that, and here's the rest of the discussion in case
someone else is interested:

On Wed, Nov 20, 2013 at 1:38 PM, Andre Esser  wrote:
> On 2013-11-20 12:23, Erik Faye-Lund wrote:
>> On Wed, Nov 20, 2013 at 12:59 PM, Andre Esser  
>> wrote:
>>>
>>> There is with POSIX extensions, and git recognises it correctly when an
>>> executable file is being committed, ie the file's exec flag is set in
>>> the repository.
>>>
>>> If that file is then however checked out to the same CIFS file system,
>>> the exec flag is not set, which seems a least inconsistent.
>>
>> OK, in that case, core.filemode probably leads down the wrong path. In
>> fact, shouldn't everything just work? I mean, normal POSIX calls
>> should behave as ususal, no? Perhaps this is a Samba issue?
>
> We've had a look at the GIT source code (after my original post) and it
> seems that after GIT identifies the file system as CIFS, it doesn't even
> try to set the x-flag. Given POSIX extensions have been around for years
> it would probably be a good idea to change GIT's behaviour. But I'm no
> developer and we don't have the spare manpower in the company to look
> into this any deeper.
>
> Thanks for your replies though!
>
> Andre
--
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: How to put tree into index

2013-11-20 Thread Johannes Sixt
Am 11/20/2013 12:47, schrieb Alexander GQ Gerasiov:
> 1. I have repository with tree like this:
> 
> dir1/
>   file1
>   file2
>   file3
> 
> dir2/
>   subdir1/
>   some files
> 
> 
> 2. Current branch is B.
> 
> 3. I want to get dir1 from branch A, and save it's content on current
> branch (B) as dir2/subdir1

> So my question is
> How to put into index tree-object with known sha1 and given name?

git rm -r --cached dir2/subdir1 &&
git read-tree --prefix=dir2/subdir1/ A:dir1

Note the trailing slash.

> PS I was able to do what I need when copied files, not tree-itself.
> Just add -r to git ls-tree, and put into index blobs/files, not tree.
> But I'm interested: is it possible to put tree-object into index?

No, because the index does not store trees, only blobs.

-- 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: Bizarre git merge behaviour

2013-11-20 Thread Matthew Cengia
On 2013-11-20 13:29, Johannes Sixt wrote:
[...]
> That's strange. I can't tell what is going on. Perhaps you have some
> criss-cross merges in your history that merge-recursive trips over?

That's as good a guess as any, but I suspect tracking it down may
involve needles and haystacks...

> 
> Sorry, I don't know how to help you further.

No worries, thanks very much for your help so far nonetheless.

-- 
Regards,
Matthew Cengia


signature.asc
Description: Digital signature


Re: Bizarre git merge behaviour

2013-11-20 Thread Johannes Sixt
Am 11/20/2013 13:03, schrieb Matthew Cengia:
> The only changes I expect are these:
> 
> mattcen@sonar:prisonpc(wtf)$ git --no-pager diff --numstat --oneline
> \ "$(git merge-base wtf origin/22869-new-kernel)"
> origin/22869-new-kernel 37  0   client/kernel/README 2797
> 0   client/kernel/config-3.5.7.20-1 0   3525
> client/kernel/config-3.5.7.21-1 -   -
> client/kernel/linux-firmware-image_3.5.7.21-1_i386.deb -   -
> client/kernel/linux-image-3.5.7.20_3.5.7.20-1_i386.deb -   -
> client/kernel/linux-image-3.5.7.21_3.5.7.21-1_i386.deb 1   0
> client/scm/20-security.scm 6   7
> client/scm/50-kernel-prisoner.scm 1   0
> client/scm/50-staff.scm 22  0
> doc/user_acceptance_tests/README 581 0
> doc/user_acceptance_tests/default.css 15480
> doc/user_acceptance_tests/user_acceptance_tests.html 42680
> doc/user_acceptance_tests/user_acceptance_tests.pdf 940 0
> doc/user_acceptance_tests/user_acceptance_tests.rst

> mattcen@sonar:prisonpc(wtf)$ git log --left-right --oneline \ 
> wtf...origin/22869-new-kernel ppcadm/modules/quarantine.py | cut
> -c1-70 < 5b5f552 Fixed msg vs msg.as_string because smtp.sendmail is
> picky < 03618f2 Use Return-Path, not X-Original-Envelope-From. <
> dc1169e Bug squashing. < 8e20216 Changed quarantine module to run
> maybe and send directly to < d79ad42 Change final port, name the
> magic number. < e44d1b3 Merge remote-tracking branch 'origin/staging'
> into 22912-ppc < a03db6d Modified quarantine UI to handle the new
> reports format

So, ppcadm/modules/quarantine.py changed only on our side, but not on
their side. This is in accordance with your listing above. Nevertheless,
the file is marked conflicted in your git status --porcelain output in
the original post:

UU ppcadm/modules/quarantine.py

That's strange. I can't tell what is going on. Perhaps you have some
criss-cross merges in your history that merge-recursive trips over?

Sorry, I don't know how to help you further.

-- 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: How to put tree into index

2013-11-20 Thread Matthew Cengia
On 2013-11-20 15:47, Alexander GQ Gerasiov wrote:
[...]
> So my question is
> How to put into index tree-object with known sha1 and given name?

I was just reading about something very similar in the Git book:
http://git-scm.com/book/en/Git-Internals-Git-Objects#Tree-Objects
(read the entire Tree Objects section), and then came up with this,
which I think does what you want except it doesn't erase the previous
contents of dir2/subdir1:

mattcen@wasp:repo$ ls -la
total 8
drwxrwxr-x 2 mattcen mattcen 4096 Nov 20 23:12 .
drwx-- 3 mattcen mattcen 4096 Nov 20 23:12 ..

mattcen@wasp:repo$ git init
Initialized empty Git repository in /tmp/with-temp-dir.ZDw19F/repo/.git/

mattcen@wasp:repo(master)$ touch .gitignore

mattcen@wasp:repo(master)$ git add .gitignore

mattcen@wasp:repo(master)$ git commit -m Initial\ commit
[master (root-commit) 43d7ee6] Initial commit
 0 files changed
 create mode 100644 .gitignore

mattcen@wasp:repo(master)$ mkdir -p dir1 dir2/subdir1

mattcen@wasp:repo(master)$ touch dir1/file{1..3} dir2/subdir1/some\ files

mattcen@wasp:repo(master)$ git add -A

mattcen@wasp:repo(master)$ git commit -m Add\ files
[master 2654d91] Add files
 0 files changed
 create mode 100644 dir1/file1
 create mode 100644 dir1/file2
 create mode 100644 dir1/file3
 create mode 100644 dir2/subdir1/some files

mattcen@wasp:repo(master)$ git branch A

mattcen@wasp:repo(master)$ git checkout -b B
Switched to a new branch 'B'

mattcen@wasp:repo(B)$ git cat-file -p A^{tree} | grep dir1
04 tree 9599fdeb034597d90d72d2f58396dee096885b79dir1

mattcen@wasp:repo(B)$ git read-tree --prefix=dir2/subdir1 
9599fdeb034597d90d72d2f58396dee096885b79

mattcen@wasp:repo(B)$ git write-tree
e1d5abac94058d1f321a3aa087f18d796efc8a0e

mattcen@wasp:repo(B)$ git checkout .

mattcen@wasp:repo(B)$ find *
dir1
dir1/file3
dir1/file1
dir1/file2
dir2
dir2/subdir1
dir2/subdir1/some files
dir2/subdir1/file3
dir2/subdir1/file1
dir2/subdir1/file2


Hope this helps.

-- 
Regards,


signature.asc
Description: Digital signature


Re: Bizarre git merge behaviour

2013-11-20 Thread Matthew Cengia
On 2013-11-20 08:20, Johannes Sixt wrote:
[...]
> Not really. It's impossible to tell what's wrong if you

Hi Hannes,

Thanks for your response, and sorry for providing insufficient
information; this is a company Git repo (it's also about 200MB), so I've
got be careful what I post, but I can certainly give more than I've
shown already.

> 
> - show only ..topic
> - but not topic..

I won't show the same (git log) output as I did previously, because
there are lots of commits and is a bit difficult to read (but can do so
as an attachment if you genuinely think it may be useful), but here's a
diff from wtf to merge-base:

mattcen@sonar:prisonpc(wtf)$ git --no-pager diff --numstat --oneline \
"$(git merge-base wtf origin/22869-new-kernel)" wtf
1   1   .gitignore
1   1   bios-monitoring/bios-alert
4   4   bios-monitoring/check_bios.sh
1   7   bios-monitoring/ppc-bios.conf
3   0   client/scm/20-security.scm
14  0   client/scm/30-apps.scm
5   10  client/scm/30-branding.scm
2   7   client/scm/30-password-reset.scm
7   9   client/scm/50-kernel-prisoner.scm
7   9   client/scm/50-kernel-staff.scm
0   10  client/scm/50-prisoner.scm
4   1   client/transmute
2   1   client/transmute.scm.in
0   1   config-files-internet.txt
66  1   debian/NEWS
24  0   debian/changelog
6   3   debian/control
0   13  debian/postinst-cfg.sh
0   6   debian/prisonpc-core.config
8   5   debian/prisonpc-core.cron.d
12  6   debian/prisonpc-core.cron.daily
2   1   debian/prisonpc-core.install
35  94  debian/prisonpc-core.postinst
25  0   debian/prisonpc-core.ppcadm-yuk.init
0   4   debian/prisonpc-core.ppcadm.upstart
14  0   debian/prisonpc-core.preinst
27  0   debian/prisonpc-core.squid-yuk.init
0   11  debian/prisonpc-core.templates
16  2   debian/prisonpc-dev.cron.weekly
6   6   debian/prisonpc-dev.install
7   0   debian/prisonpc-dev.postinst
1   0   debian/prisonpc-internet.cron.daily
6   5   debian/prisonpc-internet.install
4   19  debian/prisonpc-internet.postinst
7   1   debian/rules
11  10  disc/disc_access
7   0   doc/Test_plan.txt
11  0   doc/Vectors.txt
-   -   doc/adminguide/images/streaming_media-screen.jpg
-   -   doc/adminguide/images/streaming_media-screen.png
-   -   doc/adminguide/images/web_filtering.png
8   17  doc/adminguide/streaming_media.rst
8   3   doc/adminguide/system-status.rst
13  16  doc/adminguide/web_filtering.rst
127 0   doc/current-mail-status.txt
55  0   doc/prisonpc-mail-plan.dot
90  0   doc/prisonpc-mail.dot
2   0   doc/user_acceptance_tests/.gitignore
22  0   doc/user_acceptance_tests/README
581 0   doc/user_acceptance_tests/default.css
10110   doc/user_acceptance_tests/user_acceptance_tests.rst
2   2   dovecot/dovecot-staff.conf
0   579 dovecot/email_quarantine
8   5   eric/apps/complain.py
5   3   eric/apps/links.py
7   3   eric/apps/media.py
5   3   eric/apps/whitelist.py
4   0   eric/eric
1   0   eric/eric_cfg.py
48  0   generate-mail-stats
12  2   post-install.sh
10  0   postfix/alias-list
2   0   postfix/by-week.procmailrc
0   11  postfix/flush-queued-mail
2   0   postfix/invariant.procmailrc
49  67  postfix/main.cf.in
3   36  postfix/master-basic.cf.in
51  144 postfix/master-filter.cf.in
145 156 postfix/maxwell
0   15  postfix/maxwell.cfg.in
31  0   postfix/maybe
76  0   postfix/plainify
0   0   postfix/plainify.d/cur/00-nothing
6   0   postfix/plainify.d/cur/01-nonmime
12  0   postfix/plainify.d/cur/02-th
8   0   postfix/plainify.d/cur/02-tp
35761   0   postfix/plainify.d/cur/f0-many-types
832 0   postfix/plainify.d/cur/f0-rfc822
244 0   postfix/plainify.d/cur/f0-th_tbird_cascade
0   167 postfix/smtp_archiver
0   240 postfix/strip_quarantine_markers
119 248 postfix/whitelist
85  37  ppcadm/modules/disc_access.py
80  0   ppcadm/modules/emailstats.py
8   7   ppcadm/modules/mailfilter.py
130 49  ppcadm/modules/media.py
58  74  ppcadm/modules/quarantine.py
87  37  ppcadm/modules/wwwfilter.py
3   0   ppcadm/resources/admin.css
31  0   ppcadm/templates/disc_access.tpl
5   0   ppcadm/templates/disc_summary.tpl
7   6   ppcadm/templates/home.tpl
2   2   ppcadm/templates/mailfilter_menu.tpl
1   2   ppcadm/templates/quarantine_list.tpl
33  17  ppcadm/templates/quarantine_mail.tpl
67  0   ppcadm/templates/streaming_media.tpl
3   3   ppcadm/templates/us

Re: Check out doesn't set x-flag on CIFS

2013-11-20 Thread Andre Esser
On 2013-11-20 10:51, Erik Faye-Lund wrote:
> On Wed, May 29, 2013 at 4:16 PM, Andre Esser  
> wrote:
>> Hello,
>>
>> When on a CIFS filesystem a git checkout does not replicate the executable
>> flag from the repository:
>>
>>   $ git clone git://git/abettersqlplus
>>   Cloning into 'abettersqlplus'...
>>   remote: Counting objects: 522, done.
>>   remote: Compressing objects: 100% (342/342), done.
>>   remote: Total 522 (delta 166), reused 522 (delta 166)
>>   Receiving objects: 100% (522/522), 82.40 KiB, done.
>>   Resolving deltas: 100% (166/166), done.
>>   $ ls -l abettersqlplus/absp.py
>>   -rw-rw-r-- 1 aesser geneity 45860 May 29 14:46 abettersqlplus/absp.py
>>
>>
>> Subsequently git status reports the file as changed:
>>
>>   $ cd abettersqlplus/
>>   $ git status
>>   # On branch master
>>   # Changes not staged for commit:
>>   #   (use "git add ..." to update what will be committed)
>>   #   (use "git checkout -- ..." to discard changes in working
>>   directory)
>>   #
>>   #modified:   absp.py
>>   #
>>   no changes added to commit (use "git add" and/or "git commit -a")
>>
>>
>> If I set the x-flag manually, all is well:
>>
>>   $ chmod +x absp.py
>>   $ git status
>>   # On branch master
>>   nothing to commit (working directory clean)
>>
>>
>> This problem doesn't occur on ext3 or NFS file systems. Client is Ubuntu
>> 12.04 with git version 1.7.9.5. CIFS is exported from Ubuntu 12.04 with
>> Samba version 3.6.3.
>>
>> Since git recognises the x-flag on this CIFS file system, shouldn't it also
>> be able to set it on checkout?
>>
> 
> You might want to check out the core.filemode configuration variable.

Thanks, but unfortunately that only suppresses the error message, it
still doesn't set the x-flag when it should.

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


How to put tree into index

2013-11-20 Thread Alexander GQ Gerasiov
Hello there.

I need to realize the following scenario with git.

1. I have repository with tree like this:

dir1/
file1
file2
file3

dir2/
subdir1/
some files


2. Current branch is B.

3. I want to get dir1 from branch A, and save it's content on current
branch (B) as dir2/subdir1



>From my point I should do the following

1. Get tree-object sha1 of dir1 from A with git ls-tree
2. Put into index new tree-object with the same sha1/content, but under
the name dir2/subdir1
3. Commit index.


But it doesn't work for me. It works with blob object, but not with
tree object.

I tried the following commands:

git ls-tree A dir1 | 
sed 's#dir1#dir2/subdir1#' | 
git update-index --index-info

But after this I see in the index not tree-object with mode 04, but
commit-object with mode 16.



So my question is
How to put into index tree-object with known sha1 and given name?


PS I was able to do what I need when copied files, not tree-itself.
Just add -r to git ls-tree, and put into index blobs/files, not tree.
But I'm interested: is it possible to put tree-object into index?
--
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: Check out doesn't set x-flag on CIFS

2013-11-20 Thread Erik Faye-Lund
On Wed, May 29, 2013 at 4:16 PM, Andre Esser  wrote:
> Hello,
>
> When on a CIFS filesystem a git checkout does not replicate the executable
> flag from the repository:
>
>   $ git clone git://git/abettersqlplus
>   Cloning into 'abettersqlplus'...
>   remote: Counting objects: 522, done.
>   remote: Compressing objects: 100% (342/342), done.
>   remote: Total 522 (delta 166), reused 522 (delta 166)
>   Receiving objects: 100% (522/522), 82.40 KiB, done.
>   Resolving deltas: 100% (166/166), done.
>   $ ls -l abettersqlplus/absp.py
>   -rw-rw-r-- 1 aesser geneity 45860 May 29 14:46 abettersqlplus/absp.py
>
>
> Subsequently git status reports the file as changed:
>
>   $ cd abettersqlplus/
>   $ git status
>   # On branch master
>   # Changes not staged for commit:
>   #   (use "git add ..." to update what will be committed)
>   #   (use "git checkout -- ..." to discard changes in working
>   directory)
>   #
>   #modified:   absp.py
>   #
>   no changes added to commit (use "git add" and/or "git commit -a")
>
>
> If I set the x-flag manually, all is well:
>
>   $ chmod +x absp.py
>   $ git status
>   # On branch master
>   nothing to commit (working directory clean)
>
>
> This problem doesn't occur on ext3 or NFS file systems. Client is Ubuntu
> 12.04 with git version 1.7.9.5. CIFS is exported from Ubuntu 12.04 with
> Samba version 3.6.3.
>
> Since git recognises the x-flag on this CIFS file system, shouldn't it also
> be able to set it on checkout?
>

You might want to check out the core.filemode configuration variable.
--
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: write() _will_ fail on Mac OS X/XNU if nbytes > INT_MAX

2013-11-20 Thread Erik Faye-Lund
I know I'm extremely late to the party, and this patch has already
landed, but...

On Sat, May 11, 2013 at 1:05 AM, Junio C Hamano  wrote:
> Filipe Cabecinhas  writes:
>
>> Due to a bug in the Darwin kernel, write() calls have a maximum size of
>> INT_MAX bytes.
>>
>> This patch introduces a new compat function: clipped_write
>> This function behaves the same as write() but will write, at most, INT_MAX
>> characters.
>> It may be necessary to include this function on Windows, too.

We are already doing something similar for Windows in mingw_write (see
compat/mingw.c), but with a much smaller size.

It feels a bit pointless to duplicate this logic.

> diff --git a/compat/clipped-write.c b/compat/clipped-write.c
> new file mode 100644
> index 000..9183698
> --- /dev/null
> +++ b/compat/clipped-write.c
> @@ -0,0 +1,13 @@
> +#include 
> +#include 
> +
> +/*
> + * Version of write that will write at most INT_MAX bytes.
> + * Workaround a xnu bug on Mac OS X
> + */
> +ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)
> +{
> +   if (nbyte > INT_MAX)
> +   nbyte = INT_MAX;
> +   return write(fildes, buf, nbyte);
> +}

If we were to reuse this logic with Windows, we'd need to have some
way of overriding the max-size of the write.
--
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