Re: [PATCH] line-log: fix crash when --first-parent is used

2014-11-04 Thread Michael J Gruber
Tzvetan Mikov schrieb am 03.11.2014 um 23:09:
 On Mon, Nov 3, 2014 at 12:58 PM, Junio C Hamano gits...@pobox.com wrote:
 
 line-log tries to access all parents of a commit, but only the first
 parent has been loaded if --first-parent is specified, resulting
 in a crash.

 Limit the number of parents to one if --first-parent is specified.

 
 [...]
 Tzvetan, can we have a test for this one?
 
 Here is a sequence of commands to reproduce the crash:
 
 git init
 echo 1  a.txt  git add a.txt  git commit -m a
 git checkout -b branch
 echo 2  b.txt  git add b.txt  git commit -m b
 git checkout master
 git merge branch --no-ff -m merge
 git log --first-parent -L 1,1:a.txt
 
 I am not sure whether you have a requirement for a formal test
 included with the patch, but if there is one, I am happy to
 rework it.
 
 This is a very simple fix for the crash, though possibly
 a real comprehensive fix may be needed. I am not sure.
 The real cause is that with --first-parent some commit
 objects are never loaded from disk but may still be accessed.
 
 (sorry about my formatting - have to use GMail)
 
 regards,
 Tzvetan
 

A test would be nice, yes. Look in t/t4211-line-log.sh - you should be
OK if you add a test at the end which does the following:

git checkout parallel-change so that you have commits with more than 1
parent in the history

test git log -L1,1 b.c (it core dumps with current git)

That is, you don't have to set up merges and history, you can use the
existing one.

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


Kedves: Webmail Előfizető

2014-11-04 Thread Webmail E-mail frissítések



--
A postaláda mérete elérte határértéket kérjük kattintson ide

http://updattwesd.jigsy.com

és erősítsd meg az e-mail Köszönöm
rendszergazda

Powered by elektronikus levél rendszer.
Köszönjük az együttműködést!
Levél a Web team @ 2014
--
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/2] fetch, reflogs, and bare repositories

2014-11-04 Thread Jeff King
I ran into a rather confusing case of fetching into a bare repository
with reflogs turned on:

# make a repo with a two-component branch name
git init -q 
git commit -q --allow-empty -m one 
git branch foo/bar 

# now fetch it all into a bare repo with reflogs
git init -q --bare parent.git 
cd parent.git 
git config core.logallrefupdates true 
git fetch --prune .. +refs/*:refs/* 

# now replace the branch with one that has a d/f conflict
cd .. 
git branch -d foo/bar 
git branch foo 

# and fetch again
cd parent.git 
git fetch --prune .. +refs/*:refs/*

The final fetch fails and produces this output:

From ..
 x [deleted] (none) - foo/bar
error: Unable to append to ./logs/refs/heads/foo: Is a directory
 ! [new branch]  foo- foo  (unable to update local ref)

This turns out to be caused by two subtle bugs: one that makes git
fetch use reflogs inconsistently, and the other that causes some ref
updates to fail when reflogs are turned on and off. Details are in the
fixes themselves.

  [1/2]: fetch: load all default config at startup
  [2/2]: ignore stale directories when checking reflog existence

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


[PATCH 1/2] fetch: load all default config at startup

2014-11-04 Thread Jeff King
When we start the git-fetch program, we call git_config to
load all config, but our callback only processes the
fetch.prune option; we do not chain to git_default_config at
all.

This means that we may not load some core configuration
which will have an effect. For instance, we do not load
core.logAllRefUpdates, which impacts whether or not we
create reflogs in a bare repository.

Note that I said may above. It gets even more exciting. If
we have to transfer actual objects as part of the fetch,
then we call fetch_pack as part of the same process. That
function loads its own config, which does chain to
git_default_config, impacting global variables which are
used by the rest of fetch. But if the fetch is a pure ref
update (e.g., a new ref which is a copy of an old one), we
skip fetch_pack entirely. So we get inconsistent results
depending on whether or not we have actual objects to
transfer or not!

Let's just load the core config at the start of fetch, so we
know we have it (we may also load it again as part of
fetch_pack, but that's OK; it's designed to be idempotent).

Our tests check both cases (with and without a pack). We
also check similar behavior for push for good measure, but
it already works as expected.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/fetch.c   |  2 +-
 t/t5516-fetch-push.sh | 40 
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6ffd023..7b84d35 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -68,7 +68,7 @@ static int git_fetch_config(const char *k, const char *v, 
void *cb)
fetch_prune_config = git_config_bool(k, v);
return 0;
}
-   return 0;
+   return git_default_config(k, v, cb);
 }
 
 static int parse_refmap_arg(const struct option *opt, const char *arg, int 
unset)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 7c8a769..f4da20a 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -11,6 +11,7 @@ This test checks the following functionality:
 * hooks
 * --porcelain output format
 * hiderefs
+* reflogs
 '
 
 . ./test-lib.sh
@@ -1290,4 +1291,43 @@ test_expect_success 'pushing a tag pushes the tagged 
object' '
test_cmp expect actual
 '
 
+test_expect_success 'push into bare respects core.logallrefupdates' '
+   rm -rf dst.git 
+   git init --bare dst.git 
+   git -C dst.git config core.logallrefupdates true 
+
+   # double push to test both with and without
+   # the actual pack transfer
+   git push dst.git master:one 
+   echo one@{0} push expect 
+   git -C dst.git log -g --format=%gd %gs one actual 
+   test_cmp expect actual 
+
+   git push dst.git master:two 
+   echo two@{0} push expect 
+   git -C dst.git log -g --format=%gd %gs two actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'fetch into bare respects core.logallrefupdates' '
+   rm -rf dst.git 
+   git init --bare dst.git 
+   (
+   cd dst.git 
+   git config core.logallrefupdates true 
+
+   # as above, we double-fetch to test both
+   # with and without pack transfer
+   git fetch .. master:one 
+   echo one@{0} fetch .. master:one: storing head expect 
+   git log -g --format=%gd %gs one actual 
+   test_cmp expect actual 
+
+   git fetch .. master:two 
+   echo two@{0} fetch .. master:two: storing head expect 
+   git log -g --format=%gd %gs two actual 
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
2.1.2.596.g7379948

--
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] ignore stale directories when checking reflog existence

2014-11-04 Thread Jeff King
When we update a ref, we have two rules for whether or not
we actually update the reflog:

  1. If the reflog already exists, we will always append to
 it.

  2. If log_all_ref_updates is set, we will create a new
 reflog file if necessary.

We do the existence check by trying to open the reflog file,
either with or without O_CREAT (depending on log_all_ref_updates).
If it fails, then we check errno to see what happened.

If we were not using O_CREAT and we got ENOENT, the file
doesn't exist, and we return success (there isn't a reflog
already, and we were not told to make a new one).

If we get EISDIR, then there is likely a stale directory
that needs to be removed (e.g., there used to be foo/bar,
it was deleted, and the directory foo was left. Now we
want to create the ref foo). If O_CREAT is set, then we
catch this case, try to remove the directory, and retry our
open. So far so good.

But if we get EISDIR and O_CREAT is not set, then we treat
this as any other error, which is not right. Like ENOENT,
EISDIR is an indication that we do not have a reflog, and we
should silently return success (we were not told to create
it). Instead, the current code reports this as an error, and
we fail to update the ref at all.

Note that this is relatively unlikely to happen, as you
would have to have had reflogs turned on, and then later
turned them off (it could also happen due to a bug in fetch,
but that was fixed in the previous commit). However, it's
quite easy to fix: we just need to treat EISDIR like ENOENT
for the non-O_CREAT case, and silently return (note that
this early return means we can also simplify the O_CREAT
case).

Our new tests cover both cases (O_CREAT and non-O_CREAT).
The first one already worked, of course.

Signed-off-by: Jeff King p...@peff.net
---
Note that rs/ref-transaction-reflog tweaks this area a bit; it
introduces a separate reflog_exists() check that runs before we even get
to the first open(). Which would make the tests I introduce here run
correctly even without this patch. However, the code I am changing does
still exist in that series, and I think would handle cases where
logallrefupdates was turned on, but the ref was outside of refs/heads/.
So even with that series, we do still need this fix (and of course there
is the obvious benefit that we can probably merge this fix sooner than
that series).

 refs.c|  4 ++--
 t/t1410-reflog.sh | 34 ++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 0368ed4..5ff457e 100644
--- a/refs.c
+++ b/refs.c
@@ -2962,10 +2962,10 @@ int log_ref_setup(const char *refname, char *logfile, 
int bufsize)
 
logfd = open(logfile, oflags, 0666);
if (logfd  0) {
-   if (!(oflags  O_CREAT)  errno == ENOENT)
+   if (!(oflags  O_CREAT)  (errno == ENOENT || errno == EISDIR))
return 0;
 
-   if ((oflags  O_CREAT)  errno == EISDIR) {
+   if (errno == EISDIR) {
if (remove_empty_directories(logfile)) {
int save_errno = errno;
error(There are still logs under '%s',
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 8cab06f..485f1a7 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -253,4 +253,38 @@ test_expect_success 'checkout should not delete log for 
packed ref' '
test $(git reflog master | wc -l) = 4
 '
 
+test_expect_success 'stale dirs do not cause d/f conflicts (reflogs on)' '
+   test_when_finished git branch -d a || git branch -d a/b 
+
+   git branch a/b 
+   echo a/b@{0} branch: Created from foo expect 
+   git log -g --format=%gd %gs a/b actual 
+   test_cmp expect actual 
+   git branch -d a/b 
+
+   # now logs/refs/heads/a is a stale directory, but
+   # we should move it out of the way to create a reflog
+   git branch a 
+   echo a@{0} branch: Created from foo expect 
+   git log -g --format=%gd %gs a actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
+   test_when_finished git branch -d a || git branch -d a/b 
+
+   git branch a/b 
+   echo a/b@{0} branch: Created from foo expect 
+   git log -g --format=%gd %gs a/b actual 
+   test_cmp expect actual 
+   git branch -d a/b 
+
+   # same as before, but we only create a reflog for a if
+   # it already exists, which it does not
+   git -c core.logallrefupdates=false branch a 
+   : expect 
+   git log -g --format=%gd %gs a actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
2.1.2.596.g7379948
--
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 in log for path in case of identical commit

2014-11-04 Thread Phil Hord
On Fri, Oct 31, 2014 at 4:40 AM, Alexandre Garnier zigarn+...@gmail.com wrote:
 When merging 2 branches with the same modifications on the both sides,
 depending the merge side, one branch disappear from the file history.

 To be more clear, there is a script in attachment to reproduce, but
 here is the result :
 $ git log --graph --oneline --all --decorate --name-status
 *   63c807f (HEAD, master) Merge branch 'branch' into 'master'
 |\
 | * 5dc8785 (branch) Change line 15 on branch
 | | M   file
 | * d9cd3ce Change line 25 on branch
 | | M   file
 * | 7220d52 Change line 15 on master
 |/
 |   M   file
 * 7566672 Initial commit
   A file

 $ git log --graph --oneline --all --decorate --name-status -- file
 * 5dc8785 (branch) Change line 15 on branch
 | M file
 * d9cd3ce Change line 25 on branch
 | M file
 * 7566672 Initial commit
   A file

 = The commit 7220d52 modified the file but is not shown in file
 history anymore.
 The expected result would be:
 * 5dc8785 (branch) Change line 15 on branch
 | M file
 * d9cd3ce Change line 25 on branch
 | M file
 | * 7220d52 Change line 15 on master
 |/
 |   M   file
 * 7566672 Initial commit
   A file

 The order between the 2 commits on the branch is not important.
 If you do a 'cherry-pick 7220d52' or a 'merge --squash master' instead
 of applying the same modification for commit 5dc8785, you get the same
 result (cherry-picking was my initial use-case).
 If you do not create the commit d9cd3ce, then the file history show all 
 commits.
 If you merge 'master' into 'branch', then the file history show all commits.

This last line was confusing to me.  But I think you've misinterpreted
the results a bit.  There is no difference between merge master into
branch and merge branch into master in this case.  The real reason
the extra commit is shown in the former case is that you used
'--all' (include all refs as commandline arguments) and the commit
which was being omitted was directly referenced by a ref, 'master'.

When I remove the --all from your test script, I get consistent logs
for the two merges.

Maybe this has misled your other tests as well.  Read the History
Simplification section of git help log.
--
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 reflog --date

2014-11-04 Thread Phil Hord
On Tue, Oct 21, 2014 at 1:24 PM, Junio C Hamano gits...@pobox.com wrote:
 John Tapsell johnf...@gmail.com writes:

 Hi all,

   Could we add a default to --date so that:

 git reflog --date

 just works?  (Currently you need to do:   git reflog --date=iso)  It
 should probably obey the default in log.date?

 Hmph.  --date=style is not the way to choose between timed and
 counted output in the first place, though.

Of course not.  I always use --relative-date for this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: squash commits deep down in history

2014-11-04 Thread Phil Hord
On Thu, Oct 23, 2014 at 8:34 AM, Henning Moll newssc...@gmx.de wrote:
 Hi,

 i need to squash several commits into a single one in a automated way. I know 
 that there is interactive rebase, which can also be automated using 
 GIT_SEQUENCE_EDITOR. Unfortunately my history is very large and i need to 
 squash deep down in the history several times. So using interactive rebase 
 seems not to be the right tool.

 I wonder if i can solve this task using filter-branch? I have a file that 
 list the SHA1s for each squash operation per line. All SHA1s of a line are in 
 chronological order (youngest to oldest), or in other words: the first SHA1 
 is the child of the second, and so on.

 | ab4423e 3432343 3234252
 | 2324342 5232343
 | ...

 Lets say there are N lines in that file. Each line means: squash all SHA1s of 
 this line into the first (or last) SHA1 of this line.

I've often felt there should be some simple commands to do these kinds
of edits.  For example, it is easy to amend HEAD, but an
order-of-magnitude more difficult to amend HEAD^.   I imagine commands
like this:

   git rebase --reword HEAD^
   git rebase --edit some-old-commit
   git rebase --squash ab4423e 3432343 3234252

But each time I think of implementing one of these I am daunted by the
many exceptional cases.


 Performing this task with rebase would require N rewritings of the history. 
 So e.g. HEAD (but many others too) would be rewritten N times even if it is 
 not directly part of a line. My thinking is, that a filter-branch can do this 
 in a single rewrite and therefore would be much more performant.

 How can i solve this? Any ideas?

I know you solved this already with filter-branch, but that seems like
a complicated solution to me.  I think the --interactive rebase would
have been useful for you.  You should keep in mind that you do not
need to repeat the command multiple times for your multiple squashes.
For example, if your to-do list looks like this simple example:

pick 000
pick ab4423e
pick 3432343
pick 3234252
pick 001
pick 002
pick 2324342
pick 5232343
pick 003

I think you could get the desired effect by changing it to this:

pick 000
pick ab4423e
squash 3432343
squash 3234252
pick 001
pick 002
pick 2324342
squash 5232343
pick 003

And then running that all in one interactive rebase.  Does that make sense?

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


Re: [PATCH 0/2] fetch, reflogs, and bare repositories

2014-11-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I ran into a rather confusing case of fetching into a bare repository
 with reflogs turned on:
 ...
 This turns out to be caused by two subtle bugs: one that makes git
 fetch use reflogs inconsistently, and the other that causes some ref
 updates to fail when reflogs are turned on and off. Details are in the
 fixes themselves.

   [1/2]: fetch: load all default config at startup
   [2/2]: ignore stale directories when checking reflog existence

Must have been painful and fun at the same time to diagnose these
;-)  Both patches looked very sensible.

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] diff-highlight: exit when a pipe is broken

2014-11-04 Thread John Szakmeister
On Sat, Nov 1, 2014 at 12:04 AM, Jeff King p...@peff.net wrote:
 On Fri, Oct 31, 2014 at 07:04:04AM -0400, John Szakmeister wrote:

 While using diff-highlight with other tools, I have discovered that Python
 ignores SIGPIPE by default.  Unfortunately, this also means that tools
 attempting to launch a pager under Python--and don't realize this is
 happening--means that the subprocess inherits this setting.  In this case, it
 means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
 with those broken scripts by explicitly setting up a SIGPIPE handler and 
 exiting
 the process.

 My first thought was that this should be handled already by 7559a1b
 (unblock and unignore SIGPIPE, 2014-09-18), but after re-reading your
 message, it sounds like you are using diff-highlight with non-git
 programs?

Yes, that's correct.  It's useful, so with a few tools that use diffs,
I like to run the output through diff-highlight.

 +# Some scripts may not realize that SIGPIPE is being ignored when launching 
 the
 +# pager--for instance scripts written in Python.  Setting $SIG{PIPE} = 
 'DEFAULT'
 +# doesn't work in these instances, so we install our own signal handler 
 instead.

 Why doesn't $SIG{PIPE} = 'DEFAULT' work? I did some limited testing and
 it seemed to work fine for me. Though I simulated the condition with:

   (
 trap '' PIPE
 perl -e '$|=1; print foo\n; print STDERR bar\n'
   ) | true

 which should not ever print bar.

Hehe, now that I see you right it out, I realize my mistake: I didn't
capitalize 'default'.  Trying it out again, it does appear that does
the trick.

[snip]
 Can we exit 141 here? If we are part of a pipeline to a pager, it should
 not matter either way, but I'd rather not lose the exit code if we can
 avoid it (in case of running the script standalone).

 +$SIG{PIPE} = \pipe_handler;

 A minor nit, but would:

   $SIG{PIPE} = sub { ... };

 be nicer to avoid polluting the function namespace?

Sorry, my Perl-fu is kind of low these days.  I used to use it all the
time but switched away from it quite a while ago.  Given that
'DEFAULT' does the trick, I'll just re-roll my patch to use that.
Does that sound fair?

-John

PS  Sorry for the late response, I've been traveling.
--
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] diff-highlight: exit when a pipe is broken

2014-11-04 Thread John Szakmeister
While using diff-highlight with other tools, I have discovered that Python
ignores SIGPIPE by default.  Unfortunately, this also means that tools
attempting to launch a pager under Python--and don't realize this is
happening--means that the subprocess inherits this setting.  In this case, it
means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
with those broken scripts by restoring the default SIGPIPE handler.

Signed-off-by: John Szakmeister j...@szakmeister.net
---
Incorporates feedback from Jeff King and now we just restore the default signal
handler using the correct case of 'DEFAULT'.

 contrib/diff-highlight/diff-highlight | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index c4404d4..69a652e 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -14,6 +14,10 @@ my @removed;
 my @added;
 my $in_hunk;
 
+# Some scripts may not realize that SIGPIPE is being ignored when launching the
+# pager--for instance scripts written in Python.
+$SIG{PIPE} = 'DEFAULT';
+
 while () {
if (!$in_hunk) {
print;
-- 
2.0.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 v2] diff-highlight: exit when a pipe is broken

2014-11-04 Thread Jeff King
On Tue, Nov 04, 2014 at 03:01:12PM -0500, John Szakmeister wrote:

 While using diff-highlight with other tools, I have discovered that Python
 ignores SIGPIPE by default.  Unfortunately, this also means that tools
 attempting to launch a pager under Python--and don't realize this is
 happening--means that the subprocess inherits this setting.  In this case, it
 means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
 with those broken scripts by restoring the default SIGPIPE handler.
 
 Signed-off-by: John Szakmeister j...@szakmeister.net
 ---
 Incorporates feedback from Jeff King and now we just restore the default 
 signal
 handler using the correct case of 'DEFAULT'.

Thanks, this version looks much simpler. :)

Acked-by: Jeff King p...@peff.net

-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] submodule: Fix documentation of update subcommand

2014-11-04 Thread Jens Lehmann

Am 04.11.2014 um 00:08 schrieb Junio C Hamano:

Michal Sojka sojk...@fel.cvut.cz writes:

Or something perhaps?  Or the detailed description of
submodule.$name.update should be dropped from here and refer the
reader to config.txt instead?


I guess you mean gitmodules.txt.


Actually, I do mean the configuration.  .gitmodules is just a
template to help the user populate .git/config, and the latter of
which should be the sole source of truth.  This is an important
principle, and it becomes even more important once we start talking
about security sensitive possiblity like allowing !command as the
value.


Not quite. You're definitely right about the !command value for
the 'update' setting; this should never be taken from .gitmodules
but only from .git/config. But apart from that following this
principle would hurt submodule users a lot. The only thing that
should be set in stone in .git/config is the 'url' setting,
because an older url might not even exist anmore. But e.g. the
'branch' setting must be taken from .gitmodules. Otherwise we
could not change it on a per-superproject-branch basis. And if
the 'path' setting would only be taken from .git/config instead
of .gitmodules, we wouldn't even be able to rename submodules
(which is exactly what this setting was added for in the first
place). The same applies to 'ignore' and 'fetch'.

So I believe that gitmodules.txt should describe all ćonfig
options that can be provided by upstream (and e.g. mention that
the 'url' and 'update' values are copied into .git/config on
init), while all settings that can be overridden locally should
be documented in config.txt (which will be a subset of those
documented in gitmodules.txt).


The `!command` form is not documented in gitmodules.txt. Maybe it would
be best to fully document .update in gitmodules.txt and just refer to
there. Having documentation at two places seems to be confusing not only
for users, but also for those who send patches :)

I'm no longer able to formulate my proposal properly as a patch tonight,
but if needed I'll try it later.


That is fine.  People have lived with the current text for more than
two years without problems, so we are obviously not in a hurry.


Yup.
--
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] line-log: fix crash when --first-parent is used

2014-11-04 Thread Tzvetan Mikov
line-log tries to access all parents of a commit, but only the first
parent has been loaded if --first-parent is specified, resulting
in a crash.

Limit the number of parents to one if --first-parent is specified.

Reported-by: Eric N. Vander Weele eri...@gmail.com
Signed-off-by: Tzvetan Mikov tmi...@gmail.com
---
 PATCH v2: Add a test case (thanks Michael J Gruber)

 line-log.c  | 3 +++
 t/t4211-line-log.sh | 5 +
 2 files changed, 8 insertions(+)

diff --git a/line-log.c b/line-log.c
index 1008e72..86e7274 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info 
*rev, struct commit *comm
int i;
int nparents = commit_list_count(commit-parents);
 
+   if (nparents  1  rev-first_parent_only)
+   nparents = 1;
+
diffqueues = xmalloc(nparents * sizeof(*diffqueues));
cand = xmalloc(nparents * sizeof(*cand));
parents = xmalloc(nparents * sizeof(*parents));
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 7369d3c..0901b30 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -94,4 +94,9 @@ test_expect_success '-L ,Y (Y == nlines + 2)' '
test_must_fail git log -L ,$n:b.c
 '
 
+test_expect_success '-L with --first-parent and a merge' '
+   git checkout parallel-change 
+   git log --first-parent -L 1,1:b.c
+'
+
 test_done
-- 
1.9.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 v2] diff-highlight: exit when a pipe is broken

2014-11-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Nov 04, 2014 at 03:01:12PM -0500, John Szakmeister wrote:

 While using diff-highlight with other tools, I have discovered that Python
 ignores SIGPIPE by default.  Unfortunately, this also means that tools
 attempting to launch a pager under Python--and don't realize this is
 happening--means that the subprocess inherits this setting.  In this case, it
 means diff-highlight will be launched with SIGPIPE being ignored.  Let's work
 with those broken scripts by restoring the default SIGPIPE handler.
 
 Signed-off-by: John Szakmeister j...@szakmeister.net
 ---
 Incorporates feedback from Jeff King and now we just restore the default 
 signal
 handler using the correct case of 'DEFAULT'.

 Thanks, this version looks much simpler. :)

 Acked-by: Jeff King p...@peff.net

Thanks, both.  The patch looks good.  Will queue.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] submodule: Fix documentation of update subcommand

2014-11-04 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 So I believe that gitmodules.txt should describe all ćonfig
 options that can be provided by upstream (and e.g. mention that
 the 'url' and 'update' values are copied into .git/config on
 init), while all settings that can be overridden locally should
 be documented in config.txt (which will be a subset of those
 documented in gitmodules.txt).

Right; 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] Fix some misspellings

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


Re: [PATCH v2] line-log: fix crash when --first-parent is used

2014-11-04 Thread Junio C Hamano
Tzvetan Mikov tmi...@gmail.com writes:

 line-log tries to access all parents of a commit, but only the first
 parent has been loaded if --first-parent is specified, resulting
 in a crash.

 Limit the number of parents to one if --first-parent is specified.

 Reported-by: Eric N. Vander Weele eri...@gmail.com
 Signed-off-by: Tzvetan Mikov tmi...@gmail.com
 ---
  PATCH v2: Add a test case (thanks Michael J Gruber)

Thanks, both.  The patch looks good (modulo the indentation of
nparents assignment, which I'll locally fix up).

Will queue.

  line-log.c  | 3 +++
  t/t4211-line-log.sh | 5 +
  2 files changed, 8 insertions(+)

 diff --git a/line-log.c b/line-log.c
 index 1008e72..86e7274 100644
 --- a/line-log.c
 +++ b/line-log.c
 @@ -1141,6 +1141,9 @@ static int process_ranges_merge_commit(struct rev_info 
 *rev, struct commit *comm
   int i;
   int nparents = commit_list_count(commit-parents);
  
 + if (nparents  1  rev-first_parent_only)
 + nparents = 1;
 +
   diffqueues = xmalloc(nparents * sizeof(*diffqueues));
   cand = xmalloc(nparents * sizeof(*cand));
   parents = xmalloc(nparents * sizeof(*parents));
 diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
 index 7369d3c..0901b30 100755
 --- a/t/t4211-line-log.sh
 +++ b/t/t4211-line-log.sh
 @@ -94,4 +94,9 @@ test_expect_success '-L ,Y (Y == nlines + 2)' '
   test_must_fail git log -L ,$n:b.c
  '
  
 +test_expect_success '-L with --first-parent and a merge' '
 + git checkout parallel-change 
 + git log --first-parent -L 1,1:b.c
 +'
 +
  test_done
--
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] line-log: fix crash when --first-parent is used

2014-11-04 Thread Tzvetan Mikov
On Tue, Nov 4, 2014 at 1:24 PM, Junio C Hamano gits...@pobox.com wrote:
 Thanks, both.  The patch looks good (modulo the indentation of
 nparents assignment, which I'll locally fix up).

 Will queue.

Awesome, thanks!
(I can't believe I missed that tab. Well, at least one of my two
lines was correct :-) )

regards,
Tzvetan
--
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] use child_process_init() to initialize struct child_process variables

2014-11-04 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Jeff King p...@peff.net writes:

 I peeked at libgit2 and I think it does not support bundles at all yet,
 so that is safe. Grepping for bundle in dulwich turns up no hits,
 either.

 Looks like JGit does support them. I did a very brief test, and it seems
 to silently ignore a HEAD ref that has the NUL (I guess maybe it just
 rejects it as a malformed refname).

 We could make JGit happier either by:

   1. Only including the symref magic in ambiguous cases, so that regular
  ones Just Work as usual.

   2. Including two lines, like:

 $sha1 HEAD\0symref=refs/heads/master
  $sha1 HEAD

  which JGit does the right thing with (and git.git seems to, as
  well).

 Sounds sensible, even though it looks ugly X-.

I have a mild preference for a syntax that is more similar to the
on-wire protocol, so that connect.c::parse_feature_value() can be
reused to parse it and possibly annotate_refs_with_symref_info() can
also be reused by calling it from transport.c::get_refs_from_bundle().

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


Re: [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument

2014-11-04 Thread Stefan Beller
On Mon, Nov 3, 2014 at 11:12 AM, Ronnie Sahlberg sahlb...@google.com wrote:
 This adds support to send-pack to to negotiate and use atomic pushes

/s/to to/to/

 iff the server supports it. Atomic pushes are activated by a new command
 line flag --atomic-push.

 In order to do this we also need to change the semantics for send_pack()
 slightly. The existing send_pack() function actually don't sent all the
 refs back to the server when multiple refs are involved, for example
 when using --all. Several of the failure modes for pushes can already be
 detected locally in the send_pack client based on the information from the
 initial server side list of all the refs as generated by receive-pack.
 Any such refs that we thus know would fail to push are thus pruned from
 the list of refs we send to the server to update.

 For atomic pushes, we have to deal thus with both failures that are detected
 locally as well as failures that are reported back from the server. In order
 to do so we treat all local failures as push failures too.

 We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
 flag all refs that we would normally have tried to push to the server
 but we did not due to local failures. This is to improve the error message
 back to the end user to flag that these refs failed to update since the
 atomic push operation failed.

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  Documentation/git-send-pack.txt |  7 ++-
  builtin/send-pack.c |  6 +-
  remote.h|  3 ++-
  send-pack.c | 39 ++-
  send-pack.h |  1 +
  transport.c |  4 
  6 files changed, 52 insertions(+), 8 deletions(-)

 diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
 index 2a0de42..9296587 100644
 --- a/Documentation/git-send-pack.txt
 +++ b/Documentation/git-send-pack.txt
 @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
 repository
  SYNOPSIS
  
  [verse]
 -'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
 [ref...]
 +'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
 [host:]directory [ref...]

  DESCRIPTION
  ---
 @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a 
 flush packet.
 Send a thin pack, which records objects in deltified form based
 on objects not included in the pack to reduce network traffic.

 +--atomic-push::
 +   Use an atomic transaction for updating the refs. If any of the refs
 +   fails to update then the entire push will fail without changing any
 +   refs.
 +
  host::
 A remote host to house the repository.  When this
 part is specified, 'git-receive-pack' is invoked via
 diff --git a/builtin/send-pack.c b/builtin/send-pack.c
 index b564a77..93cb17c 100644
 --- a/builtin/send-pack.c
 +++ b/builtin/send-pack.c
 @@ -13,7 +13,7 @@
  #include sha1-array.h

  static const char send_pack_usage[] =
 -git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
 [ref...]\n
 +git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
 [host:]directory [ref...]\n
--all and explicit ref specification are mutually exclusive.;

  static struct send_pack_args args;
 @@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const 
 char *prefix)
 args.use_thin_pack = 1;
 continue;
 }
 +   if (!strcmp(arg, --atomic-push)) {
 +   args.use_atomic_push = 1;
 +   continue;
 +   }
 if (!strcmp(arg, --stateless-rpc)) {
 args.stateless_rpc = 1;
 continue;
 diff --git a/remote.h b/remote.h
 index 8b62efd..f346524 100644
 --- a/remote.h
 +++ b/remote.h
 @@ -115,7 +115,8 @@ struct ref {
 REF_STATUS_REJECT_SHALLOW,
 REF_STATUS_UPTODATE,
 REF_STATUS_REMOTE_REJECT,
 -   REF_STATUS_EXPECTING_REPORT
 +   REF_STATUS_EXPECTING_REPORT,
 +   REF_STATUS_ATOMIC_PUSH_FAILED
 } status;
 char *remote_status;
 struct ref *peer_ref; /* when renaming */
 diff --git a/send-pack.c b/send-pack.c
 index 1ccc84c..08602a8 100644
 --- a/send-pack.c
 +++ b/send-pack.c
 @@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf 
 *sb)
 for_each_commit_graft(advertise_shallow_grafts_cb, sb);
  }

 -static int ref_update_to_be_sent(const struct ref *ref, const struct 
 send_pack_args 

[PATCH] gitignore.txt: fix spelling of backslash

2014-11-04 Thread Ben North
Signed-off-by: Ben North b...@redfrontdoor.org
---
 Documentation/gitignore.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 8734c15..09e82c3 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -77,7 +77,7 @@ PATTERN FORMAT
Put a backslash (`\`) in front of the first hash for patterns
that begin with a hash.

- - Trailing spaces are ignored unless they are quoted with backlash
+ - Trailing spaces are ignored unless they are quoted with backslash
(`\`).

  - An optional prefix `!` which negates the pattern; any
-- 
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument

2014-11-04 Thread Ronnie Sahlberg
Fixed. Thanks


On Tue, Nov 4, 2014 at 2:17 PM, Stefan Beller sbel...@google.com wrote:
 On Mon, Nov 3, 2014 at 11:12 AM, Ronnie Sahlberg sahlb...@google.com wrote:
 This adds support to send-pack to to negotiate and use atomic pushes

 /s/to to/to/

 iff the server supports it. Atomic pushes are activated by a new command
 line flag --atomic-push.

 In order to do this we also need to change the semantics for send_pack()
 slightly. The existing send_pack() function actually don't sent all the
 refs back to the server when multiple refs are involved, for example
 when using --all. Several of the failure modes for pushes can already be
 detected locally in the send_pack client based on the information from the
 initial server side list of all the refs as generated by receive-pack.
 Any such refs that we thus know would fail to push are thus pruned from
 the list of refs we send to the server to update.

 For atomic pushes, we have to deal thus with both failures that are detected
 locally as well as failures that are reported back from the server. In order
 to do so we treat all local failures as push failures too.

 We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
 flag all refs that we would normally have tried to push to the server
 but we did not due to local failures. This is to improve the error message
 back to the end user to flag that these refs failed to update since the
 atomic push operation failed.

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  Documentation/git-send-pack.txt |  7 ++-
  builtin/send-pack.c |  6 +-
  remote.h|  3 ++-
  send-pack.c | 39 ++-
  send-pack.h |  1 +
  transport.c |  4 
  6 files changed, 52 insertions(+), 8 deletions(-)

 diff --git a/Documentation/git-send-pack.txt 
 b/Documentation/git-send-pack.txt
 index 2a0de42..9296587 100644
 --- a/Documentation/git-send-pack.txt
 +++ b/Documentation/git-send-pack.txt
 @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
 repository
  SYNOPSIS
  
  [verse]
 -'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] 
 [host:]directory [ref...]
 +'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
 [host:]directory [ref...]

  DESCRIPTION
  ---
 @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a 
 flush packet.
 Send a thin pack, which records objects in deltified form based
 on objects not included in the pack to reduce network traffic.

 +--atomic-push::
 +   Use an atomic transaction for updating the refs. If any of the refs
 +   fails to update then the entire push will fail without changing any
 +   refs.
 +
  host::
 A remote host to house the repository.  When this
 part is specified, 'git-receive-pack' is invoked via
 diff --git a/builtin/send-pack.c b/builtin/send-pack.c
 index b564a77..93cb17c 100644
 --- a/builtin/send-pack.c
 +++ b/builtin/send-pack.c
 @@ -13,7 +13,7 @@
  #include sha1-array.h

  static const char send_pack_usage[] =
 -git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] 
 [host:]directory [ref...]\n
 +git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
 [host:]directory [ref...]\n
--all and explicit ref specification are mutually exclusive.;

  static struct send_pack_args args;
 @@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const 
 char *prefix)
 args.use_thin_pack = 1;
 continue;
 }
 +   if (!strcmp(arg, --atomic-push)) {
 +   args.use_atomic_push = 1;
 +   continue;
 +   }
 if (!strcmp(arg, --stateless-rpc)) {
 args.stateless_rpc = 1;
 continue;
 diff --git a/remote.h b/remote.h
 index 8b62efd..f346524 100644
 --- a/remote.h
 +++ b/remote.h
 @@ -115,7 +115,8 @@ struct ref {
 REF_STATUS_REJECT_SHALLOW,
 REF_STATUS_UPTODATE,
 REF_STATUS_REMOTE_REJECT,
 -   REF_STATUS_EXPECTING_REPORT
 +   REF_STATUS_EXPECTING_REPORT,
 +   REF_STATUS_ATOMIC_PUSH_FAILED
 } status;
 char *remote_status;
 struct ref *peer_ref; /* when renaming */
 diff --git a/send-pack.c b/send-pack.c
 index 1ccc84c..08602a8 100644
 --- a/send-pack.c
 +++ b/send-pack.c
 @@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf 
 *sb)
 for_each_commit_graft(advertise_shallow_grafts_cb, sb);
 

Re: [PATCH] gitignore.txt: fix spelling of backslash

2014-11-04 Thread Junio C Hamano
Ben North b...@redfrontdoor.org writes:

 Signed-off-by: Ben North b...@redfrontdoor.org
 ---
  Documentation/gitignore.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
 index 8734c15..09e82c3 100644
 --- a/Documentation/gitignore.txt
 +++ b/Documentation/gitignore.txt
 @@ -77,7 +77,7 @@ PATTERN FORMAT
 Put a backslash (`\`) in front of the first hash for patterns
 that begin with a hash.

 - - Trailing spaces are ignored unless they are quoted with backlash
 + - Trailing spaces are ignored unless they are quoted with backslash
 (`\`).

   - An optional prefix `!` which negates the pattern; any

:-0  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] use child_process_init() to initialize struct child_process variables

2014-11-04 Thread Jeff King
On Tue, Nov 04, 2014 at 01:56:15PM -0800, Junio C Hamano wrote:

2. Including two lines, like:
 
  $sha1 HEAD\0symref=refs/heads/master
  $sha1 HEAD
 
   which JGit does the right thing with (and git.git seems to, as
   well).
 
  Sounds sensible, even though it looks ugly X-.
 
 I have a mild preference for a syntax that is more similar to the
 on-wire protocol, so that connect.c::parse_feature_value() can be
 reused to parse it and possibly annotate_refs_with_symref_info() can
 also be reused by calling it from transport.c::get_refs_from_bundle().

Yeah, what I wrote above was the simplest thing that could work, and
does not need to be the final form.  I know that you already know what
I'm about to describe below, Junio, but I want to expand on the
situation for the benefit of onlookers (and potential implementers like
Philip).

The online protocol is hampered by the if you see something after a
NUL, it is a capabilities string, and you must throw out the previous
capabilities string and replace it with this one historical rule. And
that's why we cannot do:

  $sha1 refs/heads/master\0thin-pack side-band etc
  $sha1 HEAD\0symref=refs/heads/master

as it would throw out thin-pack, side-band, etc. Instead we do it
more like:

  $sha1 refs/heads/master\0thin-pack side-band etc symref=HEAD:refs/heads/master
  $sha1 HEAD

to shove _all_ of the symref mappings into the capability string, rather
than letting them ride along with their respective refs. The downside is
that we are bounded in the number of symref mappings we can send (by the
maximum length for a single pkt-line), and therefore send only the value
of HEAD.

The bundle code is not bound by this historical legacy, and could do it
in a different (and more efficient and flexible) way. But it is probably
saner to just keep them identical. It makes the code simpler, and having
bundle as the only transport which has the extra flexibility does not
really buy us much (and probably just invites confusion).

-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] use child_process_init() to initialize struct child_process variables

2014-11-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 The bundle code is not bound by this historical legacy, and could do it
 in a different (and more efficient and flexible) way. But it is probably
 saner to just keep them identical. It makes the code simpler, and having
 bundle as the only transport which has the extra flexibility does not
 really buy us much (and probably just invites confusion).

Yeah, so let's have only symref=HEAD:refs/heads/master for now.

I would like to have the protocol update on the on-wire side during
2015 to lift various limits and correct inefficiencies (the largest
of which is the who speaks first issue).  We should make sure that
the bundle format can be enhanced to match when it happens.

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


Odd git am behavior rewriting subject, adding ASoC: prefix

2014-11-04 Thread Joe Perches
I have a patch file created by git format-patch.

Applying it via git am changes the subject prefix.
Anyone know why?

$ git --version
git version 2.1.2

$ git am -i 0002-staging-ft1000-Logging-message-neatening.patch
Commit Body is:
--
staging: ft1000: Logging message neatening

Use a more common logging style.

o Convert DEBUG macros to pr_debug
o Add pr_fmt
o Remove embedded function names from pr_debug
o Convert printks to pr_level
o Coalesce formats and align arguments
o Add missing terminating newlines

Signed-off-by: Joe Perches j...@perches.com
--
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all 

choosing Y emits:

Applying: ASoC: staging: ft1000: Logging message neatening

ASoC:? where does that come from?



--
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] t3312-notes-empty: Test that 'git notes' removes empty notes by default

2014-11-04 Thread Johan Herland
Add test cases documenting the current behavior when trying to
add/append/edit empty notes. This is in preparation for adding
--allow-empty; to allow empty notes to be stored.

Signed-off-by: Johan Herland jo...@herland.net
---
 t/t3312-notes-empty.sh | 58 ++
 1 file changed, 58 insertions(+)
 create mode 100755 t/t3312-notes-empty.sh

diff --git a/t/t3312-notes-empty.sh b/t/t3312-notes-empty.sh
new file mode 100755
index 000..2806d27
--- /dev/null
+++ b/t/t3312-notes-empty.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='Test adding/editing of empty notes'
+. ./test-lib.sh
+
+cat fake_editor.sh \EOF
+#!/bin/sh
+echo $MSG $1
+echo $MSG  2
+EOF
+chmod a+x fake_editor.sh
+GIT_EDITOR=./fake_editor.sh
+export GIT_EDITOR
+
+test_expect_success 'setup' '
+   test_commit one 
+   empty_blob=$(git hash-object -w /dev/null)
+'
+
+cleanup_notes() {
+   git update-ref -d refs/notes/commits
+}
+
+cat expect_missing \EOF
+commit d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
+Author: A U Thor aut...@example.com
+Date:   Thu Apr 7 15:13:13 2005 -0700
+
+one
+EOF
+
+verify_missing() {
+   git log -1  actual 
+   test_cmp expect_missing actual 
+   ! git notes list HEAD
+}
+
+for cmd in \
+   'add' \
+   'add -F /dev/null' \
+   'add -m ' \
+   'add -c $empty_blob' \
+   'add -C $empty_blob' \
+   'append' \
+   'append -F /dev/null' \
+   'append -m ' \
+   'append -c $empty_blob' \
+   'append -C $empty_blob' \
+   'edit'
+do
+   test_expect_success 'git notes $cmd' removes empty note 
+   cleanup_notes 
+   MSG= git notes $cmd 
+   verify_missing
+   
+done
+
+test_done
-- 
2.0.0.rc4.501.gdaf83ca

--
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] notes: Add --allow-empty, to allow storing empty notes

2014-11-04 Thread Johan Herland
Although the git notes man page advertises that we support binary-safe
notes addition (using the -C option), we currently do not support adding
the empty note (i.e. using the empty blob to annotate an object). Instead,
an empty note is always treated as an intent to remove the note
altogether.

Introduce the --allow-empty option to the add/append/edit subcommands,
to explicitly allow an empty note to be stored into the notes tree.

Also update the documentation, and add test cases for the new option.

Reported-by: James H. Fisher j...@trifork.com
Improved-by: Kyle J. McKay mack...@gmail.com
Signed-off-by: Johan Herland jo...@herland.net
---
 Documentation/git-notes.txt | 12 
 builtin/notes.c | 25 +++--
 notes.c |  3 +--
 t/t3312-notes-empty.sh  | 20 +++-
 4 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 310f0a5..851518d 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -9,10 +9,10 @@ SYNOPSIS
 
 [verse]
 'git notes' [list [object]]
-'git notes' add [-f] [-F file | -m msg | (-c | -C) object] [object]
+'git notes' add [-f] [--allow-empty] [-F file | -m msg | (-c | -C) 
object] [object]
 'git notes' copy [-f] ( --stdin | from-object to-object )
-'git notes' append [-F file | -m msg | (-c | -C) object] [object]
-'git notes' edit [object]
+'git notes' append [--allow-empty] [-F file | -m msg | (-c | -C) object] 
[object]
+'git notes' edit [--allow-empty] [object]
 'git notes' show [object]
 'git notes' merge [-v | -q] [-s strategy ] notes-ref
 'git notes' merge --commit [-v | -q]
@@ -155,6 +155,10 @@ OPTIONS
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the note message.
 
+--allow-empty::
+   Allow an empty note object to be stored. The default behavior is
+   to automatically remove empty notes.
+
 --ref ref::
Manipulate the notes tree in ref.  This overrides
'GIT_NOTES_REF' and the core.notesRef configuration.  The ref
@@ -287,7 +291,7 @@ arbitrary files using 'git hash-object':
 
 $ cc *.c
 $ blob=$(git hash-object -w a.out)
-$ git notes --ref=built add -C $blob HEAD
+$ git notes --ref=built add --allow-empty -C $blob HEAD
 
 
 (You cannot simply use `git notes --ref=built add -F a.out HEAD`
diff --git a/builtin/notes.c b/builtin/notes.c
index 68b6cd8..038a419 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -22,10 +22,10 @@
 
 static const char * const git_notes_usage[] = {
N_(git notes [--ref notes_ref] [list [object]]),
-   N_(git notes [--ref notes_ref] add [-f] [-m msg | -F file | (-c 
| -C) object] [object]),
+   N_(git notes [--ref notes_ref] add [-f] [--allow-empty] [-m msg | 
-F file | (-c | -C) object] [object]),
N_(git notes [--ref notes_ref] copy [-f] from-object to-object),
-   N_(git notes [--ref notes_ref] append [-m msg | -F file | (-c | 
-C) object] [object]),
-   N_(git notes [--ref notes_ref] edit [object]),
+   N_(git notes [--ref notes_ref] append [--allow-empty] [-m msg | -F 
file | (-c | -C) object] [object]),
+   N_(git notes [--ref notes_ref] edit [--allow-empty] [object]),
N_(git notes [--ref notes_ref] show [object]),
N_(git notes [--ref notes_ref] merge [-v | -q] [-s strategy ] 
notes_ref),
N_(git notes merge --commit [-v | -q]),
@@ -150,8 +150,8 @@ static void write_commented_object(int fd, const unsigned 
char *object)
 }
 
 static void create_note(const unsigned char *object, struct msg_arg *msg,
-   int append_only, const unsigned char *prev,
-   unsigned char *result)
+   int append_only, int allow_empty,
+   const unsigned char *prev, unsigned char *result)
 {
char *path = NULL;
 
@@ -202,7 +202,7 @@ static void create_note(const unsigned char *object, struct 
msg_arg *msg,
free(prev_buf);
}
 
-   if (!msg-buf.len) {
+   if (!allow_empty  !msg-buf.len) {
fprintf(stderr, _(Removing note for object %s\n),
sha1_to_hex(object));
hashclr(result);
@@ -266,7 +266,7 @@ static int parse_reuse_arg(const struct option *opt, const 
char *arg, int unset)
 
if (get_sha1(arg, object))
die(_(Failed to resolve '%s' as a valid ref.), arg);
-   if (!(buf = read_sha1_file(object, type, len)) || !len) {
+   if (!(buf = read_sha1_file(object, type, len))) {
free(buf);
die(_(Failed to read object '%s'.), arg);
}
@@ -397,7 +397,7 @@ static int append_edit(int argc, const char **argv, const 
char *prefix);
 
 static int add(int argc, const char **argv, const char *prefix)
 {
-   int retval = 0, force = 0;
+   int retval = 0, force = 0, allow_empty = 0;
const char *object_ref;
 

Re: [PATCH v5] lockfile.c: store absolute path

2014-11-04 Thread Scott Schmit
On Sun, Nov 02, 2014 at 07:24:37AM +0100, Michael Haggerty wrote:
 Locked paths can be saved in a linked list so that if something wrong
 happens, *.lock are removed. For relative paths, this works fine if we
 keep cwd the same, which is true 99% of time except:
 
 - update-index and read-tree hold the lock on $GIT_DIR/index really
   early, then later on may call setup_work_tree() to move cwd.
 
 - Suppose a lock is being held (e.g. by git add) then somewhere
   down the line, somebody calls real_path (e.g. link_alt_odb_entry),
   which temporarily moves cwd away and back.
 
 During that time when cwd is moved (either permanently or temporarily)
 and we decide to die(), attempts to remove relative *.lock will fail,
 and the next operation will complain that some files are still locked.
 
 Avoid this case by turning relative paths to absolute before storing
 the path in filename field.

This might be a little pathological, but it seems like this scheme would
run into trouble if the entire repo is moved while the lock is held.


smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH 1/2] t3312-notes-empty: Test that 'git notes' removes empty notes by default

2014-11-04 Thread Eric Sunshine
On Tue, Nov 4, 2014 at 8:32 PM, Johan Herland jo...@herland.net wrote:
 Add test cases documenting the current behavior when trying to
 add/append/edit empty notes. This is in preparation for adding
 --allow-empty; to allow empty notes to be stored.

 Signed-off-by: Johan Herland jo...@herland.net
 ---
  t/t3312-notes-empty.sh | 58 
 ++
  1 file changed, 58 insertions(+)
  create mode 100755 t/t3312-notes-empty.sh

 diff --git a/t/t3312-notes-empty.sh b/t/t3312-notes-empty.sh
 new file mode 100755
 index 000..2806d27
 --- /dev/null
 +++ b/t/t3312-notes-empty.sh
 @@ -0,0 +1,58 @@
 +#!/bin/sh
 +
 +test_description='Test adding/editing of empty notes'
 +. ./test-lib.sh
 +
 +cat fake_editor.sh \EOF
 +#!/bin/sh
 +echo $MSG $1
 +echo $MSG  2
 +EOF
 +chmod a+x fake_editor.sh

write_script() would allow you to drop the #!/bin/sh and chmod lines.

 +GIT_EDITOR=./fake_editor.sh
 +export GIT_EDITOR
 +
 +test_expect_success 'setup' '
 +   test_commit one 
 +   empty_blob=$(git hash-object -w /dev/null)
 +'
 +
 +cleanup_notes() {
 +   git update-ref -d refs/notes/commits
 +}
 +
 +cat expect_missing \EOF
 +commit d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
 +Author: A U Thor aut...@example.com
 +Date:   Thu Apr 7 15:13:13 2005 -0700
 +
 +one
 +EOF

Rather than hard-coding this output, generating it would make the test
script less fragile:

git log -1 expect_missing

 +verify_missing() {
 +   git log -1  actual 
 +   test_cmp expect_missing actual 
 +   ! git notes list HEAD
 +}
 +
 +for cmd in \
 +   'add' \
 +   'add -F /dev/null' \
 +   'add -m ' \
 +   'add -c $empty_blob' \
 +   'add -C $empty_blob' \
 +   'append' \
 +   'append -F /dev/null' \
 +   'append -m ' \
 +   'append -c $empty_blob' \
 +   'append -C $empty_blob' \
 +   'edit'
 +do
 +   test_expect_success 'git notes $cmd' removes empty note 
 +   cleanup_notes 
 +   MSG= git notes $cmd 
 +   verify_missing
 +   
 +done

Each -c/-C case fails for me when trying to read $empty_object. For example:

fatal: Failed to read object 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391'.
not ok 5 - 'git notes add -c $empty_blob' removes empty note

 +
 +test_done
 --
 2.0.0.rc4.501.gdaf83ca
--
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