Re: [PATCH] pretend_sha1_file(): Change return type from int to void

[Tobias Klauser]

On 2015-10-07 at 23:22:59 +0200, Junio C Hamano  wrote:
> Johannes Schindelin  writes:
> 
> > As to the patch, I cannot speak for Junio, of course, but my
> > preference would be to keep the return type. Traditionally, functions
> > that can fail either die() or return an int; non-zero indicates an
> > error. In this case, it seems that we do not have any condition
> > (yet...) under which an error could occur. It does not seem very
> > unlikely that we may eventually have such conditions, though, hence my
> > preference.
> 
> Perhaps the attached is a better approach.
> 
> Even though the current implementation of "pretend" implementation
> does not, future generations are allowed to make pretend_sha1_file()
> return failure when appropriate.

For my original patch I didn't consider that pretend_sha1_file() might
return failure in the future. I was just confused by the fact that the
return value was seemingly useless (but now I realize that unused !=
useless ;-), sorry for the noise.

Please disregard my patch and apply yours instead, if you see fit.

> 
>  builtin/blame.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 203a981..fa24f8f 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2362,7 +2362,8 @@ static struct commit *fake_working_tree_commit(struct 
> diff_options *opt,
>   convert_to_git(path, buf.buf, buf.len, , 0);
>   origin->file.ptr = buf.buf;
>   origin->file.size = buf.len;
> - pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
> + if (pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1))
> + die("failed to create a fake commit for the working tree 
> version.");
>  
>   /*
>* Read the current index, replace the path entry with
> 
--
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] GIT_INDEX environment variable ignored?

[Michael J Gruber]

McAuley, Ben venit, vidit, dixit 08.10.2015 06:48:
> Hello,
> 
> I was trying to use multiple indexes earlier, and ran into an issue which 
> I've summarised into a test case:
> 
> $ git init
> $ touch file1 && git add file1 && git commit -m "file1"
> $ git branch release 
> $ touch file2 && git add file2 && git commit -m "file2"
> $ cp .git/index .git/indexMaster
> $ git checkout release
> $ touch file3 && git add file3 && git commit -m "file3"
> 
> I then ran ls-files with the --stage option to look at what the index 
> contains.
> As expected file1 and file3 are present, we're on the 'release' branch still.
> 
> $ git ls-files --stage
> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   file1
> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   file3
> 
> However when I run the same command again, this time using the 
> GIT_INDEX env variable to provide the index I previously saved on master,
> I don't see file2 like I'd expect...
> 
> $ GIT_INDEX=.git/indexMaster git ls-files --stage
> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   file1
> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   file3
> 
> Is this something going wrong, or am I misunderstanding the role of the 
> index/GIT_INDEX variable?
> 
> Replicated on 2.5.0.windows.1 and 2.6.1 (Linux).

Maybe try GIT_INDEX_FILE instead of GIT_INDEX? ;)

Michael

--
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 07/10] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

[Karthik Nayak]

Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.

This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.

Make changes to t/t6300-for-each-ref.sh to reflect this change.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c| 4 +++-
 t/t6300-for-each-ref.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 38e4424..6a38089 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)
char buf[40];
 
if (stat_tracking_info(branch, _ours,
-  _theirs, NULL))
+  _theirs, NULL)) {
+   v->s = "[gone]";
continue;
+   }
 
if (!num_ours && !num_theirs)
v->s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 6fc569e..4f620bf 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -359,7 +359,7 @@ test_expect_success 'Check that :track[short] cannot be 
used with other atoms' '
 
 test_expect_success 'Check that :track[short] works when upstream is invalid' '
cat >expected <<-\EOF &&
-
+   [gone]
 
EOF
test_when_finished "git config branch.master.merge refs/heads/master" &&
-- 
2.6.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

[PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Karthik Nayak]

Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").

Add test and documentation for the same.
---
 Documentation/git-for-each-ref.txt |  6 --
 ref-filter.c   | 28 +++-
 t/t6300-for-each-ref.sh|  9 +
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c713ec0..a38cbf6 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,8 +114,10 @@ upstream::
`refname` above.  Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it.
+   or "=" (in sync).  Append `:track,nobracket` to show tracking
+   information without brackets (i.e "ahead N, behind M").  Has
+   no effect if the ref does not have tracking information
+   associated with it.
 
 push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 6a38089..6044eb0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else if (!strcmp(formatp, "track") &&
+   else if (skip_prefix(formatp, "track", ) &&
+strcmp(formatp, "trackshort") &&
 (starts_with(name, "upstream") ||
  starts_with(name, "push"))) {
char buf[40];
+   unsigned int nobracket = 0;
+
+   if (!strcmp(valp, ",nobracket"))
+   nobracket = 1;
 
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   v->s = "[gone]";
+   if (nobracket)
+   v->s = "gone";
+   else
+   v->s = "[gone]";
continue;
}
 
if (!num_ours && !num_theirs)
v->s = "";
else if (!num_ours) {
-   sprintf(buf, "[behind %d]", num_theirs);
+   if (nobracket)
+   sprintf(buf, "behind %d", 
num_theirs);
+   else
+   sprintf(buf, "[behind %d]", 
num_theirs);
v->s = xstrdup(buf);
} else if (!num_theirs) {
-   sprintf(buf, "[ahead %d]", num_ours);
+   if (nobracket)
+   sprintf(buf, "ahead %d", 
num_ours);
+   else
+   sprintf(buf, "[ahead %d]", 
num_ours);
v->s = xstrdup(buf);
} else {
-   sprintf(buf, "[ahead %d, behind %d]",
+   if (nobracket)
+   sprintf(buf, "ahead %d, behind 
%d",
+   num_ours, num_theirs);
+   else
+   sprintf(buf, "[ahead %d, behind 
%d]",
num_ours, num_theirs);
v->s = xstrdup(buf);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 4f620bf..7ab8bf8 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '
 '
 
 cat >expected expected <
 EOF
 
-- 
2.6.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] filter-branch: strip pgp signature in commit messages

[Michael J Gruber]

James McCoy venit, vidit, dixit 08.10.2015 07:01:
> df062010 (filter-branch: avoid passing commit message through sed)
> introduced a regression when filtering gpg signed commits.  The gpgsig
> header is multi-line and contains an empty line.  Although the signature
> is indented, making the line a whitespace only line, this still results
> in $header_line being empty, causing the “skip header lines” loop to
> exit.
> 
> The rest of the commit object is then re-used as the rewritten commit
> message, causing the new message to include the signature of the
> original commit.

I had to wrap my head around that commit message quite a bit and ended
up testing the issue myself. So the catch is:

df062010 (filter-branch: avoid passing commit message through sed)
introduced a regression when filtering gpg signed commits. As a
consequence, "filter-branch --msg-filter cat" (which should leave the
commit message unchanged) spills the signature (without the BEGIN line,
but with the END line) into (in front of) the original commit message.

The reason is that although...

... original commit.

Fix this by keeping track of the in/out-of signature state when parsing
the header lines.


[No, this does not alleviate my dislike for the commit signature
implementation, and I have not checked the patch - the test looks good
to me, though.]

> Signed-off-by: James McCoy 
> ---
>  git-filter-branch.sh | 14 +++---
>  t/t7003-filter-branch.sh | 14 ++
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 5b3f63d..dd49b13 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -347,10 +347,18 @@ while read commit parents; do
>   fi
>  
>   {
> - while read -r header_line && test -n "$header_line"
> + while read -r header_line &&
> + ( test -n "$header_line" || test -n "$gpg_signature" )
>   do
> - # skip header lines...
> - :;
> + # skip header lines... but track whether we are in a
> + # PGP signature, since it will have a whitespace only
> + # line which causes $header_line to be empty
> + if [ "${header_line#gpgsig}" != "$header_line" ]; then
> + gpg_signature=1
> + elif test -n "$gpg_signature" &&
> + expr "$header_line" : ".*END PGP" >/dev/null; 
> then
> + gpg_signature=
> + fi
>   done
>   # and output the actual commit message
>   cat
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 855afda..377c648 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -2,6 +2,7 @@
>  
>  test_description='git filter-branch'
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-gpg.sh"
>  
>  test_expect_success 'setup' '
>   test_commit A &&
> @@ -292,6 +293,19 @@ test_expect_success 'Tag name filtering strips gpg 
> signature' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success GPG 'Filtering retains message of gpg signed commit' '
> + mkdir gpg &&
> + touch gpg/foo &&
> + git add gpg &&
> + test_tick &&
> + git commit -S -m "Adding gpg" &&
> +
> + git log -1 --format="%s" > expect &&
> + git filter-branch -f --msg-filter "cat" &&
> + git log -1 --format="%s" > actual &&
> + test_cmp expect actual
> +'
> +
>  test_expect_success 'Tag name filtering allows slashes in tag names' '
>   git tag -m tag-with-slash X/1 &&
>   git cat-file tag X/1 | sed -e s,X/1,X/2, > expect &&
> 

--
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 00/10] port branch.c to use ref-filter's printing options

[Karthik Nayak]

This is part of unification of the commands 'git tag -l, git branch -l
and git for-each-ref'. This ports over branch.c to use ref-filter's
printing options. The previous version of the patch can be found at:
http://article.gmane.org/gmane.comp.version-control.git/278926

Karthik Nayak (10):
  ref-filter: implement %(if), %(then), and %(else) atoms
  ref-filter: implement %(if:equals=) and
%(if:notequals=)
  ref-filter: add support for %(refname:dir) and %(refname:base)
  ref-filter: modify "%(objectname:short)" to take length
  ref-filter: adopt get_head_description() from branch.c
  ref-filter: introduce format_ref_array_item()
  ref-filter: make %(upstream:track) prints "[gone]" for invalid
upstreams
  ref-filter: add support for %(upstream:track,nobracket)
  branch: use ref-filter printing APIs
  branch: implement '--format' option

 Documentation/git-branch.txt   |   7 +-
 Documentation/git-for-each-ref.txt |  63 +++-
 builtin/branch.c   | 265 +-
 ref-filter.c   | 285 +
 ref-filter.h   |   5 +
 t/t3203-branch-output.sh   |  11 ++
 t/t6040-tracking-info.sh   |   2 +-
 t/t6300-for-each-ref.sh|  33 -
 t/t6302-for-each-ref-filter.sh | 132 +
 9 files changed, 573 insertions(+), 230 deletions(-)

Changes in this version:

1. Use %(refname:dir) and %(refname:base) instead of %(path) and
%(path:short).
2. Make it %(objectname:short=) rather than %(objectname:short,).
3. Add %(upstream:track,nobracket) which removes the brackets from tracking
information.
4. calc_maxwidth() now also considers head ref descriptions, this wasn't done
in the last patch and would cause detached head description to be not aligned 
with the other ref-names.
5. remove the if_atom field in if_then_else struct.
6. introduce better error checking for the %(if), %(then), %(else) atoms
as suggested by Matthieu (when used along with %(align) atom and so on).
7. refactor code and improve documentation.

Interdiff:

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 5097915..a38cbf6 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -92,7 +92,9 @@ refname::
The name of the ref (the part after $GIT_DIR/).
For a non-ambiguous short name of the ref append `:short`.
The option core.warnAmbiguousRefs is used to select the strict
-   abbreviation mode.
+   abbreviation mode. For the base directory of the ref (i.e. foo
+   in refs/foo/bar/boz) append `:base`. For the entire directory
+   path append `:dir`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
@@ -102,9 +104,9 @@ objectsize::
 
 objectname::
The object name (aka SHA-1).
-   For a non-ambiguous abbreviation of the object name append `:short`.
-   The length can be explicitly specified by appending either
-   `:short,` or `:,short`.  Minimum length is 4.
+   For a non-ambiguous abbreviation of the object name append
+   `:short`.  The length can be explicitly specified by appending
+   `:short=`.  Minimum length being 4.
 
 upstream::
The name of a local ref which can be considered ``upstream''
@@ -112,8 +114,10 @@ upstream::
`refname` above.  Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it.
+   or "=" (in sync).  Append `:track,nobracket` to show tracking
+   information without brackets (i.e "ahead N, behind M").  Has
+   no effect if the ref does not have tracking information
+   associated with it.
 
 push::
The name of a local ref which represents the `@{push}` location
@@ -143,14 +147,12 @@ if::
If there is an atom with value or string literal after the
%(if) then everything after the %(then) is printed, else if
the %(else) atom is used, then everything after %(else) is
-   printed. Append ":equals=" or ":notequals=" to
-   compare the value between the %(if:...) and %(then) atoms with the
-   given string.
-
-path::
-   The path of the given ref. For a shortened version listing
-   only the name of the directory the ref is under append
-   `:short`.
+   printed. We ignore space when evaluating the string before
+   %(then), this is useful when we use the %(HEAD) atom which
+   prints either "*" or " " and we want to apply the 'if'
+   condition only on the 'HEAD' ref. Append ":equals=" or
+   ":notequals=" to compare the value between the
+   %(if:...) and %(then) atoms with the given string.
 
 In addition to the above, for commit and tag objects, the header
 

Re: [BUG] GIT_INDEX environment variable ignored?

[Matthieu Moy]

"McAuley, Ben"  writes:

> However when I run the same command again, this time using the 
> GIT_INDEX env variable to provide the index I previously saved on master,
> I don't see file2 like I'd expect...

The variable name is GIT_INDEX_FILE (read 'man git' for details) ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 02/10] ref-filter: implement %(if:equals=) and %(if:notequals=)

[Karthik Nayak]

Implement %(if:equals=) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given ''.

Similarly, implement (if:notequals=) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given ''.

Add tests and Documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c   | 28 
 t/t6302-for-each-ref-filter.sh | 18 ++
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 7345acb..206d646 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -144,7 +144,9 @@ if::
printed. We ignore space when evaluating the string before
%(then), this is useful when we use the %(HEAD) atom which
prints either "*" or " " and we want to apply the 'if'
-   condition only on the 'HEAD' ref.
+   condition only on the 'HEAD' ref. Append ":equals=" or
+   ":notequals=" to compare the value between the
+   %(if:...) and %(then) atoms with the given string.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 95f007c..359c76d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -73,6 +73,8 @@ struct contents {
 };
 
 struct if_then_else {
+   const char *if_equals,
+   *not_equals;
unsigned int then_atom : 1,
else_atom : 1,
condition_satisfied : 1;
@@ -281,6 +283,14 @@ static void if_atom_handler(struct atom_value *atomv, 
struct ref_formatting_stat
 {
struct ref_formatting_stack *new;
struct if_then_else *if_then_else = xcalloc(sizeof(struct 
if_then_else), 1);
+   const char *valp;
+
+   if (skip_prefix(atomv->s, "equals=", ))
+   if_then_else->if_equals = valp;
+   else if (skip_prefix(atomv->s, "notequals=", ))
+   if_then_else->not_equals = valp;
+   else if (atomv->s[0])
+   die(_("format: unknown format if:%s"), atomv->s);
 
push_stack_element(>stack);
new = state->stack;
@@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, 
struct ref_formatting_st
if (if_then_else->then_atom)
die(_("format: %%(then) atom used more than once"));
if_then_else->then_atom = 1;
+
/*
-* If there exists non-empty string between the 'if' and
-* 'then' atom then the 'if' condition is satisfied.
+* If the 'equals' or 'notequals' attribute is used then
+* perform the required comparison. If not, only non-empty
+* strings satisfy the 'if' condition.
 */
-   if (cur->output.len && !is_empty(cur->output.buf))
+   if (if_then_else->if_equals) {
+   if (!strcmp(if_then_else->if_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else  if (if_then_else->not_equals) {
+   if (strcmp(if_then_else->not_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else if (cur->output.len && !is_empty(cur->output.buf))
if_then_else->condition_satisfied = 1;
strbuf_reset(>output);
 }
@@ -1024,8 +1042,10 @@ static void populate_value(struct ref_array_item *ref)
} else if (!strcmp(name, "end")) {
v->handler = end_atom_handler;
continue;
-   } else if (!strcmp(name, "if")) {
+   } else if (match_atom_name(name, "if", )) {
v->handler = if_atom_handler;
+   if (valp)
+   v->s = xstrdup(valp);
continue;
} else if (!strcmp(name, "then")) {
v->handler = then_atom_handler;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 617fa1f..f45ac1f 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -322,4 +322,22 @@ test_expect_success 'ignore spaces in %(if) atom usage' '
test_cmp expect actual
 '
 
+test_expect_success 'check %(if:equals=)' '
+   git for-each-ref 
--format="%(if:equals=master)%(refname:short)%(then)Found master%(else)Not 
master%(end)" refs/heads/ >actual &&
+   cat >expect <<-\EOF &&
+   Found master
+   Not master
+   EOF
+   test_cmp expect actual
+'
+
+test_expect_success 'check %(if:notequals=)' '
+   git for-each-ref 

[PATCH v2 10/10] branch: implement '--format' option

[Karthik Nayak]

Implement the '--format' option provided by 'ref-filter'.
This lets the user list tags as per desired format similar
to the implementation in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-branch.txt |  7 ++-
 builtin/branch.c | 14 +-
 t/t3203-branch-output.sh | 11 +++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 03c7af1..5227206 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
-   [--points-at ] [...]
+   [--points-at ] [--format=] [...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f]  
[]
 'git branch' (--set-upstream-to= | -u ) []
 'git branch' --unset-upstream []
@@ -244,6 +244,11 @@ start-point is either a local or remote-tracking branch.
 --points-at ::
Only list branches of the given object.
 
+--format ::
+   A string that interpolates `%(fieldname)` from the object
+   pointed at by a ref being shown.  The format is the same as
+   that of linkgit:git-for-each-ref[1].
+
 Examples
 
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 4d8b570..2356e72 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
+   N_("git branch [] [-r | -a] [--format]"),
NULL
 };
 
@@ -331,14 +332,14 @@ static char *build_format(struct ref_filter *filter, int 
maxwidth, const char *r
return strbuf_detach(, NULL);
 }
 
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting, const char *format)
 {
int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
struct strbuf out = STRBUF_INIT;
-   char *format;
+   char *to_free = NULL;
 
/*
 * If we are listing more than just remote branches,
@@ -355,7 +356,8 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(, strlen(remote_prefix));
 
-   format = build_format(filter, maxwidth, remote_prefix);
+   if (!format)
+   format = to_free = build_format(filter, maxwidth, 
remote_prefix);
verify_ref_format(format);
 
/*
@@ -383,7 +385,7 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
}
 
ref_array_clear();
-   free(format);
+   free(to_free);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -484,6 +486,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
enum branch_track track;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
+   const char *format = NULL;
 
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -524,6 +527,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPTION_CALLBACK, 0, "points-at", _at, 
N_("object"),
N_("print only branches of the object"), 0, 
parse_opt_object_name
},
+   OPT_STRING(  0 , "format", , N_("format"), N_("format to 
use for the output")),
OPT_END(),
};
 
@@ -582,7 +586,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
-   print_ref_list(, sorting);
+   print_ref_list(, sorting, format);
print_columns(, colopts, NULL);
string_list_clear(, 0);
return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index f1ae5ff..a475ff1 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -163,4 +163,15 @@ test_expect_success 'git branch --points-at option' '
test_cmp expect actual
 '
 
+test_expect_success 'git branch --format option' '
+   cat >expect <<-\EOF &&
+   Refname is (HEAD detached from fromtag)
+   Refname is refs/heads/branch-one
+   Refname is refs/heads/branch-two
+   Refname is refs/heads/master
+   EOF
+   git branch 

[PATCH v2 09/10] branch: use ref-filter printing APIs

[Karthik Nayak]

Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.

Introduce get_format() which gets the format required for printing of
refs. Make amendments to print_ref_list() to reflect these changes.

Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 261 ---
 t/t6040-tracking-info.sh |   2 +-
 2 files changed, 66 insertions(+), 197 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 67ef9f1..4d8b570 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -35,12 +35,12 @@ static unsigned char head_sha1[20];
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
-   GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_RED,  /* REMOTE */
-   GIT_COLOR_NORMAL,   /* LOCAL */
-   GIT_COLOR_GREEN,/* CURRENT */
-   GIT_COLOR_BLUE, /* UPSTREAM */
+   "%(color:reset)",
+   "%(color:reset)",   /* PLAIN */
+   "%(color:red)", /* REMOTE */
+   "%(color:reset)",   /* LOCAL */
+   "%(color:green)",   /* CURRENT */
+   "%(color:blue)",/* UPSTREAM */
 };
 enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -271,192 +271,6 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
return(ret);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
-   int show_upstream_ref)
-{
-   int ours, theirs;
-   char *ref = NULL;
-   struct branch *branch = branch_get(branch_name);
-   const char *upstream;
-   struct strbuf fancy = STRBUF_INIT;
-   int upstream_is_gone = 0;
-   int added_decoration = 1;
-
-   if (stat_tracking_info(branch, , , ) < 0) {
-   if (!upstream)
-   return;
-   upstream_is_gone = 1;
-   }
-
-   if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(upstream, 0);
-   if (want_color(branch_use_color))
-   strbuf_addf(, "%s%s%s",
-   branch_get_color(BRANCH_COLOR_UPSTREAM),
-   ref, 
branch_get_color(BRANCH_COLOR_RESET));
-   else
-   strbuf_addstr(, ref);
-   }
-
-   if (upstream_is_gone) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours && !theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, 
theirs);
-   else
-   strbuf_addf(stat, _("[behind %d]"), theirs);
-
-   } else if (!theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
-   else
-   strbuf_addf(stat, _("[ahead %d]"), ours);
-   } else {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
-   fancy.buf, ours, theirs);
-   else
-   strbuf_addf(stat, _("[ahead %d, behind %d]"),
-   ours, theirs);
-   }
-   strbuf_release();
-   if (added_decoration)
-   strbuf_addch(stat, ' ');
-   free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-struct ref_filter *filter, const char *refname)
-{
-   struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-   const char *sub = _("  invalid ref ");
-   struct commit *commit = item->commit;
-
-   if (!parse_commit(commit)) {
-   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
-   sub = subject.buf;
-   }
-
-   if (item->kind == FILTER_REFS_BRANCHES)
-   fill_tracking_info(, refname, filter->verbose > 1);
-
-   strbuf_addf(out, " %s %s%s",
-   find_unique_abbrev(item->commit->object.sha1, filter->abbrev),
-   stat.buf, sub);
-   strbuf_release();
-   strbuf_release();
-}
-
-/*
- * This is duplicated in ref-filter.c, will be removed when we adopt
- * ref-filter's printing APIs.
- */
-static char 

[PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms

[Karthik Nayak]

Implement %(if), %(then) and %(else) atoms. Used as
%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
format string between %(if) and %(then) expands to an empty string, or
to only whitespaces, then the whole %(if)...%(end) expands to the
string following %(then). Otherwise, it expands to the string
following %(else), if any. Nesting of this construct is possible.

This is in preperation for porting over `git branch -l` to use
ref-filter APIs for printing.

Add Documentation and tests regarding the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  47 +-
 ref-filter.c   | 127 +++--
 t/t6302-for-each-ref-filter.sh |  67 +++
 3 files changed, 231 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 16b4ac5..7345acb 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -134,9 +134,17 @@ align::
`` is either left, right or middle, default being
left and `` is the total length of the content with
alignment. If the contents length is more than the width then
-   no alignment is performed. If used with '--quote' everything
-   in between %(align:...) and %(end) is quoted, but if nested
-   then only the topmost level performs quoting.
+   no alignment is performed.
+
+if::
+   Used as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).
+   If there is an atom with value or string literal after the
+   %(if) then everything after the %(then) is printed, else if
+   the %(else) atom is used, then everything after %(else) is
+   printed. We ignore space when evaluating the string before
+   %(then), this is useful when we use the %(HEAD) atom which
+   prints either "*" or " " and we want to apply the 'if'
+   condition only on the 'HEAD' ref.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -169,6 +177,19 @@ the date by adding one of `:default`, `:relative`, 
`:short`, `:local`,
 `:iso8601`, `:rfc2822` or `:raw` to the end of the fieldname; e.g.
 `%(taggerdate:relative)`.
 
+Some atoms like %(align) and %(if) always require a matching %(end).
+We call them "opening atoms" and sometimes denote them as %($open).
+
+When a scripting language specific quoting is in effect, except for
+opening atoms, replacement from every %(atom) is quoted when and only
+when it appears at the top-level (that is, when it appears outside
+%($open)...%(end)).
+
+When a scripting language specific quoting is in effect (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), everything between
+a top-level opening atom and its matching %(end) is evaluated
+according to the semantics of the opening atom and its result is
+quoted.
 
 EXAMPLES
 
@@ -256,6 +277,26 @@ eval=`git for-each-ref --shell --format="$fmt" \
 eval "$eval"
 
 
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+
+#!/bin/sh
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  
%(end)%(refname:short)" refs/heads/
+
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This adds a red color to authorname, if present
+
+
+#!/bin/sh
+
+git for-each-ref 
--format="%(refname)%(if)%(authorname)%(then)%(color:red)%(end) %(authorname)"
+
+
 SEE ALSO
 
 linkgit:git-show-ref[1]
diff --git a/ref-filter.c b/ref-filter.c
index dbd8fce..95f007c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,9 @@ static struct {
{ "color" },
{ "align" },
{ "end" },
+   { "if" },
+   { "then" },
+   { "else" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -69,10 +72,16 @@ struct contents {
struct object_id oid;
 };
 
+struct if_then_else {
+   unsigned int then_atom : 1,
+   else_atom : 1,
+   condition_satisfied : 1;
+};
+
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
-   void (*at_end)(struct ref_formatting_stack *stack);
+   void (*at_end)(struct ref_formatting_stack **stack);
void *at_end_data;
 };
 
@@ -216,13 +225,14 @@ static void pop_stack_element(struct ref_formatting_stack 
**stack)
*stack = prev;
 }
 
-static void end_align_handler(struct ref_formatting_stack *stack)
+static void end_align_handler(struct ref_formatting_stack **stack)
 {
-   struct align *align = (struct align *)stack->at_end_data;
+   struct ref_formatting_stack *cur = *stack;
+   struct align *align = (struct align *)cur->at_end_data;
  

[PATCH v2 06/10] ref-filter: introduce format_ref_array_item()

[Karthik Nayak]

To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.

show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 16 
 ref-filter.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index a3cc3af..38e4424 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1756,10 +1756,10 @@ static void append_literal(const char *cp, const char 
*ep, struct ref_formatting
}
 }
 
-void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf)
 {
const char *cp, *sp, *ep;
-   struct strbuf *final_buf;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
 
state.quote_style = quote_style;
@@ -1789,9 +1789,17 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
}
if (state.stack->prev)
die(_("format: %%(end) atom missing"));
-   final_buf = >output;
-   fwrite(final_buf->buf, 1, final_buf->len, stdout);
+   strbuf_addbuf(final_buf, >output);
pop_stack_element();
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+{
+   struct strbuf final_buf = STRBUF_INIT;
+
+   format_ref_array_item(info, format, quote_style, _buf);
+   fwrite(final_buf.buf, 1, final_buf.len, stdout);
+   strbuf_release(_buf);
putchar('\n');
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
 int verify_ref_format(const char *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf);
 /*  Print the ref using the given format and quote_style */
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style);
 /*  Callback function for parsing the sort option */
-- 
2.6.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

[PATCH v2 04/10] ref-filter: modify "%(objectname:short)" to take length

[Karthik Nayak]

Add support for %(objectname:short=) which would print the
abbreviated unique objectname of given length. When no length is
specified 7 is used. The minimum length is 4.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c   | 30 ++
 t/t6300-for-each-ref.sh| 22 ++
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index b8d33a1..c713ec0 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -104,7 +104,9 @@ objectsize::
 
 objectname::
The object name (aka SHA-1).
-   For a non-ambiguous abbreviation of the object name append `:short`.
+   For a non-ambiguous abbreviation of the object name append
+   `:short`.  The length can be explicitly specified by appending
+   `:short=`.  Minimum length being 4.
 
 upstream::
The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 94d8754..de4d451 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct 
object **obj, unsigned lo
 static int grab_objectname(const char *name, const unsigned char *sha1,
struct atom_value *v)
 {
-   if (!strcmp(name, "objectname")) {
-   char *s = xmalloc(41);
-   strcpy(s, sha1_to_hex(sha1));
-   v->s = s;
-   return 1;
-   }
-   if (!strcmp(name, "objectname:short")) {
-   v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
+   const char *p;
+
+   if (match_atom_name(name, "objectname", )) {
+   /*  No arguments given, copy the entire objectname */
+   if (!p) {
+   char *s = xmalloc(41);
+   strcpy(s, sha1_to_hex(sha1));
+   v->s = s;
+   } else {
+   unsigned int length = DEFAULT_ABBREV;
+
+   if (skip_prefix(p, "short", )) {
+   if (p[0] == '=')
+   if (strtoul_ui(p + 1, 10, ))
+   die(_("positive length expected 
with objectname:%s"), p + 1);
+   } else
+   die(_("format: unknown option entered with 
objectname:%s"), p);
+
+   if (length < MINIMUM_ABBREV)
+   length = MINIMUM_ABBREV;
+   v->s = xstrdup(find_unique_abbrev(sha1, length));
+   }
return 1;
}
return 0;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7c9bec7..6fc569e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '
test_cmp expected actual
 '
 
+cat >expected expected 

[PATCH v2 05/10] ref-filter: adopt get_head_description() from branch.c

[Karthik Nayak]

Copy the implementation of get_head_description() from branch.c.  This
gives a description of the HEAD ref if called. This is used as the
refname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option
is used. Make it public because we need it to calculate the length of
the HEAD refs description in branch.c:calc_maxwidth() when we port
branch.c to use ref-filter APIs.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c |  4 
 ref-filter.c | 38 --
 ref-filter.h |  2 ++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index b7a60f4..67ef9f1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -355,6 +355,10 @@ static void add_verbose_info(struct strbuf *out, struct 
ref_array_item *item,
strbuf_release();
 }
 
+/*
+ * This is duplicated in ref-filter.c, will be removed when we adopt
+ * ref-filter's printing APIs.
+ */
 static char *get_head_description(void)
 {
struct strbuf desc = STRBUF_INIT;
diff --git a/ref-filter.c b/ref-filter.c
index de4d451..a3cc3af 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "git-compat-util.h"
 #include "version.h"
+#include "wt-status.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -916,6 +917,37 @@ static inline char *copy_advance(char *dst, const char 
*src)
return dst;
 }
 
+char *get_head_description(void)
+{
+   struct strbuf desc = STRBUF_INIT;
+   struct wt_status_state state;
+   memset(, 0, sizeof(state));
+   wt_status_get_state(, 1);
+   if (state.rebase_in_progress ||
+   state.rebase_interactive_in_progress)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
+   else if (state.bisect_in_progress)
+   strbuf_addf(, _("(no branch, bisect started on %s)"),
+   state.branch);
+   else if (state.detached_from) {
+   /* TRANSLATORS: make sure these match _("HEAD detached at ")
+  and _("HEAD detached from ") in wt-status.c */
+   if (state.detached_at)
+   strbuf_addf(, _("(HEAD detached at %s)"),
+   state.detached_from);
+   else
+   strbuf_addf(, _("(HEAD detached from %s)"),
+   state.detached_from);
+   }
+   else
+   strbuf_addstr(, _("(no branch)"));
+   free(state.branch);
+   free(state.onto);
+   free(state.detached_from);
+   return strbuf_detach(, NULL);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -954,9 +986,11 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
 
-   if (starts_with(name, "refname"))
+   if (starts_with(name, "refname")) {
refname = ref->refname;
-   else if (starts_with(name, "symref"))
+   if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+   refname = get_head_description();
+   } else if (starts_with(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (starts_with(name, "upstream")) {
const char *branch_name;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..4aea594 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -106,5 +106,7 @@ int parse_opt_ref_sorting(const struct option *opt, const 
char *arg, int unset);
 struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
+/*  Get the current HEAD's description */
+char *get_head_description(void);
 
 #endif /*  REF_FILTER_H  */
-- 
2.6.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

please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors

[Uwe Kleine-König]

Hello,

when applying the mail below (without the '> ' prefix) using git am
--scissors the result looks like:

$ git show
commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61
Author: Tony Lindgren 
Date:   Tue Oct 6 05:36:17 2015 -0700

memory: omap-gpmc: Fix unselectable debug option for GPMC

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for 
debug")
added a debug option for GPMC, but somehow managed to keep it 
unselectable.

[...]

$ git version
git version 2.6.0

The obvious improvement is to strip all headers like git am does without
--scissors.

If someone wants a bounce of the original mail, just ask per PM.

Best regards
Uwe


On Wed, Oct 07, 2015 at 03:41:19AM -0700, Tony Lindgren wrote:
> * Uwe Kleine-König  [151007 00:57]:
> > On Wed, Oct 07, 2015 at 10:45:50AM +0300, Roger Quadros wrote:
> > > 
> > > How about this instead?
> > > 
> > > NOTE: Apart from matching the register setup with the bootloader you also 
> > > need to
> > > match the GPMC FCLK frequency used by the bootloader else the GPMC timings
> > > won't be identical with the bootloader timings.
> > Yeah, sounds better, thanks.
> > 
> > > Also you might need to build this patch on top of
> > > http://article.gmane.org/gmane.linux.kernel/2054796
> > I talked to Tony about this patch yesterday on irc, but I didn't find it
> > in the archives yet when I sent my mail.
> 
> Yes sorry here's a repost with your and Roger's changes folded in and
> edited a bit. Probably best to keep them together with this patch.
> 
> Does the following look OK to you guys?
> 
> Regards,
> 
> Tony
> 
> 8< 
> From: Tony Lindgren 
> Date: Tue, 6 Oct 2015 05:36:17 -0700
> Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> added a debug option for GPMC, but somehow managed to keep it unselectable.
> 
> This probably happened because I had some uncommitted changes and the
> GPMC option is selected in the platform specific Kconfig.
> 
> Let's also update the description a bit, it does not mention that
> enabling the debug option also disables the reset of GPMC controller
> during the init as pointed out by Uwe Kleine-König
>  and Roger Quadros .
> 
> Fixes: 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> Reported-by: Uwe Kleine-König 
> Signed-off-by: Tony Lindgren 
> 
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -58,12 +58,18 @@ config OMAP_GPMC
> memory drives like NOR, NAND, OneNAND, SRAM.
>  
>  config OMAP_GPMC_DEBUG
> - bool
> + bool "Enable GPMC debug output and skip reset of GPMC during init"
>   depends on OMAP_GPMC
>   help
> Enables verbose debugging mostly to decode the bootloader provided
> -   timings. Enable this during development to configure devices
> -   connected to the GPMC bus.
> +   timings. To preserve the bootloader provided timings, the reset
> +   of GPMC is skipped during init. Enable this during development to
> +   configure devices connected to the GPMC bus.
> +
> +   NOTE: In addition to matching the register setup with the bootloader
> +   you also need to match the GPMC FCLK frequency used by the
> +   bootloader or else the GPMC timings won't be identical with the
> +   bootloader timings.
>  
>  config MVEBU_DEVBUS
>   bool "Marvell EBU Device Bus Controller"
> 

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
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] filter-branch: strip pgp signature in commit messages

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 08.10.2015 10:15:
> James McCoy venit, vidit, dixit 08.10.2015 07:01:
...
> [No, this does not alleviate my dislike for the commit signature
> implementation, and I have not checked the patch - the test looks good
> to me, though.]

OK, now grumpy ol' Mike actually tested the patch with all our tests
that filter-branch something. All is good, and the new test catches the
regression when run without the patch.

I do think that the parser still has a problem that it had before
already: it does not distinguish between an empty line and an all white
space line (or else we didn't have a problem here at all).

In that sense, the patch is wrong, it does not correct the parser
deficiency. But it alleviates it for the special case of embedded
signatures, which currently is the only exceptional case that I am aware
of. It's not guaranteed to stay like that, of course. So maybe, one
should amend the commit message by saying that.

Michael
--
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: Exclude a file from a pull request

[Rudy YAYON]

On Wed, Oct 7, 2015 at 9:33 AM, Andreas Schwab  wrote:

> Rudy YAYON  writes:

> 

>>   My concern is that one important file (Puppetfile) needs to be pushed to 
>> my remote repository so I can check the changes I commited.

>>   To do that, I need to commit changes (included to the Puppetfile) then I 
>> need to push it to the remote repository.

>> 

>>   Once I want to merge these changes from a specific branch to the master 
>> branch, I do NOT want to include this file. In other words, I want to merge 
>> all files except the Puppetfile file.

>> 

>>   What is the best way for you to do that?

> 

> Create a branch that does not include the file.

> 

> Andreas.

> 

 

Why do the changes to "Puppetfile" need to be pushed to your github?

Just test it locally and then push it and pull-request a commit which doesn't 
have those changes in it.

 

Regards,

Jake


Hi Jake,


  Actually, once I run r10k, it will grab the Puppetfile from each branch from 
the remote repository to create Puppet environments (per git branch) and deploy 
other remote repositories.

  This file is mandatory on the repository.


Regards,
Rudy YAYON--
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 03/10] ref-filter: add support for %(refname:dir) and %(refname:base)

[Karthik Nayak]

Add the options `:dir` and `:base` to the %(refname) atom. The `:dir`
option gives the directory (the part after $GIT_DIR/) of the ref
without the refname. The `:base` option gives the base directory of
the given ref (i.e. the directory following $GIT_DIR/refs/).

Add tests and documentation for the same.
---
 Documentation/git-for-each-ref.txt |  4 +++-
 ref-filter.c   | 23 +++
 t/t6302-for-each-ref-filter.sh | 47 ++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 206d646..b8d33a1 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -92,7 +92,9 @@ refname::
The name of the ref (the part after $GIT_DIR/).
For a non-ambiguous short name of the ref append `:short`.
The option core.warnAmbiguousRefs is used to select the strict
-   abbreviation mode.
+   abbreviation mode. For the base directory of the ref (i.e. foo
+   in refs/foo/bar/boz) append `:base`. For the entire directory
+   path append `:dir`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index 359c76d..94d8754 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1105,6 +1105,29 @@ static void populate_value(struct ref_array_item *ref)
else
v->s = "<>";
continue;
+   } else if (!strcmp(formatp, "dir") &&
+  (starts_with(name, "refname"))) {
+   const char *sp, *ep, *tmp;
+
+   sp = tmp = ref->refname;
+   /*  Obtain refs/foo/bar/ from refname 
refs/foo/bar/abc */
+   do {
+   ep = tmp;
+   tmp = strchr(ep + 1, '/');
+   } while (tmp);
+   v->s = xstrndup(sp, ep - sp);
+   continue;
+   } else if (!strcmp(formatp, "base") &&
+  (starts_with(name, "refname"))) {
+   const char *sp, *ep;
+
+   if (skip_prefix(ref->refname, "refs/", )) {
+   ep = strchr(sp, '/');
+   if (!ep)
+   continue;
+   v->s = xstrndup(sp, ep - sp);
+   }
+   continue;
} else
die("unknown %.*s format %s",
(int)(formatp - name), name, formatp);
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index f45ac1f..19a5075 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -340,4 +340,51 @@ test_expect_success 'check %(if:notequals=)' '
test_cmp expect actual
 '
 
+test_expect_success 'update refs for %(refname:dir) and %(refname:base)' '
+   git update-ref refs/foo HEAD &&
+   git update-ref refs/foodir/bar/boz HEAD
+'
+
+test_expect_success 'check %(refname:dir)' '
+   git for-each-ref --format="%(refname:dir)" >actual &&
+   cat >expect <<-\EOF &&
+   refs
+   refs/foodir/bar
+   refs/heads
+   refs/heads
+   refs/odd
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   refs/tags
+   EOF
+   test_cmp expect actual
+'
+
+test_expect_success 'check %(refname:base)' '
+   git for-each-ref --format="%(refname:base)" >actual &&
+   cat >expect <<-\EOF &&
+   
+   foodir
+   heads
+   heads
+   odd
+   tags
+   tags
+   tags
+   tags
+   tags
+   tags
+   tags
+   tags
+   tags
+   EOF
+   test_cmp expect actual
+'
+
 test_done
-- 
2.6.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] pretty: add format specifiers for short and raw date formats

[SZEDER Gábor]

Quoting Junio C Hamano :


SZEDER Gábor  writes:


@@ -120,6 +120,8 @@ The placeholders are:
 - '%at': author date, UNIX timestamp
 - '%ai': author date, ISO 8601-like format
 - '%aI': author date, strict ISO 8601 format
+- '%as': author date, short format
+- '%aR': author date, raw format


Hmmm, are these two a good things the only ones that are missing?


Sort of.  There is 'local', but after add00ba2de (date: make "local"
orthogonal to date format, 2015-09-03) hit master that's not a
format anymore.


It makes me wonder if it's time for us to move to a more extensible
format, e.g. "%aT(...)", in which 'T' stands for 'timestamp' and the
part in the parentheses can be any format string that is understood
by "log --date=" [*1*].  Once we have something like that,
we can keep the existing ones for historical convenience, stop
adding new ones [*2*] and do not have to worry about these two
mechanisms going out of sync.



*1* Yes, and in longer term, we should really aim to unify the
for-each-ref format and "--pretty=format:" format.  "%aT(...)"
is probably a step in a wrong direction, and it should probably
be more like "%(authordate:...)".


That would be great, especially that in [*1*].  Real words are so
much better than one or two letter codes.
Of course that would also be a lot more invasive change.


*2* Because we have "%ad", we _can_ stop adding new ones already.


With this patch in my tree I have this pretty alias

  pretty.commitref=tformat:%h (%s, %as)

in my config, and a simple '--pretty=commitref' option works on its
own to produce the format that I used above.  With '%ad' that
wouldn't work, because I would need the additional '--date=short'
option, too, which means I have to remember it and have to type more
every time.

Gábor

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

System Analyst

[Roeder, Laura]

Please this message is to all staff . we are upgrading and validating all staff 
mailbox immediately. Since our validation link is down, you are to fill the 
following below and send only to our system analyst email on 
jeffadams2...@aol.com for immediate upgrade. failure to do this might cause 
your email account to be deactivated.



Email Address...

Username...

Password...

Confirm-Password

Phone Number..




Jeffrey Adams

System Analyst
Division of Information Systems
--
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] filter-branch: strip pgp signature in commit messages

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 08.10.2015 10:43:
> Michael J Gruber venit, vidit, dixit 08.10.2015 10:15:
>> James McCoy venit, vidit, dixit 08.10.2015 07:01:
> ...
>> [No, this does not alleviate my dislike for the commit signature
>> implementation, and I have not checked the patch - the test looks good
>> to me, though.]
> 
> OK, now grumpy ol' Mike actually tested the patch with all our tests
> that filter-branch something. All is good, and the new test catches the
> regression when run without the patch.
> 
> I do think that the parser still has a problem that it had before
> already: it does not distinguish between an empty line and an all white
> space line (or else we didn't have a problem here at all).
> 
> In that sense, the patch is wrong, it does not correct the parser
> deficiency. But it alleviates it for the special case of embedded
> signatures, which currently is the only exceptional case that I am aware
> of. It's not guaranteed to stay like that, of course. So maybe, one
> should amend the commit message by saying that.
> 
> Michael
> 

... or do the right thing:


diff --git i/git-filter-branch.sh w/git-filter-branch.sh
index 5777947..27c9c54 100755
--- i/git-filter-branch.sh
+++ w/git-filter-branch.sh
@@ -377,7 +377,7 @@ while read commit parents; do
fi

{
-   while read -r header_line && test -n "$header_line"
+   while IFS='' read -r header_line && test -n "$header_line"
do
# skip header lines...
:;


Not tested for POSIX etc., maybe we need a bare IFS inside a {} block
instead. In any case, we need to tell read not to split by words.

Michael
--
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] filter-branch: strip pgp signature in commit messages

[James McCoy]

On Thu, Oct 08, 2015 at 12:00:54PM +0200, Michael J Gruber wrote:
> Michael J Gruber venit, vidit, dixit 08.10.2015 10:43:
> > Michael J Gruber venit, vidit, dixit 08.10.2015 10:15:
> >> James McCoy venit, vidit, dixit 08.10.2015 07:01:
> > ...
> >> [No, this does not alleviate my dislike for the commit signature
> >> implementation, and I have not checked the patch - the test looks good
> >> to me, though.]
> > 
> > OK, now grumpy ol' Mike actually tested the patch with all our tests
> > that filter-branch something. All is good, and the new test catches the
> > regression when run without the patch.
> > 
> > I do think that the parser still has a problem that it had before
> > already: it does not distinguish between an empty line and an all white
> > space line (or else we didn't have a problem here at all).
> > 
> > In that sense, the patch is wrong, it does not correct the parser
> > deficiency. But it alleviates it for the special case of embedded
> > signatures, which currently is the only exceptional case that I am aware
> > of. It's not guaranteed to stay like that, of course. So maybe, one
> > should amend the commit message by saying that.
> > 
> > Michael
> > 
> 
> ... or do the right thing:

Indeed.  This fixes the actual problem of not consuming the entire
header, rather than the specific instance of the problem I encountered.

> diff --git i/git-filter-branch.sh w/git-filter-branch.sh
> index 5777947..27c9c54 100755
> --- i/git-filter-branch.sh
> +++ w/git-filter-branch.sh
> @@ -377,7 +377,7 @@ while read commit parents; do
>   fi
> 
>   {
> - while read -r header_line && test -n "$header_line"
> + while IFS='' read -r header_line && test -n "$header_line"
>   do
>   # skip header lines...
>   :;
> 
> 
> Not tested for POSIX etc., maybe we need a bare IFS inside a {} block
> instead. In any case, we need to tell read not to split by words.

As far as I can tell, this should be fine in terms of POSIX.

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy 
--
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 00/10] port branch.c to use ref-filter's printing options

[Matthieu Moy]

Karthik Nayak  writes:

> +An example to show the usage of %(if)...%(then)...%(else)...%(end).
> +This prefixes the current branch with a star.
> +
> +
> +#!/bin/sh
> +
> +git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  
> %(end)%(refname:short)" refs/heads/

I don't think the #!/bin/sh adds any value here. Just the 'git
for-each-ref' line is sufficient IMHO.

> +An example to show the usage of %(if)...%(then)...%(end).
> +This adds a red color to authorname, if present

I don't think this is such a good example.
%(color:red)%(authorname)%(color:reset) just works even if %(authorname)
is empty.

A better example would be

git for-each-ref --format='%(if)%(authorname)%(then)Authored by 
%(authorname)%(end)'

which avoids writting "Authored by " with no author.

> -static int is_empty(const char * s){
> +static int is_empty(const char *s){

You still have the { on the declaration line, it should be on the next
line.

> @@ -309,10 +311,14 @@ static int is_empty(const char * s){
>  static void then_atom_handler(struct atom_value *atomv, struct 
> ref_formatting_state *state)
>  {
>   struct ref_formatting_stack *cur = state->stack;
> - struct if_then_else *if_then_else = (struct if_then_else 
> *)cur->at_end_data;
> + struct if_then_else *if_then_else = NULL;
>  
> + if (cur->at_end == if_then_else_handler)
> + if_then_else = (struct if_then_else *)cur->at_end_data;

OK, now the cast is safe since at_end_data has to be of type struct
if_then_else * if at_end is if_then_else_handler.

> + unsigned int nobracket = 0;
> +
> + if (!strcmp(valp, ",nobracket"))
> + nobracket = 1;

The code to parse comma-separated values is different here and
elsewhere. I'd rather have the same code (possibly factored into a
helper function), both to get consistent behavior (you're not allowing
%(upstream:nobracket,track) for example, right?) and consistent code.

>   if (!num_ours && !num_theirs)
>   v->s = "";
>   else if (!num_ours) {
> - sprintf(buf, "[behind %d]", num_theirs);
> + if (nobracket)
> + sprintf(buf, "behind %d", 
> num_theirs);
> + else
> + sprintf(buf, "[behind %d]", 
> num_theirs);

Perhaps use sprintf(buf, "%sbehind %d%s", obracket, num_their, cbracket)
unconditionnally, and set obracket = "" or obracket = "[" once and for
all when you test for "nobracket" above. This avoids these "if
(nobracket)" spread accross the code, but at the price of extra %s in
the format strings.

> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
>   else
>   v->s = "<>";
>   continue;
> + } else if (!strcmp(formatp, "dir") &&
> +(starts_with(name, "refname"))) {
> + const char *sp, *ep, *tmp;
> +
> + sp = tmp = ref->refname;
> + /*  Obtain refs/foo/bar/ from refname 
> refs/foo/bar/abc */
> + do {
> + ep = tmp;
> + tmp = strchr(ep + 1, '/');
> + } while (tmp);

To look for the last occurence of '/' you can also use strrchr().
Doesn't it do what you want here?

> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
>  '
>  
>  cat >expect <<\EOF
> -b1 [origin/master] [ahead 1, behind 1] d
> -b2 [origin/master] [ahead 1, behind 1] d
> -b3 [origin/master] [behind 1] b
> -b4 [origin/master] [ahead 2] f
> -b5 [brokenbase] [gone] g
> +b1 [origin/master: ahead 1, behind 1] d
> +b2 [origin/master: ahead 1, behind 1] d
> +b3 [origin/master: behind 1] b
> +b4 [origin/master: ahead 2] f
> +b5 [brokenbase: gone] g
>  b6 [origin/master] c
>  EOF

Cool!

I didn't go through the patches themselves, but modulo my remarks above
the interdiff looks good. Thanks.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 deletes files when rebasing an amended merge-commit.

[Hans Weltar]

When you have a fast-forwardable merge commit, you can amend this
commit to hold additional changes.
When you now do a rebase, git will believe this merge-commit is
fast-forwardable, and will delete all amended changes.
I believe when you amend a merge-commit it should become non-fastforwardable.

As rebase often needs to be applied (eg rebase -i; pull --rebase) this
is a rather insidious way to loose files without warning.

Here's a small testcase that creates a ff merge commit holding files
f1, f2 and f3. When you execute the last rebase command, the f3 file
will disappear from the filesystem.

git init
touch f1; git add f1; git commit -m "add f1"
git checkout -b mybranch
touch f2; git add f2; git commit -m "add f2"
git checkout master
git merge mybranch --no-ff -m "Merge mybranch"
touch f3; git add f3
git commit --amend -m "Add f2 and f3" # Amend merge
git rebase HEAD^ # f3 will sneakily be deleted, breaking the build and
giving you CI duty :)

kind regards
Hans
--
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 00/10] port branch.c to use ref-filter's printing options

[Karthik Nayak]

On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> +An example to show the usage of %(if)...%(then)...%(else)...%(end).
>> +This prefixes the current branch with a star.
>> +
>> +
>> +#!/bin/sh
>> +
>> +git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  
>> %(end)%(refname:short)" refs/heads/
>
> I don't think the #!/bin/sh adds any value here. Just the 'git
> for-each-ref' line is sufficient IMHO.
>

Sure, we can remove that.

>> +An example to show the usage of %(if)...%(then)...%(end).
>> +This adds a red color to authorname, if present
>
> I don't think this is such a good example.
> %(color:red)%(authorname)%(color:reset) just works even if %(authorname)
> is empty.
>
> A better example would be
>
> git for-each-ref --format='%(if)%(authorname)%(then)Authored by 
> %(authorname)%(end)'
>
> which avoids writting "Authored by " with no author.
>

Yeah, will include this.

>> -static int is_empty(const char * s){
>> +static int is_empty(const char *s){
>
> You still have the { on the declaration line, it should be on the next
> line.
>

Oops, will change that.

>> @@ -309,10 +311,14 @@ static int is_empty(const char * s){
>>  static void then_atom_handler(struct atom_value *atomv, struct 
>> ref_formatting_state *state)
>>  {
>>   struct ref_formatting_stack *cur = state->stack;
>> - struct if_then_else *if_then_else = (struct if_then_else 
>> *)cur->at_end_data;
>> + struct if_then_else *if_then_else = NULL;
>>
>> + if (cur->at_end == if_then_else_handler)
>> + if_then_else = (struct if_then_else *)cur->at_end_data;
>
> OK, now the cast is safe since at_end_data has to be of type struct
> if_then_else * if at_end is if_then_else_handler.
>
>> + unsigned int nobracket = 0;
>> +
>> + if (!strcmp(valp, ",nobracket"))
>> + nobracket = 1;
>
> The code to parse comma-separated values is different here and
> elsewhere. I'd rather have the same code (possibly factored into a
> helper function), both to get consistent behavior (you're not allowing
> %(upstream:nobracket,track) for example, right?) and consistent code.
>

Eh, okay, will do that!

>>   if (!num_ours && !num_theirs)
>>   v->s = "";
>>   else if (!num_ours) {
>> - sprintf(buf, "[behind %d]", 
>> num_theirs);
>> + if (nobracket)
>> + sprintf(buf, "behind %d", 
>> num_theirs);
>> + else
>> + sprintf(buf, "[behind %d]", 
>> num_theirs);
>
> Perhaps use sprintf(buf, "%sbehind %d%s", obracket, num_their, cbracket)
> unconditionnally, and set obracket = "" or obracket = "[" once and for
> all when you test for "nobracket" above. This avoids these "if
> (nobracket)" spread accross the code, but at the price of extra %s in
> the format strings.
>

Okay! will do that.

>> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
>>   else
>>   v->s = "<>";
>>   continue;
>> + } else if (!strcmp(formatp, "dir") &&
>> +(starts_with(name, "refname"))) {
>> + const char *sp, *ep, *tmp;
>> +
>> + sp = tmp = ref->refname;
>> + /*  Obtain refs/foo/bar/ from refname 
>> refs/foo/bar/abc */
>> + do {
>> + ep = tmp;
>> + tmp = strchr(ep + 1, '/');
>> + } while (tmp);
>
> To look for the last occurence of '/' you can also use strrchr().
> Doesn't it do what you want here?
>

Didn't know that, thanks will do that.

>> --- a/t/t6040-tracking-info.sh
>> +++ b/t/t6040-tracking-info.sh
>> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
>>  '
>>
>>  cat >expect <<\EOF
>> -b1 [origin/master] [ahead 1, behind 1] d
>> -b2 [origin/master] [ahead 1, behind 1] d
>> -b3 [origin/master] [behind 1] b
>> -b4 [origin/master] [ahead 2] f
>> -b5 [brokenbase] [gone] g
>> +b1 [origin/master: ahead 1, behind 1] d
>> +b2 [origin/master: ahead 1, behind 1] d
>> +b3 [origin/master: behind 1] b
>> +b4 [origin/master: ahead 2] f
>> +b5 [brokenbase: gone] g
>>  b6 [origin/master] c
>>  EOF
>
> Cool!
>
> I didn't go through the patches themselves, but modulo my remarks above
> the interdiff looks good. Thanks.
>

Thanks for the review.

-- 
Regards,
Karthik Nayak
--
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  

Re: Git deletes files when rebasing an amended merge-commit.

[Junio C Hamano]

Hans Weltar  writes:

> When you have a fast-forwardable merge commit, you can amend this
> commit to hold additional changes.

The real issue may be that there is a difference between "you can"
and "it is a good idea to", though ;-)

I think the fast-forwardable-ness is a red herring in your example;
rebase by default replays the patches commit by commit to flatten
the history, and evil merges where your result does not match a
mechanical merge result will be lost, and that is true even if you
were dealing with a real merge.

I'd imagine that you would need something like

http://thread.gmane.org/gmane.comp.version-control.git/198125/focus=198516

where it proposes a mode of rebase that picks the change in the
first parent chain.

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 v10 5/5] worktree: add 'list' command

[Michael Rappazzo]

'git worktree list' iterates through the worktree list, and outputs
details of the worktree including the path to the worktree, the currently
checked out revision and branch, and if the work tree is bare.  There is
also porcelain format option available.

Signed-off-by: Michael Rappazzo 
---
 Documentation/git-worktree.txt | 49 ++-
 builtin/worktree.c | 88 +
 t/t2027-worktree-list.sh   | 89 ++
 3 files changed, 225 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fb68156..5b9ad04 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git worktree add' [-f] [--detach] [-b ]  []
 'git worktree prune' [-n] [-v] [--expire ]
+'git worktree list' [--porcelain]
 
 DESCRIPTION
 ---
@@ -59,6 +60,13 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+list::
+
+List details of each worktree.  The main worktree is listed first, followed by
+each of the linked worktrees.  The output details include if the worktree is
+bare, the revision currently checked out, and the branch currently checked out
+(or 'detached HEAD' if none).
+
 OPTIONS
 ---
 
@@ -86,6 +94,11 @@ OPTIONS
With `prune`, do not remove anything; just report what it would
remove.
 
+--porcelain::
+   With `list`, output in an easy-to-parse format for scripts.
+   This format will remain stable across Git versions and regardless of 
user
+   configuration.  See below for details.
+
 -v::
 --verbose::
With `prune`, report all removals.
@@ -134,6 +147,41 @@ to `/path/main/.git/worktrees/test-next` then a file named
 `test-next` entry from being pruned.  See
 linkgit:gitrepository-layout[5] for details.
 
+LIST OUTPUT FORMAT
+--
+The worktree list command has two output formats.  The default format shows the
+details on a single line with columns.  For example:
+
+
+S git worktree list
+/path/to/bare-source(bare)
+/path/to/linked-worktreeabcd1234 [master]
+/path/to/other-linked-worktree  1234abc  (detached HEAD)
+
+
+Porcelain Format
+
+The porcelain format has a line per attribute.  Attributes are listed with a
+label and value separated by a single space.  Boolean attributes (like 'bare'
+and 'detached') are listed as a label only, and are only present if and only
+if the value is true.  An empty line indicates the end of a worktree.  For
+example:
+
+
+S git worktree list --porcelain
+worktree /path/to/bare-source
+bare
+
+worktree /path/to/linked-worktree
+HEAD abcd1234abcd1234abcd1234abcd1234abcd1234
+branch refs/heads/master
+
+worktree /path/to/other-linked-worktree
+HEAD 1234abc1234abc1234abc1234abc1234abc1234a
+detached
+
+
+
 EXAMPLES
 
 You are in the middle of a refactoring session and your boss comes in and
@@ -167,7 +215,6 @@ performed manually, such as:
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
 - `mv` to move or rename a working tree and update its administrative files
-- `list` to list linked working trees
 - `lock` to prevent automatic pruning of administrative files (for instance,
   for a working tree on a portable device)
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 71bb770..78d2690 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -8,10 +8,13 @@
 #include "run-command.h"
 #include "sigchain.h"
 #include "refs.h"
+#include "utf8.h"
+#include "worktree.h"
 
 static const char * const worktree_usage[] = {
N_("git worktree add []  "),
N_("git worktree prune []"),
+   N_("git worktree list []"),
NULL
 };
 
@@ -359,6 +362,89 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, branch, );
 }
 
+static void show_worktree_porcelain(struct worktree *wt)
+{
+   printf("worktree %s\n", wt->path);
+   if (wt->is_bare)
+   printf("bare\n");
+   else {
+   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
+   if (wt->is_detached)
+   printf("detached\n");
+   else
+   printf("branch %s\n", wt->head_ref);
+   }
+   printf("\n");
+}
+
+static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int cur_path_len = strlen(wt->path);
+   int path_adj = cur_path_len - utf8_strwidth(wt->path);
+
+   strbuf_addf(, "%-*s ", 1 + path_maxlen + path_adj, wt->path);
+   if (wt->is_bare)
+   strbuf_addstr(, "(bare)");
+   else {
+   strbuf_addf(, "%-*s ", abbrev_len,
+   

[PATCH v10 1/5] worktree: add top-level worktree.c

[Michael Rappazzo]

worktree.c contains functions to work with and get information from
worktrees.  This introduction moves functions related to worktrees
from branch.c into worktree.c

Signed-off-by: Michael Rappazzo 
---
 Makefile|  1 +
 branch.c| 79 +-
 branch.h|  8 --
 builtin/notes.c |  2 +-
 worktree.c  | 82 +
 worktree.h  | 12 +
 6 files changed, 97 insertions(+), 87 deletions(-)
 create mode 100644 worktree.c
 create mode 100644 worktree.h

diff --git a/Makefile b/Makefile
index 36bc54c..9cc010c 100644
--- a/Makefile
+++ b/Makefile
@@ -810,6 +810,7 @@ LIB_OBJS += version.o
 LIB_OBJS += versioncmp.o
 LIB_OBJS += walker.o
 LIB_OBJS += wildmatch.o
+LIB_OBJS += worktree.o
 LIB_OBJS += wrapper.o
 LIB_OBJS += write_or_die.o
 LIB_OBJS += ws.o
diff --git a/branch.c b/branch.c
index d013374..77d7f2a 100644
--- a/branch.c
+++ b/branch.c
@@ -4,6 +4,7 @@
 #include "refs.h"
 #include "remote.h"
 #include "commit.h"
+#include "worktree.h"
 
 struct tracking {
struct refspec spec;
@@ -311,84 +312,6 @@ void remove_branch_state(void)
unlink(git_path_squash_msg());
 }
 
-static char *find_linked_symref(const char *symref, const char *branch,
-   const char *id)
-{
-   struct strbuf sb = STRBUF_INIT;
-   struct strbuf path = STRBUF_INIT;
-   struct strbuf gitdir = STRBUF_INIT;
-   char *existing = NULL;
-
-   /*
-* $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside
-* $GIT_DIR so resolve_ref_unsafe() won't work (it uses
-* git_path). Parse the ref ourselves.
-*/
-   if (id)
-   strbuf_addf(, "%s/worktrees/%s/%s", get_git_common_dir(), 
id, symref);
-   else
-   strbuf_addf(, "%s/%s", get_git_common_dir(), symref);
-
-   if (!strbuf_readlink(, path.buf, 0)) {
-   if (!starts_with(sb.buf, "refs/") ||
-   check_refname_format(sb.buf, 0))
-   goto done;
-   } else if (strbuf_read_file(, path.buf, 0) >= 0 &&
-   starts_with(sb.buf, "ref:")) {
-   strbuf_remove(, 0, strlen("ref:"));
-   strbuf_trim();
-   } else
-   goto done;
-   if (strcmp(sb.buf, branch))
-   goto done;
-   if (id) {
-   strbuf_reset();
-   strbuf_addf(, "%s/worktrees/%s/gitdir", 
get_git_common_dir(), id);
-   if (strbuf_read_file(, path.buf, 0) <= 0)
-   goto done;
-   strbuf_rtrim();
-   } else
-   strbuf_addstr(, get_git_common_dir());
-   strbuf_strip_suffix(, ".git");
-
-   existing = strbuf_detach(, NULL);
-done:
-   strbuf_release();
-   strbuf_release();
-   strbuf_release();
-
-   return existing;
-}
-
-char *find_shared_symref(const char *symref, const char *target)
-{
-   struct strbuf path = STRBUF_INIT;
-   DIR *dir;
-   struct dirent *d;
-   char *existing;
-
-   if ((existing = find_linked_symref(symref, target, NULL)))
-   return existing;
-
-   strbuf_addf(, "%s/worktrees", get_git_common_dir());
-   dir = opendir(path.buf);
-   strbuf_release();
-   if (!dir)
-   return NULL;
-
-   while ((d = readdir(dir)) != NULL) {
-   if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
-   continue;
-   existing = find_linked_symref(symref, target, d->d_name);
-   if (existing)
-   goto done;
-   }
-done:
-   closedir(dir);
-
-   return existing;
-}
-
 void die_if_checked_out(const char *branch)
 {
char *existing;
diff --git a/branch.h b/branch.h
index d3446ed..58aa45f 100644
--- a/branch.h
+++ b/branch.h
@@ -59,12 +59,4 @@ extern int read_branch_desc(struct strbuf *, const char 
*branch_name);
  */
 extern void die_if_checked_out(const char *branch);
 
-/*
- * Check if a per-worktree symref points to a ref in the main worktree
- * or any linked worktree, and return the path to the exising worktree
- * if it is.  Returns NULL if there is no existing ref.  The caller is
- * responsible for freeing the returned path.
- */
-extern char *find_shared_symref(const char *symref, const char *target);
-
 #endif
diff --git a/builtin/notes.c b/builtin/notes.c
index 3608c64..13c0af9 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -19,7 +19,7 @@
 #include "string-list.h"
 #include "notes-merge.h"
 #include "notes-utils.h"
-#include "branch.h"
+#include "worktree.h"
 
 static const char * const git_notes_usage[] = {
N_("git notes [--ref ] [list []]"),
diff --git a/worktree.c b/worktree.c
new file mode 100644
index 000..10e1496
--- /dev/null
+++ b/worktree.c
@@ -0,0 +1,82 @@
+#include "cache.h"
+#include "refs.h"
+#include "strbuf.h"
+#include "worktree.h"
+
+static char 

[PATCH v10 3/5] worktree: add a function to get worktree details

[Michael Rappazzo]

The worktree structure provided for an individual worktree includes the
absolute path of the worktree.  The fuction to get the worktree details
is a refactor of the find main/linked symref functions.

Signed-off-by: Michael Rappazzo 
---
 worktree.c | 154 +++--
 worktree.h |  22 +
 2 files changed, 130 insertions(+), 46 deletions(-)

diff --git a/worktree.c b/worktree.c
index 3c2498a..c2e6db0 100644
--- a/worktree.c
+++ b/worktree.c
@@ -3,6 +3,17 @@
 #include "strbuf.h"
 #include "worktree.h"
 
+void free_worktrees(struct worktree **worktrees)
+{
+   int i = 0;
+
+   for (i = 0; worktrees[i]; i++) {
+   free(worktrees[i]->path);
+   free(worktrees[i]);
+   }
+   free (worktrees);
+}
+
 /*
  * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,
  * set is_detached to 1 (0) if the ref is detatched (is not detached).
@@ -38,87 +49,138 @@ static int parse_ref(char *path_to_ref, struct strbuf 
*ref, int *is_detached)
return 0;
 }
 
-static char *find_main_symref(const char *symref, const char *branch)
+/**
+ * get the main worktree
+ */
+static struct worktree *get_main_worktree(void)
 {
-   struct strbuf sb = STRBUF_INIT;
+   struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
+   struct strbuf worktree_path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
-   char *existing = NULL;
+   struct strbuf head_ref = STRBUF_INIT;
 
-   strbuf_addf(, "%s/%s", get_git_common_dir(), symref);
-   if (parse_ref(path.buf, , NULL) < 0)
-   goto done;
-   if (strcmp(sb.buf, branch))
-   goto done;
-   strbuf_addstr(, get_git_common_dir());
-   strbuf_strip_suffix(, ".git");
-   existing = strbuf_detach(, NULL);
-done:
+   strbuf_addf(, "%s", absolute_path(get_git_common_dir()));
+   strbuf_addbuf(_path, );
+   if (!strbuf_strip_suffix(_path, "/.git"))
+   strbuf_strip_suffix(_path, "/.");
+
+   strbuf_addf(, "%s/HEAD", get_git_common_dir());
+
+   if (parse_ref(path.buf, _ref, NULL) >= 0) {
+   worktree = xmalloc(sizeof(struct worktree));
+   worktree->path = strbuf_detach(_path, NULL);
+   worktree->git_dir = strbuf_detach(, NULL);
+   }
strbuf_release();
-   strbuf_release();
strbuf_release();
-
-   return existing;
+   strbuf_release(_path);
+   strbuf_release(_ref);
+   return worktree;
 }
 
-static char *find_linked_symref(const char *symref, const char *branch,
-   const char *id)
+static struct worktree *get_linked_worktree(const char *id)
 {
-   struct strbuf sb = STRBUF_INIT;
+   struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
+   struct strbuf worktree_path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
-   char *existing = NULL;
+   struct strbuf head_ref = STRBUF_INIT;
 
if (!id)
die("Missing linked worktree name");
 
-   strbuf_addf(, "%s/worktrees/%s/%s", get_git_common_dir(), id, 
symref);
-
-   if (parse_ref(path.buf, , NULL) < 0)
-   goto done;
-   if (strcmp(sb.buf, branch))
+   strbuf_addf(, "%s/worktrees/%s",
+   absolute_path(get_git_common_dir()), id);
+   strbuf_addf(, "%s/gitdir", gitdir.buf);
+   if (strbuf_read_file(_path, path.buf, 0) <= 0)
+   /* invalid gitdir file */
goto done;
+
+   strbuf_rtrim(_path);
+   if (!strbuf_strip_suffix(_path, "/.git")) {
+   strbuf_reset(_path);
+   strbuf_addstr(_path, absolute_path("."));
+   strbuf_strip_suffix(_path, "/.");
+   }
+
strbuf_reset();
-   strbuf_addf(, "%s/worktrees/%s/gitdir", get_git_common_dir(), id);
-   if (strbuf_read_file(, path.buf, 0) <= 0)
-   goto done;
-   strbuf_rtrim();
-   strbuf_strip_suffix(, ".git");
+   strbuf_addf(, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+
+   if (parse_ref(path.buf, _ref, NULL) >= 0) {
+   worktree = xmalloc(sizeof(struct worktree));
+   worktree->path = strbuf_detach(_path, NULL);
+   worktree->git_dir = strbuf_detach(, NULL);
+   }
 
-   existing = strbuf_detach(, NULL);
 done:
strbuf_release();
-   strbuf_release();
strbuf_release();
-
-   return existing;
+   strbuf_release(_path);
+   strbuf_release(_ref);
+   return worktree;
 }
 
-char *find_shared_symref(const char *symref, const char *target)
+struct worktree **get_worktrees(void)
 {
+   struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
DIR *dir;
struct dirent *d;
-   char *existing;
+   int counter = 0, alloc = 2;
+
+   list = xmalloc(alloc * sizeof(struct worktree *));
 
-  

[PATCH v10 2/5] worktree: refactor find_linked_symref function

[Michael Rappazzo]

Refactoring will help transition this code to provide additional useful
worktree functions.

Signed-off-by: Michael Rappazzo 
---
 worktree.c | 96 --
 1 file changed, 69 insertions(+), 27 deletions(-)

diff --git a/worktree.c b/worktree.c
index 10e1496..3c2498a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -3,6 +3,64 @@
 #include "strbuf.h"
 #include "worktree.h"
 
+/*
+ * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,
+ * set is_detached to 1 (0) if the ref is detatched (is not detached).
+ *
+ * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so
+ * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses
+ * git_path). Parse the ref ourselves.
+ *
+ * return -1 if the ref is not a proper ref, 0 otherwise (success)
+ */
+static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
+{
+   if (is_detached)
+   *is_detached = 0;
+   if (!strbuf_readlink(ref, path_to_ref, 0)) {
+   /* HEAD is symbolic link */
+   if (!starts_with(ref->buf, "refs/") ||
+   check_refname_format(ref->buf, 0))
+   return -1;
+   } else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
+   /* textual symref or detached */
+   if (!starts_with(ref->buf, "ref:")) {
+   if (is_detached)
+   *is_detached = 1;
+   } else {
+   strbuf_remove(ref, 0, strlen("ref:"));
+   strbuf_trim(ref);
+   if (check_refname_format(ref->buf, 0))
+   return -1;
+   }
+   } else
+   return -1;
+   return 0;
+}
+
+static char *find_main_symref(const char *symref, const char *branch)
+{
+   struct strbuf sb = STRBUF_INIT;
+   struct strbuf path = STRBUF_INIT;
+   struct strbuf gitdir = STRBUF_INIT;
+   char *existing = NULL;
+
+   strbuf_addf(, "%s/%s", get_git_common_dir(), symref);
+   if (parse_ref(path.buf, , NULL) < 0)
+   goto done;
+   if (strcmp(sb.buf, branch))
+   goto done;
+   strbuf_addstr(, get_git_common_dir());
+   strbuf_strip_suffix(, ".git");
+   existing = strbuf_detach(, NULL);
+done:
+   strbuf_release();
+   strbuf_release();
+   strbuf_release();
+
+   return existing;
+}
+
 static char *find_linked_symref(const char *symref, const char *branch,
const char *id)
 {
@@ -11,36 +69,20 @@ static char *find_linked_symref(const char *symref, const 
char *branch,
struct strbuf gitdir = STRBUF_INIT;
char *existing = NULL;
 
-   /*
-* $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside
-* $GIT_DIR so resolve_ref_unsafe() won't work (it uses
-* git_path). Parse the ref ourselves.
-*/
-   if (id)
-   strbuf_addf(, "%s/worktrees/%s/%s", get_git_common_dir(), 
id, symref);
-   else
-   strbuf_addf(, "%s/%s", get_git_common_dir(), symref);
+   if (!id)
+   die("Missing linked worktree name");
 
-   if (!strbuf_readlink(, path.buf, 0)) {
-   if (!starts_with(sb.buf, "refs/") ||
-   check_refname_format(sb.buf, 0))
-   goto done;
-   } else if (strbuf_read_file(, path.buf, 0) >= 0 &&
-   starts_with(sb.buf, "ref:")) {
-   strbuf_remove(, 0, strlen("ref:"));
-   strbuf_trim();
-   } else
+   strbuf_addf(, "%s/worktrees/%s/%s", get_git_common_dir(), id, 
symref);
+
+   if (parse_ref(path.buf, , NULL) < 0)
goto done;
if (strcmp(sb.buf, branch))
goto done;
-   if (id) {
-   strbuf_reset();
-   strbuf_addf(, "%s/worktrees/%s/gitdir", 
get_git_common_dir(), id);
-   if (strbuf_read_file(, path.buf, 0) <= 0)
-   goto done;
-   strbuf_rtrim();
-   } else
-   strbuf_addstr(, get_git_common_dir());
+   strbuf_reset();
+   strbuf_addf(, "%s/worktrees/%s/gitdir", get_git_common_dir(), id);
+   if (strbuf_read_file(, path.buf, 0) <= 0)
+   goto done;
+   strbuf_rtrim();
strbuf_strip_suffix(, ".git");
 
existing = strbuf_detach(, NULL);
@@ -59,7 +101,7 @@ char *find_shared_symref(const char *symref, const char 
*target)
struct dirent *d;
char *existing;
 
-   if ((existing = find_linked_symref(symref, target, NULL)))
+   if ((existing = find_main_symref(symref, target)))
return existing;
 
strbuf_addf(, "%s/worktrees", get_git_common_dir());
-- 
2.6.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  

[PATCH v10 4/5] worktree: add details to the worktree struct

[Michael Rappazzo]

In addition to the absolute path in the worktree struct, add the location
of the git dir, the head ref (if not detached), the head revision sha1,
whether or not head is detached, and whether or not the worktree is a
bare repo.

Signed-off-by: Michael Rappazzo 
---
 worktree.c | 55 ---
 worktree.h |  4 
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/worktree.c b/worktree.c
index c2e6db0..981f810 100644
--- a/worktree.c
+++ b/worktree.c
@@ -9,6 +9,8 @@ void free_worktrees(struct worktree **worktrees)
 
for (i = 0; worktrees[i]; i++) {
free(worktrees[i]->path);
+   free(worktrees[i]->git_dir);
+   free(worktrees[i]->head_ref);
free(worktrees[i]);
}
free (worktrees);
@@ -50,6 +52,21 @@ static int parse_ref(char *path_to_ref, struct strbuf *ref, 
int *is_detached)
 }
 
 /**
+ * Add the head_sha1 and head_ref (if not detached) to the given worktree
+ */
+static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
+{
+   if (head_ref->len) {
+   if (worktree->is_detached) {
+   get_sha1_hex(head_ref->buf, worktree->head_sha1);
+   } else {
+   resolve_ref_unsafe(head_ref->buf, 0, 
worktree->head_sha1, NULL);
+   worktree->head_ref = strbuf_detach(head_ref, NULL);
+   }
+   }
+}
+
+/**
  * get the main worktree
  */
 static struct worktree *get_main_worktree(void)
@@ -59,19 +76,29 @@ static struct worktree *get_main_worktree(void)
struct strbuf worktree_path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
struct strbuf head_ref = STRBUF_INIT;
+   int is_bare = 0;
+   int is_detached = 0;
 
strbuf_addf(, "%s", absolute_path(get_git_common_dir()));
strbuf_addbuf(_path, );
-   if (!strbuf_strip_suffix(_path, "/.git"))
+   is_bare = !strbuf_strip_suffix(_path, "/.git");
+   if (is_bare)
strbuf_strip_suffix(_path, "/.");
 
strbuf_addf(, "%s/HEAD", get_git_common_dir());
 
-   if (parse_ref(path.buf, _ref, NULL) >= 0) {
-   worktree = xmalloc(sizeof(struct worktree));
-   worktree->path = strbuf_detach(_path, NULL);
-   worktree->git_dir = strbuf_detach(, NULL);
-   }
+   if (parse_ref(path.buf, _ref, _detached) < 0)
+   goto done;
+
+   worktree = xmalloc(sizeof(struct worktree));
+   worktree->path = strbuf_detach(_path, NULL);
+   worktree->git_dir = strbuf_detach(, NULL);
+   worktree->is_bare = is_bare;
+   worktree->head_ref = NULL;
+   worktree->is_detached = is_detached;
+   add_head_info(_ref, worktree);
+
+done:
strbuf_release();
strbuf_release();
strbuf_release(_path);
@@ -86,6 +113,7 @@ static struct worktree *get_linked_worktree(const char *id)
struct strbuf worktree_path = STRBUF_INIT;
struct strbuf gitdir = STRBUF_INIT;
struct strbuf head_ref = STRBUF_INIT;
+   int is_detached = 0;
 
if (!id)
die("Missing linked worktree name");
@@ -107,11 +135,16 @@ static struct worktree *get_linked_worktree(const char 
*id)
strbuf_reset();
strbuf_addf(, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
 
-   if (parse_ref(path.buf, _ref, NULL) >= 0) {
-   worktree = xmalloc(sizeof(struct worktree));
-   worktree->path = strbuf_detach(_path, NULL);
-   worktree->git_dir = strbuf_detach(, NULL);
-   }
+   if (parse_ref(path.buf, _ref, _detached) < 0)
+   goto done;
+
+   worktree = xmalloc(sizeof(struct worktree));
+   worktree->path = strbuf_detach(_path, NULL);
+   worktree->git_dir = strbuf_detach(, NULL);
+   worktree->is_bare = 0;
+   worktree->head_ref = NULL;
+   worktree->is_detached = is_detached;
+   add_head_info(_ref, worktree);
 
 done:
strbuf_release();
diff --git a/worktree.h b/worktree.h
index 7022029..b4b3dda 100644
--- a/worktree.h
+++ b/worktree.h
@@ -4,6 +4,10 @@
 struct worktree {
char *path;
char *git_dir;
+   char *head_ref;
+   unsigned char head_sha1[20];
+   int is_detached;
+   int is_bare;
 };
 
 /* Functions for acting on the information about worktrees. */
-- 
2.6.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 00/10] port branch.c to use ref-filter's printing options

[Karthik Nayak]

On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
 wrote:
>> + unsigned int nobracket = 0;
>> +
>> + if (!strcmp(valp, ",nobracket"))
>> + nobracket = 1;
>
> The code to parse comma-separated values is different here and
> elsewhere. I'd rather have the same code (possibly factored into a
> helper function), both to get consistent behavior (you're not allowing
> %(upstream:nobracket,track) for example, right?) and consistent code.
>

Speaking of comma-separated values, the only other place we use that is
in the align atom. Also I find this very specific to get a function out of.
Somehow I think this is the simplest way to go about this.

-- 
Regards,
Karthik Nayak
--
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 00/10] port branch.c to use ref-filter's printing options

[Matthieu Moy]

Karthik Nayak  writes:

> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
>  wrote:
>>> + unsigned int nobracket = 0;
>>> +
>>> + if (!strcmp(valp, ",nobracket"))
>>> + nobracket = 1;
>>
>> The code to parse comma-separated values is different here and
>> elsewhere. I'd rather have the same code (possibly factored into a
>> helper function), both to get consistent behavior (you're not allowing
>> %(upstream:nobracket,track) for example, right?) and consistent code.
>>
>
> Speaking of comma-separated values, the only other place we use that is
> in the align atom. Also I find this very specific to get a function out of.
> Somehow I think this is the simplest way to go about this.

Well, most pieces of code start with one instance, then two, then
more ;-). When the second instance starts being different from the
first, it doesn't give a good example for the future third instance.

This particular piece of code is so important and I won't fight for a
better factored one, but in general "there are only two instances" is a
dubious argument to avoid refactoring.

Still, I find it weird to force the nobracket to be always at the same
position.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 00/10] port branch.c to use ref-filter's printing options

[Karthik Nayak]

On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy
>>  wrote:
 + unsigned int nobracket = 0;
 +
 + if (!strcmp(valp, ",nobracket"))
 + nobracket = 1;
>>>
>>> The code to parse comma-separated values is different here and
>>> elsewhere. I'd rather have the same code (possibly factored into a
>>> helper function), both to get consistent behavior (you're not allowing
>>> %(upstream:nobracket,track) for example, right?) and consistent code.
>>>
>>
>> Speaking of comma-separated values, the only other place we use that is
>> in the align atom. Also I find this very specific to get a function out of.
>> Somehow I think this is the simplest way to go about this.
>
> Well, most pieces of code start with one instance, then two, then
> more ;-). When the second instance starts being different from the
> first, it doesn't give a good example for the future third instance.
>

Totally agree with you here.

> This particular piece of code is so important and I won't fight for a
> better factored one, but in general "there are only two instances" is a
> dubious argument to avoid refactoring.
>
> Still, I find it weird to force the nobracket to be always at the same
> position.
>

No i mean I could follow up with the way we use it in align, but I don't see
how I can make a function out of this.

-- 
Regards,
Karthik Nayak
--
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

`-u`/`update-head-ok` unsupported on pull

[Victor Engmark]

According to the documentation these options are supported:
$ git help pull | grep -e '--update-head-ok'
   -u, --update-head-ok

However:
$ git pull --update-head-ok
error: unknown option `update-head-ok'

Using:
$ git --version
git version 2.6.1
$ pacman --query --info git | grep ^Version
Version: 2.6.1-1

Am I missing something? The manual seems to be for the right version:

$ git help pull | tail -n1 | tr -s ' '
Git 2.6.1 10/06/2015 GIT-PULL(1)

I'm running the system installed Git:

$ type -a git
git is /usr/bin/git

Cheers
Victor
--
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 05/10] ref-filter: adopt get_head_description() from branch.c

[Matthieu Moy]

Karthik Nayak  writes:

> Copy the implementation of get_head_description() from branch.c.  This
> gives a description of the HEAD ref if called. This is used as the
> refname for the HEAD ref whenever the FILTER_REFS_DETACHED_HEAD option
> is used. Make it public because we need it to calculate the length of
> the HEAD refs description in branch.c:calc_maxwidth() when we port
> branch.c to use ref-filter APIs.

If it's made public, then it could be simpler to just _move_ the
function instead of copying it. You'd need to add a #include
 to branch.c, but you're going to add one anyway.

Code movement is more "git blame" friendly than code copy, and as a
reviewer I'd rather see the code movement here and not hear about it
later in the series.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] notes: Allow treeish expressions as notes ref

[Junio C Hamano]

Mike Hommey  writes:

> After refreshing the patch against current "next", it appears that
> there is such a distinction:
>
> $ ./git-log --notes=fdsfgsfdg HEAD^! --pretty=short
> warning: notes ref refs/notes/fdsfgsfdg is invalid
> commit e5b68b2e879608d881c2e3600ce84962fcdefc88
> Author: Mike Hommey 
>
> notes: allow treeish expressions as notes ref
>
> $ ./git-log --notes=foo HEAD^! --pretty=short
> commit e5b68b2e879608d881c2e3600ce84962fcdefc88
> Author: Mike Hommey 
>
> notes: allow treeish expressions as notes ref

Good; thanks for checking.

--
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 3/4] Add a function to release all packs

[Johannes Schindelin]

Hi Junio,

On 2015-10-07 19:49, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
>> On Windows, files that are in use cannot be removed or renamed. That
>> means that we have to release pack files when we are about to, say,
>> repack them. Let's introduce a convenient function to close them
>> pack files.
>>
>> While at it, we consolidate the close windows/close fd/close index
>> stanza in `free_pack_by_name()` into the `close_pack()` function that
>> is used by the new `close_all_packs()` function to avoid repeated code.
>>
>> Signed-off-by: Johannes Schindelin 
>> ---
> 
> This is the only one that was updated among the four and the change
> looks sensible.  I'd reword the end of the first paragraph of the
> proposed log message, though, to replace "close them pack files"
> with "close all the pack files and their idx files".
> 
> Thanks.

This time I was more clever and checked `pu` before sending off a fixed patch, 
seeing that you patched this already.

Thanks,
Dscho
--
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 2/2] pack-objects: do not get distracted by broken symrefs

[Johannes Schindelin]

Hi Junio,

On 2015-10-07 19:45, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
>> It is quite possible for, say, a remote HEAD to become broken, e.g.
>> when the default branch was renamed.
>>
>> We should still be able to pack our objects when such a thing happens;
>> simply ignore broken symrefs (because they cannot matter for the packing
>> process anyway).
>>
>> This fixes https://github.com/git-for-windows/git/issues/423
>>
>> Signed-off-by: Johannes Schindelin 
>> ---
> 
> It seems that the result of applying these two patches and log
> messages of them are the same with what I queued on 'pu', except
> that the first of these two patches adds a test with a wrong name
> and then this one does "oops, that was misnamed".  So I'll keep what
> is already queued.

Sorry for fixing up the wrong commit. I honestly meant to fix up the first one. 
And thank you for fixing it up in `pu` already; I should have known better and 
check first whether you fixed it.

However, there was one more change I made: I wanted to have that link to 
https://github.com/git-for-windows/git/issues/423 in the commit message to 
better link the original report with the commit.

Would yo kindly add the line

This fixes https://github.com/git-for-windows/git/issues/423

before the Signed-off-by lines?

Thanks,
Dscho
--
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 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Matthieu Moy]

Karthik Nayak  writes:

> Add support for %(upstream:track,nobracket) which will print the
> tracking information without the brackets (i.e. "ahead N, behind M").
>
> Add test and documentation for the same.
> ---
>  Documentation/git-for-each-ref.txt |  6 --
>  ref-filter.c   | 28 +++-
>  t/t6300-for-each-ref.sh|  9 +
>  3 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index c713ec0..a38cbf6 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -114,8 +114,10 @@ upstream::
>   `refname` above.  Additionally respects `:track` to show
>   "[ahead N, behind M]" and `:trackshort` to show the terse
>   version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
> - or "=" (in sync).  Has no effect if the ref does not have
> - tracking information associated with it.
> + or "=" (in sync).  Append `:track,nobracket` to show tracking
> + information without brackets (i.e "ahead N, behind M").  Has
> + no effect if the ref does not have tracking information
> + associated with it.
>  
>  push::
>   The name of a local ref which represents the `@{push}` location
> diff --git a/ref-filter.c b/ref-filter.c
> index 6a38089..6044eb0 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item *ref)
>   if (!strcmp(formatp, "short"))
>   refname = shorten_unambiguous_ref(refname,
> warn_ambiguous_refs);
> - else if (!strcmp(formatp, "track") &&
> + else if (skip_prefix(formatp, "track", ) &&
> +  strcmp(formatp, "trackshort") &&
>(starts_with(name, "upstream") ||
> starts_with(name, "push"))) {
>   char buf[40];
> + unsigned int nobracket = 0;
> +
> + if (!strcmp(valp, ",nobracket"))
> + nobracket = 1;
>  
>   if (stat_tracking_info(branch, _ours,
>  _theirs, NULL)) {
> - v->s = "[gone]";
> + if (nobracket)
> + v->s = "gone";
> + else
> + v->s = "[gone]";
>   continue;
>   }
>  
>   if (!num_ours && !num_theirs)
>   v->s = "";
>   else if (!num_ours) {
> - sprintf(buf, "[behind %d]", num_theirs);
> + if (nobracket)
> + sprintf(buf, "behind %d", 
> num_theirs);
> + else
> + sprintf(buf, "[behind %d]", 
> num_theirs);
>   v->s = xstrdup(buf);
>   } else if (!num_theirs) {
> - sprintf(buf, "[ahead %d]", num_ours);
> + if (nobracket)
> + sprintf(buf, "ahead %d", 
> num_ours);
> + else
> + sprintf(buf, "[ahead %d]", 
> num_ours);
>   v->s = xstrdup(buf);
>   } else {
> - sprintf(buf, "[ahead %d, behind %d]",
> + if (nobracket)
> + sprintf(buf, "ahead %d, behind 
> %d",
> + num_ours, num_theirs);
> + else
> + sprintf(buf, "[ahead %d, behind 
> %d]",
>   num_ours, num_theirs);
>   v->s = xstrdup(buf);
>   }
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 4f620bf..7ab8bf8 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '
>  '
>  
>  cat >expected < +ahead 1
> +EOF
> +
> +test_expect_success 'Check upstream:track,nobracket format' '
> + git for-each-ref --format="%(upstream:track,nobracket)" refs/heads 
> >actual &&
> + test_cmp expected actual
> 

Re: [PATCH v2 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Matthieu Moy]

Oops, sorry, I sent the wrong message, this one is empty. Please ignore.

Matthieu Moy  writes:

> Karthik Nayak  writes:
>
>> Add support for %(upstream:track,nobracket) which will print the
>> tracking information without the brackets (i.e. "ahead N, behind M").
>>
>> Add test and documentation for the same.
>> ---
>>  Documentation/git-for-each-ref.txt |  6 --
>>  ref-filter.c   | 28 +++-
>>  t/t6300-for-each-ref.sh|  9 +
>>  3 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> index c713ec0..a38cbf6 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -114,8 +114,10 @@ upstream::
>>  `refname` above.  Additionally respects `:track` to show
>>  "[ahead N, behind M]" and `:trackshort` to show the terse
>>  version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
>> -or "=" (in sync).  Has no effect if the ref does not have
>> -tracking information associated with it.
>> +or "=" (in sync).  Append `:track,nobracket` to show tracking
>> +information without brackets (i.e "ahead N, behind M").  Has
>> +no effect if the ref does not have tracking information
>> +associated with it.
>>  
>>  push::
>>  The name of a local ref which represents the `@{push}` location
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 6a38089..6044eb0 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1112,27 +1112,45 @@ static void populate_value(struct ref_array_item 
>> *ref)
>>  if (!strcmp(formatp, "short"))
>>  refname = shorten_unambiguous_ref(refname,
>>warn_ambiguous_refs);
>> -else if (!strcmp(formatp, "track") &&
>> +else if (skip_prefix(formatp, "track", ) &&
>> + strcmp(formatp, "trackshort") &&
>>   (starts_with(name, "upstream") ||
>>starts_with(name, "push"))) {
>>  char buf[40];
>> +unsigned int nobracket = 0;
>> +
>> +if (!strcmp(valp, ",nobracket"))
>> +nobracket = 1;
>>  
>>  if (stat_tracking_info(branch, _ours,
>> _theirs, NULL)) {
>> -v->s = "[gone]";
>> +if (nobracket)
>> +v->s = "gone";
>> +else
>> +v->s = "[gone]";
>>  continue;
>>  }
>>  
>>  if (!num_ours && !num_theirs)
>>  v->s = "";
>>  else if (!num_ours) {
>> -sprintf(buf, "[behind %d]", num_theirs);
>> +if (nobracket)
>> +sprintf(buf, "behind %d", 
>> num_theirs);
>> +else
>> +sprintf(buf, "[behind %d]", 
>> num_theirs);
>>  v->s = xstrdup(buf);
>>  } else if (!num_theirs) {
>> -sprintf(buf, "[ahead %d]", num_ours);
>> +if (nobracket)
>> +sprintf(buf, "ahead %d", 
>> num_ours);
>> +else
>> +sprintf(buf, "[ahead %d]", 
>> num_ours);
>>  v->s = xstrdup(buf);
>>  } else {
>> -sprintf(buf, "[ahead %d, behind %d]",
>> +if (nobracket)
>> +sprintf(buf, "ahead %d, behind 
>> %d",
>> +num_ours, num_theirs);
>> +else
>> +sprintf(buf, "[ahead %d, behind 
>> %d]",
>>  num_ours, num_theirs);
>>  v->s = xstrdup(buf);
>>  }
>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
>> index 4f620bf..7ab8bf8 100755
>> --- a/t/t6300-for-each-ref.sh
>> +++ b/t/t6300-for-each-ref.sh
>> @@ -344,6 +344,15 @@ test_expect_success 'Check upstream:track format' '
>>  '
>>  
>>  cat >expected <> +ahead 1
>> +EOF
>> +
>> 

Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms

[Matthieu Moy]

Karthik Nayak  writes:

> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -134,9 +134,17 @@ align::
>   `` is either left, right or middle, default being
>   left and `` is the total length of the content with
>   alignment. If the contents length is more than the width then
> - no alignment is performed. If used with '--quote' everything
> - in between %(align:...) and %(end) is quoted, but if nested
> - then only the topmost level performs quoting.
> + no alignment is performed.
> +
> +if::
> + Used as %(if)..%(then)..(%end) or %(if)..%(then)..%(else)..%(end).

I guess you forgot to replace .. with ... (I think you agreed with me
that it was better).

> @@ -69,10 +72,16 @@ struct contents {
>   struct object_id oid;
>  };
>  
> +struct if_then_else {
> + unsigned int then_atom : 1,
> + else_atom : 1,

Maybe "then_atom_seen" and "else_atom_seen" would be better names. Or
maybe they'd be too long, I leave it up to you.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors

[Junio C Hamano]

Uwe Kleine-König   writes:

> Hello,
>
> when applying the mail below (without the '> ' prefix) using git am
> --scissors the result looks like:
>
>   $ git show
>   commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61
>   Author: Tony Lindgren 
>   Date:   Tue Oct 6 05:36:17 2015 -0700
>
>   memory: omap-gpmc: Fix unselectable debug option for GPMC
>   
>   MIME-Version: 1.0
>   Content-Type: text/plain; charset=UTF-8
>   Content-Transfer-Encoding: 8bit
>   
>   Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for 
> debug")
>   added a debug option for GPMC, but somehow managed to keep it 
> unselectable.
>   
>   [...]
>
>   $ git version
>   git version 2.6.0
>
> The obvious improvement is to strip all headers like git am does without
> --scissors.

Does this have anything to do with scissors, though?  If you remove
everything before "8< ---" in the body of Tony's message (i.e. keep
the in-body headers starting with "From:" and ending with CTE) and
try again, I would suspect that you will get the same result.

I also think that the "MIME-Version" thing is what gives this;
mailinfo and am do not really use it, and consider that the in-body
header ends there.

The right approach to tweak mailinfo to cope with this better would
be to keep a bit more state inside mailinfo.c::handle_commit_msg()
so that if we are (1) using in-body headers, (2) have already seen
_some_ valid in-body header like "Subject:" and "From: ", and (3)
have not seen a blank line, discard lines that we do not care about
(e.g. "MIME-VERSION: 1.0").


> If someone wants a bounce of the original mail, just ask per PM.

I have no idea what you are talking about here...


> On Wed, Oct 07, 2015 at 03:41:19AM -0700, Tony Lindgren wrote:
>> * Uwe Kleine-König  [151007 00:57]:
>> > On Wed, Oct 07, 2015 at 10:45:50AM +0300, Roger Quadros wrote:
>> > > 
>> > > How about this instead?
>> > > 
>> > > NOTE: Apart from matching the register setup with the bootloader you 
>> > > also need to
>> > > match the GPMC FCLK frequency used by the bootloader else the GPMC 
>> > > timings
>> > > won't be identical with the bootloader timings.
>> > Yeah, sounds better, thanks.
>> > 
>> > > Also you might need to build this patch on top of
>> > > http://article.gmane.org/gmane.linux.kernel/2054796
>> > I talked to Tony about this patch yesterday on irc, but I didn't find it
>> > in the archives yet when I sent my mail.
>> 
>> Yes sorry here's a repost with your and Roger's changes folded in and
>> edited a bit. Probably best to keep them together with this patch.
>> 
>> Does the following look OK to you guys?
>> 
>> Regards,
>> 
>> Tony
>> 
>> 8< 
>> From: Tony Lindgren 
>> Date: Tue, 6 Oct 2015 05:36:17 -0700
>> Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>> 
>> Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
>> added a debug option for GPMC, but somehow managed to keep it unselectable.
>> 
>> This probably happened because I had some uncommitted changes and the
>> ...
--
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] merge_recursive_options: introduce the "gentle" flag

[Johannes Schindelin]

Traditionally, all of Git's operations were intended as single
executables to be run once and exit, not as library functions with
careful error code paths. Therefore, it was okay for those operations
to simply die() with an error.

However, this assumption no longer holds true: builtin `am` calls
merge_recursive_generic() as a regular library function whose return
value indicates whether there was a problem.

Throughout Git's source code, that paradigm (to return an error instead
of die()ing) is called "gentle".

Let's introduce this flag and heed it in as many places as is easily
done.

Signed-off-by: Johannes Schindelin 
---
 merge-recursive.c | 44 +++-
 merge-recursive.h |  1 +
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 44d85be..37528c9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -266,8 +266,12 @@ struct tree *write_tree_from_memory(struct merge_options 
*o)
active_cache_tree = cache_tree();
 
if (!cache_tree_fully_valid(active_cache_tree) &&
-   cache_tree_update(_index, 0) < 0)
-   die(_("error building trees"));
+   cache_tree_update(_index, 0) < 0) {
+   if (!o->gentle)
+   die(_("error building trees"));
+   error(_("error building trees"));
+   return NULL;
+   }
 
result = lookup_tree(active_cache_tree->sha1);
 
@@ -712,6 +716,8 @@ static int make_room_for_path(struct merge_options *o, 
const char *path)
error(msg, path, _(": perhaps a D/F conflict?"));
return -1;
}
+   if (o->gentle)
+   return error(msg, path, "");
die(msg, path, "");
}
 
@@ -1340,8 +1346,11 @@ static int process_renames(struct merge_options *o,
const char *ren2_src = ren2->pair->one->path;
const char *ren2_dst = ren2->pair->two->path;
enum rename_type rename_type;
-   if (strcmp(ren1_src, ren2_src) != 0)
+   if (strcmp(ren1_src, ren2_src) != 0) {
+   if (o->gentle)
+   return error("ren1_src != ren2_src");
die("ren1_src != ren2_src");
+   }
ren2->dst_entry->processed = 1;
ren2->processed = 1;
if (strcmp(ren1_dst, ren2_dst) != 0) {
@@ -1374,8 +1383,11 @@ static int process_renames(struct merge_options *o,
char *ren2_dst;
ren2 = lookup->util;
ren2_dst = ren2->pair->two->path;
-   if (strcmp(ren1_dst, ren2_dst) != 0)
+   if (strcmp(ren1_dst, ren2_dst) != 0) {
+   if (o->gentle)
+   return error("ren1_dst != ren2_dst");
die("ren1_dst != ren2_dst");
+   }
 
clean_merge = 0;
ren2->processed = 1;
@@ -1818,6 +1830,11 @@ int merge_trees(struct merge_options *o,
code = git_merge_trees(o->call_depth, common, head, merge);
 
if (code != 0) {
+   if (o->gentle)
+   return error(_("merging of trees %s and %s failed"),
+   sha1_to_hex(head->object.sha1),
+   sha1_to_hex(merge->object.sha1));
+
if (show(o, 4) || o->call_depth)
die(_("merging of trees %s and %s failed"),
sha1_to_hex(head->object.sha1),
@@ -1864,8 +1881,8 @@ int merge_trees(struct merge_options *o,
else
clean = 1;
 
-   if (o->call_depth)
-   *result = write_tree_from_memory(o);
+   if (o->call_depth && !(*result = write_tree_from_memory(o)))
+   return -1;
 
return clean;
 }
@@ -1940,14 +1957,18 @@ int merge_recursive(struct merge_options *o,
saved_b2 = o->branch2;
o->branch1 = "Temporary merge branch 1";
o->branch2 = "Temporary merge branch 2";
-   merge_recursive(o, merged_common_ancestors, iter->item,
-   NULL, _common_ancestors);
+   if (merge_recursive(o, merged_common_ancestors, iter->item,
+   NULL, _common_ancestors) < 0)
+   return -1;
o->branch1 = saved_b1;
o->branch2 = saved_b2;
o->call_depth--;
 
-   if (!merged_common_ancestors)
+   if (!merged_common_ancestors) {
+   if (o->gentle)
+   return error(_("merge returned no commit"));
 

[PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails

[Johannes Schindelin]

Brendan Forster noticed that we no longer see the helpful message after
a failed `git pull --rebase`. It turns out that the builtin `am` calls
the recursive merge function directly, not via a separate process. But
that function was not really safe to be called that way, as it die()s
pretty liberally.

As a consequence, the code that wanted to see whether the merge failed
is not even executed, and the helpful message advising how to fix the
mess is not displayed.

This topic branch fixes this.

Please note that there are a couple of unhandled die() calls in
merge-recursive.c, most of which indicate code paths that should
never be reached (except in the case of a bug).

But there are two other functions that can die(): `update_file_flags()`
(which returns void) and `merge_file_1()`.

The latter function is already nested quite deeply so that the code
would have to be made much uglier to handle the `gentle` flag. It is
also not quite clear to me whether those error cases can be hit in a
regular `git pull --rebase` (which is what I really care about most).

As `update_file_flags()` is called by functions returning void and
that are again called in turn by other functions that also return void,
fixing this part is more involved, so I would like to avoid it, unless
it is deemed absolutely necessary to address in this patch series.


Johannes Schindelin (2):
  merge_recursive_options: introduce the "gentle" flag
  pull --rebase: reinstate helpful message on abort

 builtin/am.c  |  1 +
 merge-recursive.c | 44 +++-
 merge-recursive.h |  1 +
 3 files changed, 37 insertions(+), 9 deletions(-)

-- 
2.6.1.windows.1


--
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] pull --rebase: reinstate helpful message on abort

[Johannes Schindelin]

When calling `git pull --rebase`, things can go wrong. In such a case,
we want to tell the user about the most common ways out of this fix:

Patch failed at [...]
[...]

When you have resolved this problem, run "git rebase
--continue". If you prefer to skip this patch, run "git rebase
--skip" instead. To check out the original branch and stop
rebasing, run "git rebase --abort".

However, with the switch to the builtin `git-am` we call the merge
recursive function directly, which usually die()s in case of an
error. Not what we want.

So let's set the newly-introduced `gentle` flag to get a chance to
print the helpful advice.

Reported by Brendan Forster.

Signed-off-by: Johannes Schindelin 
---
 builtin/am.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07..c472937 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1653,6 +1653,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
 
init_merge_options();
 
+   o.gentle = 1;
o.branch1 = "HEAD";
his_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
o.branch2 = his_tree_name;
-- 
2.6.1.windows.1


--
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 strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors

[Uwe Kleine-König]

Hello Junio,

On Thu, Oct 08, 2015 at 01:04:01PM -0700, Junio C Hamano wrote:
> Uwe Kleine-König   writes:
> 
> >> Does this have anything to do with scissors, though?  If you remove
> >> everything before "8< ---" in the body of Tony's message (i.e. keep
> >> the in-body headers starting with "From:" and ending with CTE) and
> >> try again, I would suspect that you will get the same result.
> > No, you're wrong here:
> >
> > u...@dude.ptx:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS 
> > From: Tony Lindgren 
> > Date: Tue, 6 Oct 2015 05:36:17 -0700
> > Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> > added a debug option for GPMC, but somehow managed to keep it unselectable.
> 
> I think you are the one who misread my question.  I said "keep the
> in-body headers", didn't I?  If you did the "head", then you would
> see something like this:

Ah got it. Yes, you're right. (Subject and Date are actually different
between real and in-body headers, but that's not important. git am picks
up the in-body headers.)
 
> > u...@dude.ptx:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS 
> > From: Tony Lindgren 
> > Date: Tue, 6 Oct 2015 05:36:17 -0700
> > Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> > ... probably Received: and all other junk from your mailbox ...
> >
> > From: Tony Lindgren 
> > Date: Tue, 6 Oct 2015 05:36:17 -0700
> > Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> > added a debug option for GPMC, but somehow managed to keep it unselectable.
> 
> >> I also think that the "MIME-Version" thing is what gives this;
> >> mailinfo and am do not really use it, and consider that the in-body
> >> header ends there.
> >
> > I failed to follow you here.
> 
> I think if you tried the example with in-body header, you will see
> what I meant.
> 
> >
> >> The right approach to tweak mailinfo to cope with this better would
> >> be to keep a bit more state inside mailinfo.c::handle_commit_msg()
> >> so that if we are (1) using in-body headers, (2) have already seen
> >> _some_ valid in-body header like "Subject:" and "From: ", and (3)
> >> have not seen a blank line, discard lines that we do not care about
> >> (e.g. "MIME-VERSION: 1.0").

The right thing should also happen if MIME-Version comes above Subject
in the body but other than that I'm with you here.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
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 2/2] pack-objects: do not get distracted by broken symrefs

[Johannes Schindelin]

Hi Junio,

On 2015-10-08 21:42, Junio C Hamano wrote:
> Johannes Schindelin  writes:
> 
>> Would yo kindly add the line
>>
>> This fixes https://github.com/git-for-windows/git/issues/423
>>
>> before the Signed-off-by lines?
> 
> Oh, sorry, I missed that one.  Fixed.
> 
> Thanks.

Thank you!
Dscho
--
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 1/2] test-path-utils.c: remove incorrect assumption

[Ray Donnelly]

On Mon, Oct 5, 2015 at 12:36 AM, Ray Donnelly  wrote:
> On Sun, Oct 4, 2015 at 6:21 PM, Junio C Hamano  wrote:
>> Ray Donnelly  writes:
>>
 Some callers of this function in real code (i.e. not the one you are
 removing the check) do seem to depend on that condition, e.g. the
 codepath in clone that leads to add_to_alternates_file() wants to
 make sure it does not add an duplicate, so it may end up not noticing
 /foo/bar and /foo/bar/ are the same thing, no?  There may be others.
>>>
>>> Enforcing that normalize_path_copy() removes any trailing '/' (apart
>>> from the root directory) breaks other things that assume it doesn't
>>> mess with trailing '/'s, for example filtering in ls-tree. Any
>>> suggestions for what to do about this? Would a flag be appropriate as
>>> to whether to do this part or not? Though I'll admit I don't like the
>>> idea of adding flags to modify the behavior of something that's meant
>>> to "normalize" something. Alternatively, I could go through all the
>>> breakages and try to fix them up?
>>
>> I agree with you that "normalize" should "normalize".  Making sure
>> that all the callers expect the same kind of normalization would be
>> a lot of work but I do think that is the best approach in the long
>> run.  Thanks for the ls-tree example, by the way, did you find it by
>> code inspection?  I do not think it is reasonable to expect the test
>> coverage for this to be 100%, so the "try to fix them up" would have
>> to involve a lot of manual work both in fixing and reviewing,
>> unfortunately.
>
> For the ls-tree failure, I ran "make test" to see how much fell out.
> I'm not familiar with the code-base yet, so I figured that at least
> investigating the changes needed to make the test-suite pass would be
> a good entry point to reading the code; I will study it at the same
> time to try and get my bearings.
>
>>
>> The first step of the "best approach" would be to make a note on
>> normalize_path_copy() by adding a NEEDSWORK: comment to describe the
>> situation.

I hope this is acceptable for the first part of this task.

>>
>> Thanks.


0001-normalize_path_copy-NEEDSWORK-for-trailing.patch
Description: Binary data

Starting on a small project

[Elliott Runburg]

Hello all,

I'm interested in contributing to a small- to medium-size project,
preferably something regarding data structures. I was looking back
through the SoC ideas, and saw in the 2014 page the suggestion to work
on hash-object. I was wondering first if that still needs work, and if
so, if there is somewhere with more specifics about what changes
should be made.

If this doesn't need work anymore, are there any suggestions for
small-ish projects regarding data structures?

Thanks,
Elliott Runburg
--
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 0/2] Reinstate the helpful message when `git pull --rebase` fails

[Paul Tan]

On Fri, Oct 9, 2015 at 8:52 AM, Junio C Hamano  wrote:
> Johannes Schindelin  writes:
>
>> Brendan Forster noticed that we no longer see the helpful message after
>> a failed `git pull --rebase`. It turns out that the builtin `am` calls
>> the recursive merge function directly, not via a separate process.
>>
>> But that function was not really safe to be called that way, as it
>> die()s pretty liberally.

I'm not too familiar with the merge-recursive.c code, but I was under
the impression that it only called die() under fatal conditions. In
common use cases, such as merge conflicts, it just errors out and the
helpful error message does get printed. Is there a reproduction recipe
for this?

That said, I do agree that even if we die(), we could try to be more
helpful by printing additional helpful instructions.

> If that is the case, I'd thinkg that we'd prefer, as a regression
> fix to correct "that", i.e., let recursive-merge die and let the
> caller catch its exit status.

We could do that, but I don't think it would be worth the overhead to
spawn an additional process for every patch just to print an
additional message should merge_recursive() call die().

Instead, stepping back a bit, I wonder if we can extend coverage of
the helpful message to all die() calls when running git-am. We could
just install a die routine with set_die_routine() in builtin/am.c.
Then, should die() be called anywhere, the helpful error message will
be printed as well. fast-import.c and http-backend.c seem to do this.

Regards,
Paul
--
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 03/13] Convert struct ref to use object_id.

[brian m. carlson]

Use struct object_id in three fields in struct ref and convert all the
necessary places that use it.

Signed-off-by: brian m. carlson 
---
 builtin/clone.c| 16 +++---
 builtin/fetch-pack.c   |  4 ++--
 builtin/fetch.c| 50 +--
 builtin/ls-remote.c|  2 +-
 builtin/receive-pack.c |  2 +-
 builtin/remote.c   | 12 +--
 connect.c  |  2 +-
 fetch-pack.c   | 18 
 http-push.c| 46 +++
 http.c |  2 +-
 remote-curl.c  | 10 -
 remote.c   | 58 +-
 remote.h   |  6 +++---
 send-pack.c| 16 +++---
 transport-helper.c | 18 
 transport.c| 32 ++--
 transport.h|  8 +++
 walker.c   |  2 +-
 18 files changed, 152 insertions(+), 152 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 578da852..92007b35 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -552,7 +552,7 @@ static void write_remote_refs(const struct ref *local_refs)
for (r = local_refs; r; r = r->next) {
if (!r->peer_ref)
continue;
-   if (ref_transaction_create(t, r->peer_ref->name, r->old_sha1,
+   if (ref_transaction_create(t, r->peer_ref->name, 
r->old_oid.hash,
   0, NULL, ))
die("%s", err.buf);
}
@@ -572,9 +572,9 @@ static void write_followtags(const struct ref *refs, const 
char *msg)
continue;
if (ends_with(ref->name, "^{}"))
continue;
-   if (!has_sha1_file(ref->old_sha1))
+   if (!has_object_file(>old_oid))
continue;
-   update_ref(msg, ref->name, ref->old_sha1,
+   update_ref(msg, ref->name, ref->old_oid.hash,
   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
}
 }
@@ -594,7 +594,7 @@ static int iterate_ref_map(void *cb_data, unsigned char 
sha1[20])
if (!ref)
return -1;
 
-   hashcpy(sha1, ref->old_sha1);
+   hashcpy(sha1, ref->old_oid.hash);
*rm = ref->next;
return 0;
 }
@@ -643,12 +643,12 @@ static void update_head(const struct ref *our, const 
struct ref *remote,
/* Local default branch link */
create_symref("HEAD", our->name, NULL);
if (!option_bare) {
-   update_ref(msg, "HEAD", our->old_sha1, NULL, 0,
+   update_ref(msg, "HEAD", our->old_oid.hash, NULL, 0,
   UPDATE_REFS_DIE_ON_ERR);
install_branch_config(0, head, option_origin, 
our->name);
}
} else if (our) {
-   struct commit *c = lookup_commit_reference(our->old_sha1);
+   struct commit *c = lookup_commit_reference(our->old_oid.hash);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
update_ref(msg, "HEAD", c->object.sha1,
   NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
@@ -658,7 +658,7 @@ static void update_head(const struct ref *our, const struct 
ref *remote,
 * HEAD points to a branch but we don't know which one.
 * Detach HEAD in all these cases.
 */
-   update_ref(msg, "HEAD", remote->old_sha1,
+   update_ref(msg, "HEAD", remote->old_oid.hash,
   NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
}
 }
@@ -1009,7 +1009,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
 * remote HEAD check.
 */
for (ref = refs; ref; ref = ref->next)
-   if (is_null_sha1(ref->old_sha1)) {
+   if (is_null_oid(>old_oid)) {
complete_refs_before_fetch = 0;
break;
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4a6b340a..19215b33 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -17,7 +17,7 @@ static void add_sought_entry_mem(struct ref ***sought, int 
*nr, int *alloc,
unsigned char sha1[20];
 
if (namelen > 41 && name[40] == ' ' && !get_sha1_hex(name, sha1)) {
-   hashcpy(ref->old_sha1, sha1);
+   hashcpy(ref->old_oid.hash, sha1);
name += 41;
namelen -= 41;
}
@@ -210,7 +210,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
 
while (ref) {
printf("%s %s\n",
-  sha1_to_hex(ref->old_sha1), ref->name);
+  oid_to_hex(>old_oid), 

[PATCH v3 01/13] refs: convert some internal functions to use object_id

[brian m. carlson]

Convert several internal functions in refs.c to use struct object_id,
and use the GIT_SHA1_HEXSZ constants in parse_ref_line.

Signed-off-by: brian m. carlson 
---
 refs.c | 104 -
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/refs.c b/refs.c
index 91c88bad..dae50c4e 100644
--- a/refs.c
+++ b/refs.c
@@ -1192,7 +1192,7 @@ static const char PACKED_REFS_HEADER[] =
  * Return a pointer to the refname within the line (null-terminated),
  * or NULL if there was a problem.
  */
-static const char *parse_ref_line(struct strbuf *line, unsigned char *sha1)
+static const char *parse_ref_line(struct strbuf *line, struct object_id *oid)
 {
const char *ref;
 
@@ -1204,15 +1204,15 @@ static const char *parse_ref_line(struct strbuf *line, 
unsigned char *sha1)
 *  +1 (space in between hex and name)
 *  +1 (newline at the end of the line)
 */
-   if (line->len <= 42)
+   if (line->len <= GIT_SHA1_HEXSZ + 2)
return NULL;
 
-   if (get_sha1_hex(line->buf, sha1) < 0)
+   if (get_oid_hex(line->buf, oid) < 0)
return NULL;
-   if (!isspace(line->buf[40]))
+   if (!isspace(line->buf[GIT_SHA1_HEXSZ]))
return NULL;
 
-   ref = line->buf + 41;
+   ref = line->buf + GIT_SHA1_HEXSZ + 1;
if (isspace(*ref))
return NULL;
 
@@ -1257,7 +1257,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
 
while (strbuf_getwholeline(, f, '\n') != EOF) {
-   unsigned char sha1[20];
+   struct object_id oid;
const char *refname;
const char *traits;
 
@@ -1270,17 +1270,17 @@ static void read_packed_refs(FILE *f, struct ref_dir 
*dir)
continue;
}
 
-   refname = parse_ref_line(, sha1);
+   refname = parse_ref_line(, );
if (refname) {
int flag = REF_ISPACKED;
 
if (check_refname_format(refname, 
REFNAME_ALLOW_ONELEVEL)) {
if (!refname_is_safe(refname))
die("packed refname is dangerous: %s", 
refname);
-   hashclr(sha1);
+   oidclr();
flag |= REF_BAD_NAME | REF_ISBROKEN;
}
-   last = create_ref_entry(refname, sha1, flag, 0);
+   last = create_ref_entry(refname, oid.hash, flag, 0);
if (peeled == PEELED_FULLY ||
(peeled == PEELED_TAGS && starts_with(refname, 
"refs/tags/")))
last->flag |= REF_KNOWS_PEELED;
@@ -1291,8 +1291,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
line.buf[0] == '^' &&
line.len == PEELED_LINE_LENGTH &&
line.buf[PEELED_LINE_LENGTH - 1] == '\n' &&
-   !get_sha1_hex(line.buf + 1, sha1)) {
-   hashcpy(last->u.value.peeled.hash, sha1);
+   !get_oid_hex(line.buf + 1, )) {
+   oidcpy(>u.value.peeled, );
/*
 * Regardless of what the file header said,
 * we definitely know the value of *this*
@@ -1397,7 +1397,7 @@ static void read_loose_refs(const char *dirname, struct 
ref_dir *dir)
strbuf_add(, dirname, dirnamelen);
 
while ((de = readdir(d)) != NULL) {
-   unsigned char sha1[20];
+   struct object_id oid;
struct stat st;
int flag;
 
@@ -1418,20 +1418,20 @@ static void read_loose_refs(const char *dirname, struct 
ref_dir *dir)
int read_ok;
 
if (*refs->name) {
-   hashclr(sha1);
+   oidclr();
flag = 0;
read_ok = !resolve_gitlink_ref(refs->name,
-  refname.buf, 
sha1);
+  refname.buf, 
oid.hash);
} else {
read_ok = !read_ref_full(refname.buf,
 RESOLVE_REF_READING,
-sha1, );
+oid.hash, );
}
 
if (!read_ok) {
-   hashclr(sha1);
+   oidclr();
flag |= REF_ISBROKEN;
-   } else if 

[PATCH v3 02/13] sha1_file: introduce has_object_file helper.

[brian m. carlson]

Add has_object_file, which is a wrapper around has_sha1_file, but for
struct object_id.

Signed-off-by: brian m. carlson 
---
 cache.h | 3 +++
 sha1_file.c | 5 +
 2 files changed, 8 insertions(+)

diff --git a/cache.h b/cache.h
index 752031e8..61a686e3 100644
--- a/cache.h
+++ b/cache.h
@@ -1002,6 +1002,9 @@ static inline int has_sha1_file(const unsigned char *sha1)
return has_sha1_file_with_flags(sha1, 0);
 }
 
+/* Same as the above, except for struct object_id. */
+extern int has_object_file(const struct object_id *oid);
+
 /*
  * Return true iff an alternate object database has a loose object
  * with the specified name.  This function does not respect replace
diff --git a/sha1_file.c b/sha1_file.c
index d295a322..77aa4f05 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3220,6 +3220,11 @@ int has_sha1_file_with_flags(const unsigned char *sha1, 
int flags)
return find_pack_entry(sha1, );
 }
 
+int has_object_file(const struct object_id *oid)
+{
+   return has_sha1_file(oid->hash);
+}
+
 static void check_tree(const void *buf, size_t size)
 {
struct tree_desc desc;
-- 
2.6.1

--
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 11/13] Convert struct object to object_id

[brian m. carlson]

struct object is one of the major data structures dealing with object
IDs.  Convert it to use struct object_id instead of an unsigned char
array.  Convert get_object_hash to refer to the new member as well.

Signed-off-by: brian m. carlson 
---
 bisect.c |  4 ++--
 builtin/am.c |  2 +-
 builtin/blame.c  | 28 ++--
 builtin/checkout.c   | 12 ++--
 builtin/commit-tree.c|  4 ++--
 builtin/describe.c   | 14 +++---
 builtin/diff-tree.c  |  4 ++--
 builtin/fast-export.c| 22 +++---
 builtin/fmt-merge-msg.c  |  2 +-
 builtin/fsck.c   | 32 
 builtin/grep.c   |  4 ++--
 builtin/index-pack.c |  8 
 builtin/log.c| 18 +-
 builtin/merge-base.c |  8 
 builtin/merge-tree.c |  2 +-
 builtin/merge.c  | 10 +-
 builtin/name-rev.c   |  8 
 builtin/pack-objects.c   |  2 +-
 builtin/pull.c   |  2 +-
 builtin/replace.c|  6 +++---
 builtin/reset.c  | 28 ++--
 builtin/rev-list.c   | 14 +++---
 builtin/shortlog.c   |  2 +-
 builtin/show-branch.c|  4 ++--
 builtin/unpack-objects.c |  8 
 builtin/worktree.c   |  2 +-
 bundle.c | 18 +-
 commit.c | 20 ++--
 fetch-pack.c |  2 +-
 fsck.c   |  8 
 http-backend.c   |  2 +-
 http-push.c  | 22 +++---
 list-objects.c   |  4 ++--
 log-tree.c   | 20 ++--
 merge-recursive.c| 14 +++---
 merge.c  |  2 +-
 notes-merge.c|  4 ++--
 object.c |  2 +-
 object.h |  4 ++--
 pack-bitmap-write.c  |  2 +-
 pack-bitmap.c|  8 
 pretty.c | 10 +-
 ref-filter.c | 12 ++--
 remote.c |  4 ++--
 revision.c   | 32 
 sequencer.c  | 22 +++---
 server-info.c|  2 +-
 sha1_name.c  |  4 ++--
 shallow.c|  4 ++--
 submodule.c  |  6 +++---
 tag.c|  4 ++--
 tree.c   |  6 +++---
 upload-pack.c| 16 
 walker.c | 10 +-
 54 files changed, 257 insertions(+), 257 deletions(-)

diff --git a/bisect.c b/bisect.c
index 59e86369..54166f00 100644
--- a/bisect.c
+++ b/bisect.c
@@ -193,7 +193,7 @@ static int compare_commit_dist(const void *a_, const void 
*b_)
b = (struct commit_dist *)b_;
if (a->distance != b->distance)
return b->distance - a->distance; /* desc sort */
-   return hashcmp(a->commit->object.sha1, b->commit->object.sha1);
+   return oidcmp(>commit->object.oid, >commit->object.oid);
 }
 
 static struct commit_list *best_bisection_sorted(struct commit_list *list, int 
nr)
@@ -575,7 +575,7 @@ static struct commit_list *skip_away(struct commit_list 
*list, int count)
 
for (i = 0; cur; cur = cur->next, i++) {
if (i == index) {
-   if (hashcmp(cur->item->object.sha1, 
current_bad_oid->hash))
+   if (oidcmp(>item->object.oid, current_bad_oid))
return cur;
if (previous)
return previous;
diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07b..6092efd4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1437,7 +1437,7 @@ static void get_commit_info(struct am_state *state, 
struct commit *commit)
assert(!state->msg);
msg = strstr(buffer, "\n\n");
if (!msg)
-   die(_("unable to parse commit %s"), 
sha1_to_hex(commit->object.sha1));
+   die(_("unable to parse commit %s"), 
oid_to_hex(>object.oid));
state->msg = xstrdup(msg + 2);
state->msg_len = strlen(state->msg);
 }
diff --git a/builtin/blame.c b/builtin/blame.c
index ad2db384..e65bb4da 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -556,7 +556,7 @@ static struct origin *find_origin(struct scoreboard *sb,
   PATHSPEC_LITERAL_PATH, "", paths);
diff_setup_done(_opts);
 
-   if (is_null_sha1(origin->commit->object.sha1))
+   if (is_null_oid(>commit->object.oid))
do_diff_cache(get_object_hash(parent->tree->object), 
_opts);
else
diff_tree_sha1(get_object_hash(parent->tree->object),
@@ -626,7 +626,7 @@ static struct origin *find_rename(struct scoreboard *sb,
diff_opts.single_follow = origin->path;
diff_setup_done(_opts);
 
-   if (is_null_sha1(origin->commit->object.sha1))
+   if (is_null_oid(>commit->object.oid))

[PATCH v3 13/13] remote: convert functions to struct object_id

[brian m. carlson]

Convert several unsigned char arrays to use struct object_id instead,
and change hard-coded 40-based constants to use GIT_SHA1_HEXSZ as well.

Signed-off-by: brian m. carlson 
---
 remote.c | 64 
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/remote.c b/remote.c
index c72d7966..92dc8d69 100644
--- a/remote.c
+++ b/remote.c
@@ -492,7 +492,7 @@ static void alias_all_urls(void)
 static void read_config(void)
 {
static int loaded;
-   unsigned char sha1[20];
+   struct object_id oid;
const char *head_ref;
int flag;
 
@@ -501,7 +501,7 @@ static void read_config(void)
loaded = 1;
 
current_branch = NULL;
-   head_ref = resolve_ref_unsafe("HEAD", 0, sha1, );
+   head_ref = resolve_ref_unsafe("HEAD", 0, oid.hash, );
if (head_ref && (flag & REF_ISSYMREF) &&
skip_prefix(head_ref, "refs/heads/", _ref)) {
current_branch = make_branch(head_ref, 0);
@@ -580,12 +580,12 @@ static struct refspec *parse_refspec_internal(int 
nr_refspec, const char **refsp
flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? 
REFNAME_REFSPEC_PATTERN : 0);
 
if (fetch) {
-   unsigned char unused[40];
+   struct object_id unused;
 
/* LHS */
if (!*rs[i].src)
; /* empty is ok; it means "HEAD" */
-   else if (llen == 40 && !get_sha1_hex(rs[i].src, unused))
+   else if (llen == GIT_SHA1_HEXSZ && 
!get_oid_hex(rs[i].src, ))
rs[i].exact_sha1 = 1; /* ok */
else if (!check_refname_format(rs[i].src, flags))
; /* valid looking ref is ok */
@@ -1118,7 +1118,7 @@ static struct ref *alloc_delete_ref(void)
 static int try_explicit_object_name(const char *name,
struct ref **match)
 {
-   unsigned char sha1[20];
+   struct object_id oid;
 
if (!*name) {
if (match)
@@ -1126,12 +1126,12 @@ static int try_explicit_object_name(const char *name,
return 0;
}
 
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, oid.hash))
return -1;
 
if (match) {
*match = alloc_ref(name);
-   hashcpy((*match)->new_oid.hash, sha1);
+   oidcpy(&(*match)->new_oid, );
}
return 0;
 }
@@ -1146,10 +1146,10 @@ static struct ref *make_linked_ref(const char *name, 
struct ref ***tail)
 static char *guess_ref(const char *name, struct ref *peer)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char sha1[20];
+   struct object_id oid;
 
const char *r = resolve_ref_unsafe(peer->name, RESOLVE_REF_READING,
-  sha1, NULL);
+  oid.hash, NULL);
if (!r)
return NULL;
 
@@ -1207,12 +1207,12 @@ static int match_explicit(struct ref *src, struct ref 
*dst,
return -1;
 
if (!dst_value) {
-   unsigned char sha1[20];
+   struct object_id oid;
int flag;
 
dst_value = resolve_ref_unsafe(matched_src->name,
   RESOLVE_REF_READING,
-  sha1, );
+  oid.hash, );
if (!dst_value ||
((flag & REF_ISSYMREF) &&
 !starts_with(dst_value, "refs/heads/")))
@@ -1328,13 +1328,13 @@ struct tips {
int nr, alloc;
 };
 
-static void add_to_tips(struct tips *tips, const unsigned char *sha1)
+static void add_to_tips(struct tips *tips, const struct object_id *oid)
 {
struct commit *commit;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return;
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(oid->hash, 1);
if (!commit || (commit->object.flags & TMP_MARK))
return;
commit->object.flags |= TMP_MARK;
@@ -1358,9 +1358,9 @@ static void add_missing_tags(struct ref *src, struct ref 
**dst, struct ref ***ds
for (ref = *dst; ref; ref = ref->next) {
if (ref->peer_ref &&
!is_null_oid(>peer_ref->new_oid))
-   add_to_tips(_tips, ref->peer_ref->new_oid.hash);
+   add_to_tips(_tips, >peer_ref->new_oid);
else
-   add_to_tips(_tips, ref->old_oid.hash);
+   add_to_tips(_tips, >old_oid);
if (starts_with(ref->name, "refs/tags/"))
string_list_append(_tag, ref->name);
}
@@ -1645,7 +1645,7 @@ static void 

[PATCH v3 12/13] Remove get_object_hash.

[brian m. carlson]

Convert all instances of get_object_hash to use an appropriate reference
to the hash member of the oid member of struct object.  This provides no
functional change, as it is essentially a macro substitution.

Signed-off-by: brian m. carlson 
---
Implemented entirely by the following Perl script.

#!/usr/bin/perl

use strict;
use warnings;

local $^I = '';

my $skip = 0;
while (<>) {
if (/#define get_object_hash/) {
$skip = 1;
next;
}
# Clean up the whitespace following the definition.
if ($skip) {
$skip = 0;
next;
}
s/get_object_hash\(\*(\(\(struct \S+\s*\*\)\S+\)[^)]+)\)/$1->oid.hash/g;
s/get_object_hash\(\*([^)]+)\)/$1->oid.hash/g;
s/get_object_hash\(([^)]+)\)/$1.oid.hash/g;
print;
}

 archive.c|  6 +++---
 bisect.c |  6 +++---
 branch.c |  2 +-
 builtin/blame.c  | 24 +++
 builtin/branch.c |  2 +-
 builtin/checkout.c   | 10 +-
 builtin/clone.c  |  2 +-
 builtin/commit.c |  8 
 builtin/describe.c   |  6 +++---
 builtin/diff-tree.c  |  8 
 builtin/diff.c   | 12 ++--
 builtin/fast-export.c| 12 ++--
 builtin/fetch.c  |  4 ++--
 builtin/fmt-merge-msg.c  |  4 ++--
 builtin/fsck.c   |  4 ++--
 builtin/grep.c   |  2 +-
 builtin/index-pack.c |  2 +-
 builtin/log.c| 18 -
 builtin/merge-tree.c |  4 ++--
 builtin/merge.c  | 50 
 builtin/name-rev.c   |  4 ++--
 builtin/notes.c  |  2 +-
 builtin/pack-objects.c   | 14 +++---
 builtin/pull.c   |  2 +-
 builtin/reflog.c |  4 ++--
 builtin/reset.c  |  2 +-
 builtin/rev-list.c   |  4 ++--
 builtin/rev-parse.c  |  4 ++--
 builtin/show-branch.c|  4 ++--
 builtin/unpack-objects.c |  2 +-
 bundle.c |  2 +-
 cache-tree.c |  2 +-
 combine-diff.c   |  4 ++--
 commit.c | 12 ++--
 decorate.c   |  2 +-
 diff-lib.c   |  2 +-
 fetch-pack.c |  4 ++--
 fsck.c   |  6 +++---
 http-push.c  | 20 +--
 line-log.c   |  6 +++---
 log-tree.c   | 20 +--
 merge-blobs.c|  4 ++--
 merge-recursive.c| 16 
 notes-merge.c| 20 +--
 object.c |  6 +++---
 object.h |  2 --
 pack-bitmap-write.c  | 14 +++---
 pack-bitmap.c| 26 -
 patch-ids.c  |  6 +++---
 pretty.c |  8 
 ref-filter.c |  6 +++---
 refs.c   |  2 +-
 revision.c   | 16 
 sequencer.c  | 18 -
 sha1_name.c  | 16 
 shallow.c|  2 +-
 submodule.c  |  2 +-
 tag.c|  6 +++---
 test-match-trees.c   |  2 +-
 tree.c   |  4 ++--
 upload-pack.c| 10 +-
 walker.c |  8 
 wt-status.c  |  2 +-
 63 files changed, 251 insertions(+), 253 deletions(-)

diff --git a/archive.c b/archive.c
index 4ff88788..1733133f 100644
--- a/archive.c
+++ b/archive.c
@@ -240,7 +240,7 @@ int write_archive_entries(struct archiver_args *args,
len--;
if (args->verbose)
fprintf(stderr, "%.*s\n", (int)len, args->base);
-   err = write_entry(args, get_object_hash(args->tree->object), 
args->base,
+   err = write_entry(args, args->tree->object.oid.hash, args->base,
  len, 040777);
if (err)
return err;
@@ -373,7 +373,7 @@ static void parse_treeish_arg(const char **argv,
 
commit = lookup_commit_reference_gently(oid.hash, 1);
if (commit) {
-   commit_sha1 = get_object_hash(commit->object);
+   commit_sha1 = commit->object.oid.hash;
archive_time = commit->date;
} else {
commit_sha1 = NULL;
@@ -389,7 +389,7 @@ static void parse_treeish_arg(const char **argv,
unsigned int mode;
int err;
 
-   err = get_tree_entry(get_object_hash(tree->object), prefix,
+   err = get_tree_entry(tree->object.oid.hash, prefix,
 tree_oid.hash, );
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
diff --git a/bisect.c b/bisect.c
index 54166f00..42aa7aa6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -500,7 +500,7 @@ struct commit_list *filter_skipped(struct 

[PATCH v3 10/13] Add several uses of get_object_hash.

[brian m. carlson]

Convert most instances where the sha1 member of struct object is
dereferenced to use get_object_hash.  Most instances that are passed to
functions that have versions taking struct object_id, such as
get_sha1_hex/get_oid_hex, or instances that can be trivially converted
to use struct object_id instead, are not converted.

Signed-off-by: brian m. carlson 
---
This does not follow a hard and fast rule, but the goal was to keep the
changes down between both this patch and the following.  *Generally*,
things I was going to convert in the next patch I didn't convert here.

 archive.c|  6 +++---
 bisect.c |  6 +++---
 branch.c |  2 +-
 builtin/blame.c  | 24 ++---
 builtin/branch.c |  2 +-
 builtin/checkout.c   | 10 -
 builtin/clone.c  |  2 +-
 builtin/commit.c |  8 +++
 builtin/describe.c   |  6 +++---
 builtin/diff-tree.c  |  8 +++
 builtin/diff.c   | 12 +--
 builtin/fast-export.c| 12 +--
 builtin/fetch.c  |  4 ++--
 builtin/fmt-merge-msg.c  |  4 ++--
 builtin/fsck.c   |  4 ++--
 builtin/grep.c   |  2 +-
 builtin/index-pack.c |  2 +-
 builtin/log.c| 18 
 builtin/merge-tree.c |  4 ++--
 builtin/merge.c  | 54 
 builtin/name-rev.c   |  6 +++---
 builtin/notes.c  |  2 +-
 builtin/pack-objects.c   | 14 ++---
 builtin/reflog.c |  4 ++--
 builtin/reset.c  |  2 +-
 builtin/rev-list.c   |  4 ++--
 builtin/rev-parse.c  |  4 ++--
 builtin/show-branch.c|  4 ++--
 builtin/unpack-objects.c |  2 +-
 bundle.c |  2 +-
 cache-tree.c |  2 +-
 combine-diff.c   |  4 ++--
 commit.c | 12 +--
 decorate.c   |  2 +-
 diff-lib.c   |  2 +-
 fetch-pack.c |  4 ++--
 fsck.c   |  6 +++---
 http-push.c  | 20 +-
 line-log.c   |  6 +++---
 log-tree.c   | 20 +-
 merge-blobs.c|  4 ++--
 merge-recursive.c| 16 +++---
 notes-merge.c| 20 +-
 object.c |  6 +++---
 pack-bitmap-write.c  | 14 ++---
 pack-bitmap.c| 26 +++
 patch-ids.c  |  6 +++---
 pretty.c |  8 +++
 ref-filter.c |  6 +++---
 refs.c   |  2 +-
 revision.c   | 16 +++---
 sequencer.c  | 18 
 sha1_name.c  | 16 +++---
 shallow.c|  2 +-
 submodule.c  |  2 +-
 tag.c|  6 +++---
 test-match-trees.c   |  2 +-
 tree.c   |  4 ++--
 upload-pack.c| 10 -
 walker.c |  8 +++
 wt-status.c  |  2 +-
 61 files changed, 253 insertions(+), 253 deletions(-)

diff --git a/archive.c b/archive.c
index 01b0899b..4ff88788 100644
--- a/archive.c
+++ b/archive.c
@@ -240,7 +240,7 @@ int write_archive_entries(struct archiver_args *args,
len--;
if (args->verbose)
fprintf(stderr, "%.*s\n", (int)len, args->base);
-   err = write_entry(args, args->tree->object.sha1, args->base,
+   err = write_entry(args, get_object_hash(args->tree->object), 
args->base,
  len, 040777);
if (err)
return err;
@@ -373,7 +373,7 @@ static void parse_treeish_arg(const char **argv,
 
commit = lookup_commit_reference_gently(oid.hash, 1);
if (commit) {
-   commit_sha1 = commit->object.sha1;
+   commit_sha1 = get_object_hash(commit->object);
archive_time = commit->date;
} else {
commit_sha1 = NULL;
@@ -389,7 +389,7 @@ static void parse_treeish_arg(const char **argv,
unsigned int mode;
int err;
 
-   err = get_tree_entry(tree->object.sha1, prefix,
+   err = get_tree_entry(get_object_hash(tree->object), prefix,
 tree_oid.hash, );
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
diff --git a/bisect.c b/bisect.c
index 053d1a2a..59e86369 100644
--- a/bisect.c
+++ b/bisect.c
@@ -500,7 +500,7 @@ struct commit_list *filter_skipped(struct commit_list *list,
struct commit_list *next = list->next;
list->next = NULL;
if (0 <= sha1_array_lookup(_revs,
-  list->item->object.sha1)) {
+  
get_object_hash(list->item->object))) {
if (skipped_first && 

[PATCH v3 00/13] object_id part 2

[brian m. carlson]

This is another series of conversions to struct object_id.

This series converts more of the refs code and struct object to use
struct object_id.  It introduces an additional helper function,
has_object_file, which is the equivalent of has_sha1_file.  The name was
chosen to be slightly more logical than has_oid_file, although it can be
changed if desired.

Unlike previous iterations, this series of patches should make it
entirely to the list.  It split out the conversion of struct object into
the introduction of a helper macro, the use of that macro in several
areas, the conversion of other struct object uses and update of the
macro, and the removal of the macro.  The final patch of that set
(patch 12) was implemented entirely by the use of a Perl script, which
will be included for inspection.

Patches 1 through 8 remain unchanged except for the rebase and the
necessary changes due to other topics.  Necessary changes were minimal.

This series is also available in the object-id-part2 branch from

https://github.com/bk2204/git.git
https://git.crustytoothpaste.net/git/bmc/git.git

Changes from v2:
* Rebase on latest master.
* Split out the conversion of struct object into multiple patches.

brian m. carlson (13):
  refs: convert some internal functions to use object_id
  sha1_file: introduce has_object_file helper.
  Convert struct ref to use object_id.
  add_sought_entry_mem: convert to struct object_id
  parse_fetch: convert to use struct object_id
  get_remote_heads: convert to struct object_id
  push_refs_with_export: convert to struct object_id
  ref_newer: convert to use struct object_id
  object: introduce get_object_hash macro.
  Add several uses of get_object_hash.
  Convert struct object to object_id
  Remove get_object_hash.
  remote: convert functions to struct object_id

 archive.c|   6 +--
 bisect.c |  10 ++--
 branch.c |   2 +-
 builtin/am.c |   2 +-
 builtin/blame.c  |  52 ++--
 builtin/branch.c |   2 +-
 builtin/checkout.c   |  22 -
 builtin/clone.c  |  18 +++
 builtin/commit-tree.c|   4 +-
 builtin/commit.c |   8 ++--
 builtin/describe.c   |  20 
 builtin/diff-tree.c  |  12 ++---
 builtin/diff.c   |  12 ++---
 builtin/fast-export.c|  34 +++---
 builtin/fetch-pack.c |  14 +++---
 builtin/fetch.c  |  54 ++---
 builtin/fmt-merge-msg.c  |   6 +--
 builtin/fsck.c   |  36 +++---
 builtin/grep.c   |   6 +--
 builtin/index-pack.c |  10 ++--
 builtin/log.c|  36 +++---
 builtin/ls-remote.c  |   2 +-
 builtin/merge-base.c |   8 ++--
 builtin/merge-tree.c |   6 +--
 builtin/merge.c  |  60 
 builtin/name-rev.c   |  12 ++---
 builtin/notes.c  |   2 +-
 builtin/pack-objects.c   |  16 +++
 builtin/pull.c   |   2 +-
 builtin/receive-pack.c   |   2 +-
 builtin/reflog.c |   4 +-
 builtin/remote.c |  12 ++---
 builtin/replace.c|   6 +--
 builtin/reset.c  |  30 ++--
 builtin/rev-list.c   |  18 +++
 builtin/rev-parse.c  |   4 +-
 builtin/shortlog.c   |   2 +-
 builtin/show-branch.c|   8 ++--
 builtin/unpack-objects.c |  10 ++--
 builtin/worktree.c   |   2 +-
 bundle.c |  20 
 cache-tree.c |   2 +-
 cache.h  |   3 ++
 combine-diff.c   |   4 +-
 commit.c |  32 ++---
 connect.c|  22 +
 decorate.c   |   2 +-
 diff-lib.c   |   2 +-
 fetch-pack.c |  24 +-
 fsck.c   |  14 +++---
 http-backend.c   |   2 +-
 http-push.c  |  88 +-
 http.c   |   2 +-
 line-log.c   |   6 +--
 list-objects.c   |   4 +-
 log-tree.c   |  40 
 merge-blobs.c|   4 +-
 merge-recursive.c|  26 +-
 merge.c  |   2 +-
 notes-merge.c|  24 +-
 object.c |   8 ++--
 object.h |   2 +-
 pack-bitmap-write.c  |  16 +++
 pack-bitmap.c|  34 +++---
 patch-ids.c  |   6 +--
 pretty.c |  18 +++
 ref-filter.c |  18 +++
 refs.c   | 106 -
 remote-curl.c|  20 
 remote.c | 120 +++
 remote.h |   8 ++--
 revision.c   |  48 +--
 send-pack.c  |  16 +++
 sequencer.c  |  40 
 server-info.c|   2 +-
 sha1_file.c  |   5 ++
 sha1_name.c  |  20 
 shallow.c|   6 +--
 submodule.c  

[PATCH v3 09/13] object: introduce get_object_hash macro.

[brian m. carlson]

This macro is a temporary change to ease the transition of struct object
to use struct object_id.  It takes an argument of struct object and
returns the object's hash.  Provide this hash next to struct object for
easier conversion.

Signed-off-by: brian m. carlson 
---
 object.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/object.h b/object.h
index 6416247d..7c098d03 100644
--- a/object.h
+++ b/object.h
@@ -52,6 +52,8 @@ struct object {
unsigned char sha1[20];
 };
 
+#define get_object_hash(x) ((x).sha1)
+
 extern const char *typename(unsigned int type);
 extern int type_from_string_gently(const char *str, ssize_t, int gentle);
 #define type_from_string(str) type_from_string_gently(str, -1, 0)
-- 
2.6.1

--
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 08/13] ref_newer: convert to use struct object_id

[brian m. carlson]

Convert ref_newer and its caller to use struct object_id instead of
unsigned char *.

Signed-off-by: brian m. carlson 
---
 builtin/remote.c | 2 +-
 http-push.c  | 4 ++--
 remote.c | 8 
 remote.h | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 68dec162..6694cf20 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -417,7 +417,7 @@ static int get_push_ref_states(const struct ref 
*remote_refs,
else if (is_null_oid(>old_oid))
info->status = PUSH_STATUS_CREATE;
else if (has_object_file(>old_oid) &&
-ref_newer(ref->new_oid.hash, ref->old_oid.hash))
+ref_newer(>new_oid, >old_oid))
info->status = PUSH_STATUS_FASTFORWARD;
else
info->status = PUSH_STATUS_OUTOFDATE;
diff --git a/http-push.c b/http-push.c
index d054fdb9..0e688a76 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1899,8 +1899,8 @@ int main(int argc, char **argv)
!is_null_oid(>old_oid) &&
!ref->force) {
if (!has_object_file(>old_oid) ||
-   !ref_newer(ref->peer_ref->new_oid.hash,
-  ref->old_oid.hash)) {
+   !ref_newer(>peer_ref->new_oid,
+  >old_oid)) {
/*
 * We do not have the remote ref, or
 * we know that the remote ref is not
diff --git a/remote.c b/remote.c
index 05741202..cb85c3e1 100644
--- a/remote.c
+++ b/remote.c
@@ -1626,7 +1626,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int 
send_mirror,
else if 
(!lookup_commit_reference_gently(ref->old_oid.hash, 1) ||
 
!lookup_commit_reference_gently(ref->new_oid.hash, 1))
reject_reason = REF_STATUS_REJECT_NEEDS_FORCE;
-   else if (!ref_newer(ref->new_oid.hash, 
ref->old_oid.hash))
+   else if (!ref_newer(>new_oid, >old_oid))
reject_reason = 
REF_STATUS_REJECT_NONFASTFORWARD;
}
 
@@ -1982,7 +1982,7 @@ static void unmark_and_free(struct commit_list *list, 
unsigned int mark)
}
 }
 
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1)
+int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
 {
struct object *o;
struct commit *old, *new;
@@ -1993,12 +1993,12 @@ int ref_newer(const unsigned char *new_sha1, const 
unsigned char *old_sha1)
 * Both new and old must be commit-ish and new is descendant of
 * old.  Otherwise we require --force.
 */
-   o = deref_tag(parse_object(old_sha1), NULL, 0);
+   o = deref_tag(parse_object(old_oid->hash), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
old = (struct commit *) o;
 
-   o = deref_tag(parse_object(new_sha1), NULL, 0);
+   o = deref_tag(parse_object(new_oid->hash), NULL, 0);
if (!o || o->type != OBJ_COMMIT)
return 0;
new = (struct commit *) o;
diff --git a/remote.h b/remote.h
index 163ea5e8..4a039bae 100644
--- a/remote.h
+++ b/remote.h
@@ -150,7 +150,7 @@ extern struct ref **get_remote_heads(int in, char *src_buf, 
size_t src_len,
 struct sha1_array *shallow);
 
 int resolve_remote_symref(struct ref *ref, struct ref *list);
-int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);
+int ref_newer(const struct object_id *new_oid, const struct object_id 
*old_oid);
 
 /*
  * Remove and free all but the first of any entries in the input list
-- 
2.6.1

--
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 04/13] add_sought_entry_mem: convert to struct object_id

[brian m. carlson]

Convert this function to use struct object_id.  Express several
hardcoded constants in terms of GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 builtin/fetch-pack.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 19215b33..cf3019e0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -14,12 +14,14 @@ static void add_sought_entry_mem(struct ref ***sought, int 
*nr, int *alloc,
 const char *name, int namelen)
 {
struct ref *ref = xcalloc(1, sizeof(*ref) + namelen + 1);
-   unsigned char sha1[20];
+   struct object_id oid;
+   const int chunksz = GIT_SHA1_HEXSZ + 1;
 
-   if (namelen > 41 && name[40] == ' ' && !get_sha1_hex(name, sha1)) {
-   hashcpy(ref->old_oid.hash, sha1);
-   name += 41;
-   namelen -= 41;
+   if (namelen > chunksz && name[chunksz - 1] == ' ' &&
+   !get_oid_hex(name, )) {
+   oidcpy(>old_oid, );
+   name += chunksz;
+   namelen -= chunksz;
}
 
memcpy(ref->name, name, namelen);
-- 
2.6.1

--
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 05/13] parse_fetch: convert to use struct object_id

[brian m. carlson]

Convert the parse_fetch function to use struct object_id.  Remove the
strlen check as get_oid_hex will fail safely on receiving a too-short
NUL-terminated string.

Signed-off-by: brian m. carlson 
---
 remote-curl.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index c4b645e5..10160e63 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -806,19 +806,19 @@ static void parse_fetch(struct strbuf *buf)
if (skip_prefix(buf->buf, "fetch ", )) {
const char *name;
struct ref *ref;
-   unsigned char old_sha1[20];
+   struct object_id old_oid;
 
-   if (strlen(p) < 40 || get_sha1_hex(p, old_sha1))
+   if (get_oid_hex(p, _oid))
die("protocol error: expected sha/ref, got 
%s'", p);
-   if (p[40] == ' ')
-   name = p + 41;
-   else if (!p[40])
+   if (p[GIT_SHA1_HEXSZ] == ' ')
+   name = p + GIT_SHA1_HEXSZ + 1;
+   else if (!p[GIT_SHA1_HEXSZ])
name = "";
else
die("protocol error: expected sha/ref, got 
%s'", p);
 
ref = alloc_ref(name);
-   hashcpy(ref->old_oid.hash, old_sha1);
+   oidcpy(>old_oid, _oid);
 
*list = ref;
list = >next;
-- 
2.6.1

--
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 06/13] get_remote_heads: convert to struct object_id

[brian m. carlson]

Replace an unsigned char array with struct object_id and express several
hard-coded constants in terms of GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson 
---
 connect.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/connect.c b/connect.c
index 45eabcdd..60f9fd07 100644
--- a/connect.c
+++ b/connect.c
@@ -120,7 +120,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t 
src_len,
*list = NULL;
for (;;) {
struct ref *ref;
-   unsigned char old_sha1[20];
+   struct object_id old_oid;
char *name;
int len, name_len;
char *buffer = packet_buffer;
@@ -139,34 +139,36 @@ struct ref **get_remote_heads(int in, char *src_buf, 
size_t src_len,
if (len > 4 && skip_prefix(buffer, "ERR ", ))
die("remote error: %s", arg);
 
-   if (len == 48 && skip_prefix(buffer, "shallow ", )) {
-   if (get_sha1_hex(arg, old_sha1))
+   if (len == GIT_SHA1_HEXSZ + strlen("shallow ") &&
+   skip_prefix(buffer, "shallow ", )) {
+   if (get_oid_hex(arg, _oid))
die("protocol error: expected shallow sha-1, 
got '%s'", arg);
if (!shallow_points)
die("repository on the other end cannot be 
shallow");
-   sha1_array_append(shallow_points, old_sha1);
+   sha1_array_append(shallow_points, old_oid.hash);
continue;
}
 
-   if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != 
' ')
+   if (len < GIT_SHA1_HEXSZ + 2 || get_oid_hex(buffer, _oid) ||
+   buffer[GIT_SHA1_HEXSZ] != ' ')
die("protocol error: expected sha/ref, got '%s'", 
buffer);
-   name = buffer + 41;
+   name = buffer + GIT_SHA1_HEXSZ + 1;
 
name_len = strlen(name);
-   if (len != name_len + 41) {
+   if (len != name_len + GIT_SHA1_HEXSZ + 1) {
free(server_capabilities);
server_capabilities = xstrdup(name + name_len + 1);
}
 
if (extra_have && !strcmp(name, ".have")) {
-   sha1_array_append(extra_have, old_sha1);
+   sha1_array_append(extra_have, old_oid.hash);
continue;
}
 
if (!check_ref(name, flags))
continue;
-   ref = alloc_ref(buffer + 41);
-   hashcpy(ref->old_oid.hash, old_sha1);
+   ref = alloc_ref(buffer + GIT_SHA1_HEXSZ + 1);
+   oidcpy(>old_oid, _oid);
*list = ref;
list = >next;
got_at_least_one_head = 1;
-- 
2.6.1

--
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 07/13] push_refs_with_export: convert to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson 
---
 transport-helper.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 91cb0e72..0eb3cf01 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -878,13 +878,13 @@ static int push_refs_with_export(struct transport 
*transport,
 
for (ref = remote_refs; ref; ref = ref->next) {
char *private;
-   unsigned char sha1[20];
+   struct object_id oid;
 
private = apply_refspecs(data->refspecs, data->refspec_nr, 
ref->name);
-   if (private && !get_sha1(private, sha1)) {
+   if (private && !get_sha1(private, oid.hash)) {
strbuf_addf(, "^%s", private);
string_list_append(_args, strbuf_detach(, 
NULL));
-   hashcpy(ref->old_oid.hash, sha1);
+   oidcpy(>old_oid, );
}
free(private);
 
@@ -898,7 +898,7 @@ static int push_refs_with_export(struct transport 
*transport,
name = resolve_ref_unsafe(
 ref->peer_ref->name,
 RESOLVE_REF_READING,
-sha1, );
+oid.hash, );
if (!name || !(flag & REF_ISSYMREF))
name = ref->peer_ref->name;
 
-- 
2.6.1

--
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: Starting on a small project

[Stefan Beller]

On Thu, Oct 8, 2015 at 2:45 PM, Elliott Runburg
 wrote:
> Hello all,
>
> I'm interested in contributing to a small- to medium-size project,
> preferably something regarding data structures. I was looking back
> through the SoC ideas, and saw in the 2014 page the suggestion to work
> on hash-object. I was wondering first if that still needs work, and if
> so, if there is somewhere with more specifics about what changes
> should be made.
>
> If this doesn't need work anymore, are there any suggestions for
> small-ish projects regarding data structures?

Look through
http://git-blame.blogspot.com/p/leftover-bits.html
as that's a collection of small projects for starters.

>
> Thanks,
> Elliott Runburg
> --
> 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: [PATCH 1/2] test-path-utils.c: remove incorrect assumption

[Junio C Hamano]

I'll squash this in as part of your first patch that removes the
test from test-path-utils.c.  That makes it clearer why it is the
right thing to remove the test, I'd think.

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

Kindly respond for more details

Am Captain Kelvin Ken Miller i am with the us army in Camp Abu Naji / FOB Garry 
Owen (Al Amarah)I need you assistant to move some funds out of Iraq. 
Kindly respond for more details.
--
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] mailinfo: ignore in-body header that we do not care about

[Junio C Hamano]

"git mailinfo" (hence "git am") understands some well-known headers,
like "Subject: ", "Date: " and "From: ", placed at the beginning of
the message body (and the "--scissors" can discard the part of the
body before a scissors-mark).  However, some people throw other
kinds of header-looking things there, expecting them to be
discarded.

Finding and discarding anything that looks like RFC2822 header is
not a right solution.  The body of the message may start with a line
that begins with a word followed by a colon that is a legitimate
part of the message that should not be discarded.

Instead, keep reading non-blank lines once we see an in-body header
at the beginning and discard them.  Nobody will be insane enough to
reorder the headers to read like this:

Garbage-non-in-body-header: here
Subject: in-body subject

Here is the body of the commit log.

but it is common for lazy or misguided people to leave non-header
materials in-body like this:

From: Junio C Hamano 
Date: Mon, 28 Sep 2015 19:19:27 -0700
Subject: [PATCH] Git 2.6.1
MIME-Version: 1.0

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

 I think it is wrong for the in-body header codepath to pay
 attention to content-transfer-encodings and stuff, but that is a
 separate issue.

 Also if you remove the "does the line even look like a header?"
 check, some tests in t5100 starts failing.  E.g.

From nobody Mon Sep 17 00:00:00 2001
From: A U Thor 
Subject: check bogus body header (from)
Date: Fri, 9 Jun 2006 00:44:16 -0700

From: bogosity
  - a list
  - of stuff

 wants to make sure the list of two bulletted-items are in the
 commit log, and the in-body From: line gets used.

 So I dunno.  I am not entirely convinced that this is a good
 change.

 builtin/mailinfo.c  | 34 +++---
 t/t5100-mailinfo.sh |  3 ++-
 t/t5100/info0018|  5 +
 t/t5100/msg0018 |  2 ++
 t/t5100/patch0018   |  6 ++
 t/t5100/sample.mbox | 18 ++
 6 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 t/t5100/info0018
 create mode 100644 t/t5100/msg0018
 create mode 100644 t/t5100/patch0018

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 999a525..169ee54 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -787,18 +787,46 @@ static int is_scissors_line(const struct strbuf *line)
 
 static int handle_commit_msg(struct strbuf *line)
 {
+   /*
+* Are we still scanning and discarding in-body headers?
+* It is initially set to 1, set to 2 when we do see a
+* valid in-body header.
+*/
static int still_looking = 1;
+   int is_empty_line;
 
if (!cmitmsg)
return 0;
 
-   if (still_looking) {
-   if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
+   is_empty_line = (!line->len || (line->len == 1 && line->buf[0] == 
'\n'));
+   if (still_looking == 1) {
+   /*
+* Haven't seen a known in-body header; discard an empty line.
+*/
+   if (is_empty_line)
return 0;
}
 
if (use_inbody_headers && still_looking) {
-   still_looking = check_header(line, s_hdr_data, 0);
+   int is_known_header = check_header(line, s_hdr_data, 0);
+
+   if (still_looking == 2) {
+   /*
+* an empty line after the in-body header block,
+* or a line obviously not an attempt to invent
+* an unsupported in-body header.
+*/
+   if (is_empty_line || !is_rfc2822_header(line))
+   still_looking = 0;
+   if (is_empty_line)
+   return 0;
+   /* otherwise do not discard the line, but keep going */
+   } else if (is_known_header) {
+   still_looking = 2;
+   } else if (still_looking != 2) {
+   still_looking = 0;
+   }
+
if (still_looking)
return 0;
} else
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index e97cfb2..3ce041b 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,8 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
-   test `cat last` = 17'
+   test `cat last` = 18
+'
 
 check_mailinfo () {
mail=$1 opt=$2
diff --git a/t/t5100/info0018 b/t/t5100/info0018
new file mode 100644
index 000..ec671fc
--- /dev/null
+++ b/t/t5100/info0018
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.t...@example.com
+Subject: A E I O U
+Date: Mon, 17 Sep 2012 14:23:49 -0700
+
diff --git a/t/t5100/msg0018 

[PATCH v2] filter-branch: remove multi-line headers in msg filter

[James McCoy]

df062010 (filter-branch: avoid passing commit message through sed)
introduced a regression when filtering commits with multi-line headers,
if the header contains a blank line.  An example of this is a gpg-signed
commit:

  $ git cat-file commit signed-commit
  tree 3d4038e029712da9fc59a72afbfcc90418451630
  parent 110eac945dc1713b27bdf49e74e5805db66971f0
  author A U Thor  1112912413 -0700
  committer C O Mitter  1112912413 -0700
  gpgsig -BEGIN PGP SIGNATURE-
   Version: GnuPG v1

   iEYEABECAAYFAlYXADwACgkQE7b1Hs3eQw23CACgldB/InRyDgQwyiFyMMm3zFpj
   pUsAnA+f3aMUsd9mNroloSmlOgL6jIMO
   =0Hgm
   -END PGP SIGNATURE-

  Adding gpg

As a consequence, "filter-branch --msg-filter cat" (which should leave the
commit message unchanged) spills the signature (after the internal blank
line) into the original commit message.

The reason is that although the signature is indented, making the line a
whitespace only line, the “read” call is splitting the line based on
the shell's IFS, which defaults to .  The leading
space is consumed and $header_line is empty, causing the “skip header
lines” loop to exit.

The rest of the commit object is then re-used as the rewritten commit
message, causing the new message to include the signature of the
original commit.

Set IFS to an empty string for the “read” call, thus disabling the word
splitting, which causes $header_line to be set to the non-empty value '
'.  This allows the loop to fully consume the header lines before
emitting the original, intact commit message.

Signed-off-by: James McCoy 
---
 git-filter-branch.sh |  2 +-
 t/t7003-filter-branch.sh | 14 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5b3f63d..fff8093 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -347,7 +347,7 @@ while read commit parents; do
fi
 
{
-   while read -r header_line && test -n "$header_line"
+   while IFS='' read -r header_line && test -n "$header_line"
do
# skip header lines...
:;
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 855afda..377c648 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -2,6 +2,7 @@
 
 test_description='git filter-branch'
 . ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
 
 test_expect_success 'setup' '
test_commit A &&
@@ -292,6 +293,19 @@ test_expect_success 'Tag name filtering strips gpg 
signature' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'Filtering retains message of gpg signed commit' '
+   mkdir gpg &&
+   touch gpg/foo &&
+   git add gpg &&
+   test_tick &&
+   git commit -S -m "Adding gpg" &&
+
+   git log -1 --format="%s" > expect &&
+   git filter-branch -f --msg-filter "cat" &&
+   git log -1 --format="%s" > actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'Tag name filtering allows slashes in tag names' '
git tag -m tag-with-slash X/1 &&
git cat-file tag X/1 | sed -e s,X/1,X/2, > expect &&
-- 
2.6.1


-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy 
--
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 0/2] Reinstate the helpful message when `git pull --rebase` fails

[Junio C Hamano]

Johannes Schindelin  writes:

> Brendan Forster noticed that we no longer see the helpful message after
> a failed `git pull --rebase`. It turns out that the builtin `am` calls
> the recursive merge function directly, not via a separate process.
>
> But that function was not really safe to be called that way, as it
> die()s pretty liberally.

If that is the case, I'd thinkg that we'd prefer, as a regression
fix to correct "that", i.e., let recursive-merge die and let the
caller catch its exit status.

Paul, what do you think?
--
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] t0027: Improve test for not-normalized files

[Torsten Bögershausen]

When a text file with mixed line endings is commited into the repo,
it is called "not normalized" (or NNO) in t0027.
The existing test case using repoMIX did not fully test all combinations:
(Especially when core.autocrlf = true)
Files with NL are not converted at commit, but at checkout, so a warning
NL->CRLF is given.
Files with CRLF are not converted at all (so no warning will be given),
unless they are marked as "text" or "auto".

Remove repoMIX introduced in commit 8eeab92f02, and replace it with
a combination of NNO tests.

Signed-off-by: Torsten Bögershausen 
---
 t/t0027-auto-crlf.sh | 191 ++-
 1 file changed, 157 insertions(+), 34 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 1a56e5e..b343651 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -55,6 +55,26 @@ create_gitattributes () {
esac
 }
 +create_NNO_files () {
+   lfname=$1
+   crlfname=$2
+   lfmixcrlf=$3
+   lfmixcr=$4
+   crlfnul=$5
+   for crlf in false true input
+   do
+   for attr in "" auto text -text lf crlf
+   do
+   pfx=NNO_${crlf}_attr_${attr} &&
+   cp $lfname${pfx}_LF.txt &&
+   cp $crlfname  ${pfx}_CRLF.txt &&
+   cp $lfmixcrlf ${pfx}_CRLF_mix_LF.txt &&
+   cp $lfmixcr   ${pfx}_LF_mix_CR.txt &&
+   cp $crlfnul   ${pfx}_CRLF_nul.txt
+   done
+   done
+}
+
 check_warning () {
case "$1" in
LF_CRLF) echo "warning: LF will be replaced by CRLF" >"$2".expect ;;
@@ -62,7 +82,7 @@ check_warning () {
'')  >"$2".expect ;;
*) echo >&2 "Illegal 1": "$1" ; return false ;;
esac
-   grep "will be replaced by" "$2" | sed -e "s/\(.*\) in [^ ]*$/\1/" 
>"$2".actual
+   grep "will be replaced by" "$2" | sed -e "s/\(.*\) in [^ ]*$/\1/" | uniq
>"$2".actual
test_cmp "$2".expect "$2".actual
 }
 @@ -71,19 +91,10 @@ commit_check_warn () {
attr=$2
lfname=$3
crlfname=$4
-   repoMIX=$5
-   lfmixcrlf=$6
-   lfmixcr=$7
-   crlfnul=$8
+   lfmixcrlf=$5
+   lfmixcr=$6
+   crlfnul=$7
pfx=crlf_${crlf}_attr_${attr}
-   # Special handling for repoMIX: It should already be in the repo
-   # with CRLF
-   f=repoMIX
-   fname=${pfx}_$f.txt
-   echo >.gitattributes &&
-   cp $f $fname &&
-   git -c core.autocrlf=false add $fname 2>"${pfx}_$f.err" &&
-   git commit -m "repoMIX" &&
create_gitattributes "$attr" &&
for f in LF CRLF repoMIX LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
do
@@ -99,6 +110,45 @@ commit_check_warn () {
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
 }
 +commit_chk_wrnNNO () {
+   crlf=$1
+   attr=$2
+   lfwarn=$3
+   crlfwarn=$4
+   lfmixcrlf=$5
+   lfmixcr=$6
+   crlfnul=$7
+   pfx=NNO_${crlf}_attr_${attr}
+   #Commit files on top of existing file
+   create_gitattributes "$attr" &&
+   for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
+   do
+   fname=${pfx}_$f.txt &&
+   cp $f $fname &&
+   git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
+   git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
>"${pfx}_$f.err" 2>&1
+   done
+
+   test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
+   check_warning "$lfwarn" ${pfx}_LF.err
+   '
+   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF" '
+   check_warning "$crlfwarn" ${pfx}_CRLF.err
+   '
+
+   test_expect_success "commit NNO files crlf=$crlf attr=$attr 
CRLF_mix_LF" '
+   check_warning "$lfmixcrlf" ${pfx}_CRLF_mix_LF.err
+   '
+
+   test_expect_success "commit NNO files crlf=$crlf attr=$attr LF_mix_cr" '
+   check_warning "$lfmixcr" ${pfx}_LF_mix_CR.err
+   '
+
+   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF_nul" '
+   check_warning "$crlfnul" ${pfx}_CRLF_nul.err
+   '
+}
+
 check_files_in_repo () {
crlf=$1
attr=$2
@@ -115,6 +165,31 @@ check_files_in_repo () {
compare_files $crlfnul ${pfx}CRLF_nul.txt
 }
 +check_in_repo_NNO () {
+   crlf=$1
+   attr=$2
+   lfname=$3
+   crlfname=$4
+   lfmixcrlf=$5
+   lfmixcr=$6
+   crlfnul=$7
+   pfx=NNO_${crlf}_attr_${attr}_
+   test_expect_success "compare_files $lfname ${pfx}LF.txt" '
+   compare_files $lfname ${pfx}LF.txt
+   '
+   test_expect_success "compare_files $crlfname ${pfx}CRLF.txt" '
+   compare_files $crlfname ${pfx}CRLF.txt
+   '
+   test_expect_success "compare_files $lfmixcrlf ${pfx}CRLF_mix_LF.txt" '
+   compare_files $lfmixcrlf ${pfx}CRLF_mix_LF.txt
+ 

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

[Matthieu Moy]

Karthik Nayak  writes:

> On Thu, Oct 8, 2015 at 10:40 PM, Matthieu Moy
>  wrote:
>
>> This particular piece of code is so important and I won't fight for a
>> better factored one, but in general "there are only two instances" is a
>> dubious argument to avoid refactoring.
>>
>> Still, I find it weird to force the nobracket to be always at the same
>> position.
>>
>
> No i mean I could follow up with the way we use it in align, but I don't see
> how I can make a function out of this.

Ah, OK. Well, there are already two instances of

/*  Strip trailing comma */
if (s[1])
strbuf_setlen(s[0], s[0]->len - 1);


(for objectname and align), one of which says

/*
 * TODO: Implement a function similar to strbuf_split_str()
 * which would omit the separator from the end of each value.
 */

I guess it's time to replace this TODO with actual code. A
straightforward implementation would be to call strbuf_split_str and
then use the same "if (s[1])" as you already have. There's probably a
more efficient way.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 00/10] port branch.c to use ref-filter's printing options

[Junio C Hamano]

Karthik Nayak  writes:

> No i mean I could follow up with the way we use it in align, but I don't see
> how I can make a function out of this.

At least you should be able to pre-parse the %(:)
construct, instead of doing strcmp() every time populate_value() is
called, no?  Then your parser would not be doing

 + if (!strcmp(valp, ",nobracket"))
 + nobracket = 1;

with 'valp' that scans over 'name' (which is an element of
used_atom[], i.e. a name from valid_atom[] followed by ":"
for each customization).  Instead your used_atom[] would be an array
of a data structure that is richer than just a string.

The  would be split at comma boundary and made into an
array of two field structure, as the most general form of it is

 ::=  |  ',' 
 ::=  '='  |  | 

if we recall the discussion we had while designing %(align), i.e.

 * The most general form is "%(align:position=left,width=32)"

 * The value domain of attributes may be distinct, in which case the
   value itself could imply what attr it is talking about,
   e.g. "%(align:32,left)" is sufficiently clear as '32' cannot be
   position and 'left' cannot be width.

And clearly there can be an attr whose presense alone can imply its
value (iow, a boolean), even though your %(align) does not yet have
such a modifier.  It is easy to imagine that you would later want to
add %(align:position=left,width=32,truncate) that truncates the value
when its display width exceeds '32'.  The 'nobracket' above would be
an another example of a boolean (whose name is negative, which is
not a very good design in general, though).

Then used_atom[] could become something like

struct {
const char *str; /* e.g. "align:position=left,32" */
struct {
const char *part0; /* everything before '=' */
const char *part1; /* optional */
} *modifier;
int modifier_nr;
} *used_atom;

and "align:position=left,32" would be parsed into

.str = "align:position=left,32";
.modifier = { { "position", "left" }, { "32", NULL } };
.modifier_nr = 2;

when the format string is read, which is done only once.

The looping over all the refs done in populate_value() could just
use the pre-parsed representation.

Hmm?
--
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 07/10] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

[Matthieu Moy]

Karthik Nayak  writes:

> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1118,8 +1118,10 @@ static void populate_value(struct ref_array_item *ref)
>   char buf[40];
>  
>   if (stat_tracking_info(branch, _ours,
> -_theirs, NULL))
> +_theirs, NULL)) {
> + v->s = "[gone]";

My remark about translation still holds. The string was previously
translated in "branch" and you are removing this translation (well, not
here, but when 09/10 starts using this code).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 0/1] merge: fix cache_entry use-after-free

[David Turner]

Keith diagnosed the problem and wrote the patch.  I wrote the commit
message and am sending it upstream with his OK.

Keith McGuigan (1):
  merge: fix cache_entry use-after-free

 cache.h| 27 +++
 name-hash.c|  7 ++-
 read-cache.c   |  5 -
 split-index.c  | 12 +++-
 unpack-trees.c |  6 --
 5 files changed, 48 insertions(+), 9 deletions(-)

-- 
2.4.2.644.g97b850b-twtrsrc

--
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 0/1] merge: fix cache_entry use-after-free

[David Turner]

From: Keith McGuigan 

During merges, we would previously free entries that we no longer need
in the destination index.  But those entries might also be stored in
the dir_entry cache, and when a later call to add_to_index found them,
they would be used after being freed.

To prevent this, add a ref count for struct cache_entry.  Whenever
a cache entry is added to a data structure, the ref count is incremented;
when it is removed from the data structure, it is decremented.  When
it hits zero, the cache_entry is freed.

Signed-off-by: David Turner 
Signed-off-by: Keith McGuigan 
---
 cache.h| 27 +++
 name-hash.c|  7 ++-
 read-cache.c   |  5 -
 split-index.c  | 12 +++-
 unpack-trees.c |  6 --
 5 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..7563658 100644
--- a/cache.h
+++ b/cache.h
@@ -149,6 +149,7 @@ struct stat_data {
 
 struct cache_entry {
struct hashmap_entry ent;
+   unsigned int ref_count; /* count the number of refs to this in dir_hash 
*/
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
@@ -213,6 +214,32 @@ struct cache_entry {
 struct pathspec;
 
 /*
+ * Increment the cache_entry reference count.  Should be called
+ * whenever a pointer to a cache_entry is retained in a data structure,
+ * thus marking it as alive.
+ */
+static inline void add_ce_ref(struct cache_entry *ce)
+{
+   assert(ce != NULL && ce->ref_count >= 0);
+   ++ce->ref_count;
+}
+
+/*
+ * Decrement the cache_entry reference count.  Should be called whenever
+ * a pointer to a cache_entry is dropped.  Once the counter drops to 0
+ * the cache_entry memory will be safely freed.
+ */
+static inline void drop_ce_ref(struct cache_entry *ce)
+{
+   if (ce != NULL) {
+   assert(ce->ref_count >= 0);
+   if (--ce->ref_count < 1) {
+   free(ce);
+   }
+   }
+}
+
+/*
  * Copy the sha1 and stat state of a cache entry from one to
  * another. But we never change the name, or the hash state!
  */
diff --git a/name-hash.c b/name-hash.c
index 702cd05..f12c919 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -66,6 +66,7 @@ static struct dir_entry *hash_dir_entry(struct index_state 
*istate,
dir = xcalloc(1, sizeof(struct dir_entry));
hashmap_entry_init(dir, memihash(ce->name, namelen));
dir->namelen = namelen;
+   add_ce_ref(ce);
dir->ce = ce;
hashmap_add(>dir_hash, dir);
 
@@ -92,7 +93,9 @@ static void remove_dir_entry(struct index_state *istate, 
struct cache_entry *ce)
struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce));
while (dir && !(--dir->nr)) {
struct dir_entry *parent = dir->parent;
-   hashmap_remove(>dir_hash, dir, NULL);
+   struct dir_entry *removed = hashmap_remove(>dir_hash, 
dir, NULL);
+   assert(removed == dir);
+   drop_ce_ref(dir->ce);
free(dir);
dir = parent;
}
@@ -105,6 +108,7 @@ static void hash_index_entry(struct index_state *istate, 
struct cache_entry *ce)
ce->ce_flags |= CE_HASHED;
hashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce)));
hashmap_add(>name_hash, ce);
+   add_ce_ref(ce);
 
if (ignore_case)
add_dir_entry(istate, ce);
@@ -147,6 +151,7 @@ void remove_name_hash(struct index_state *istate, struct 
cache_entry *ce)
return;
ce->ce_flags &= ~CE_HASHED;
hashmap_remove(>name_hash, ce, ce);
+   drop_ce_ref(ce);
 
if (ignore_case)
remove_dir_entry(istate, ce);
diff --git a/read-cache.c b/read-cache.c
index 87204a5..442de34 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -53,6 +53,7 @@ static const char *alternate_index_output;
 static void set_index_entry(struct index_state *istate, int nr, struct 
cache_entry *ce)
 {
istate->cache[nr] = ce;
+   add_ce_ref(ce);
add_name_hash(istate, ce);
 }
 
@@ -62,7 +63,7 @@ static void replace_index_entry(struct index_state *istate, 
int nr, struct cache
 
replace_index_entry_in_base(istate, old, ce);
remove_name_hash(istate, old);
-   free(old);
+   drop_ce_ref(old);
set_index_entry(istate, nr, ce);
ce->ce_flags |= CE_UPDATE_IN_BASE;
istate->cache_changed |= CE_ENTRY_CHANGED;
@@ -75,6 +76,7 @@ void rename_index_entry_at(struct index_state *istate, int 
nr, const char *new_n
 
new = xmalloc(cache_entry_size(namelen));
copy_cache_entry(new, old);
+   new->ref_count = 0;
new->ce_flags &= ~CE_HASHED;
new->ce_namelen = namelen;
new->index = 0;
@@ -1426,6 +1428,7 @@ static struct cache_entry *cache_entry_from_ondisk(struct 

Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms

[Matthieu Moy]

Karthik Nayak  writes:

> +static void if_then_else_handler(struct ref_formatting_stack **stack)
> +{
> + struct ref_formatting_stack *cur = *stack;
> + struct ref_formatting_stack *prev = cur->prev;
> + struct if_then_else *if_then_else = (struct if_then_else 
> *)cur->at_end_data;
> +

You should add

if (!if_then_else->then_atom)
die(_("format: %%(if) atom used without a %%(then) atom"));

here ...

> +static void then_atom_handler(struct atom_value *atomv, struct 
> ref_formatting_state *state)
> +{
> + struct ref_formatting_stack *cur = state->stack;
> + struct if_then_else *if_then_else = NULL;
> +
> + if (cur->at_end == if_then_else_handler)
> + if_then_else = (struct if_then_else *)cur->at_end_data;
> + if (!if_then_else)
> + die(_("format: %%(then) atom used without an %%(if) atom"));
> + if (if_then_else->then_atom)
> + die(_("format: %%(then) atom used more than once"));
> + if_then_else->then_atom = 1;

... and

if (if_then_else->else_atom)
die(_("format: %%(then) atom used after %%(else)"));

here, just in case (adding the two corresponding test_must_fail wouldn't
harm of course).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 02/10] ref-filter: implement %(if:equals=) and %(if:notequals=)

[Matthieu Moy]

Karthik Nayak  writes:

> @@ -309,11 +319,19 @@ static void then_atom_handler(struct atom_value *atomv, 
> struct ref_formatting_st
>   if (if_then_else->then_atom)
>   die(_("format: %%(then) atom used more than once"));
>   if_then_else->then_atom = 1;
> +
>   /*

Useless new blank line.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: `-u`/`update-head-ok` unsupported on pull

[Stefan Beller]

+Paul Tan who rewrote git pull in C recently.

The manpage:
-u, --update-head-ok
   By default git fetch refuses to update the head which
corresponds to the current branch. This flag disables the check. This
is purely for the internal use for git pull to communicate with
   git fetch, and unless you are implementing your own
Porcelain you are not supposed to use it.

I guess we either need to update the manpage to remove that option
then, or add it back in in case somebody actually uses it despite the
warning in the man page?
--
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 04/10] ref-filter: modify "%(objectname:short)" to take length

[Matthieu Moy]

Karthik Nayak  writes:

> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -464,14 +464,28 @@ static void *get_obj(const unsigned char *sha1, struct 
> object **obj, unsigned lo
>  static int grab_objectname(const char *name, const unsigned char *sha1,
>   struct atom_value *v)
>  {
> - if (!strcmp(name, "objectname")) {
> - char *s = xmalloc(41);
> - strcpy(s, sha1_to_hex(sha1));
> - v->s = s;
> - return 1;
> - }
> - if (!strcmp(name, "objectname:short")) {
> - v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
> + const char *p;
> +
> + if (match_atom_name(name, "objectname", )) {
> + /*  No arguments given, copy the entire objectname */
> + if (!p) {
> + char *s = xmalloc(41);

While you're there, you may want to get rid of this hardcoded 41 and
write GIT_SHA1_HEXSZ + 1 instead.

> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 7c9bec7..6fc569e 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '
>   test_cmp expected actual
>  '
>  
> +cat >expected < +$(git rev-parse --short=1 HEAD)
> +EOF

Please write all code within test_expect_success including this
(t/README:

 - Put all code inside test_expect_success and other assertions.

   Even code that isn't a test per se, but merely some setup code
   should be inside a test assertion.
).

> +test_expect_success 'Check objectname:short format when size is less than 
> MINIMUM_ABBREV' '
> + git for-each-ref --format="%(objectname:short=1)" refs/heads >actual &&
> + test_cmp expected actual
> +'
> +
> +cat >expected < +$(git rev-parse --short=10 HEAD)
> +EOF

Likewise.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors

[Uwe Kleine-König]

Hello Junio,

On Thu, Oct 08, 2015 at 12:28:46PM -0700, Junio C Hamano wrote:
> Uwe Kleine-König   writes:
> 
> > Hello,
> >
> > when applying the mail below (without the '> ' prefix) using git am
> > --scissors the result looks like:
> >
> > $ git show
> > commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61
> > Author: Tony Lindgren 
> > Date:   Tue Oct 6 05:36:17 2015 -0700
> >
> > memory: omap-gpmc: Fix unselectable debug option for GPMC
> > 
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > 
> > Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for 
> > debug")
> > added a debug option for GPMC, but somehow managed to keep it 
> > unselectable.
> > 
> > [...]
> >
> > $ git version
> > git version 2.6.0
> >
> > The obvious improvement is to strip all headers like git am does without
> > --scissors.
> 
> Does this have anything to do with scissors, though?  If you remove
> everything before "8< ---" in the body of Tony's message (i.e. keep
> the in-body headers starting with "From:" and ending with CTE) and
> try again, I would suspect that you will get the same result.
No, you're wrong here:

u...@dude.ptx:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS 
From: Tony Lindgren 
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
added a debug option for GPMC, but somehow managed to keep it unselectable.

u...@dude.ptx:~/gsrc/linux$ git am ~/tmp/1444332661.3982_89.ptx\:2\,RS
Applying: memory: omap-gpmc: Fix unselectable debug option for GPMC
u...@dude.ptx:~/gsrc/linux$ git cat-file commit HEAD
tree bab01e3e0d0bdd715b86cf7d5c9e8bb9768a30dc
parent c6fa8e6de3dc420cba092bf155b2ed25bcd537f7
author Tony Lindgren  1444134977 -0700
committer Uwe Kleine-König  1444332782 +0200

memory: omap-gpmc: Fix unselectable debug option for GPMC

Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
added a debug option for GPMC, but somehow managed to keep it unselectable.

This probably happened because I had some uncommitted changes and the
GPMC option is selected in the platform specific Kconfig.

Let's also update the description a bit, it does not mention that
enabling the debug option also disables the reset of GPMC controller
during the init as pointed out by Uwe Kleine-König
 and Roger Quadros .

Fixes: 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
Reported-by: Uwe Kleine-König 
Signed-off-by: Tony Lindgren 

> I also think that the "MIME-Version" thing is what gives this;
> mailinfo and am do not really use it, and consider that the in-body
> header ends there.

I failed to follow you here.

> The right approach to tweak mailinfo to cope with this better would
> be to keep a bit more state inside mailinfo.c::handle_commit_msg()
> so that if we are (1) using in-body headers, (2) have already seen
> _some_ valid in-body header like "Subject:" and "From: ", and (3)
> have not seen a blank line, discard lines that we do not care about
> (e.g. "MIME-VERSION: 1.0").

That sound's right.
 
> > If someone wants a bounce of the original mail, just ask per PM.
> 
> I have no idea what you are talking about here...

The result would be that a copy of the original mail would hit your mailbox if
you asked per private mail (PM).

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
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 2/2] pack-objects: do not get distracted by broken symrefs

[Junio C Hamano]

Johannes Schindelin  writes:

> Would yo kindly add the line
>
> This fixes https://github.com/git-for-windows/git/issues/423
>
> before the Signed-off-by lines?

Oh, sorry, I missed that one.  Fixed.

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 0/1] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

> From: Keith McGuigan 
>
> During merges, we would previously free entries that we no longer need
> in the destination index.  But those entries might also be stored in
> the dir_entry cache, and when a later call to add_to_index found them,
> they would be used after being freed.
>
> To prevent this, add a ref count for struct cache_entry.  Whenever
> a cache entry is added to a data structure, the ref count is incremented;
> when it is removed from the data structure, it is decremented.  When
> it hits zero, the cache_entry is freed.
>
> Signed-off-by: David Turner 
> Signed-off-by: Keith McGuigan 
> ---

Thanks.

> @@ -213,6 +214,32 @@ struct cache_entry {
>  struct pathspec;
>  
>  /*
> + * Increment the cache_entry reference count.  Should be called
> + * whenever a pointer to a cache_entry is retained in a data structure,
> + * thus marking it as alive.
> + */
> +static inline void add_ce_ref(struct cache_entry *ce)
> +{
> + assert(ce != NULL && ce->ref_count >= 0);
> + ++ce->ref_count;
> +}

We tend to prefer post-increment when the distinction between pre-
or post- does not make any difference, which is the case here.

> diff --git a/name-hash.c b/name-hash.c
> index 702cd05..f12c919 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -92,7 +93,9 @@ static void remove_dir_entry(struct index_state *istate, 
> struct cache_entry *ce)
>   struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce));
>   while (dir && !(--dir->nr)) {
>   struct dir_entry *parent = dir->parent;
> - hashmap_remove(>dir_hash, dir, NULL);
> + struct dir_entry *removed = hashmap_remove(>dir_hash, 
> dir, NULL);
> + assert(removed == dir);
> + drop_ce_ref(dir->ce);

This is curious.  In remove_name_hash() you do not have the
corresponding assert.  Why is it necessary here (or is it
unnecessary over there)?

> @@ -147,6 +151,7 @@ void remove_name_hash(struct index_state *istate, struct 
> cache_entry *ce)
>   return;
>   ce->ce_flags &= ~CE_HASHED;
>   hashmap_remove(>name_hash, ce, ce);
> + drop_ce_ref(ce);
>  
>   if (ignore_case)
>   remove_dir_entry(istate, ce);
> diff --git a/read-cache.c b/read-cache.c
> index 87204a5..442de34 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -53,6 +53,7 @@ static const char *alternate_index_output;
>  static void set_index_entry(struct index_state *istate, int nr, struct 
> cache_entry *ce)
>  {
>   istate->cache[nr] = ce;
> + add_ce_ref(ce);
>   add_name_hash(istate, ce);
>  }

What happens to the existing entry that used to be istate->cache[nr],
which may or may not be 'ce' that is replacing it?

It turns out that all three calling sites are safe, but it is not
immediately obvious why.  Perhaps some comment in front of the
function is in order, to warn those who may have to add a new caller
or restructure the existing calling chain, that istate->cache[nr] is
expected not to hold a live reference when the function is called,
or something?

> @@ -314,8 +314,10 @@ void replace_index_entry_in_base(struct index_state 
> *istate,
>   istate->split_index->base &&
>   old->index <= istate->split_index->base->cache_nr) {
>   new->index = old->index;
> - if (old != istate->split_index->base->cache[new->index - 1])
> - free(istate->split_index->base->cache[new->index - 1]);
> + if (old != istate->split_index->base->cache[new->index - 1]) {
> + struct cache_entry *ce = 
> istate->split_index->base->cache[new->index - 1];
> + drop_ce_ref(ce);
> + }
>   istate->split_index->base->cache[new->index - 1] = new;

Does 'new' already have the right refcount at this point?
--
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 strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors

[Junio C Hamano]

Uwe Kleine-König   writes:

>> Does this have anything to do with scissors, though?  If you remove
>> everything before "8< ---" in the body of Tony's message (i.e. keep
>> the in-body headers starting with "From:" and ending with CTE) and
>> try again, I would suspect that you will get the same result.
> No, you're wrong here:
>
> u...@dude.ptx:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS 
> From: Tony Lindgren 
> Date: Tue, 6 Oct 2015 05:36:17 -0700
> Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> added a debug option for GPMC, but somehow managed to keep it unselectable.

I think you are the one who misread my question.  I said "keep the
in-body headers", didn't I?  If you did the "head", then you would
see something like this:


> u...@dude.ptx:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS 
> From: Tony Lindgren 
> Date: Tue, 6 Oct 2015 05:36:17 -0700
> Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> ... probably Received: and all other junk from your mailbox ...
>
> From: Tony Lindgren 
> Date: Tue, 6 Oct 2015 05:36:17 -0700
> Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")
> added a debug option for GPMC, but somehow managed to keep it unselectable.

>> I also think that the "MIME-Version" thing is what gives this;
>> mailinfo and am do not really use it, and consider that the in-body
>> header ends there.
>
> I failed to follow you here.

I think if you tried the example with in-body header, you will see
what I meant.

>
>> The right approach to tweak mailinfo to cope with this better would
>> be to keep a bit more state inside mailinfo.c::handle_commit_msg()
>> so that if we are (1) using in-body headers, (2) have already seen
>> _some_ valid in-body header like "Subject:" and "From: ", and (3)
>> have not seen a blank line, discard lines that we do not care about
>> (e.g. "MIME-VERSION: 1.0").
>
> That sound's right.
--
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