* Re: body-CC-comment regression
From: Linus Torvalds @ 2017-02-17 17:38 UTC (permalink / raw)
To: Matthieu Moy
Cc: Johan Hovold, Git Mailing List, Jeff King, Kevin Daudt,
Junio C Hamano, Larry Finger
In-Reply-To: <vpq7f4pdkjp.fsf@anie.imag.fr>
On Fri, Feb 17, 2017 at 5:16 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>
> I mostly agree for the SoB, but why should a Cc tag have only one email?
Because changing that clearly broke real and useful behavior.
The "multiple email addresses" thing is bogus and wrong. Just don't do it.
How would you even parse it sanely? Are the Cc: lines now
SMTP-compliant with the whole escaping and all the usual "next line"
rules?
For example, in email, the rule for "next line" is that if you're in a
header block, and it starts with whitespace, then it's a continuation
of the last line.
That's *not* how Cc: lines work in commit messages. They are all
individual lines, and we have lots of tools (mainly just scripts with
grepping) that simply depend on it.
So this notion that the bottom of the commit message is some email
header crap is WRONG.
Stop it. It caused bugs. It's wrong. Don't do it.
Linus
^ permalink raw reply
* Re: body-CC-comment regression
From: Johan Hovold @ 2017-02-17 17:30 UTC (permalink / raw)
To: Matthieu Moy
Cc: Johan Hovold, git, Jeff King, Kevin Daudt, Junio C Hamano,
Larry Finger
In-Reply-To: <vpq4lzs7o0s.fsf@anie.imag.fr>
On Fri, Feb 17, 2017 at 05:58:11PM +0100, Matthieu Moy wrote:
> > On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:
> >> Johan Hovold <johan@kernel.org> writes:
> >
> >> The "multiple emails per Cc: field" has been there for a while already
> >> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got
> >> used to it. What you are proposing breaks their flow.
> >
> > Note that that commit never mentions multiple addresses in either
> > headers or body-tags -- it's all about being able to specify multiple
> > entries on the command line.
>
> Indeed. I'm not the author of the patch, but I was supervising the
> students who wrote it and "multiple addresses in Cc:" was not the goal,
> but a (IMHO positive) side effect we discovered after the fact.
Yeah, and the broken --suppress-cc=self I mention below is indicative
of that too.
> If I had a time machine, I'd probably go back then and forbid multiple
> addresses there, but ...
>
> > There does not seem to be single commit in the kernel where multiple
> > address are specified in a CC tag since after git-send-email started
> > allowing it, but there are ten commits before (to my surprise), and that
> > should be contrasted with at least 4178 commits with trailing comments
> > including a # sign.
>
> Hey, there's a life outside the kernel ;-).
Sure, but it's the origin of git as well as the tags we're discussing (I
believe).
My point of bringing it up was that the multiple addresses in a CC-tag
was indeed an unintended (and undocumented) side-effect and I doubt many
people have started using it given that it's sort of counter-intuitive
(again, compare with SoB).
If either the trailing comments or multiple addresses in a CC-tag has to
go, I think dropping the latter is clearly the best choice.
> >> 1) Stop calling Mail::Address even if available.[...]
> >
> > Right, that sounds like the right thing to do regardless.
> >
> >> 2) Modify our in-house parser to discard garbage after the >. [...]
> >
> > Sounds perfectly fine to me, and seems to work too after quick test.
>
> OK, sounds like the way to go.
>
> Do you want to work on a patch? If not, I should be able to do that
> myself. The code changes are straightforward, but we probably want a
> proper test for that.
Feel free to implement it this way if that's what people prefer. As long
as trailing comments are supported and discarded, I don't really have a
preference.
> > addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess
> > that can be fixed separately.
>
> OK. If it's unrelated enough, please start a separate thread to explain
> the problem (and/or write a patch ;-) ).
Well, it's related to the "offending" patch that added support for
multiple addresses in tags. By disallowing that, as my fix does, the
problem goes away.
# Now parse the message body
while(<$fh>) {
$message .= $_;
if (/^(Signed-off-by|Cc): (.*)$/i) {
chomp;
my ($what, $c) = ($1, $2);
chomp $c;
my $sc = sanitize_address($c);
if ($sc eq $sender) {
next if ($suppress_cc{'self'});
The problem here is that $sc will never match $sender when there are more
than one address in a tag. For example:
From: Johan Hovold <johan@kernel.org>
...
Cc: alpha <test1@a.com>, Johan Hovold <johan@kernel.org>
results in
sc = alpha <test1@a.com>, Johan Hovold <johan@kernel.org>
sender = Johan Hovold <johan@kernel.org>
so that --suppress-cc=self is not honoured.
Thanks,
Johan
^ permalink raw reply
* Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs
From: Junio C Hamano @ 2017-02-17 17:11 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ...
All good review comments.
I briefly wondered if recording the deletion of the current branch
in HEAD's reflog has much practical values (Porcelain "git branch"
would not even allow deletion in the first place), but because it is
a rare and unusual event, it probably makes more sense to have a
record of it.
^ permalink raw reply
* [L10N] Kickoff for Git 2.12.0 l10n round 2
From: Jiang Xin @ 2017-02-17 17:11 UTC (permalink / raw)
To: Alexander Shopov, Jordi Mas, Ralf Thielow, Jean-Noël Avila,
Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
Peter Krefting, Trần Ngọc Quân, Jiang Xin
Cc: Git List
Hi guys,
Git v2.12.0-rc1 introduced another two new messages need to be translated,
so let's start l10n for Git 2.12.0 round 2:
l10n: git.pot: v2.12.0 round 2 (2 new)
Generate po/git.pot from v2.12.0-rc1 for git v2.12.0 l10n round 2.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
You can get it from the usual place:
https://github.com/git-l10n/git-po/
As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH/RFC 00/15] Fix git-gc losing objects in multi worktree
From: Johannes Schindelin @ 2017-02-17 17:09 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Michael Haggerty, Stefan Beller
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 484 bytes --]
Hi Duy,
On Fri, 17 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> So here is my latest attempt on fixing this issue. For people who are
> not aware of it, git-gc does not take per-worktree refs, reflogs and
> indexes into account. An odb prune may leave HEAD and references in
> other worktrees pointing to nowhere.
Thank you so much for working on this. The bug really affects my daily
work very, very negatively.
Will try to review as soon as possible.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] delete_refs(): accept a reflog message argument
From: Junio C Hamano @ 2017-02-17 17:05 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217081205.zn7j6d5cffgdvfbn@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> diff --git a/refs.h b/refs.h
>> index 9fbff90e7..81627a63d 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -277,7 +277,7 @@ int reflog_exists(const char *refname);
>> * be NULL_SHA1. flags is passed through to ref_transaction_delete().
>> */
>> int delete_ref(const char *refname, const unsigned char *old_sha1,
>> - unsigned int flags);
>> + unsigned int flags, const char *msg);
>
> Should the "msg" argument go at the beginning, to match update_ref()?
Probably. rename/create have the message at the end but their
parameters are very different from update/delete. The parameters
update and delete take are not identical, but we can view them as a
lot more similar than the other two. So I think it makes sense for
delete to try matching update, even though trying to make all four
the same may proabably be pointless.
^ permalink raw reply
* [PATCH v3 2/2] rev-parse: fix several options when running in a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Michael Rappazzo
In-Reply-To: <cover.1487350582.git.johannes.schindelin@gmx.de>
In addition to making git_path() aware of certain file names that need
to be handled differently e.g. when running in worktrees, the commit
557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
2014-11-30) also snuck in a new option for `git rev-parse`:
`--git-path`.
On the face of it, there is no obvious bug in that commit's diff: it
faithfully calls git_path() on the argument and prints it out, i.e. `git
rev-parse --git-path <filename>` has the same precise behavior as
calling `git_path("<filename>")` in C.
The problem lies deeper, much deeper. In hindsight (which is always
unfair), implementing the .git/ directory discovery in
`setup_git_directory()` by changing the working directory may have
allowed us to avoid passing around a struct that contains information
about the current repository, but it bought us many, many problems.
In this case, when being called in a subdirectory, `git rev-parse`
changes the working directory to the top-level directory before calling
`git_path()`. In the new working directory, the result is correct. But
in the working directory of the calling script, it is incorrect.
Example: when calling `git rev-parse --git-path HEAD` in, say, the
Documentation/ subdirectory of Git's own source code, the string
`.git/HEAD` is printed.
Side note: that bug is hidden when running in a subdirectory of a
worktree that was added by the `git worktree` command: in that case, the
(correct) absolute path of the `HEAD` file is printed.
In the interest of time, this patch does not go the "correct" route to
introduce a struct with repository information (and removing global
state in the process), instead this patch chooses to detect when the
command was called in a subdirectory and forces the result to be an
absolute path.
While at it, we are also fixing the output of --git-common-dir and
--shared-index-path.
Lastly, please note that we reuse the same strbuf for all of the
relative_path() calls; this avoids frequent allocation (and duplicated
code), and it does not risk memory leaks, for two reasons: 1) the
cmd_rev_parse() function does not return anywhere between the use of
the new strbuf instance and its final release, and 2) git-rev-parse is
one of these "one-shot" programs in Git, i.e. it exits after running
for a very short time, meaning that all allocated memory is released
with the exit() call anyway.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rev-parse.c | 15 +++++++++++----
t/t1500-rev-parse.sh | 4 ++--
t/t1700-split-index.sh | 2 +-
t/t2027-worktree-list.sh | 4 ++--
4 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1db..2cfd8d2aae4 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -545,6 +545,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
unsigned int flags = 0;
const char *name = NULL;
struct object_context unused;
+ struct strbuf buf = STRBUF_INIT;
if (argc > 1 && !strcmp("--parseopt", argv[1]))
return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -599,7 +600,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
if (!strcmp(arg, "--git-path")) {
if (!argv[i + 1])
die("--git-path requires an argument");
- puts(git_path("%s", argv[i + 1]));
+ strbuf_reset(&buf);
+ puts(relative_path(git_path("%s", argv[i + 1]),
+ prefix, &buf));
i++;
continue;
}
@@ -821,8 +824,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
- const char *pfx = prefix ? prefix : "";
- puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
+ strbuf_reset(&buf);
+ puts(relative_path(get_git_common_dir(),
+ prefix, &buf));
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -845,7 +849,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die(_("Could not read the index"));
if (the_index.split_index) {
const unsigned char *sha1 = the_index.split_index->base_sha1;
- puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));
+ const char *path = git_path("sharedindex.%s", sha1_to_hex(sha1));
+ strbuf_reset(&buf);
+ puts(relative_path(path, prefix, &buf));
}
continue;
}
@@ -897,6 +903,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
verify_filename(prefix, arg, 1);
}
+ strbuf_release(&buf);
if (verify) {
if (revs_count == 1) {
show_rev(type, sha1, name);
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index f39f783f2db..d74f09ad93e 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -93,7 +93,7 @@ test_expect_success 'git-common-dir from worktree root' '
test_cmp expect actual
'
-test_expect_failure 'git-common-dir inside sub-dir' '
+test_expect_success 'git-common-dir inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
@@ -107,7 +107,7 @@ test_expect_success 'git-path from worktree root' '
test_cmp expect actual
'
-test_expect_failure 'git-path inside sub-dir' '
+test_expect_success 'git-path inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b754865a618..6096f2c6309 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,7 +200,7 @@ EOF
test_cmp expect actual
'
-test_expect_failure 'rev-parse --shared-index-path' '
+test_expect_success 'rev-parse --shared-index-path' '
test_create_repo split-index &&
(
cd split-index &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index c1a072348e7..848da5f3684 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,7 +8,7 @@ test_expect_success 'setup' '
test_commit init
'
-test_expect_failure 'rev-parse --git-common-dir on main worktree' '
+test_expect_success 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main worktree' '
test_cmp expected2 actual2
'
-test_expect_failure 'rev-parse --git-path objects linked worktree' '
+test_expect_success 'rev-parse --git-path objects linked worktree' '
echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&
test_when_finished "rm -rf linked-tree && git worktree prune" &&
git worktree add --detach linked-tree master &&
--
2.11.1.windows.1.2.g87ad093.dirty
^ permalink raw reply related
* [PATCH v3 1/2] rev-parse tests: add tests executed from a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:59 UTC (permalink / raw)
To: git; +Cc: Michael Rappazzo, Junio C Hamano,
Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1487350582.git.johannes.schindelin@gmx.de>
From: Michael Rappazzo <rappazzo@gmail.com>
t2027-worktree-list has an incorrect expectation for --git-common-dir
which has been adjusted and marked to expect failure.
Some of the tests added have been marked to expect failure. These
demonstrate a problem with the way that some options to git rev-parse
behave when executed from a subdirectory of the main worktree.
[jes: fixed incorrect assumption that objects/ lives in the
worktree-specific git-dir (it lives in the common dir instead). Also
adjusted t1700 so that the test case does not *need* to be the last
one in that script.]
Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1500-rev-parse.sh | 28 ++++++++++++++++++++++++++++
t/t1700-split-index.sh | 16 ++++++++++++++++
t/t2027-worktree-list.sh | 12 ++++++++++--
3 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c4014..f39f783f2db 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -87,4 +87,32 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru
test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
+test_expect_success 'git-common-dir from worktree root' '
+ echo .git >expect &&
+ git rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-common-dir inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+ git -C path/to/child rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+ echo .git/objects >expect &&
+ git rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-path inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
+ git -C path/to/child rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 292a0720fcc..b754865a618 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,20 @@ EOF
test_cmp expect actual
'
+test_expect_failure 'rev-parse --shared-index-path' '
+ test_create_repo split-index &&
+ (
+ cd split-index &&
+ git update-index --split-index &&
+ echo .git/sharedindex* >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual &&
+ mkdir subdirectory &&
+ cd subdirectory &&
+ echo ../.git/sharedindex* >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 465eeeacd3d..c1a072348e7 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,16 +8,24 @@ test_expect_success 'setup' '
test_commit init
'
-test_expect_success 'rev-parse --git-common-dir on main worktree' '
+test_expect_failure 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
mkdir sub &&
git -C sub rev-parse --git-common-dir >actual2 &&
- echo sub/.git >expected2 &&
+ echo ../.git >expected2 &&
test_cmp expected2 actual2
'
+test_expect_failure 'rev-parse --git-path objects linked worktree' '
+ echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&
+ test_when_finished "rm -rf linked-tree && git worktree prune" &&
+ git worktree add --detach linked-tree master &&
+ git -C linked-tree rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '"list" all worktrees from main' '
echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
test_when_finished "rm -rf here && git worktree prune" &&
--
2.11.1.windows.1.2.g87ad093.dirty
^ permalink raw reply related
* [PATCH v3 0/2] Fix bugs in rev-parse's output when run in a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Michael Rappazzo
In-Reply-To: <cover.1486740772.git.johannes.schindelin@gmx.de>
The bug that bit me (hard!) and that triggered not only a long series of
curses but also my writing a patch and sending it to the list was that
`git rev-parse --git-path HEAD` would give *incorrect* output when run
in a subdirectory of a regular checkout, but *correct* output when run
in a subdirectory of an associated *worktree*.
I had tested the script in question quite a bit, but in a worktree. And
in production, it quietly did exactly the wrong thing.
Changes relative to v2:
- the "iffy" test in t1700 was made "uniffy"
- clarified in the commit message of 2/2 why we can get away with the
"reset then use" pattern
Johannes Schindelin (1):
rev-parse: fix several options when running in a subdirectory
Michael Rappazzo (1):
rev-parse tests: add tests executed from a subdirectory
builtin/rev-parse.c | 15 +++++++++++----
t/t1500-rev-parse.sh | 28 ++++++++++++++++++++++++++++
t/t1700-split-index.sh | 16 ++++++++++++++++
t/t2027-worktree-list.sh | 10 +++++++++-
4 files changed, 64 insertions(+), 5 deletions(-)
base-commit: 076c05393a047247ea723896289b48d6549ed7d0
Published-As: https://github.com/dscho/git/releases/tag/git-path-in-subdir-v3
Fetch-It-Via: git fetch https://github.com/dscho/git git-path-in-subdir-v3
Interdiff vs v2:
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 84af2802f6f..2cfd8d2aae4 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -903,6 +903,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
verify_filename(prefix, arg, 1);
}
+ strbuf_release(&buf);
if (verify) {
if (revs_count == 1) {
show_rev(type, sha1, name);
@@ -912,6 +913,5 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die_no_single_rev(quiet);
} else
show_default();
- strbuf_release(&buf);
return 0;
}
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 446ff34f966..6096f2c6309 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -201,17 +201,16 @@ EOF
'
test_expect_success 'rev-parse --shared-index-path' '
- rm -rf .git &&
- test_create_repo . &&
- git update-index --split-index &&
- ls -t .git/sharedindex* | tail -n 1 >expect &&
- git rev-parse --shared-index-path >actual &&
- test_cmp expect actual &&
- mkdir work &&
- test_when_finished "rm -rf work" &&
+ test_create_repo split-index &&
(
- cd work &&
- ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+ cd split-index &&
+ git update-index --split-index &&
+ echo .git/sharedindex* >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual &&
+ mkdir subdirectory &&
+ cd subdirectory &&
+ echo ../.git/sharedindex* >expect &&
git rev-parse --shared-index-path >actual &&
test_cmp expect actual
)
--
2.11.1.windows.1.2.g87ad093.dirty
^ permalink raw reply
* Re: [PATCH v2 1/2] rev-parse tests: add tests executed from a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Michael Rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqpoipkd3l.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 10 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> > index 292a0720fcc..1d6e27a09d8 100755
> > --- a/t/t1700-split-index.sh
> > +++ b/t/t1700-split-index.sh
> > @@ -200,4 +200,21 @@ EOF
> > test_cmp expect actual
> > '
> >
> > +test_expect_failure 'rev-parse --shared-index-path' '
> > + rm -rf .git &&
> > + test_create_repo . &&
>
> Another thing that I notice only after merging this and other topics
> to 'pu' was that this piece needs to always come at the end of the
> script because of this removal. It would make the test more robust
> to create a test repository for this test and work inside it.
Yes, this is indeed unnecessary and even undesirable.
I waited a couple of days to give the original author a chance to fix
this. As I want a fix really badly (because I am really embarrassed of
this bug), I adjusted t1700 appropriately and will send out v3 in a
second.
Ciao,
Johannes
^ permalink raw reply
* Re: body-CC-comment regression
From: Matthieu Moy @ 2017-02-17 16:58 UTC (permalink / raw)
To: Johan Hovold; +Cc: git, Jeff King, Kevin Daudt, Junio C Hamano, Larry Finger
In-Reply-To: <20170217164241.GE2625@localhost>
> On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:
>> Johan Hovold <johan@kernel.org> writes:
>
>> The "multiple emails per Cc: field" has been there for a while already
>> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got
>> used to it. What you are proposing breaks their flow.
>
> Note that that commit never mentions multiple addresses in either
> headers or body-tags -- it's all about being able to specify multiple
> entries on the command line.
Indeed. I'm not the author of the patch, but I was supervising the
students who wrote it and "multiple addresses in Cc:" was not the goal,
but a (IMHO positive) side effect we discovered after the fact.
If I had a time machine, I'd probably go back then and forbid multiple
addresses there, but ...
> There does not seem to be single commit in the kernel where multiple
> address are specified in a CC tag since after git-send-email started
> allowing it, but there are ten commits before (to my surprise), and that
> should be contrasted with at least 4178 commits with trailing comments
> including a # sign.
Hey, there's a life outside the kernel ;-).
>> 1) Stop calling Mail::Address even if available.[...]
>
> Right, that sounds like the right thing to do regardless.
>
>> 2) Modify our in-house parser to discard garbage after the >. [...]
>
> Sounds perfectly fine to me, and seems to work too after quick test.
OK, sounds like the way to go.
Do you want to work on a patch? If not, I should be able to do that
myself. The code changes are straightforward, but we probably want a
proper test for that.
> addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess
> that can be fixed separately.
OK. If it's unrelated enough, please start a separate thread to explain
the problem (and/or write a patch ;-) ).
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v2 1/2] rev-parse tests: add tests executed from a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Michael Rappazzo, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqshnllw2f.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 10 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> > index 292a0720fcc..1d6e27a09d8 100755
> > --- a/t/t1700-split-index.sh
> > +++ b/t/t1700-split-index.sh
> > @@ -200,4 +200,21 @@ EOF
> > test_cmp expect actual
> > '
> >
> > +test_expect_failure 'rev-parse --shared-index-path' '
> > + rm -rf .git &&
> > + test_create_repo . &&
> > + git update-index --split-index &&
> > + ls -t .git/sharedindex* | tail -n 1 >expect &&
> > + git rev-parse --shared-index-path >actual &&
> > + test_cmp expect actual &&
> > + mkdir work &&
> > + test_when_finished "rm -rf work" &&
> > + (
> > + cd work &&
> > + ls -t ../.git/sharedindex* | tail -n 1 >expect &&
> > + git rev-parse --shared-index-path >actual &&
> > + test_cmp expect actual
> > + )
>
> This looks iffy.
Indeed.
> If we expect multiple sharedindex* files, the first output from "ls -t"
> may or may not match the real one in use (multiple things do happen
> within a single second or whatever your filesystem's time granularity
> is). Two "ls -t" run in this test would (hopefully) give stable
> results, but I suspect that the chance the first line in the output
> matches what --shared-index-path reports is 50% if we expect to have two
> sharedindex* files.
>
> On the other hand, if we do not expect multiple sharedindex* files,
> use of "ls" piped to "tail" is simply misleading.
>
> If this test can be written in such a way that there is only one
> such file that match the pattern, it would be the cleanest to
> understand and explain. As there is only a single invocation of
> "update-index --split-index" immediately after a new repository is
> created, I suspect that the expectation to see only one sharedindex*
> file already holds (because its name is unpredictable, we still need
> to catch it with wildcard), and if that is the case, we can just
> lose "-t" and pipe to tail, i.e. "ls .git/sharedindex* >expect".
Indeed. We can expect only one sharedindex file to be present, and we do
not even have to call out to `ls` but can get away with calling `echo`
(which is a builtin in Bash).
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] rev-parse: fix several options when running in a subdirectory
From: Johannes Schindelin @ 2017-02-17 16:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Nguyễn Thái Ngọc Duy, Michael Rappazzo
In-Reply-To: <xmqqo9y9lvqk.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 10 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> > index ff13e59e1db..84af2802f6f 100644
> > --- a/builtin/rev-parse.c
> > +++ b/builtin/rev-parse.c
> > @@ -545,6 +545,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > unsigned int flags = 0;
> > const char *name = NULL;
> > struct object_context unused;
> > + struct strbuf buf = STRBUF_INIT;
> >
> > if (argc > 1 && !strcmp("--parseopt", argv[1]))
> > return cmd_parseopt(argc - 1, argv + 1, prefix);
> > @@ -599,7 +600,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > if (!strcmp(arg, "--git-path")) {
> > if (!argv[i + 1])
> > die("--git-path requires an argument");
> > - puts(git_path("%s", argv[i + 1]));
> > + strbuf_reset(&buf);
> > + puts(relative_path(git_path("%s", argv[i + 1]),
> > + prefix, &buf));
> > i++;
> > continue;
> > }
> > @@ -821,8 +824,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > continue;
> > }
> > if (!strcmp(arg, "--git-common-dir")) {
> > - const char *pfx = prefix ? prefix : "";
> > - puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
> > + strbuf_reset(&buf);
> > + puts(relative_path(get_git_common_dir(),
> > + prefix, &buf));
> > continue;
> > }
> > if (!strcmp(arg, "--is-inside-git-dir")) {
> > @@ -845,7 +849,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > die(_("Could not read the index"));
> > if (the_index.split_index) {
> > const unsigned char *sha1 = the_index.split_index->base_sha1;
> > - puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));
> > + const char *path = git_path("sharedindex.%s", sha1_to_hex(sha1));
> > + strbuf_reset(&buf);
> > + puts(relative_path(path, prefix, &buf));
> > }
> > continue;
> > }
> > @@ -906,5 +912,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > die_no_single_rev(quiet);
> > } else
> > show_default();
> > + strbuf_release(&buf);
>
> This uses "reset then use" pattern for repeated use of strbuf, and
> causes the string last held in the strbuf to leak on early return,
... which cannot happen due to the lack of an early return...
> which can be mitigated by using "use then reset" pattern. I.e.
>
> if (!strcmp(arg, "--git-common-dir")) {
> puts(relative_path(get_git_common_dir(),
> prefix, &buf));
> strbuf_reset(&buf);
> continue;
> }
>
> I'd think.
This would not release the memory, though:
#define strbuf_reset(sb) strbuf_setlen(sb, 0)
and
static inline void strbuf_setlen(struct strbuf *sb, size_t len)
{
if (len > (sb->alloc ? sb->alloc - 1 : 0))
die("BUG: strbuf_setlen() beyond buffer");
sb->len = len;
sb->buf[len] = '\0';
}
There is not a single free() statement there.
So the "use then reset" scheme would leak *just the same*.
> You'd still want to release it at the end anyway for good code hygiene,
> though.
Which I do.
Technically, this is not even necessary because all of the cmd_*()
functions are immediately followed by a call to exit(). Wasn't that the
genius idea in the early Git days, that we could simply get away with
sloppy memory management because the program exit()s shortly afterwards,
anyway? ;-)
In any case, I adjusted the commit message to clarify why the "reset then
use" scheme is correct here.
Ciao,
Johannes
^ permalink raw reply
* Re: body-CC-comment regression
From: Johan Hovold @ 2017-02-17 16:42 UTC (permalink / raw)
To: Matthieu Moy
Cc: Johan Hovold, git, Jeff King, Kevin Daudt, Junio C Hamano,
Larry Finger
In-Reply-To: <vpq7f4pdkjp.fsf@anie.imag.fr>
On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:
> Johan Hovold <johan@kernel.org> writes:
>
> > There is another option, namely to only accept a single address for tags
> > in the body. I understand that being able to copy a CC-header to either
> > the header section or to the command line could be useful, but I don't
> > really see the point in allowing this in the tags in the body (a SoB
> > always has one address, and so should a CC-tag).
>
> I mostly agree for the SoB, but why should a Cc tag have only one email?
For symmetry (with SoB) and readability reasons (one tag per line).
These are body tags, not mail headers, after all.
> The "multiple emails per Cc: field" has been there for a while already
> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got
> used to it. What you are proposing breaks their flow.
Note that that commit never mentions multiple addresses in either
headers or body-tags -- it's all about being able to specify multiple
entries on the command line.
There does not seem to be single commit in the kernel where multiple
address are specified in a CC tag since after git-send-email started
allowing it, but there are ten commits before (to my surprise), and that
should be contrasted with at least 4178 commits with trailing comments
including a # sign.
> > And since this is a regression for something that has been working for
> > years that was introduced by a new feature, I also think it's reasonable
> > to (partially) revert the feature.
>
> I'd find it rather ironic to fix your case by breaking a feature that
> has been working for more than a year :-(. What would you answer to a
> contributor comming one year from now and proposing to revert your
> reversion because it breaks his flow?
Such conflicts are not uncommon when dealing with regressions introduced
by new features, and need to be dealt with on a case-by-case basis. But
the fact that trailing comments have been properly supported for more
than four years should carry some weight.
> All that said, I think another fix would be both satisfactory for
> everyone and rather simple:
>
> 1) Stop calling Mail::Address even if available. It used to make sense
> to do that when our in-house parser was really poor, but we now have
> something essentially as good as Mail::Address. We test our parser
> against Mail::Address and we do have a few known differences (see
> t9000), but they are really corner-cases and shouldn't matter.
>
> A good consequence of this is that we stop depending on the way Perl
> is installed to parse emails. Regardless of the current issue, I
> think it is a good thing.
Right, that sounds like the right thing to do regardless.
> 2) Modify our in-house parser to discard garbage after the >. The patch
> should look like (untested):
>
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -903,11 +903,11 @@ sub parse_mailboxes {
> my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> foreach my $token (@tokens) {
> if ($token =~ /^[,;]$/) {
> - # if buffer still contains undeterminated strings
> - # append it at the end of @address or @phrase
> - if ($end_of_addr_seen) {
> - push @phrase, @buffer;
> - } else {
> + # if buffer still contains undeterminated
> + # strings append it at the end of @address,
> + # unless we already saw the closing >, in
> + # which case we discard it.
> + if (!$end_of_addr_seen) {
> push @address, @buffer;
> }
>
> What do you think?
Sounds perfectly fine to me, and seems to work too after quick test.
Note however that there's another minor issue with using multiple
addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess
that can be fixed separately.
Thanks,
Johan
^ permalink raw reply
* RE: new git-diff switch to eliminate leading "+" and "-" characters
From: Vanderhoof, Tzadik @ 2017-02-17 16:06 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8CTL1GUreqNVBYyu2EqqdiUxj-dOOE9=Rr0ivK6-7yKjw@mail.gmail.com>
> From: Duy Nguyen [mailto:pclouds@gmail.com]
> Sent: Friday, February 17, 2017 6:54 AM
> To: Vanderhoof, Tzadik
> Cc: git@vger.kernel.org
>
> > Would it make sense to develop such a switch or has there been work on
> that already?
>
> I face this "problem" every day, but most editors nowadays have block-
> based editing that would allow you to remove a column of "+/-"
> easily. At least it has not bothered me to think of improving it.
Would a patch be welcome?
Tzadik
> --
> Duy
This e-mail, including attachments, may include confidential and/or
proprietary information, and may be used only by the person or entity
to which it is addressed. If the reader of this e-mail is not the intended
recipient or his or her authorized agent, the reader is hereby notified
that any dissemination, distribution or copying of this e-mail is
prohibited. If you have received this e-mail in error, please notify the
sender by replying to this message and delete this e-mail immediately.
^ permalink raw reply
* RE: new git-diff switch to eliminate leading "+" and "-" characters
From: Vanderhoof, Tzadik @ 2017-02-17 16:04 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8CTL1GUreqNVBYyu2EqqdiUxj-dOOE9=Rr0ivK6-7yKjw@mail.gmail.com>
> From: Duy Nguyen [mailto:pclouds@gmail.com]
> Sent: Friday, February 17, 2017 6:54 AM
> To: Vanderhoof, Tzadik
> Cc: git@vger.kernel.org
>
> On Tue, Feb 14, 2017 at 6:01 AM, Vanderhoof, Tzadik
> <tzadik.vanderhoof@optum360.com> wrote:
> > The output of git-diff includes lines beginning with "+" and "-" to indicate
> added and deleted lines. A somewhat common task (at least for me) is to
> want to copy output from a "diff" (usually the deleted lines) and paste it back
> into my code.
> >
> > This is quite inconvenient because of the leading "+" and "-" characters. I
> know there are shell and IDE / editor workarounds but it would be nice if
> there was a switch to git-diff to make it leave out those characters, especially
> since "--color" kind of makes those leading characters obsolete.
>
> Color wouldn't show you new/old empty lines though (unless you use
> different background color, but I doubt that looks readable).
>
> > Would it make sense to develop such a switch or has there been work on
> that already?
>
> I face this "problem" every day, but most editors nowadays have block-
> based editing that would allow you to remove a column of "+/-"
> easily. At least it has not bothered me to think of improving it.
Would a patch be welcome?
Tzadik
> --
> Duy
This e-mail, including attachments, may include confidential and/or
proprietary information, and may be used only by the person or entity
to which it is addressed. If the reader of this e-mail is not the intended
recipient or his or her authorized agent, the reader is hereby notified
that any dissemination, distribution or copying of this e-mail is
prohibited. If you have received this e-mail in error, please notify the
sender by replying to this message and delete this e-mail immediately.
^ permalink raw reply
* Re: [PATCH] mingw: make stderr unbuffered again
From: Johannes Schindelin @ 2017-02-17 16:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt, Jeff Hostetler
In-Reply-To: <xmqqlgt7110q.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 15 Feb 2017, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >> FWIW I wish it were different, that git.git's `master` reflected more
> >> closely what the current Git for Windows version has.
> >
> > Well, we two wishing the same thing together without doing anything
> > else would make it happen.
>
> ehh, would *not* make it happen, of course.
That is the reason why set aside a substantial part of my maintainer time
to upstream those patches. You may not see how much time it costs me, say,
to get the putty-w-args stuff upstream, but rest assured that I would not
be able to do this were it not for a company paying me to do exactly that.
> > As an experiment to see if our process can be improved, I've been
> > meaning to suggest (which is what was behind my "question at a bit
> > higher level" to Hannes [*1*]) asking you to throw me occasional
> > pull requests for changes that are only about Windows specific
> > issues, bypassing "patches on the list" for things like a hotfix to
> > js/mingw-isatty [*2*] and use of OpenSSL SHA-1 on Windows [*3*],
> > essentially treating Windows specific changes as "a sub-maintainer
> > makes pull requests" we already do with Paul, Eric and Pat.
The problem here is that Git for Windows is not a subsystem. It touches
pretty much all of Git. That is very different from the Tcl/Tk code, or
from git-svn (whose code is not shared by anything else in Git's source
code, despite the fact that it is written in Perl).
And even if you were to accept the occasional Pull Request from me, it
would not solve the even bigger problem that we essentially reject so much
expertise out there, so many potential contributions by very competent
developers who just have no desire to fight the contribution process.
> While this may ease the flow of upstreaming windows specific
> changes, we need a separate thing to address the on-going issue you
> raised in your message. A Windows-less person would not know his
... or her...
> change to a generic code that is innocuous-looking has fallouts on
> Windows (read this sentence with "Windows" replaced with any
> specific platform name). When somebody writes c == '/' that should
> have been written as is_dir_sep(c), you or Hannes often finds it
> during the review here, and after repeatedly seeing such reviews,
> that (slowly) rubs off on other Window-less folks. A new code may
> still hit 'next' and 'master' with such an issue if it goes
> unnoticed during the review.
Apart from the fact that we have no prayer at coming up with a system that
keeps track of open issues (because we do not use any tracker, but instead
rely on people to remember that some thing still may not have been
addressed), there is a different problem here: you stated very explicitly
that it is okay for `pu` to be broken [*1*]. If it were different, if any
breakage would imply that a fix is really, really required lest the patch
series be evicted from `pu`, we could easily modify my Continuous Testing
setup to report more visibly what is broken. But since it is okay for `pu`
to be broken, that would just annoy everybody and people would learn to
ignore/procmail those reports.
> The CI you are setting up [*1*]
which *1*?
And... I already set it up. I just did not bother to make it more public
because the builds were broken more often than not. IIRC the entire month
of October was a solid red.
> may certainly be a step in the good direction. Having more people like
> Hannes working off of upstream may also be a great way to help the
> "forget 'next' and upstream in general" issue. Any other ideas?
The Continuous Integration is actually not so much a Continuous
Integration as it is a Continuous Testing. If it became more of a CI, that
would certainly reduce the impression that Git's bleeding edge only ever
works on Linux.
Ciao,
Johannes
Footnote *1*: You probably read between the lines that this is
unfortunate, in my opinion. It sets the mood that lets experimental (if
useful) features such as worktrees be broken for the better part of a
year.
^ permalink raw reply
* libgit2::git_describe_workdir() fails with depth-cloned + detached head
From: Lukas Lueg @ 2017-02-17 15:39 UTC (permalink / raw)
To: git
Hi,
I have a build on Travis failing with a weird error that happens while
trying to get a description from a repo. One of the integration tests
that are executed on Travis executes libgit2::git_describe_workdir()
on it's own repo and fails with code: -3, klass: 9, message: "object
not found - no match for id
(77f95f14776deb7e120a2a26f7b56abf2903bc62)".
The id 77f95f1 from the error message refers to a commit that is of no
particular interest and actually weeks behind; I never asked for it.
I was finally able to reproduce the problem by very carefully
following how Travis clones the repo:
git clone --depth=50 https://... repo_root
cd repo_root
git fetch origin +refs/pull/414/merge:
git checkout -qf FETCH_HEAD
If and only if the repo is cloned recursively, the call to
libgit2::git_describe_workdir() results in this error, always
referring to 77f95f1. If the repo is cloned without '--depth 50', no
error occurs. The repo does not even have submodules.
My local git client does the right thing and get's the latest commit
oid without problems.
Any idea what's going on here?
Best regards
Lukas
^ permalink raw reply
* Re: new git-diff switch to eliminate leading "+" and "-" characters
From: Duy Nguyen @ 2017-02-17 14:53 UTC (permalink / raw)
To: Vanderhoof, Tzadik; +Cc: git@vger.kernel.org
In-Reply-To: <2C8817BDA27E034F8E9A669458E375EF11886C5A@APSWP0428.ms.ds.uhc.com>
On Tue, Feb 14, 2017 at 6:01 AM, Vanderhoof, Tzadik
<tzadik.vanderhoof@optum360.com> wrote:
> The output of git-diff includes lines beginning with "+" and "-" to indicate added and deleted lines. A somewhat common task (at least for me) is to want to copy output from a "diff" (usually the deleted lines) and paste it back into my code.
>
> This is quite inconvenient because of the leading "+" and "-" characters. I know there are shell and IDE / editor workarounds but it would be nice if there was a switch to git-diff to make it leave out those characters, especially since "--color" kind of makes those leading characters obsolete.
Color wouldn't show you new/old empty lines though (unless you use
different background color, but I doubt that looks readable).
> Would it make sense to develop such a switch or has there been work on that already?
I face this "problem" every day, but most editors nowadays have
block-based editing that would allow you to remove a column of "+/-"
easily. At least it has not bothered me to think of improving it.
--
Duy
^ permalink raw reply
* [PATCH 15/15] rev-list: expose and document --single-worktree
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/rev-list-options.txt | 8 ++++++++
revision.c | 2 ++
2 files changed, 10 insertions(+)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5da7cf5a8..dd773f97c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -179,6 +179,14 @@ explicitly.
Pretend as if all objects mentioned by reflogs are listed on the
command line as `<commit>`.
+--single-worktree::
+ By default, all working trees will be examined by the
+ following options when there are more than one (see
+ linkgit:git-worktree[1]): `--all`, `--reflog` and
+ `--indexed-objects`.
+ This option forces them to examine the current working tree
+ only.
+
--ignore-missing::
Upon seeing an invalid object name in the input, pretend as if
the bad input was not given.
diff --git a/revision.c b/revision.c
index ecfd6fea6..2a27e55fe 100644
--- a/revision.c
+++ b/revision.c
@@ -2222,6 +2222,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
return error("invalid argument to --no-walk");
} else if (!strcmp(arg, "--do-walk")) {
revs->no_walk = 0;
+ } else if (!strcmp(arg, "--single-worktree")) {
+ revs->single_worktree = 1;
} else {
return 0;
}
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 14/15] revision.c: --reflog add HEAD reflog from all worktrees
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
Note that add_other_reflogs_to_pending() is a bit inefficient, since
it scans reflog for all refs of each worktree, including shared refs,
so the shared ref's reflog is scanned over and over again.
We could update refs API to pass "per-worktree only" flag to avoid
that. But long term we should be able to obtain a "per-worktree only"
ref store and would need to revert the changes in reflog iteration
API. So let's just wait until then.
add_reflogs_to_pending() is called by reachable.c so by default "git
prune" will examine reflog from all worktrees.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
revision.c | 28 +++++++++++++++++++++++++++-
t/t5304-prune.sh | 16 ++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 8ee929cef..ecfd6fea6 100644
--- a/revision.c
+++ b/revision.c
@@ -1134,6 +1134,7 @@ struct all_refs_cb {
int warned_bad_reflog;
struct rev_info *all_revs;
const char *name_for_errormsg;
+ struct ref_store *refs;
};
int ref_excluded(struct string_list *ref_excludes, const char *path)
@@ -1169,6 +1170,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
{
cb->all_revs = revs;
cb->all_flags = flags;
+ cb->refs = NULL;
}
void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@ -1237,17 +1239,41 @@ static int handle_one_reflog(const char *path, const struct object_id *oid,
struct all_refs_cb *cb = cb_data;
cb->warned_bad_reflog = 0;
cb->name_for_errormsg = path;
- for_each_reflog_ent(path, handle_one_reflog_ent, cb_data);
+ refs_for_each_reflog_ent(cb->refs, path,
+ handle_one_reflog_ent, cb_data);
return 0;
}
+static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
+{
+ struct worktree **worktrees, **p;
+
+ worktrees = get_worktrees(0);
+ for (p = worktrees; *p; p++) {
+ struct worktree *wt = *p;
+
+ if (wt->is_current)
+ continue;
+
+ cb->refs = get_worktree_ref_store(wt);
+ refs_for_each_reflog(cb->refs,
+ handle_one_reflog,
+ cb);
+ }
+ free_worktrees(worktrees);
+}
+
void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
{
struct all_refs_cb cb;
cb.all_revs = revs;
cb.all_flags = flags;
+ cb.refs = get_main_ref_store();
for_each_reflog(handle_one_reflog, &cb);
+
+ if (!revs->single_worktree)
+ add_other_reflogs_to_pending(&cb);
}
static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 683bdb031..6694c19a1 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -304,4 +304,20 @@ test_expect_success 'prune: handle HEAD in multiple worktrees' '
test_cmp third-worktree/blob actual
'
+test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '
+ git config core.logAllRefUpdates true &&
+ echo "lost blob for third-worktree" >expected &&
+ (
+ cd third-worktree &&
+ cat ../expected >blob &&
+ git add blob &&
+ git commit -m "second commit in third" &&
+ git reset --hard HEAD^
+ ) &&
+ git prune --expire=now &&
+ SHA1=`git hash-object expected` &&
+ git -C third-worktree show "$SHA1" >actual &&
+ test_cmp expected actual
+'
+
test_done
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 13/15] files-backend: make reflog iterator go through per-worktree reflog
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs/files-backend.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 011a7e256..d429f8713 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3314,6 +3314,7 @@ struct files_reflog_iterator {
struct ref_iterator base;
struct dir_iterator *dir_iterator;
+ struct dir_iterator *dir_iterator2;
struct object_id oid;
};
@@ -3334,6 +3335,10 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
if (ends_with(diter->basename, ".lock"))
continue;
+ if (iter->dir_iterator2 &&
+ starts_with(diter->relative_path, "refs/bisect/"))
+ continue;
+
if (read_ref_full(diter->relative_path, 0,
iter->oid.hash, &flags)) {
error("bad ref for %s", diter->path.buf);
@@ -3346,7 +3351,11 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
return ITER_OK;
}
- iter->dir_iterator = NULL;
+ iter->dir_iterator = iter->dir_iterator2;
+ if (iter->dir_iterator2) {
+ iter->dir_iterator2 = NULL;
+ return files_reflog_iterator_advance(ref_iterator);
+ }
if (ref_iterator_abort(ref_iterator) == ITER_ERROR)
ok = ITER_ERROR;
return ok;
@@ -3367,6 +3376,12 @@ static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)
if (iter->dir_iterator)
ok = dir_iterator_abort(iter->dir_iterator);
+ if (iter->dir_iterator2) {
+ int ok2 = dir_iterator_abort(iter->dir_iterator2);
+ if (ok2 == ITER_ERROR)
+ ok = ok2;
+ }
+
base_ref_iterator_free(ref_iterator);
return ok;
}
@@ -3389,6 +3404,13 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
files_path(refs, &sb, "logs");
iter->dir_iterator = dir_iterator_begin(sb.buf);
strbuf_release(&sb);
+
+ if (strcmp(refs->gitdir.buf, refs->gitcommondir.buf)) {
+ strbuf_addf(&sb, "%s/logs", refs->gitdir.buf);
+ iter->dir_iterator2 = dir_iterator_begin(sb.buf);
+ strbuf_release(&sb);
+ }
+
return ref_iterator;
}
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 11/15] revision.c: --all adds HEAD from all worktrees
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
Unless single_worktree is set, --all now adds HEAD from all worktrees.
Since reachable.c code does not use setup_revisions(), we need to call
other_head_refs_submodule() explicitly there to have the same effect on
"git prune", so that we won't accidentally delete objects needed by some
other HEADs.
A new FIXME is added because we would need something like
int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);
in addition to other_head_refs() to handle it, which might require
int get_submodule_worktrees(const char *submodule, int flags);
It could be a separate topic to reduce the scope of this one.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
reachable.c | 1 +
refs.c | 22 ++++++++++++++++++++++
refs.h | 1 +
revision.c | 13 +++++++++++++
submodule.c | 2 ++
t/t5304-prune.sh | 12 ++++++++++++
6 files changed, 51 insertions(+)
diff --git a/reachable.c b/reachable.c
index d0199cace..61a6ec05c 100644
--- a/reachable.c
+++ b/reachable.c
@@ -177,6 +177,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
/* detached HEAD is not included in the list above */
head_ref(add_one_ref, revs);
+ other_head_refs(add_one_ref, revs);
/* Add all reflog info */
if (mark_reflog)
diff --git a/refs.c b/refs.c
index fa2df7a1d..ce165c0ea 100644
--- a/refs.c
+++ b/refs.c
@@ -1676,3 +1676,25 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
return refs->be->rename_ref(refs, oldref, newref, logmsg);
}
+
+int other_head_refs(each_ref_fn fn, void *cb_data)
+{
+ struct worktree **worktrees, **p;
+ int ret = 0;
+
+ worktrees = get_worktrees(0);
+ for (p = worktrees; *p; p++) {
+ struct worktree *wt = *p;
+ struct ref_store *refs;
+
+ if (wt->is_current)
+ continue;
+
+ refs = get_worktree_ref_store(wt);
+ ret = refs_head_ref(refs, fn, cb_data);
+ if (ret)
+ break;
+ }
+ free_worktrees(worktrees);
+ return ret;
+}
diff --git a/refs.h b/refs.h
index 986d408bd..6665e5c57 100644
--- a/refs.h
+++ b/refs.h
@@ -190,6 +190,7 @@ typedef int each_ref_fn(const char *refname,
* stop the iteration.
*/
int head_ref(each_ref_fn fn, void *cb_data);
+int other_head_refs(each_ref_fn fn, void *cb_data);
int for_each_ref(each_ref_fn fn, void *cb_data);
int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
diff --git a/revision.c b/revision.c
index d82c37b44..8ee929cef 100644
--- a/revision.c
+++ b/revision.c
@@ -2105,6 +2105,13 @@ static int handle_revision_pseudo_opt(const char *submodule,
int argcount;
if (submodule) {
+ /*
+ * We need some something like get_submodule_worktrees()
+ * before we can go through all worktrees of a submodule,
+ * .e.g with adding all HEADs from --all, which is not
+ * supported right now, so stick to single worktree.
+ */
+ assert(revs->single_worktree != 0);
refs = get_submodule_ref_store(submodule);
} else
refs = get_main_ref_store();
@@ -2122,6 +2129,12 @@ static int handle_revision_pseudo_opt(const char *submodule,
if (!strcmp(arg, "--all")) {
handle_refs(refs, revs, *flags, refs_for_each_ref);
handle_refs(refs, revs, *flags, refs_head_ref);
+ if (!revs->single_worktree) {
+ struct all_refs_cb cb;
+
+ init_all_refs_cb(&cb, revs, *flags);
+ other_head_refs(handle_one_ref, &cb);
+ }
clear_ref_exclusion(&revs->ref_excludes);
} else if (!strcmp(arg, "--branches")) {
handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
diff --git a/submodule.c b/submodule.c
index 3ce589d55..aed47baaf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1155,6 +1155,8 @@ static int find_first_merges(struct object_array *result, const char *path,
oid_to_hex(&a->object.oid));
init_revisions(&revs, NULL);
rev_opts.submodule = path;
+ /* FIXME: can't handle linked worktrees in submodules yet */
+ revs.single_worktree = path != NULL;
setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);
/* save all revisions from the above list that contain b */
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index cba45c7be..683bdb031 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -292,4 +292,16 @@ test_expect_success 'prune: handle index in multiple worktrees' '
test_cmp second-worktree/blob actual
'
+test_expect_success 'prune: handle HEAD in multiple worktrees' '
+ git worktree add --detach third-worktree &&
+ echo "new blob for third-worktree" >third-worktree/blob &&
+ git -C third-worktree add blob &&
+ git -C third-worktree commit -m "third" &&
+ rm .git/worktrees/third-worktree/index &&
+ test_must_fail git -C third-worktree show :blob &&
+ git prune --expire=now &&
+ git -C third-worktree show HEAD:blob >actual &&
+ test_cmp third-worktree/blob actual
+'
+
test_done
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 12/15] refs: add refs_for_each_reflog[_ent]()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 19 ++++++++++++++-----
refs.h | 3 +++
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index ce165c0ea..622c6b669 100644
--- a/refs.c
+++ b/refs.c
@@ -1592,9 +1592,8 @@ int verify_refname_available(const char *refname,
return refs->be->verify_refname_available(refs, refname, extra, skip, err);
}
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
{
- struct ref_store *refs = get_main_ref_store();
struct ref_iterator *iter;
iter = refs->be->reflog_iterator_begin(refs);
@@ -1602,6 +1601,11 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return do_for_each_ref_iterator(iter, fn, cb_data);
}
+int for_each_reflog(each_ref_fn fn, void *cb_data)
+{
+ return refs_for_each_reflog(get_main_ref_store(), fn, cb_data);
+}
+
int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
void *cb_data)
{
@@ -1611,12 +1615,17 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
fn, cb_data);
}
+int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
+ each_reflog_ent_fn fn, void *cb_data)
+{
+ return refs->be->for_each_reflog_ent(refs, refname, fn, cb_data);
+}
+
int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
void *cb_data)
{
- struct ref_store *refs = get_main_ref_store();
-
- return refs->be->for_each_reflog_ent(refs, refname, fn, cb_data);
+ return refs_for_each_reflog_ent(get_main_ref_store(),
+ refname, fn, cb_data);
}
int reflog_exists(const char *refname)
diff --git a/refs.h b/refs.h
index 6665e5c57..5c1b99596 100644
--- a/refs.h
+++ b/refs.h
@@ -572,5 +572,8 @@ int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
+ each_reflog_ent_fn fn, void *cb_data);
#endif /* REFS_H */
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH 10/15] refs: remove dead for_each_*_submodule()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Michael Haggerty, Stefan Beller,
Johannes Schindelin, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217141908.18012-1-pclouds@gmail.com>
These are used in revision.c. After the last patch they are replaced
with the refs_ version. Delete them (except for_each_remote_ref_submodule
which is still used by submodule.c)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 30 ------------------------------
refs.h | 9 ---------
2 files changed, 39 deletions(-)
diff --git a/refs.c b/refs.c
index 37b03d4ff..fa2df7a1d 100644
--- a/refs.c
+++ b/refs.c
@@ -310,12 +310,6 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)
return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
}
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return refs_for_each_tag_ref(get_submodule_ref_store(submodule),
- fn, cb_data);
-}
-
int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
{
return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data);
@@ -326,12 +320,6 @@ int for_each_branch_ref(each_ref_fn fn, void *cb_data)
return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
}
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return refs_for_each_branch_ref(get_submodule_ref_store(submodule),
- fn, cb_data);
-}
-
int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
{
return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data);
@@ -1169,11 +1157,6 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
return 0;
}
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return refs_head_ref(get_submodule_ref_store(submodule), fn, cb_data);
-}
-
int head_ref(each_ref_fn fn, void *cb_data)
{
return refs_head_ref(get_main_ref_store(), fn, cb_data);
@@ -1212,12 +1195,6 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(get_main_ref_store(), "", fn, 0, 0, cb_data);
}
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(get_submodule_ref_store(submodule),
- "", fn, 0, 0, cb_data);
-}
-
int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
each_ref_fn fn, void *cb_data)
{
@@ -1239,13 +1216,6 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
prefix, fn, 0, flag, cb_data);
}
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(get_submodule_ref_store(submodule),
- prefix, fn, strlen(prefix), 0, cb_data);
-}
-
int for_each_replace_ref(each_ref_fn fn, void *cb_data)
{
return do_for_each_ref(get_main_ref_store(),
diff --git a/refs.h b/refs.h
index 8fc82deda..986d408bd 100644
--- a/refs.h
+++ b/refs.h
@@ -202,15 +202,6 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
const char *prefix, void *cb_data);
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
-int for_each_ref_submodule(const char *submodule,
- each_ref_fn fn, void *cb_data);
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn fn, void *cb_data);
-int for_each_tag_ref_submodule(const char *submodule,
- each_ref_fn fn, void *cb_data);
-int for_each_branch_ref_submodule(const char *submodule,
- each_ref_fn fn, void *cb_data);
int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn fn, void *cb_data);
--
2.11.0.157.gd943d85
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox