[PATCH] gitk: show detached HEAD if --all is specified

2014-09-09 Thread Max Kirillov
If HEAD is detached, 'gitk --all' does not show it. This is inconvenient
for frontend program, and for example git log does show the detached HEAD.

gitk uses git rev-parse to find a list of branches to show.
Apparently, the command does not include detached HEAD to output if
--all argument is specified. This has been discussed in [1] and stated
as expected behavior. So rev-parse's parameters should be tuned in gitk.

[1] http://thread.gmane.org/gmane.comp.version-control.git/255996

Signed-off-by: Max Kirillov 
---
 gitk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gitk b/gitk
index c8df35d..a5d98bf 100755
--- a/gitk
+++ b/gitk
@@ -294,6 +294,8 @@ proc parseviewrevs {view revs} {
 
 if {$revs eq {}} {
set revs HEAD
+} elseif {[lsearch -exact $revs --all] >= 0} {
+   lappend revs HEAD
 }
 if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
# we get stdout followed by stderr in $err
-- 
2.0.1.1697.g73c6810

--
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] diff --no-index: allow pathspec after --

2014-09-09 Thread Nguyễn Thái Ngọc Duy
Another patch to test the water before I put more effort into it.

Commit d516c2d (Teach git-diff-files the new option `--no-index` -
2007-02-22) brings the bells and whistles of git-diff to the world
outside a git repository. This patch continues that direction and adds
a new syntax

git diff --no-index [--]   -- 

It's easy to guess that the two directories will be filtered by
pathspec, which could be useful for filtering out generated files
(negative pathspec to rescue!), or simply to limit diff output to a
selection of files.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/diff.c  | 26 ++
 diff-no-index.c | 49 ++---
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 0f247d2..4e37876 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -321,23 +321,25 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 
init_revisions(&rev, prefix);
 
-   if (no_index && argc != i + 2) {
-   if (no_index == DIFF_NO_INDEX_IMPLICIT) {
-   /*
-* There was no --no-index and there were not two
-* paths. It is possible that the user intended
-* to do an inside-repository operation.
-*/
-   fprintf(stderr, "Not a git repository\n");
-   fprintf(stderr,
-   "To compare two paths outside a working 
tree:\n");
-   }
+   if (no_index == DIFF_NO_INDEX_IMPLICIT && argc != i + 2) {
+   /*
+* There was no --no-index and there were not two
+* paths. It is possible that the user intended
+* to do an inside-repository operation.
+*/
+   fprintf(stderr, "Not a git repository\n");
+   fprintf(stderr,
+   "To compare two paths outside a working tree:\n");
/* Give the usage message for non-repository usage and exit. */
usagef("git diff %s  ",
   no_index == DIFF_NO_INDEX_EXPLICIT ?
   "--no-index" : "[--no-index]");
-
}
+   if (no_index == DIFF_NO_INDEX_EXPLICIT && argc < i + 2)
+   /* Give the usage message for non-repository usage and exit. */
+   usagef("git diff %s  ",
+  no_index == DIFF_NO_INDEX_EXPLICIT ?
+  "--no-index" : "[--no-index]");
if (no_index)
/* If this is a no-index diff, just run it and exit there. */
diff_no_index(&rev, argc, argv, prefix);
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..7f5cd0f 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -89,8 +89,20 @@ static struct diff_filespec *noindex_filespec(const char 
*name, int mode)
return s;
 }
 
+static int path_ok(struct diff_options *o, const char *name, int prefix)
+{
+   if (!name)
+   return 0;
+   name += prefix;
+   if (*name == '/')
+   name++;
+   return match_pathspec(&o->pathspec, name, strlen(name),
+ 0, NULL, 0);
+}
+
 static int queue_diff(struct diff_options *o,
- const char *name1, const char *name2)
+ const char *name1, int prefix1,
+ const char *name2, int prefix2)
 {
int mode1 = 0, mode2 = 0;
 
@@ -157,7 +169,7 @@ static int queue_diff(struct diff_options *o,
n2 = buffer2.buf;
}
 
-   ret = queue_diff(o, n1, n2);
+   ret = queue_diff(o, n1, prefix1, n2, prefix2);
}
string_list_clear(&p1, 0);
string_list_clear(&p2, 0);
@@ -165,7 +177,7 @@ static int queue_diff(struct diff_options *o,
strbuf_release(&buffer2);
 
return ret;
-   } else {
+   } else if (path_ok(o, name1, prefix1) || path_ok(o, name2, prefix2)) {
struct diff_filespec *d1, *d2;
 
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
@@ -180,13 +192,14 @@ static int queue_diff(struct diff_options *o,
diff_queue(&diff_queued_diff, d1, d2);
return 0;
}
+   return 0;
 }
 
 void diff_no_index(struct rev_info *revs,
   int argc, const char **argv,
   const char *prefix)
 {
-   int i, prefixlen;
+   int i, j, prefixlen;
const char *paths[2];
 
diff_setup(&revs->diffopt);
@@ -194,19 +207,23 @@ void diff_no_index(struct rev_info *revs,
int j;
if (!strcmp(argv[i], "--no-index"))
i++;
-   else if (!strcmp(argv[i], "--"))
+   else if (!strcmp(argv[i], "--")) {
i++;
-   else

[ANNOUNCE] git-as-svn: subversion frontend server for git repository

2014-09-09 Thread Marat Radchenko
Some time ago I complained [1] about troubles using Git
on a project with high ratio of non-programmers.

Since then, a conclusion was made: Git is too complex.
While Git provides many nice advanced stuff, its simplest
workflow isn't simple enough.

So we examined other options:

  * Splitting project in two repos (Git + SVN). It was
thought to be the worst idea because we lost commit
atomicity

  * Use GitHub SVN integration [2]. Rejected due to security
considerations: our closed-source project isn't allowed to be
hosted outside.

  * Use GitHub Enterprise: rejected due to pricing

  * Use SubGit [3]: rejected because of its architecture.

Then, a lost'n'forgotten git_svn_server [4] was found. After playing
with it, we found out that its approach can work, though several
decisions (Python and extensive forking of `git`) made it very slow.

So we thought "we're programmers, after all".

And that's when *git-as-svn* [5] was born. It is a daemon that sits
on top of Git repository and talks svn:// protocol.

Features supported:

  * checkout/update

  * log

  * blame

 
  * commit (!)  
 

 
  * rename detection (though a bit slow yet)
 

 
  * svn:eol-style   
 

 
  * Git pre-receive hooks   
 

 
  * simple or LDAP authentication   
 

 
  * partial checkout
 

 
  * sparse working copy (svn --depth/--set-depth)   
 

 
  * git submodules  
 

 
Current limitations:
 

 
  * Only a single Git branch from a single repository

  * Needs at least one commit in Git

  * Parses whole history on startup and doesn't cache it anywhere

  * You must not do 'inverted merges'. Old HEAD must be reachable from
new HEAD by first-parent traversal.

[1]: http://marc.info/?l=git&m=13980018802
[2]: https://help.github.com/articles/support-for-subversion-clients
[3]: http://subgit.com/
[4]: http://git.q42.co.uk/git_svn_server.git
[5]: https://github.com/bozaro/git-as-svn/
--
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


GET A QUICK CASH LOAN DEAL

2014-09-09 Thread GET A QUICK CASH LOAN DEAL
VIEW THE ATTACHMENT

GET A QUICK LOAN - CASH DEAL LOAN.pdf
Description: Adobe PDF document


Re: Next Git conference or meeting

2014-09-09 Thread Christian Couder
Hi Shawn and Peff,

On Wed, Sep 3, 2014 at 10:59 PM, Jeff King  wrote:
> On Wed, Sep 03, 2014 at 10:15:15AM -0700, Shawn Pearce wrote:
>
>> I hadn't realized Git is turning 10 next year. Just been too busy
>> using Git to pay attention to its upcoming anniversary. Let me talk to
>> some folks at Google and see if we can organize something here in
>> Mountain View, or help the LinuxFoundation sponsor something.

Did you talk to some folks?

> Christian mentioned that he talked to some GitHub folks at LinuxCon.
> Those folks have also started thinking about things. :)
>
> Things are still very tentative at this point, but I think they are
> considering something like the Git Merge conference we did earlier, and
> doing it in June in Europe (maybe Paris). I know they were going to
> reach out to Linux Foundation folks to try to jointly plan something,
> but I don't know if that has happened yet.

Could you ask if they talked to Linux Foundation folks?

It would be very easy for me to come if it was in Paris, but I'd
rather make it easy for Junio, Linus, you guys, and other
contributors, unless everyone wants to take advantage of the
conference to visit Paris :-)

> So it seems there are a lot of different people who are all potentially
> interested in planning or taking part, and they should all be talking to
> each other. :)

Maybe we should have a mailing list for that.

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


Re: [ANNOUNCE] git-as-svn: subversion frontend server for git repository

2014-09-09 Thread Junio C Hamano
Marat Radchenko  writes:

> Some time ago I complained [1] about troubles using Git
> on a project with high ratio of non-programmers.
> ...
> Then, a lost'n'forgotten git_svn_server [4] was found.
> ...

Interesting.

> Current limitations:
> ...
>   * You must not do 'inverted merges'. Old HEAD must be reachable from
> new HEAD by first-parent traversal.

I am not sure what you mean by this to properly assess how
significant this limitation is.  Care to draw a simple picture?
--
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/RFC 0/2] custom format for interactive rebase todo

2014-09-09 Thread Junio C Hamano
William Clifford  writes:

> A couple of examples:
>
> - `git config sequence.format "%<(12,trunc)%ae %s"`
> - `git config sequence.format "%s <%aN %aE>"`
> - `git config sequence.format "%s%n%%n%b"`
> ... I'm unsure what would happen if I tried to rebase with the
> third style unedited or uncommented.

It should be simply forbidden.  The body part may have a line that
is similar enough (i.e. starting with one of the command words and
then a hexadecimal string) to confuse the sequencing machinery.

Other than that safety issue, I am not fundamentally opposed to the
idea.

As to the implementation in 1/2, your unconditional use of ">%h" is
wrong (you would end up including the commits from the left side).

Use '%m' instead of a hardcoded '>', perhaps?



--
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] gitk: show detached HEAD if --all is specified

2014-09-09 Thread Junio C Hamano
Max Kirillov  writes:

> If HEAD is detached, 'gitk --all' does not show it. This is inconvenient
> for frontend program, and for example git log does show the detached HEAD.

"git log" does use the same revision machinery as rev-parse uses
internally to parse its command line arguments.  What does it do
differently?
--
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] gitk: show detached HEAD if --all is specified

2014-09-09 Thread Junio C Hamano
Junio C Hamano  writes:

> Max Kirillov  writes:
>
>> If HEAD is detached, 'gitk --all' does not show it. This is inconvenient
>> for frontend program, and for example git log does show the detached HEAD.
>
> "git log" does use the same revision machinery as rev-parse uses
> internally to parse its command line arguments.  What does it do
> differently?

Ah, nevermind.  I misremembered that rev-parse does not use the
revision machinery.
--
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/RFC 0/2] custom format for interactive rebase todo

2014-09-09 Thread Junio C Hamano
Junio C Hamano  writes:

> William Clifford  writes:
>
>> A couple of examples:
>>
>> - `git config sequence.format "%<(12,trunc)%ae %s"`
>> - `git config sequence.format "%s <%aN %aE>"`
>> - `git config sequence.format "%s%n%%n%b"`
>> ... I'm unsure what would happen if I tried to rebase with the
>> third style unedited or uncommented.
>
> It should be simply forbidden.  The body part may have a line that
> is similar enough (i.e. starting with one of the command words and
> then a hexadecimal string) to confuse the sequencing machinery.
>
> Other than that safety issue, I am not fundamentally opposed to the
> idea.
>
> As to the implementation in 1/2, your unconditional use of ">%h" is
> wrong (you would end up including the commits from the left side).
>
> Use '%m' instead of a hardcoded '>', perhaps?

Also, I do not think you want to make the prepending of "%m%h "
conditional.  If the user for whatever silly reason asks to use a
format "%m%h %m%h %m%h", let her have that _after_ the "%m%h " the
machinery needs to operate, i.e. "%m%h %m%h %m%h %m%h".  It is far
easier to explain to the users and you can lose three lines from the
second patch.
--
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 6/8] merge-recursive: allow storing conflict hunks in index

2014-09-09 Thread Junio C Hamano
Thomas Rast  writes:

> diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
> index be07705..39841a9 100755
> --- a/t/t3030-merge-recursive.sh
> +++ b/t/t3030-merge-recursive.sh
> @@ -310,6 +310,26 @@ test_expect_success 'merge-recursive --index-only' '
>   test_cmp expected-diff actual-diff
>  '
>  
> +test_expect_success 'merge-recursive --index-only --conflicts-in-index' '
> + # first pass: do a merge as usual to obtain "expected"
> + rm -fr [abcd] &&
> + git checkout -f "$c2" &&
> + test_expect_code 1 git merge-recursive "$c0" -- "$c2" "$c1" &&
> + git add [abcd] &&
> + git ls-files -s >expected &&
> + # second pass: actual test
> + rm -fr [abcd] &&
> + git checkout -f "$c2" &&
> + test_expect_code 1 \
> + git merge-recursive --index-only --conflicts-in-index \
> + "$c0" -- "$c2" "$c1" &&
> + git ls-files -s >actual &&
> + test_cmp expected actual &&

At this point what is the expected output from "git diff-files"?

> + git diff HEAD >actual-diff &&
> + : >expected-diff &&
> + test_cmp expected-diff actual-diff
> +'
> +
>  test_expect_success 'fail if the index has unresolved entries' '
>  
>   rm -fr [abcd] &&
--
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 7/8] name-hash: allow dir hashing even when !ignore_case

2014-09-09 Thread Junio C Hamano
Thomas Rast  writes:

> The directory hash (for fast checks if the index already has a
> directory) was only used in ignore_case mode and so depended on that
> flag.
>
> Make it generally available on request.
>
> Signed-off-by: Thomas Rast 
> ---
>  cache.h |  2 ++
>  name-hash.c | 13 -
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 4d5b76c..c54b2e1 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -306,6 +306,7 @@ struct index_state {
>   struct split_index *split_index;
>   struct cache_time timestamp;
>   unsigned name_hash_initialized : 1,
> +  has_dir_hash : 1,
>initialized : 1;
>   struct hashmap name_hash;
>   struct hashmap dir_hash;
> @@ -315,6 +316,7 @@ struct index_state {
>  extern struct index_state the_index;
>  
>  /* Name hashing */
> +extern void init_name_hash(struct index_state *istate, int force_dir_hash);
>  extern void add_name_hash(struct index_state *istate, struct cache_entry 
> *ce);
>  extern void remove_name_hash(struct index_state *istate, struct cache_entry 
> *ce);
>  extern void free_name_hash(struct index_state *istate);
> diff --git a/name-hash.c b/name-hash.c
> index 702cd05..22e3ec6 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -106,7 +106,7 @@ static void hash_index_entry(struct index_state *istate, 
> struct cache_entry *ce)
>   hashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce)));
>   hashmap_add(&istate->name_hash, ce);
>  
> - if (ignore_case)
> + if (istate->has_dir_hash)
>   add_dir_entry(istate, ce);

This smells more like needs_dir_hash than has_dir_hash to me.  For
ignore-case, we need dir_hash to make sure we do not end up adding
two entries that cannot be represented on the filesystem.

--
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] pretty-format: add append line-feed format specifier

2014-09-09 Thread Harry Jeffery

Add a new format prefix `_` that causes a line-feed to be inserted
immediately after an expansion if the expansion expands to a non-empty
string. This is useful for when you would like a line for an expansion
to be prepended, but only when the expansion expands to a non empty
string, such as inserting a '%_d' expansion before a commit to show any
refs pointing towards it.

Signed-off-by: Harry Jeffery 
---
 Documentation/pretty-formats.txt |  4 
 pretty.c | 10 --
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt 
b/Documentation/pretty-formats.txt

index 85d6353..842cd17 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -197,6 +197,10 @@ If you add a ` ` (space) after '%' of a 
placeholder, a space

 is inserted immediately before the expansion if and only if the
 placeholder expands to a non-empty string.

+If you add a `_` (underscore) after '%' of a placeholder, a line-feed
+is inserted immediately after the expansion if and only if the
+placeholder expands to a non-empty string.
+
 * 'tformat:'
 +
 The 'tformat:' format works exactly like 'format:', except that it
diff --git a/pretty.c b/pretty.c
index 44b9f64..ddb930d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1416,7 +1416,8 @@ static size_t format_commit_item(struct strbuf 
*sb, /* in UTF-8 */

NO_MAGIC,
ADD_LF_BEFORE_NON_EMPTY,
DEL_LF_BEFORE_EMPTY,
-   ADD_SP_BEFORE_NON_EMPTY
+   ADD_SP_BEFORE_NON_EMPTY,
+   ADD_LF_AFTER_NON_EMPTY
} magic = NO_MAGIC;

switch (placeholder[0]) {
@@ -1429,6 +1430,9 @@ static size_t format_commit_item(struct strbuf 
*sb, /* in UTF-8 */

case ' ':
magic = ADD_SP_BEFORE_NON_EMPTY;
break;
+   case '_':
+   magic = ADD_LF_AFTER_NON_EMPTY;
+   break;
default:
break;
}
@@ -1449,6 +1453,8 @@ static size_t format_commit_item(struct strbuf 
*sb, /* in UTF-8 */

} else if (orig_len != sb->len) {
if (magic == ADD_LF_BEFORE_NON_EMPTY)
strbuf_insert(sb, orig_len, "\n", 1);
+   else if (magic == ADD_LF_AFTER_NON_EMPTY)
+   strbuf_addch(sb, '\n');
else if (magic == ADD_SP_BEFORE_NON_EMPTY)
strbuf_insert(sb, orig_len, " ", 1);
}
@@ -1460,7 +1466,7 @@ static size_t userformat_want_item(struct strbuf 
*sb, const char *placeholder,

 {
struct userformat_want *w = context;

-   if (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ')
+	if (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ' 
|| *placeholder == '_')

placeholder++;

switch (*placeholder) {
--
2.1.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: [ANNOUNCE] git-as-svn: subversion frontend server for git repository

2014-09-09 Thread Marat Radchenko
On Tue, Sep 09, 2014 at 09:49:03AM -0700, Junio C Hamano wrote:
> Marat Radchenko  writes:
> 
> > Some time ago I complained [1] about troubles using Git
> > on a project with high ratio of non-programmers.
> > ...
> > Then, a lost'n'forgotten git_svn_server [4] was found.
> > ...
> 
> Interesting.

Actually, no. As I said, git_svn_server made several ineffective
architectural choices. It can be viewed as a proof-of-concept work though.

> > Current limitations:
> > ...
> >   * You must not do 'inverted merges'. Old HEAD must be reachable from
> > new HEAD by first-parent traversal.
> 
> I am not sure what you mean by this to properly assess how
> significant this limitation is.  Care to draw a simple picture?

SVN doesn't support nonlinear history (except merge-info crutch).

Thus, we only expose "main" history line to SVN where "main" means
"reachable through first-parent traversal from branch tip".

To keep SVN history consistent, commits that once became visible to SVN
have to remain visible. This limitation will be removed when git-as-svn
gets persistent storage and will be able to remember what *was* main line.

Imagine you had following history:

--- time --->

A -- B -- C

Now you merge (via Git) a feature branch:

A -- B -- C -- G
 \/
  D -- E --- F

For SVN, history will look this way:

A -- B -- C -- F

We might introduce merge-info support for this one day.

And now the *bad* case. You have the same initial history but do *inverted 
merge*:

A -- D -- E -- F -- G'
 \ /
  B -- C -/
   ^
   |
Previous branch tip

That's where things brake because for SVN, history transforms from

A -- B -- C

to

A -- D -- E -- F -- G'

And all users who checked out revisions B & C get their working copies screwed.

This also means that push --force also must not be performed.

Quoting my initial post [1] about inverted merges (you might call them
"merges with swapped parents").

> I call it "swapped/reverse merge problem".
>
> In short:
> 1. Hack, hack, hack
> 2. Commit
> 3. Push, woops, reject (non-ff)
> 4. Pull
> 5. Push
>
> The root of evil is step #4 that creates a merge commit with "swapped" 
> parents - 
> local commits become first parent, remote commits become second. If one would 
> want to 
> make proper parent order, he would have to: 1. git fetch
> 2. git checkout origin/master -b tmp
> 3. git merge master
> 4. git push
> 5. git checkout master
> 6. git merge origin/master
> 7. git branch -d tmp
> 
> And all this branch dance produces exactly the same commit (content-wise) as 
> simple
> "pull, push" sequence with the only difference in parent order. And things 
> become
> even worse if comeone pushes more commits to remote repo while you perform 
> this
> dance.
>
> We can't expect all developers (especially, designers and artist) to do it. 
> They
> don't want to use branches and just work on mainline. This is especially 
> important on
> early development stages when new features (that designers' work depends 
> upon) are
> added every day.
>
> Additionally, many git-related tools depend on first-parent convention and 
> show wrong
> graphs/diffs.

[1] http://marc.info/?l=git&m=13980018802
--
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 8/8] log --remerge-diff: show what the conflict resolution changed

2014-09-09 Thread Junio C Hamano
Thomas Rast  writes:

Thomas Rast  writes:

> Git has --cc as a very fast inspection tool that shows a brief summary
> of what a conflicted merge "looks like", and -c/-m as "give me the
> full information" data dumps.
>
> But --cc actually loses information: if the merge lost(!) some changes
> from one side, that hunk would fully agree with the other side, and
> therefore be elided.  So --cc cannot be used to investigate mismerges.
> Indeed it is rather hard to find a merge that has lost changes, unless
> one knows where to look.
>
> The new option --remerge-diff is an attempt at filling this gap,
> admittedly at the cost of a lot of CPU cycles.  For each merge commit,
> it diffs the merge result against a recursive merge of the merge's
> parents.
>
> For files that can be auto-merged cleanly, it will typically show
> nothing.  However, it will make it obvious when the merge introduces
> extra changes.
>
> For files that result in merge conflicts, we diff against the
> representation with conflict hunks (what the user would usually see in
> the worktree).  So the diff will show what was changed in the conflict
> hunks to resolve the conflict.
>
> It still takes a bit of staring to tell an evil from a regular merge.
> But at least the information is there, unlike with --cc; and the
> output is usually much shorter than with -c.
>
> Signed-off-by: Thomas Rast 
> ---
>  Documentation/rev-list-options.txt |   7 +
>  log-tree.c | 297 
> +
>  merge-recursive.c  |   3 +-
>  merge-recursive.h  |   1 +
>  revision.c |   2 +
>  revision.h |   4 +-
>  t/t4213-log-remerge-diff.sh| 222 +++
>  7 files changed, 534 insertions(+), 2 deletions(-)
>  create mode 100755 t/t4213-log-remerge-diff.sh
>
> diff --git a/Documentation/rev-list-options.txt 
> b/Documentation/rev-list-options.txt
> index deb8cca..7128350 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -805,6 +805,13 @@ options may be given. See linkgit:git-diff-files[1] for 
> more options.
>   in that case, the output represents the changes the merge
>   brought _into_ the then-current branch.
>  
> +--remerge-diff::
> + Diff merge commits against a recursive merge of their parents,
> + with conflict hunks.  Intuitively speaking, this shows what
> + the author of the merge changed to resolve the merge.  It
> + assumes that all (or most) merges are recursive merges; other
> + strategies are not supported.
> +
>  -r::
>   Show recursive diffs.
>  
> diff --git a/log-tree.c b/log-tree.c
> index 8f57651..4db1385 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -11,6 +11,8 @@
>  #include "gpg-interface.h"
>  #include "sequencer.h"
>  #include "line-log.h"
> +#include "cache-tree.h"
> +#include "merge-recursive.h"
>  
>  struct decoration name_decoration = { "object names" };
>  
> @@ -719,6 +721,299 @@ static int do_diff_combined(struct rev_info *opt, 
> struct commit *commit)
>  }
>  
>  /*
> + * Helpers for make_asymmetric_conflict_entries() below.
> + */
> +static char *load_cache_entry_blob(struct cache_entry *entry,
> +unsigned long *size)
> +{
> + enum object_type type;
> + void *data;
> +
> + if (!entry)
> + return NULL;
> +
> + data = read_sha1_file(entry->sha1, &type, size);
> + if (type != OBJ_BLOB)
> + die("BUG: load_cache_entry_blob for non-blob");
> +
> + return data;
> +}
> +
> +static void strbuf_append_cache_entry_blob(struct strbuf *sb,
> +struct cache_entry *entry)
> +{
> + unsigned long size;
> + char *data = load_cache_entry_blob(entry, &size);;
> +
> + if (!data)
> + return;
> +
> + strbuf_add(sb, data, size);
> + free(data);
> +}
> +
> +static void assemble_conflict_entry(struct strbuf *sb,
> + const char *branch1,
> + const char *branch2,
> + struct cache_entry *entry1,
> + struct cache_entry *entry2)
> +{
> + strbuf_addf(sb, "<<< %s\n", branch1);
> + strbuf_append_cache_entry_blob(sb, entry1);
> + strbuf_addstr(sb, "===\n");
> + strbuf_append_cache_entry_blob(sb, entry2);
> + strbuf_addf(sb, ">>> %s\n", branch2);
> +}

Hmm, what is this one doing?  I would have expected that you would
give file-level three-way merge using ll_merge machinery here.  For
a conflicted path with two entries, using an empty buffer as the
common ancestor would give you a reasonable-looking two-way
no-parent merge.

> +/*
> + * For --remerge-diff, we need conflicted (<<< ... >>>)
> + * representations of as many conflicts as possible.  Default conflict
> + * generation only applies to files that have all thr

Re: [PATCH] Improve English grammar

2014-09-09 Thread Alex Henrie
I see it's been accepted now. Thank you!

-Alex
--
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 8/8] log --remerge-diff: show what the conflict resolution changed

2014-09-09 Thread Junio C Hamano
Thomas Rast  writes:

> + assemble_conflict_entry(&content,
> + branch1, branch2,
> + stage2, stage3);
> + if (write_sha1_file(content.buf, content.len,
> + typename(OBJ_BLOB), sha1))
> + die("write_sha1_file failed");
> +...
> + if (cache_tree_update(&the_index, WRITE_TREE_SILENT) < 0) {
> + printf("BUG: merge conflicts not fully folded, cannot diff.\n");
> + return !opt->loginfo;
> + }

Another worry I have on this change is that it breaks the
expectation that "log [-p]" is a read-only operation.  I do not know
how big a breakage this will be viewed as by those uninitiated who
do not know how --remerge-diff (or Git in general) works internally.
--
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-format: add append line-feed format specifier

2014-09-09 Thread Junio C Hamano
Harry Jeffery  writes:

> Add a new format prefix `_` that causes a line-feed to be inserted
> immediately after an expansion if the expansion expands to a non-empty
> string. This is useful for when you would like a line for an expansion
> to be prepended, but only when the expansion expands to a non empty
> string, such as inserting a '%_d' expansion before a commit to show any
> refs pointing towards it.

Is this different from "%n%-d"?
--
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-format: add append line-feed format specifier

2014-09-09 Thread Harry Jeffery

On 09/09/14 20:15, Junio C Hamano wrote:

Is this different from "%n%-d"?



Yes. "%n%-d" will place the newline before the expansion, not after.

log --decorate --pretty=format:"%n%-d%h\\ %t\\ [%cn]\\ %s"
---

 (HEAD, upstream/master, master)85f0837 c29da1d [Junio C Hamano] Start 
the post-2.1 cycle

 f655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd'
 51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config'
 4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode'
---

log --decorate --pretty=format:"%_d%%h\\ %t\\ [%cn]\\ %s"
---
 (HEAD, upstream/master, master)
85f0837 c29da1d [Junio C Hamano] Start the post-2.1 cycle
f655651 4027a43 [Junio C Hamano] Merge branch 'rs/strbuf-getcwd'
51eeaea 1f4970c [Junio C Hamano] Merge branch 'ta/pretty-parse-config'
4740891 8961621 [Junio C Hamano] Merge branch 'bc/archive-pax-header-mode'
---

The latter is the output I've been trying to accomplish, and as far as I 
can tell, this patch is the only way to achieve it.


Well, you can do "%d%n" but that will put a blank line before every 
commit that doesn't have a ref.

--
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: [ANNOUNCE] git-as-svn: subversion frontend server for git repository

2014-09-09 Thread Junio C Hamano
Marat Radchenko  writes:

>> >   * You must not do 'inverted merges'. Old HEAD must be reachable from
>> > new HEAD by first-parent traversal.
>> 
>> I am not sure what you mean by this to properly assess how
>> significant this limitation is.  Care to draw a simple picture?
>
> SVN doesn't support nonlinear history (except merge-info crutch).
> ...
> And now the *bad* case. You have the same initial history but do *inverted 
> merge*:

That is a bad way to answer a question that asks "what do you mean
by an 'inverted merge', which is not in our normal lexicon?" ;-)

You must not merge the current tip of SVN server *into* the
work you did on top of a past state you obtained from the
SVN server.  Check out the current state from the SVN side,
and merge your work into it instead.

or something like that is what people would understand without
introducing a new/unused word to the world.  And

> A -- D -- E -- F -- G'
>  \ /
>   B -- C -/
>^
>|
> Previous branch tip

this illustrates the topology you meant reasonably well, especially
if you marked D, E and F as "your own work" (as opposed to what the
server side did in the meantime while you worked, i.e. B and C).

Thanks for a clarification.


--
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-format: add append line-feed format specifier

2014-09-09 Thread Junio C Hamano
Harry Jeffery  writes:

> On 09/09/14 20:15, Junio C Hamano wrote:
>> Is this different from "%n%-d"?
>>
>
> Yes. "%n%-d" will place the newline before the expansion, not after.

Maybe "%[-+ ]" needs to be rethought, instead of making things worse
by turning it into "%[-_+ ]", as the next person who comes would
want to add space after the expansion and would need to find yet
another letter like you did 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: [RFC PATCH 2/2] headers: include dependent headers

2014-09-09 Thread René Scharfe

Am 08.09.2014 um 19:50 schrieb Junio C Hamano:

René Scharfe  writes:


Am 06.09.2014 um 21:20 schrieb David Aguilar:

Add dependent headers so that including a header does not
require including additional headers.

This makes it so that "gcc -c $header" succeeds for each header.

Signed-off-by: David Aguilar 
---



diff --git a/branch.h b/branch.h
index 64173ab..a61fd1a 100644
--- a/branch.h
+++ b/branch.h
@@ -3,6 +3,9 @@

   /* Functions for acting on the information about branches. */

+#include "cache.h"
+#include "strbuf.h"


cache.h includes strbuf.h, so the line above isn't necessary.


True, but is "gcc -c $header" something we want to please in the
first place (not an objection, but request for opinions)?


It *sounds* useful, but we did without that feature so far.  Hmm.  It 
would make it easier to use headers -- any dependency to other files are 
already taken care of.  Since we have less .h files than .c files this 
seems to make sense.


Would it perhaps also make using precompiled headers easier (or possible 
in the first place)?  Do we care?


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


What's cooking in git.git (Sep 2014, #02; Tue, 9)

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

The second batch of topics have graduated to 'master'.  There are
too many topics waiting to be in 'next' but without comments and
reviews on the list, which is somewhat disturbing.

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

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

--
[Graduated to "master"]

* bc/imap-send-doc (2014-08-05) 1 commit
  (merged to 'next' on 2014-08-29 at 2ea514b)
 + imap-send doc: omit confusing "to use imap-send" modifier


* jc/apply-ws-prefix (2014-08-07) 3 commits
  (merged to 'next' on 2014-08-29 at 67967d3)
 + apply: omit ws check for excluded paths
 + apply: hoist use_patch() helper for path exclusion up
 + apply: use the right attribute for paths in non-Git patches

 Applying a patch not generated by Git in a subdirectory used to
 check the whitespace breakage using the attributes for incorrect
 paths. Also whitespace checks were performed even for paths
 excluded via "git apply --exclude=" mechanism.


* jc/config-mak-document-darwin-vs-macosx (2014-08-15) 1 commit
  (merged to 'next' on 2014-08-29 at 55e28af)
 + config.mak.uname: add hint on uname_R for MacOS X
 (this branch uses km/no-apple-common-crypto-on-darwin-8-and-below.)


* jc/not-mingw-cygwin (2014-07-21) 2 commits
  (merged to 'next' on 2014-08-29 at 0d00bb7)
 + test prerequisites: enumerate with commas
 + test prerequisites: eradicate NOT_FOO

 We have been using NOT_{MINGW,CYGWIN} test prerequisites long
 before Peff invented support for negated prerequisites e.g. !MINGW
 and we still add more uses of the former.  Convert them to the
 latter to avoid confusion.


* jk/command-line-config-empty-string (2014-08-05) 1 commit
  (merged to 'next' on 2014-08-29 at 74f04af)
 + config: teach "git -c" to recognize an empty string

 "git -c section.var command" and "git -c section.var= command"
 should pass the configuration differently (the former should be
 a boolean true, the latter should be an empty string).


* jk/prompt-stash-could-be-packed (2014-08-25) 1 commit
  (merged to 'next' on 2014-08-29 at 526e3bd)
 + git-prompt: do not look for refs/stash in $GIT_DIR

 The prompt script checked $GIT_DIR/ref/stash file to see if there
 is a stash, which was a no-no.


* jk/stash-list-p (2014-08-07) 1 commit
  (merged to 'next' on 2014-08-29 at db94527)
 + stash: default listing to working-tree diff

 Teach "git stash list -p" to show the difference between the base
 commit version and the working tree version, which is in line with
 what "git show" gives.


* km/no-apple-common-crypto-on-darwin-8-and-below (2014-08-15) 1 commit
  (merged to 'next' on 2014-08-29 at 8abb416)
 + config.mak.uname: set NO_APPLE_COMMON_CRYPTO on older systems
 (this branch is used by jc/config-mak-document-darwin-vs-macosx.)

 Build automation for older versions of MacOS X.


* la/init-doc (2014-08-08) 7 commits
  (merged to 'next' on 2014-08-29 at 2cf846b)
 + Documentation: git-init: flesh out example
 + Documentation: git-init: template directory: reword and cross-reference
 + Documentation: git-init: reword parenthetical statements
 + Documentation: git-init: --separate-git-dir: clarify
 + Documentation: git-init: template directory: reword
 + Documentation: git-init: list items facelift
 + Documentation: git-init: typographical fixes


* lf/bundle-exclusion (2014-08-07) 1 commit
  (merged to 'next' on 2014-08-29 at d84b102)
 + bundle: fix exclusion of annotated tags

 "git bundle create" with date-range specification were meant to
 exclude tags outside the range, but it didn't.


* mm/log-branch-desc-plug-leak (2014-08-07) 1 commit
  (merged to 'next' on 2014-08-29 at 3580add)
 + builtin/log.c: fix minor memory leak


* nd/strbuf-utf8-replace (2014-08-11) 1 commit
  (merged to 'next' on 2014-08-29 at 5d1ddf4)
 + utf8.c: fix strbuf_utf8_replace() consuming data beyond input string


* rs/clean-menu-item-defn (2014-08-18) 1 commit
  (merged to 'next' on 2014-08-29 at 88c1a9d)
 + clean: use f(void) instead of f() to declare a pointer to a function without 
arguments


* rs/inline-compat-path-macros (2014-08-18) 1 commit
  (merged to 'next' on 2014-08-29 at 5705ad5)
 + turn path macros into inline function


* rs/refresh-beyond-symlink (2014-08-10) 1 commit
  (merged to 'next' on 2014-08-29 at 90a4a8b)
 + read-cache: check for leading symlinks when refreshing index

 "git add x" where x that used to be a directory has become a
 symbolic link to a directory misbehaved.


* sb/blame-msg-i18n (2014-08-12) 1 commit
  (merged to 'next' on 2014-08-29 at 5b33466)
 + builtin/blame.c: add translation to warning about failed revision walk


* sb/mailsplit-dead-code-removal (2014-08-13) 1 commit
  (merged to 'next' on 2014-08-29 at 4f9ca4f)
 + mailsplit.c: remove dead code


* sb/pl

git 2.1.0: make fails

2014-09-09 Thread Gerry Reno
Downloaded the git-2.1.0.tar.gz  tarball.

Cannot build git 2.1.0:

$ V=1 make prefix=/usr/local all
...
cc -o xdiff/xpatience.o -c -MF xdiff/.depend/xpatience.o.d -MQ 
xdiff/xpatience.o -MMD -MP  -g -O2 -Wall -I.
-DHAVE_ALLOCA_H  -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME 
-DSHA1_HEADER=''  -DNO_STRLCPY
-DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"'  xdiff/xpatience.c
cc -o xdiff/xhistogram.o -c -MF xdiff/.depend/xhistogram.o.d -MQ 
xdiff/xhistogram.o -MMD -MP  -g -O2 -Wall -I.
-DHAVE_ALLOCA_H  -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME 
-DSHA1_HEADER=''  -DNO_STRLCPY
-DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"'  xdiff/xhistogram.c
rm -f xdiff/lib.a && ar rcs xdiff/lib.a xdiff/xdiffi.o xdiff/xprepare.o 
xdiff/xutils.o xdiff/xemit.o xdiff/xmerge.o
xdiff/xpatience.o xdiff/xhistogram.o
cc  -g -O2 -Wall -I. -DHAVE_ALLOCA_H  -DHAVE_PATHS_H -DHAVE_DEV_TTY 
-DHAVE_CLOCK_GETTIME
-DSHA1_HEADER=''  -DNO_STRLCPY -DNO_MKSTEMPS 
-DSHELL_PATH='"/bin/sh"' -o git-credential-store  
credential-store.o libgit.a xdiff/lib.a  -lz  -lcrypto -lpthread -lrt
collect2: ld returned 1 exit status
make: *** [git-credential-store] Error 1


$ ls -l credential.o libgit.a xdiff/lib.a /usr/lib/libz.a 
/usr/lib/libcrypt.a /usr/lib/libpthread.a /usr/lib/librt.a
-rw-rw-r-- 1 greno greno   20332 Sep  9 16:21 credential.o
-rw-rw-r-- 1 greno greno 4658874 Sep  9 16:23 libgit.a
-rw-r--r-- 1 root  root24272 Jan  5  2007 /usr/lib/libcrypt.a
-rw-r--r-- 1 root  root   228586 Jan  5  2007 /usr/lib/libpthread.a
-rw-r--r-- 1 root  root52128 Jan  5  2007 /usr/lib/librt.a
-rwxr-xr-x 1 root  root92622 Jul 19  2006 /usr/lib/libz.a
-rw-rw-r-- 1 greno greno  122456 Sep  9 16:23 xdiff/lib.a




--
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-format: add append line-feed format specifier

2014-09-09 Thread Jeff King
On Tue, Sep 09, 2014 at 12:37:48PM -0700, Junio C Hamano wrote:

> Harry Jeffery  writes:
> 
> > On 09/09/14 20:15, Junio C Hamano wrote:
> >> Is this different from "%n%-d"?
> >>
> >
> > Yes. "%n%-d" will place the newline before the expansion, not after.
> 
> Maybe "%[-+ ]" needs to be rethought, instead of making things worse
> by turning it into "%[-_+ ]", as the next person who comes would
> want to add space after the expansion and would need to find yet
> another letter like you did with '_'.

Yeah, that was my thought on reading the initial patch, too. Why limit
ourselves to newlines and spaces. I'd much rather have full conditional
expansion, like "${foo:+prefix $foo suffix}" in the shell.

Something like the patch below might work, but I didn't test it very
thoroughly (and note the comments, which might need dealing with). Maybe
it would make a sensible base for Harry to build on if he wants to
pursue this.

With it, you can do:

  git log --format='%h %s%if(%d,%n  Decoration:%d)' origin

to get:

  85f0837 Start the post-2.1 cycle
Decoration: (origin/master, origin/HEAD, github/foo)
  f655651 Merge branch 'rs/strbuf-getcwd'
  51eeaea Merge branch 'ta/pretty-parse-config'
  4740891 Merge branch 'bc/archive-pax-header-mode'
  0e28161 Merge branch 'pr/remotes-in-hashmap'
  44ceb79 Merge branch 'jk/pretty-empty-format'
  56f214e Merge branch 'ta/config-set'
  e8e4ce7 Merge branch 'rs/init-no-duplicate-real-path'
  1d8a6f6 Merge branch 'mm/config-edit-global'
  c518279 Merge branch 'jc/reopen-lock-file'
  96db324 Merge git://github.com/git-l10n/git-po
Decoration: (origin/maint)

You could also make "%d" more flexible with it. We unconditionally
include the " (...)" wrapper when expanding it. But assuming we
introduced a "%D" that is _just_ the decoration names, you could do:

  %if(%D, (%D))

to get the same effect with much more flexibility.

---
diff --git a/pretty.c b/pretty.c
index fe34ddc..96cd512 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1398,6 +1398,42 @@ static size_t format_commit_item(struct strbuf *sb, /* 
in UTF-8 */
ADD_SP_BEFORE_NON_EMPTY
} magic = NO_MAGIC;
 
+   if (starts_with(placeholder, "if(")) {
+   const char *cond_beg, *cond_end;
+   const char *data_beg, *data_end;
+   char *buf;
+   struct strbuf scratch = STRBUF_INIT;
+
+   /* can't handle commas in conditions; allow backslash-escaping? 
*/
+   cond_beg = cond_end = placeholder + 3;
+   for (cond_end = cond_beg; *cond_end != ','; cond_end++)
+   if (!*cond_end)
+   return 0;
+
+   /* ditto for nested parentheses; backslash escaping? */
+   data_beg = cond_end + 1;
+   for (data_end = data_beg; *data_end != ')'; data_end++)
+   if (!*data_end)
+   return 0;
+
+   /*
+* Should teach formatters to return size only without
+* actually writing to scratch buffer?
+*/
+   buf = xmemdupz(cond_beg, cond_end - cond_beg);
+   strbuf_expand(&scratch, buf, format_commit_item, context);
+   free(buf);
+
+   if (scratch.len) {
+   buf = xmemdupz(data_beg, data_end - data_beg);
+   strbuf_expand(sb, buf, format_commit_item, context);
+   free(buf);
+   }
+   strbuf_release(&scratch);
+
+   return data_end - placeholder + 1;
+   }
+
switch (placeholder[0]) {
case '-':
magic = DEL_LF_BEFORE_EMPTY;
--
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] fsck: exit with non-zero status upon error from fsck_obj()

2014-09-09 Thread Junio C Hamano
From: Jeff King 
Date: Fri, 29 Aug 2014 16:31:46 -0400

Upon finding a corrupt loose object, we forgot to note the error to
signal it with the exit status of the entire process.

[jc: adjusted t1450 and added another test]

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

 * I think your fix is a right one that catches all the "we can
   parse minimally for the purpose of 'struct object' class system,
   but the object is semantically broken" cases, as fsck_obj() is
   where such a validation should all happen.

   I can haz a sign off?  Thanks.

 builtin/fsck.c  |  3 ++-
 t/t1450-fsck.sh | 30 +-
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1affdd5..8abe644 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -389,7 +389,8 @@ static void fsck_sha1_list(void)
unsigned char *sha1 = entry->sha1;
 
sha1_list.entry[i] = NULL;
-   fsck_sha1(sha1);
+   if (fsck_sha1(sha1))
+   errors_found |= ERROR_OBJECT;
free(entry);
}
sha1_list.nr = 0;
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 8c739c9..c8dff9c 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -101,7 +101,7 @@ test_expect_success 'email with embedded > is not okay' '
test_when_finished "remove_object $new" &&
git update-ref refs/heads/bogus "$new" &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
-   git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "error in commit $new" out
 '
@@ -113,7 +113,7 @@ test_expect_success 'missing < email delimiter is reported 
nicely' '
test_when_finished "remove_object $new" &&
git update-ref refs/heads/bogus "$new" &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
-   git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "error in commit $new.* - bad name" out
 '
@@ -125,7 +125,7 @@ test_expect_success 'missing email is reported nicely' '
test_when_finished "remove_object $new" &&
git update-ref refs/heads/bogus "$new" &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
-   git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "error in commit $new.* - missing email" out
 '
@@ -137,7 +137,7 @@ test_expect_success '> in name is reported' '
test_when_finished "remove_object $new" &&
git update-ref refs/heads/bogus "$new" &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
-   git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "error in commit $new" out
 '
@@ -151,11 +151,31 @@ test_expect_success 'integer overflow in timestamps is 
reported' '
test_when_finished "remove_object $new" &&
git update-ref refs/heads/bogus "$new" &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
-   git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "error in commit $new.*integer overflow" out
 '
 
+test_expect_success 'malformatted tree object' '
+   test_when_finished "git update-ref -d refs/tags/wrong" &&
+   test_when_finished "remove_object \$T" &&
+   T=$(
+   GIT_INDEX_FILE=test-index &&
+   export GIT_INDEX_FILE &&
+   rm -f test-index &&
+   >x &&
+   git add x &&
+   T=$(git write-tree) &&
+   (
+   git cat-file tree $T &&
+   git cat-file tree $T
+   ) |
+   git hash-object -w -t tree --stdin
+   ) &&
+   test_must_fail git fsck 2>out &&
+   grep "error in tree .*contains duplicate file entries" out
+'
+
 test_expect_success 'tag pointing to nonexistent' '
cat >invalid-tag <<-\EOF &&
object 
-- 
2.1.0-449-g93bbe5b

--
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] fsck: exit with non-zero status upon error from fsck_obj()

2014-09-09 Thread Jeff King
On Tue, Sep 09, 2014 at 03:03:33PM -0700, Junio C Hamano wrote:

> From: Jeff King 
> Date: Fri, 29 Aug 2014 16:31:46 -0400
> 
> Upon finding a corrupt loose object, we forgot to note the error to
> signal it with the exit status of the entire process.
> 
> [jc: adjusted t1450 and added another test]
> 
> Signed-off-by: Junio C Hamano 
> ---
> 
>  * I think your fix is a right one that catches all the "we can
>parse minimally for the purpose of 'struct object' class system,
>but the object is semantically broken" cases, as fsck_obj() is
>where such a validation should all happen.
> 
>I can haz a sign off?  Thanks.

Thanks, I think this is a step forward regardless of other conversation
on the exit code, as it is just harmonizing loose and packed object
behavior.

Signed-off-by: Jeff King 

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


Re: git fsck exit code?

2014-09-09 Thread Jeff King
On Mon, Sep 01, 2014 at 02:17:43PM -0400, David Turner wrote:

> > I don't think git fsck should return !0 in this case. Yes, it's an
> > inconsistency in the repo, but it's sometimes due to erroneous
> > conversions from another SCM or some other (non-standard) implementation
> > of the git client. I've seen things like this (and other inconsistencies
> > in repos, like wrong date formats, non-standard Author fields, unsorted
> > trees, zero-padded file modes and so on), and the thing is, owners of
> > public repos with these errors tend to avoid fixing it because it
> > changes the commit SHAs. If git fsck starts to return !0 on these
> > errors, it will always return error on that repo, which in practise
> > means that the error code is rendered useless. IMHO git fsck should only
> > return !0 on errors that can be fixed without changing the commit
> > history, for example missing or invalid objects.
> 
> We could have one exit code for errors which can be fixed without
> rewriting history, and another for errors that can't.  Or different
> command-line arguments to suppress errors of this sort.
> 
> In my case, I actually could fix the issue, because it was in a
> newly-created branch; I just rewrote the script that created the branch
> to be a little smarter.

I think there's obviously some disagreement or room for interpretation
on the exit code. Perhaps a better path forward is to have a
machine-readable output from fsck in the first place, and then we can
annotate each warning/error with extra information that a caller can
use.

As it is now, you have to scrape fsck's stderr stream to figure out what
happened (which is a thing I have done, and it felt dirty and wrong).

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


Re: [PATCH] pretty-format: add append line-feed format specifier

2014-09-09 Thread Harry Jeffery

On 09/09/14 22:45, Jeff King wrote:

Yeah, that was my thought on reading the initial patch, too. Why limit
ourselves to newlines and spaces. I'd much rather have full conditional
expansion, like "${foo:+prefix $foo suffix}" in the shell.

Something like the patch below might work, but I didn't test it very
thoroughly (and note the comments, which might need dealing with). Maybe
it would make a sensible base for Harry to build on if he wants to
pursue this.


I definitely prefer your more general solution to my 
bare-minimum-to-scratch-itch patch. I'd certainly be willing to take 
your patch and expand upon it (pun unintended) once Junio has weighed in 
on your suggestions.



You could also make "%d" more flexible with it. We unconditionally
include the " (...)" wrapper when expanding it. But assuming we
introduced a "%D" that is _just_ the decoration names, you could do:

   %if(%D, (%D))

to get the same effect with much more flexibility.


Regardless of what happens with the conditional expansion I think it 
would definitely be a useful addition to be able to print the decorators 
without the " (...)" wrapper. I think it's general enough that it'd 
warrant its own separate patch rather than being part of a patch series 
for the conditional expansion.

--
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: What's cooking in git.git (Sep 2014, #02; Tue, 9)

2014-09-09 Thread Jeff King
On Tue, Sep 09, 2014 at 02:26:22PM -0700, Junio C Hamano wrote:

> * jk/command-line-config-empty-string (2014-08-05) 1 commit
>   (merged to 'next' on 2014-08-29 at 74f04af)
>  + config: teach "git -c" to recognize an empty string
> 
>  "git -c section.var command" and "git -c section.var= command"
>  should pass the configuration differently (the former should be
>  a boolean true, the latter should be an empty string).

Hmm, there is something funny about the authorship on the sole commit in
that topic. It's too late to fix now (and I do not care too much), but
you may want to puzzle out what happened (stray --reset-author, weird
use of "git am"?).

> * jk/index-pack-threading-races (2014-08-29) 1 commit
>  - index-pack: fix race condition with duplicate bases
> 
>  When receiving an invalid pack stream that records the same object
>  twice, multiple threads got confused due to a race.  We should
>  reject or correct such a stream upon receiving, but that will be a
>  larger change.
> 
>  Will merge to 'next'.

I have been meaning to revisit this topic since the earlier discussion.
At the very least, we need to improve the error message for this case. I
think it's OK to merge this first patch, though, as anything else can
build on it.

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


Re: [PATCH] fsck: exit with non-zero status upon error from fsck_obj()

2014-09-09 Thread Junio C Hamano
Junio C Hamano  writes:

> From: Jeff King 
> Date: Fri, 29 Aug 2014 16:31:46 -0400
>
> Upon finding a corrupt loose object, we forgot to note the error to
> signal it with the exit status of the entire process.
>
> [jc: adjusted t1450 and added another test]

Spoke too soon.  If found another instance where we expected fsck to
notice a corruption.  We'd need to squash this in.

By the way, Jonathan, with dbedf8bf (t1450 (fsck): remove dangling
objects, 2010-09-06) you added a 'test_might_fail git fsck' to the
1450 test that catches an object corruption.  Do you remember if
there was some flakiness in this test that necessitated it, or is it
merely "I think this should fail, but it does not, and we may fix it
some day but I am not doing that in this patch?"  Assuming that it
is the latter, I am including an update to 1450 as well.

There is another test_might_fail that runs "git rev-list" in the
same test, but that is not part of the said patch and unrelated to
the current topic, so I didn't dig further; we may want to audit the
hits from "git grep test_might_fail t/" as a separate topic (hint,
hint).


 t/t1450-fsck.sh| 2 +-
 t/t4212-log-corrupt.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index c8dff9c..0de755c 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -69,7 +69,7 @@ test_expect_success 'object with bad sha1' '
git update-ref refs/heads/bogus $cmt &&
test_when_finished "git update-ref -d refs/heads/bogus" &&
 
-   test_might_fail git fsck 2>out &&
+   test_must_fail git fsck 2>out &&
cat out &&
grep "$sha.*corrupt" out
 '
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index 58b792b..67bd8ec 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -14,7 +14,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'fsck notices broken commit' '
-   git fsck 2>actual &&
+   test_must_fail git fsck 2>actual &&
test_i18ngrep invalid.author actual
 '
 
--
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] fsck: exit with non-zero status upon error from fsck_obj()

2014-09-09 Thread Jonathan Nieder
Junio C Hamano wrote:

> By the way, Jonathan, with dbedf8bf (t1450 (fsck): remove dangling
> objects, 2010-09-06) you added a 'test_might_fail git fsck' to the
> 1450 test that catches an object corruption.  Do you remember if
> there was some flakiness in this test that necessitated it, or is it
> merely "I think this should fail, but it does not, and we may fix it
> some day but I am not doing that in this patch?"

Thomas is the person to ask. :)  See v1.6.3-rc0~176^2~3 (Test fsck a
bit harder, 2009-02-19):

> + (git fsck 2>out; true) &&

which that cleanup tightened to test_might_fail.

But yes, I'm pretty sure it was for futureproofing, not for hiding
flakiness.  I think your patch does the right thing in changing it to
test_must_fail now that fsck exits nonzero.

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


Re: [PATCH] pretty-format: add append line-feed format specifier

2014-09-09 Thread Jeff King
On Tue, Sep 09, 2014 at 11:17:20PM +0100, Harry Jeffery wrote:

> I definitely prefer your more general solution to my
> bare-minimum-to-scratch-itch patch. I'd certainly be willing to take your
> patch and expand upon it (pun unintended) once Junio has weighed in on your
> suggestions.

Thanks. I am always happy to see contributors willing to pick up and run
with ideas.

It is probably out-of-scope for what you want, but while we are talking
about %d, it may be worth considering whether there is something simple
we can do to make formatting list-like items more flexible. E.g., even
with "%D", you are stuck with the format "foo, bar, baz" for multiple
decorations. Some kind of "%join(%d,; )" might work to produce "foo;
bar; baz" (or whatever you want). But that may also be crossing the line
into insanity, and we would be better to allow some Turing-complete
embedded language like lua. For that matter, conditionals might be
crossing that insanity line, too.

> Regardless of what happens with the conditional expansion I think it would
> definitely be a useful addition to be able to print the decorators without
> the " (...)" wrapper. I think it's general enough that it'd warrant its own
> separate patch rather than being part of a patch series for the conditional
> expansion.

Yeah, I agree it can be separate.

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


Re: [PATCH v4 06/32] lockfile: unlock file if lockfile permissions cannot be adjusted

2014-09-09 Thread Junio C Hamano
Michael Haggerty  writes:

> If the call to adjust_shared_perm() fails, lock_file returns -1, which
> to the caller looks like any other failure to lock the file.  So in
> this case, roll back the lockfile before returning so that the lock
> file is deleted immediately and the lockfile object is left in a
> predictable state (namely, unlocked).  Previously, the lockfile was
> retained until process cleanup in this situation.

... which would mean that other processes can grab a lock on the
same file a bit earlier. Is there any negative implication caused by
that difference?  I do not think of any but I could be missing
something.

>
> Signed-off-by: Michael Haggerty 
> ---
>  lockfile.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lockfile.c b/lockfile.c
> index b1c4ba0..d4f165d 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -153,6 +153,7 @@ static int lock_file(struct lock_file *lk, const char 
> *path, int flags)
>   int save_errno = errno;
>   error("cannot fix permission bits on %s",
> lk->filename);
> + rollback_lock_file(lk);
>   errno = save_errno;
>   return -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: [PATCH v4 07/32] hold_lock_file_for_append(): release lock on errors

2014-09-09 Thread Junio C Hamano
Michael Haggerty  writes:

> If there is an error copying the old contents to the lockfile, roll
> back the lockfile before exiting so that the lockfile is not held
> until process cleanup.

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


Re: [PATCH v4 02/32] api-lockfile: expand the documentation

2014-09-09 Thread Junio C Hamano
Michael Haggerty  writes:

> +LOCK_NODEREF::

I think I've seen this mentioned already but LOCK_NO_DEREF to avoid
"lock the node ref?" misreading?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git 2.1.0: make fails

2014-09-09 Thread Jeff King
On Tue, Sep 09, 2014 at 04:34:02PM -0400, Gerry Reno wrote:

> Downloaded the git-2.1.0.tar.gz  tarball.
> 
> Cannot build git 2.1.0:

Weird. It works fine for me on Debian unstable. What platform are you on?

> cc  -g -O2 -Wall -I. -DHAVE_ALLOCA_H  -DHAVE_PATHS_H -DHAVE_DEV_TTY 
> -DHAVE_CLOCK_GETTIME
> -DSHA1_HEADER=''  -DNO_STRLCPY -DNO_MKSTEMPS 
> -DSHELL_PATH='"/bin/sh"' -o git-credential-store  
> credential-store.o libgit.a xdiff/lib.a  -lz  -lcrypto -lpthread -lrt
> collect2: ld returned 1 exit status
> make: *** [git-credential-store] Error 1

So the linker failed, but it didn't actually tell us why. That's not
much to go on. Are you sure there was no other output on stderr?

I assume the reason it broke on git-credential-store is just because it
is the first thing we try to link. What does:

  make V=1 prefix=/usr/local git

output? Does it successfully link "git"?

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


Re:

2014-09-09 Thread David Aguilar
On Mon, Sep 08, 2014 at 04:36:49PM +0200, R. Klomp wrote:
> Ok great! That indeed fixed the issue.
> Although I still don't understand why it didn't work without -solo..
> since it didn't work when no instance of Beyond Compare was running as
> well.
> 
> There must be something not quite right in either Git or Beyond Compare.
> 
> On Mon, Sep 8, 2014 at 3:37 PM, Jim Naslund  wrote:
> >
> > On Sep 8, 2014 7:39 AM, "R. Klomp"  wrote:
> >>
> >> It seems like there's a bug involving git difftool's -d flag and Beyond
> >> Compare.
> >>
> >> When using the difftool Beyond Compare, git difftool <..> <..> -d
> >> immidiatly shuts down once the diff tree has been created. Beyond
> >> Compare successfully shows the files that differ.
> >> However, since git difftool doesn't wait for Beyond Compare to shut
> >> down, all temporary files are gone. Due to this it's impossible to
> >> view changes made inside files using the -d flag.
> >>
> >> I haven't tested if this issue also happens with other difftools.
> >>
> >> I'm using the latest versions of both Beyond Compare 3 (3.3.12, Pro
> >> Edition for Windows) and Git (1.9.4 for Windows).
> >>
> >>
> >> Thanks in advance for your help!
> >
> > I see the same behavior. For me it had something to do with the diff opening
> > in a new tab in an existing window. Adding -solo to difftool.cmd will make
> > beyond compare use a new window which fixes the issue for me.

Interesting. Would it be worth changing difftool to use -solo by default, or
are there any downsides to doing so?

Is -solo a new feature that only exists in new versions of beyond compare?
I would be okay saying that the user should use a fairly new version.

Can we rely on -solo being available on all platforms?
If so, I'd be okay with changing the default if there are no other downsides.

The --dir-diff feature is not the only one that needs this blocking behavior.
Does this issue also happen in the normal difftool mode without -d?
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v2 1/2] Makefile: add check-headers target

2014-09-09 Thread David Aguilar
On Mon, Sep 08, 2014 at 12:57:46PM -0700, Junio C Hamano wrote:
> Matthieu Moy  writes:
> 
> > Junio C Hamano  writes:
> >
> >> David Aguilar  writes:
> >>
> >>> +IFS='
> >>> +'
> >>> +git ls-files *.h ewah/*.h vcs-svn/*.h xdiff/*.h |
> >>
> >> Hmm.  This is only for true developers (not one who merely compiles
> >> after expanding a tarball), so "git ls-files" may probably be OK.
> >>
> >> But "/bin/ls" would be equally fine for that, no?
> >
> > Actually, since this is "| while read header", I have to wonder why this
> > is not written as
> >
> > for header in .h ewah/*.h vcs-svn/*.h xdiff/*.h
> > do
> > ...
> > done
> 
> Yes, that would be even better.  Then you wouldn't even have to
> worry about $IFS dance.

The original motivation was to avoid picking up the generated
common-cmds.h header file.

It was the N_() function that was messing it up.

Would it make sense to split out a separate patch that makes common-cmds.h
check-headers clean?
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 0/1] Use absolute paths of lockfiles

2014-09-09 Thread Yue Lin Ho
Hi Duy, Michael, Junio C Hamano:

Thanks for working on lock file issue.

Thank you!  Thank you~

^_^

Yue Lin Ho



--
View this message in context: 
http://git.661346.n2.nabble.com/What-s-cooking-in-git-git-Aug-2014-02-Fri-8-tp7616651p7618314.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html