Re: [PATCH v14 01/16] test: add test cases for relative_path
2013/6/25 Junio C Hamano gits...@pobox.com: Add prefix _ to workaround the absolute path rewritten issue in msysGit is interesting, but these test cases have already been tested in Linux, right? The most important thing is what we want to test in these tests. The special test program is to try running the underlying relative_path() by driving it directly, but the only real caller is in setup_work_tree(), where two return values from real_path() is compared. On POSIX systems, we know we are feeding paths that both begin with /. Now, on Windows systems, what do we get back from real_path()? C:\git\Documentation? /git/Documentation? Confirm that on Windows real_path() returns absolute path in Windows style, such as: C:/msysgit/git/.git Unlike relative_path() used in setup.c:setup_work_tree(), for path_relative() from quote.c, IIRC, the expected inputs are both full pathnames within the working tree. A typical question the callers to this function asks is like The current directory obtained from prefix is the Documentation/ directory and we need to show that compat/mkdir.c is modified, relative to the current directory. ../compat/mkdir.c is what I want to show. So in that sense, it does not matter if /a/b/c is given as /a/b/c or C:\a\b\c as we do not care the leading common part (either / or C:\) very much. On the other hand, the test vector you preoared in the first test that all begin with / may not be very useful to make sure that the function behaves the same way before and after your rewrite. Yes, I should add more test cases without the leading '/'. -- Jiang Xin -- 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: Splitting a rev list into 2 sets
Hello Thomas,
On Mon, Jun 24, 2013 at 11:59 AM, Thomas Rast tr...@inf.ethz.ch wrote:
Francis Moreau francis.m...@gmail.com writes:
On Thu, Jun 20, 2013 at 3:20 PM, Thomas Rast tr...@inf.ethz.ch wrote:
positive=$(git rev-parse $@ | grep -v '^\^')
negative=$(git rev-parse $@ | grep '^\^')
boundary=$(git rev-list --boundary $positive ^master | sed -n 's/^-//p')
# the intersection is
git rev-list $boundary $negative
I think there's a minor issue here, when boundary is empty. Please
correct me if I'm wrong but I think it can only happen if positive is
simply master or a subset of master. In that case I think the solution
is just make boundary equal to positive:
# the intersection is
git rev-list ${boundary:-$positive} $negative
Now I'm going to see if that solution is faster than the initial one.
Jan jast Krüger pointed out on #git that
git log $(git merge-base --all A B)
is exactly the set of commits reachable from both A and B; so there's
your intersection operator :-)
nice :)
So it would seem that a much simpler approach is
git rev-list $(git merge-base --all master $positive) --not $negative
avoiding the boundary handling and special-case. It relies on the
(weird?) property that $(git merge-base --all A B1 B2 ...) shows the
merge bases of A with a hypothetical merge of B1, B2, ..., which is just
what you need here.
Thank you Thomas, that's exactly what I was asking for :)
--
Francis
--
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: Another core.safecrlf behavor with git diff/git status
Hi, Le 24.06.2013 18:55, Junio C Hamano a écrit : Yann Droneaud ydrone...@opteya.com writes: - Why git diff does not always report the CRLF/LF mismatch ? Most likely because you are telling safecrlf not to error out but just warn, and then you are not fixing the cause of the warning? So diff would say Ok, you must know what you are doing, so I trust what is in the index, perhaps? - Why git status does not report about the CRLF/LF mismatch before updating the index: My suspicion is the same as diff. I'm ok with theses answers regarding the test case provided first: the warning was emitted when the files were commited. (But still I would like git diff/git status to behave the same regarding the index: emit the warning and update the index, I suppose it's not related to core.safecrlf but inner Git way of working). Could you have a look at the other test case I've sent later in this thread / the rebase problem I've sent earlier in another thread. Regards. -- Yann Droneaud OPTEYA -- 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 v6 2/5] t7102 (reset): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t7102-reset.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index df82ec9..05dfb27 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -192,7 +192,8 @@ test_expect_success \ 'changing files and redo the last commit should succeed' ' echo 3rd line 2nd file secondfile git commit -a -C ORIG_HEAD - check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d + head4=$(git rev-parse --verify HEAD) + check_changes $head4 test $(git rev-parse ORIG_HEAD) = \ $head5 ' @@ -211,7 +212,7 @@ test_expect_success \ git reset --hard HEAD~2 check_changes ddaefe00f1da16864591c61fdc7adb5d7cd6b74e test $(git rev-parse ORIG_HEAD) = \ - 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d + $head4 ' .diff_expect @@ -326,10 +327,11 @@ test_expect_success '--hard reset to HEAD should clear a failed merge' ' git checkout branch2 echo 3rd line in branch2 secondfile git commit -a -m change in branch2 + head3=$(git rev-parse --verify HEAD) test_must_fail git pull . branch1 git reset --hard - check_changes 77abb337073fb4369a7ad69ff6f5ec0e4d6b54bb + check_changes $head3 ' .diff_expect -- 1.8.3.1.15.g5c23c1e -- 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 v6 0/5] Reroll patches against v1.8.3.1
It's all started here [http://thread.gmane.org/gmane.comp.version-control.git/177634] and recently continued later [http://thread.gmane.org/gmane.comp.version-control.git/214419] v6 of this patch series includes Junio's suggestions against v5: 1. [PATCH v6 1/5] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs using modern 'git rev-parse HEAD:' git syntax to get commit tree ID 2. [PATCH v6 2/5] t7102 (reset): don't hardcode SHA-1 in expected outputs untouched 3. [PATCH v6 3/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs using 'rev-parse --short' instead of 'git rev-list --max-count=1 --abbrev-commit' 4. [PATCH v6 4/5] pretty: Add failing tests: --format output should honor logOutputEncoding iso8859-5 encoding reverted back to cp1251 encoding (as it was in v4 series) reworded log message 5. [PATCH v6 5/5] pretty: --format output should honor logOutputEncoding reworded log message Alexey Shumkin (5): t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs t7102 (reset): don't hardcode SHA-1 in expected outputs t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs pretty: Add failing tests: --format output should honor logOutputEncoding pretty: --format output should honor logOutputEncoding builtin/reset.c | 6 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 1 + log-tree.c | 1 + submodule.c | 1 + t/t4041-diff-submodule-option.sh | 25 +++-- t/t4205-log-pretty-formats.sh| 179 - t/t6006-rev-list-format.sh | 207 --- t/t7102-reset.sh | 37 ++- 9 files changed, 293 insertions(+), 165 deletions(-) -- 1.8.3.1.15.g5c23c1e -- 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 v6 5/5] pretty: --format output should honor logOutputEncoding
One can set an alias
$ git config [--global] alias.lg log --graph
--pretty=format:'%Cred%h%Creset
-%C(yellow)%d%Creset %s %Cgreen(%cd) %C(bold blue)%an%Creset'
--abbrev-commit --date=local
to see the log as a pretty tree (like *gitk* but in a terminal).
However, log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted even when i18n.logOutputEncoding
and terminal encoding are the same (e.g. log messages committed on a Cygwin box
with Windows-1251 encoding seen on a Linux box with a UTF-8 encoding and vice
versa).
To simplify an example we can say the following two commands are expected
to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it formats %s.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
and
$ git reset --hard
This patch makes pretty --format honor logOutputEncoding when it formats
log message.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
builtin/reset.c | 6 --
builtin/rev-list.c | 1 +
builtin/shortlog.c | 1 +
log-tree.c | 1 +
submodule.c | 1 +
t/t4041-diff-submodule-option.sh | 10 +-
t/t4205-log-pretty-formats.sh| 34 +-
t/t6006-rev-list-format.sh | 8
t/t7102-reset.sh | 2 +-
9 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..a6cacc6 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -92,11 +92,12 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
static void print_new_head_line(struct commit *commit)
{
- const char *hex, *body;
+ const char *hex, *body, *msg;
hex = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
printf(_(HEAD is now at %s), hex);
- body = strstr(commit-buffer, \n\n);
+ msg = logmsg_reencode(commit, NULL, get_log_output_encoding());
+ body = strstr(msg, \n\n);
if (body) {
const char *eol;
size_t len;
@@ -107,6 +108,7 @@ static void print_new_head_line(struct commit *commit)
}
else
printf(\n);
+ logmsg_free(msg, commit);
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 67701be..a5ec30d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -111,6 +111,7 @@ static void show_commit(struct commit *commit, void *data)
ctx.date_mode = revs-date_mode;
ctx.date_mode_explicit = revs-date_mode_explicit;
ctx.fmt = revs-commit_format;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, buf);
if (revs-graph) {
if (buf.len) {
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1fd6f8a..1434f8f 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -137,6 +137,7 @@ void shortlog_add_commit(struct shortlog *log, struct
commit *commit)
ctx.subject = ;
ctx.after_subject = ;
ctx.date_mode = DATE_NORMAL;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, ufbuf);
buffer = ufbuf.buf;
} else if (*buffer) {
diff --git a/log-tree.c b/log-tree.c
index 1946e9c..5277d3e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -616,6 +616,7 @@ void show_log(struct rev_info *opt)
ctx.fmt = opt-commit_format;
ctx.mailmap = opt-mailmap;
ctx.color = opt-diffopt.use_color;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(ctx, commit, msgbuf);
if (opt-add_signoff)
diff --git a/submodule.c b/submodule.c
index 1821a5b..78734e1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -226,6 +226,7 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
while ((commit = get_revision(rev))) {
struct pretty_print_context ctx = {0};
ctx.date_mode = rev-date_mode;
+ ctx.output_encoding = get_log_output_encoding();
strbuf_setlen(sb, 0);
strbuf_addstr(sb, line_prefix);
if (commit-object.flags SYMMETRIC_LEFT) {
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 67afb86..9ba4b8e 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -94,7 +94,7 @@ test_expect_success 'diff.submodule does not affect plumbing'
'
commit_file sm1
head2=$(add_file sm1 foo3)
[PATCH v6 4/5] pretty: Add failing tests: --format output should honor logOutputEncoding
One can set an alias
$ git config alias.lg log --graph --pretty=format:'%Cred%h%Creset
-%C(yellow)%d%Creset %s %Cgreen(%cd) %C(bold blue)%an%Creset'
--abbrev-commit --date=local
to see the log as a pretty tree (like *gitk* but in a terminal).
However, log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted even when i18n.logOutputEncoding
and terminal encoding are the same (e.g. log messages committed on a Cygwin box
with Windows-1251 encoding seen on a Linux box with a UTF-8 encoding and vice
versa).
To simplify an example we can say the following two commands are expected
to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it formats %s.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
and
$ git reset --hard
This patch adds failing tests for the next patch that fixes them.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
t/t4041-diff-submodule-option.sh | 35 +
t/t4205-log-pretty-formats.sh| 149 ---
t/t6006-rev-list-format.sh | 83 +++---
t/t7102-reset.sh | 29 +++-
4 files changed, 199 insertions(+), 97 deletions(-)
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 32d4a60..67afb86 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -1,6 +1,7 @@
#!/bin/sh
#
# Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)
#
test_description='Support for verbose submodule differences in git diff
@@ -10,6 +11,9 @@ This test tries to verify the sanity of the --submodule
option of git diff.
. ./test-lib.sh
+# String added in Russian, encoded in UTF-8, used in
+# sample commit log messages in add_file() function below.
+added=$(printf
\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275)
add_file () {
(
cd $1
@@ -19,7 +23,8 @@ add_file () {
echo $name $name
git add $name
test_tick
- git commit -m Add $name || exit
+ msg_added_cp1251=$(echo Add $name ($added $name) |
iconv -f utf-8 -t cp1251)
+ git -c 'i18n.commitEncoding=cp1251' commit -m
$msg_added_cp1251
done /dev/null
git rev-parse --short --verify HEAD
)
@@ -89,29 +94,29 @@ test_expect_success 'diff.submodule does not affect
plumbing' '
commit_file sm1
head2=$(add_file sm1 foo3)
-test_expect_success 'modified submodule(forward)' '
+test_expect_failure 'modified submodule(forward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
-test_expect_success 'modified submodule(forward)' '
+test_expect_failure 'modified submodule(forward)' '
git diff --submodule=log actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
-test_expect_success 'modified submodule(forward) --submodule' '
+test_expect_failure 'modified submodule(forward) --submodule' '
git diff --submodule actual
cat expected -EOF
Submodule sm1 $head1..$head2:
- Add foo3
+ Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
@@ -138,25 +143,25 @@ head3=$(
git rev-parse --short --verify HEAD
)
-test_expect_success 'modified submodule(backward)' '
+test_expect_failure 'modified submodule(backward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head2..$head3 (rewind):
- Add foo3
- Add foo2
+ Add foo3 ($added foo3)
+ Add foo2 ($added foo2)
EOF
test_cmp expected actual
'
head4=$(add_file sm1 foo4 foo5)
-test_expect_success 'modified submodule(backward and forward)' '
+test_expect_failure 'modified submodule(backward and forward)' '
git diff-index -p --submodule=log HEAD actual
cat expected -EOF
Submodule sm1 $head2...$head4:
- Add foo5
- Add foo4
- Add foo3
- Add foo2
+ Add foo5 ($added foo5)
+ Add foo4 ($added foo4)
+ Add foo3 ($added foo3)
+ Add foo2 ($added foo2)
EOF
test_cmp expected actual
'
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index
[PATCH v6 1/5] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use
them instead of hardcoding.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
t/t6006-rev-list-format.sh | 140 +
1 file changed, 77 insertions(+), 63 deletions(-)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 0393c9f..cc1008d 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -7,8 +7,19 @@ test_description='git rev-list --pretty=format test'
test_tick
test_expect_success 'setup' '
-touch foo git add foo git commit -m added foo
- echo changed foo git commit -a -m changed foo
+ : foo
+ git add foo
+ git commit -m added foo
+ head1=$(git rev-parse --verify HEAD)
+ head1_short=$(git rev-parse --verify --short $head1)
+ tree1=$(git rev-parse --verify HEAD:)
+ tree1_short=$(git rev-parse --verify --short $tree1)
+ echo changed foo
+ git commit -a -m changed foo
+ head2=$(git rev-parse --verify HEAD)
+ head2_short=$(git rev-parse --verify --short $head2)
+ tree2=$(git rev-parse --verify HEAD:)
+ tree2_short=$(git rev-parse --verify --short $tree2)
'
# usage: test_format name format_string expected_output
@@ -32,49 +43,49 @@ has_no_color () {
test_cmp expect $1
}
-test_format percent %%h 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format percent %%h EOF
+commit $head2
%h
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
%h
EOF
-test_format hash %H%n%h 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-131a310eb913d107dd3c09a65d1651175898735d
-131a310
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
+test_format hash %H%n%h EOF
+commit $head2
+$head2
+$head2_short
+commit $head1
+$head1
+$head1_short
EOF
-test_format tree %T%n%t 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-fe722612f26da5064c32ca3843aa154bdb0b08a0
-fe72261
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-4d5fcadc293a348e88f777dc0920f11e7d71441c
-4d5fcad
+test_format tree %T%n%t EOF
+commit $head2
+$tree2
+$tree2_short
+commit $head1
+$tree1
+$tree1_short
EOF
-test_format parents %P%n%p 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format parents %P%n%p EOF
+commit $head2
+$head1
+$head1_short
+commit $head1
EOF
# we don't test relative here
-test_format author %an%n%ae%n%ad%n%aD%n%at 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format author %an%n%ae%n%ad%n%aD%n%at EOF
+commit $head2
A U Thor
aut...@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
A U Thor
aut...@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -82,14 +93,14 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format committer %cn%n%ce%n%cd%n%cD%n%ct 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format committer %cn%n%ce%n%cd%n%cD%n%ct EOF
+commit $head2
C O Mitter
commit...@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
C O Mitter
commit...@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -97,43 +108,43 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format encoding %e 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format encoding %e EOF
+commit $head2
+commit $head1
EOF
-test_format subject %s 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format subject %s EOF
+commit $head2
changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
added foo
EOF
-test_format body %b 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format body %b EOF
+commit $head2
+commit $head1
EOF
-test_format raw-body %B 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format raw-body %B EOF
+commit $head2
changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
added foo
EOF
-test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy EOF
+commit $head2
[31mfoo[32mbar[34mbaz[mxyzzy
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
[31mfoo[32mbar[34mbaz[mxyzzy
EOF
-test_format advanced-colors '%C(red yellow bold)foo%C(reset)' 'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format advanced-colors '%C(red yellow bold)foo%C(reset)' EOF
+commit $head2
[1;31;43mfoo[m
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
[1;31;43mfoo[m
EOF
@@ -186,39 +197,42 @@ This commit
[PATCH v6 3/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com --- t/t4205-log-pretty-formats.sh | 48 +++ 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 26fbfde..73ba5e8 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -101,7 +101,11 @@ test_expect_failure 'NUL termination with --stat' ' test_expect_success 'setup more commits' ' test_commit message one one one message-one - test_commit message two two two message-two + test_commit message two two two message-two + head1=$(git rev-parse --verify --short HEAD~0) + head2=$(git rev-parse --verify --short HEAD~1) + head3=$(git rev-parse --verify --short HEAD~2) + head4=$(git rev-parse --verify --short HEAD~3) ' test_expect_success 'left alignment formatting' ' @@ -117,18 +121,18 @@ EOF test_cmp expected actual ' -test_expect_success 'left alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'left alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message twoZ -7cd6c63 message oneZ -1711bf9 add barZ -af20c06 initialZ +$head1 message twoZ +$head2 message oneZ +$head3 add barZ +$head4 initialZ EOF test_cmp expected actual -' + test_expect_success 'left alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual @@ -195,18 +199,18 @@ EOF test_cmp expected actual ' -test_expect_success 'right alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'right alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message two -7cd6c63 message one -1711bf9 add bar -af20c06 initial +$head1 message two +$head2 message one +$head3 add bar +$head4 initial EOF test_cmp expected actual -' + test_expect_success 'right alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual @@ -234,18 +238,18 @@ EOF test_cmp expected actual ' -test_expect_success 'center alignment formatting at the nth column' ' - git log --pretty=format:%h %|(40)%s actual +test_expect_success 'center alignment formatting at the nth column' + git log --pretty='format:%h %|(40)%s' actual # complete the incomplete line at the end echo actual qz_to_tab_space \EOF expected -fa33ab1 message two Z -7cd6c63 message one Z -1711bf9 add barZ -af20c06 initialZ +$head1 message two Z +$head2 message one Z +$head3 add barZ +$head4 initialZ EOF test_cmp expected actual -' + test_expect_success 'center alignment formatting with no padding' ' git log --pretty=format:%(1)%s actual -- 1.8.3.1.15.g5c23c1e -- 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 13/14] t/t5515-fetch-merge-logic: don't use {branches,remotes}-file
Santi Béjar wrote:
If you think it's not worth testing the merge logic with remotes in
{branches,remotes}-files you can consider just removing these remotes and
the associated result files.
Thanks, but Junio has already decided to keep them and dropped these patches.
--
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 v14 03/16] quote.c: remove path_relative, use relative_path instead
2013/6/25 Junio C Hamano gits...@pobox.com:
Jiang Xin worldhello@gmail.com writes:
Since there is an enhanced version of relative_path() in path.c,
remove duplicate counterpart path_relative() in quote.c.
There is no nice comparison chart before and after like you had in
patch 02/16?
You mean drawing a table to compare output of path_relative and
relative_path?
I will rewrite the commit log for patch 03/16 like the following. Need
to polish spellings and grammars.
quote.c: substitute path_relative with relative_path
Substitute the function path_relative in quote.c with the function
relative_path. Function relative_path can be treated as an enhanced
and robust version of path_relative.
Outputs of path_relative and it's replacement (relative_path) are the
same for the following cases:
path prefix output of path_relative output of relative_path
= === ===
/a/b/c/ /a/b/ c/ c/
/a/b/c/a/b/ cc
/a/ /a/b/ ../ ../
/ /a/b/ ../../ ../../
/a/c /a/b/ ../c ../c
/x/y /a/b/ ../../x/y../../x/y
a/b/c/a/b/ c/ c/
a/a/b/ ../ ../
x/y a/b/ ../../x/y ../../x/y
/a/b (empty)/a/b /a/b
/a/b (null) /a/b /a/b
a/b (empty)a/b a/b
a/b (null) a/b a/b
But if both of the path and the prefix are the same, or the returned
relative path should be the current directory, the outputs of both
functions are different. Function relative_path returns ./, while
function path_relative returns empty string.
path prefix output of path_relative output of relative_path
= === ===
/a/b/ /a/b/ (empty) ./
a/b/ a/b/ (empty) ./
(empty) (null) (empty) ./
(empty) (empty)(empty) ./
But not panic, the callers of path_relative can handle such cases, or
never encounter this issue at all. E.g.
* In function quote_path_relative, if the output of path_relative is
empty, append ./ to it, like:
if (!out-len)
strbuf_addstr(out, ./);
* Another caller is write_name_quoted_relative, which is only used by
builtin/ls-files.c. git-ls-files only show files, so path of files
will never be identical with the prefix of a directory.
The following differences show that path_relative can not handle
extra slashes properly.
path prefix output of path_relative output of relative_path
= === ===
/a//b//c/ //a/b//../../../../a//b//c/ c/
a/b//ca//b ../b//c c
And if prefix has no trailing slash, path_relative can not work properly
either. But since prefix always has a trailing slash, so it's not a
problem.
path prefix output of path_relative output of relative_path
= === ===
/a/b/c/ /a/b b/c/ c/
/a/b /a/b b./
/a/b/ /a/b b/ ./
/a/a/b/ ../../a ../
a/b/c/a/bb/c/ c/
a/b/ a/bb/ ./
a a/b../a ../
x/y a/b/ ../x/y ../../x/y
a/c a/bc../c
/a/ /a/b (empty) ../
(empty) /a/b ../../ ./
void write_name_quoted_relative(const char *name, size_t len,
const char *prefix, size_t prefix_len,
FILE *fp, int terminator)
{
struct strbuf sb = STRBUF_INIT;
- name = path_relative(name, len, sb, prefix, prefix_len);
+ name = relative_path(name, prefix, sb);
Are we sure nobody calls prefix_len pointing into the middle of
string, not at the end of prefix? This is unsafe for such a
caller, and to make sure we catch them, we should remove the
now-unused prefix_len parameter from this function.
Next two commits will remove the unused parameters, and
make this series of patches easy to review. But indeed this
commit has flaws, next two commits are fixes. Should I squash
them back?
--
Jiang Xin
--
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 v14 04/16] Refactor quote_path_relative, remove unused params
2013/6/25 Junio C Hamano gits...@pobox.com:
Jiang Xin worldhello@gmail.com writes:
After substitute path_relative() in quote.c with relative_path() from
path.c, parameters (such as len and prefix_len) are obsolete in function
quote_path_relative(). Remove unused parameters and change the order of
parameters for quote_path_relative() function.
...
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..f77f95 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (remove_dirs(directory, prefix, rm_flags,
dry_run, quiet, gone))
errors++;
if (gone !quiet) {
- qname =
quote_path_relative(directory.buf, directory.len, buf, prefix);
+ qname =
quote_path_relative(directory.buf, prefix, buf);
printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
This one needed a bit closer look to make sure directory.len is
already pointing at the end of directory.buf (it is) to verify that
this is a safe conversion. Everywhere else we lost either -1 or
strlen() of the string we feed quote_path() and quote_path_relative()
so they look fine.
When tracking in remove_dirs(directory, prefix, rm_flags, dry_run,
quiet, gone),
there are some suspect strbuf_setlens, like:
strbuf_setlen(path, len);
strbuf_addstr(path, e-d_name);
and at the end of remove_dirs, there is:
strbuf_setlen(path, original_len);
So both the but and len will be restored in the end, and directory.len always
points to strlen of directory.buf.
So it's safe for the following refactor:
- qname =
quote_path_relative(directory.buf, directory.len, buf, prefix);
+ qname =
quote_path_relative(directory.buf, prefix, buf);
--
Jiang Xin
--
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 v14 05/16] Refactor write_name_quoted_relative, remove unused params
2013/6/25 Junio C Hamano gits...@pobox.com:
Jiang Xin worldhello@gmail.com writes:
-static void write_name(const char* name, size_t len)
+static void write_name(const char *name)
{
- write_name_quoted_relative(name, len, prefix, prefix_len, stdout,
- line_terminator);
+
+ /* turn off prefix, if run with --full-name */
+ write_name_quoted_relative(name, prefix_len ? prefix : NULL,
+stdout, line_terminator);
Hmmm
Does this mean that ls-files has been broken in 03/16, because
write_name_quoted_relative() was made to ignore prefix_len and
measure the length of prefix with strlen(prefix), and this change
fixes it?
I will rewrite and move this chunk to patch 03/16, but leave others
in this patch for clearity.
--
蒋鑫
北京群英汇信息技术有限公司
邮件: worldhello@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 18601196889
--
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 10/16] pack-objects: use bitmaps when packing objects
Vicent Marti wrote:
$ time ../git/git pack-objects --all --stdout
Counting objects: 3053537, done.
Compressing objects: 100% (495706/495706), done.
Total 3053537 (delta 2529614), reused 3053537 (delta 2529614)
real0m36.686s
user0m34.440s
sys 0m2.184s
$ time ../git/git pack-objects --all --stdout
Counting objects: 3053537, done.
Compressing objects: 100% (495706/495706), done.
Total 3053537 (delta 2529614), reused 3053537 (delta 2529614)
real0m7.255s
user0m6.892s
sys 0m0.444s
Awesome work! Can you put up this series on gh:vmg so I can try it
out for myself?
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b7cab18..469b8da 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
+ if (!strcmp(k, pack.usebitmaps)) {
+ bitmap_support = git_config_bool(k, v);
+ return 0;
+ }
Not using config_error_nonbool() to indicate an error?
+ if (use_bitmap_index) {
+ uint32_t size_hint;
+
+ if (!prepare_bitmap_walk(revs, size_hint)) {
+ khint_t new_hash_size = (size_hint * (1.0 /
__ac_HASH_UPPER)) + 0.5;
How does this work? You've taken the inverse of __ac_HASH_UPPER,
multiplied it by the size_hint you get from prepare_bitmap_walk(), and
add 0.5?
+ kh_resize_sha1(packed_objects, new_hash_size);
So packed_objects is a hashtable of type kh_sha1_t * (you introduced
in [03/16]) that you're now resizing to new_hash_size. To find out
what the significance of this new_hash_size is, it looks like I have
to read prepare_bitmap_walk().
+ nr_alloc = (size_hint + 63) ~63;
+ objects = xrealloc(objects, nr_alloc * sizeof(struct
object_entry *));
Interesting. The only other place where we realloc the objects in
this file is in pack-objects.c:949, and we do that because nr_object
= nr_alloc. What is this 63 magic?
if (prepare_revision_walk(revs))
die(revision walk setup failed);
+
Stray newline.
+ if (bitmap_support) {
+ if (use_internal_rev_list pack_to_stdout)
+ use_bitmap_index = 1;
+ }
+
Wait, what does pack_to_stdout have to do with deciding whether or not
to walk the bitmap?
diff --git a/pack-bitmap.c b/pack-bitmap.c
new file mode 100644
index 000..090db15
--- /dev/null
+++ b/pack-bitmap.c
+struct stored_bitmap {
+ unsigned char sha1[20];
+ struct ewah_bitmap *root;
+ struct stored_bitmap *xor;
+ int flags;
+};
What exactly is this? What is stored_bitmap *xor? It looks like some
sort of next-pointer, but why is it named xor?
+struct bitmap_index {
Okay, the bitmap index.
+ struct ewah_bitmap *commits;
+ struct ewah_bitmap *trees;
+ struct ewah_bitmap *blobs;
+ struct ewah_bitmap *tags;
I might be asking a really stupid question here, but why do you have
different bitmaps for different object types? Unless I'm mistaken,
the packfile index doesn't make this differentiation: it sorts and
stores the SHA-1s of the various objects; you request a SHA-1, it does
a binary search and returns the object.
+ khash_sha1 *bitmaps;
A hashmap keyed with the SHA-1, I presume.
+ struct packed_git *pack;
You're defining which pack this bitmap index is for, right?
+ struct {
+ struct object_array entries;
+ khash_sha1 *map;
+ } fake_index;
What is this?
+ struct bitmap *result;
No clue what this is about.
+ int entry_count;
No clue what this is, but I'm assuming it can't be important because
it's an int.
+ char pack_checksum[20];
+
+ int version;
Use something invariant like uint32_t? Also, there is no clear
indication about where this information is going to go (header,
presumably?). Look at pack.h:
struct pack_idx_header {
uint32_t idx_signature;
uint32_t idx_version;
};
+ unsigned loaded : 1,
+native_bitmaps : 1,
+has_hash_cache : 1;
Booleans, but I don't know what they're doing here even after reading
your bitmap-format.txt.
+ struct ewah_bitmap *(*read_bitmap)(struct bitmap_index *index);
I'm very confused now. Each bitmap_index has a specialized read_bitmap()?
+ void *map;
+ size_t map_size, map_pos;
+
+ uint32_t *delta_hashes;
I'll give up on the rest.
+static struct bitmap_index bitmap_git;
You could have made the struct static to begin with and ended it with
a } bitmap_git;
+static struct ewah_bitmap *
+lookup_stored_bitmap(struct stored_bitmap *st)
Please conform to Linux style and make it easier for us to grep by
putting this in one line?
+{
+ struct ewah_bitmap *parent;
+ struct
Re: [PATCH 07/16] compat: add endinanness helpers
Vicent Marti: The POSIX standard doesn't currently define a `nothll`/`htonll` function pair to perform network-to-host and host-to-network swaps of 64-bit data. These 64-bit swaps are necessary for the on-disk storage of EWAH bitmaps if they are not in native byte order. endian(3) claims that glibc 2.9+ define be64toh() and htobe64() which should do what you are looking for. The manual page does mention them being named differently across OSes, though, so you may need to be careful with that. -- \\// Peter - http://www.softwolves.pp.se/ -- 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 07/16] compat: add endinanness helpers
On Tue, Jun 25, 2013 at 3:08 PM, Peter Krefting pe...@softwolves.pp.se wrote: endian(3) claims that glibc 2.9+ define be64toh() and htobe64() which should do what you are looking for. The manual page does mention them being named differently across OSes, though, so you may need to be careful with that. I'm aware of that, but Git needs to build with glibc 2.7+ (or was it 2.6?), hence the need for this compat layer. -- 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: Basic git-archive --remote question
On Mon, 2013-06-24 at 15:53 -0400, Greg Freemyer wrote: I'm trying to create a tarball from a git tag and I can't get the syntax right. The documentation is not very clear. Can someone help me? == details git v1.8.1.4 The upstream git repo is at: https://github.com/dkovar/analyzeMFT Here's a few attempts using git as the protocol: git archive --format=tar --remote=github.com:dkovar/analyzeMFT.git v2.0.4 Permission denied (publickey). fatal: The remote end hung up unexpectedly Assuming you haven't set up any ssh rules for the github.com host, you're trying to log in with ssh with your local username, which isn't going to work. git archive --format=tar --remote=git://github.com/dkovar/analyzeMFT v2.0.4 fatal: remote error Your Git client has made an invalid request: 003agit-upload-archive /dkovar/analyzeMFT This is the right format. GitHub doesn't allow remote archive requests, which is why it's complaining. If you want a tarball from GitHub, you need to download over HTTP from the links they provide (which you can find e.g. through the web UI). The github page also says I can use ssh with git as the user, but that complains I don't have the private key (which I don't): git archive --format=tar --remote=ssh://git@github/com/dkovar/analyzeMFT.git v2.0.4 Using git as the ssh user is the right thing (if you want to talk git over ssh) with GitHub and a few other sites/hosting programs, as your public key is used to determine which user is trying to connect without giving each user an account on the underlying OS. cmn -- 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 v15 02/16] path.c: refactor relative_path(), not only strip prefix
Original design of relative_path() is simple, just strip the prefix
(*base) from the absolute path (*abs). In most cases, we need a real
relative path, such as: ../foo, ../../bar. That's why there is another
reimplementation (path_relative()) in quote.c.
Borrowed some codes from path_relative() in quote.c to refactor
relative_path() in path.c, so that it could return real relative path,
and user can reuse this function without reimplement his/her own.
The function path_relative() in quote.c will be substituted, and I
would use the new relative_path() function when implement the
interactive git-clean later.
Different results for relative_path() before and after this refactor:
abs path base path relative (original) relative (refactor)
= === ===
/a/b/c/ /a/b c/ c/
/a/b//c/ //a///b/ c/ c/
/a/b /a/b ../
/a/b/ /a/b ../
/a/a/b/ /a ../
/ /a/b/ /../../
/a/c /a/b/ /a/c ../c
/x/y /a/b/ /x/y ../../x/y
/a/b (empty)/a/b /a/b
/a/b (null) /a/b /a/b
a/b/c/a/b/ c/ c/
a/b/c/a/bc/ c/
a/b//ca//b cc
a/b/ a/b/ ../
a/b/ a/b../
a a/ba../
x/y a/b/ x/y ../../x/y
a/c a/ba/c ../c
a/b (empty)a/b a/b
a/b (null) a/b a/b
(empty) (null) (empty) ./
(empty) (empty)(empty) ./
(empty) /a/b (empty) ./
(null)(null) (null) ./
(null)(empty)(null) ./
(null)/a/b (segfault) ./
You may notice that return value . has been changed to ./.
It is because:
* Function quote_path_relative() in quote.c will show the relative
path as ./ if abs(in) and base(prefix) are the same.
* Function relative_path() is called only once (in setup.c), and
it will be OK for the return value as ./ instead of ..
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 2 +-
path.c| 116 +-
setup.c | 5 ++-
t/t0060-path-utils.sh | 39 -
test-path-utils.c | 4 +-
5 files changed, 113 insertions(+), 53 deletions(-)
diff --git a/cache.h b/cache.h
index dd0fb..c2886 100644
--- a/cache.h
+++ b/cache.h
@@ -758,7 +758,7 @@ int is_directory(const char *);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
-const char *relative_path(const char *abs, const char *base);
+const char *relative_path(const char *in, const char *prefix, struct strbuf
*sb);
int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
diff --git a/path.c b/path.c
index 04ff..fbe52c9b 100644
--- a/path.c
+++ b/path.c
@@ -441,42 +441,100 @@ int adjust_shared_perm(const char *path)
return 0;
}
-const char *relative_path(const char *abs, const char *base)
+/*
+ * Give path as relative to prefix.
+ *
+ * The strbuf may or may not be used, so do not assume it contains the
+ * returned path.
+ */
+const char *relative_path(const char *in, const char *prefix,
+ struct strbuf *sb)
{
- static char buf[PATH_MAX + 1];
- int i = 0, j = 0;
-
- if (!base || !base[0])
- return abs;
- while (base[i]) {
- if (is_dir_sep(base[i])) {
- if (!is_dir_sep(abs[j]))
- return abs;
- while (is_dir_sep(base[i]))
+ int in_len = in ? strlen(in) : 0;
+ int prefix_len = prefix ? strlen(prefix) : 0;
+ int in_off = 0;
+ int prefix_off = 0;
+ int i = 0,j = 0;
+
+ if (!in_len)
+ return ./;
+ else if (!prefix_len)
+ return in;
+
+ while (i prefix_len j in_len prefix[i] == in[j]) {
+ if (is_dir_sep(prefix[i])) {
+ while (is_dir_sep(prefix[i]))
i++;
- while (is_dir_sep(abs[j]))
+ while (is_dir_sep(in[j]))
j++;
+ prefix_off = i;
+ in_off = j;
+ } else {
+
[PATCH v15 00/16] Interactive git clean
Update since v14: * Add more testcases for relative_path. See patch 01/16. * Refactor: change arguments name for relative_path (in path.c), i.e. abc - in, base - prefix. This is because the first argument is not restricted to absolute path any more. See patch 02/16. * Move git-ls-files fix from patch 05/16 to patch 03/16. See patch 03/16. * Remove NO_MINGW and related functions from t0060 script. See patch 16/16. Jiang Xin (16): test: add test cases for relative_path path.c: refactor relative_path(), not only strip prefix quote.c: substitute path_relative with relative_path Refactor quote_path_relative, remove unused params Refactor write_name_quoted_relative, remove unused params git-clean: refactor git-clean into two phases git-clean: add support for -i/--interactive git-clean: show items of del_list in columns git-clean: add colors to interactive git-clean git-clean: use a git-add-interactive compatible UI git-clean: add filter by pattern interactive action git-clean: add select by numbers interactive action git-clean: add ask each interactive action git-clean: add documentation for interactive git-clean test: add t7301 for git-clean--interactive test: run testcases with POSIX absolute paths on Windows Documentation/config.txt | 21 +- Documentation/git-clean.txt | 71 +++- builtin/clean.c | 778 +-- builtin/grep.c | 5 +- builtin/ls-files.c | 17 +- cache.h | 2 +- path.c | 116 +-- quote.c | 65 +--- quote.h | 7 +- setup.c | 5 +- t/t0060-path-utils.sh| 72 +++- t/t7301-clean-interactive.sh | 439 test-path-utils.c| 32 ++ wt-status.c | 17 +- 14 files changed, 1474 insertions(+), 173 deletions(-) create mode 100755 t/t7301-clean-interactive.sh -- 1.8.3.1.756.g2e9b71f -- 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 v15 03/16] quote.c: substitute path_relative with relative_path
Substitute the function path_relative in quote.c with the function
relative_path. Function relative_path can be treated as an enhanced
and robust version of path_relative.
Outputs of path_relative and it's replacement (relative_path) are the
same for the following cases:
path prefix output of path_relative output of relative_path
= === ===
/a/b/c/ /a/b/ c/ c/
/a/b/c/a/b/ cc
/a/ /a/b/ ../ ../
/ /a/b/ ../../ ../../
/a/c /a/b/ ../c ../c
/x/y /a/b/ ../../x/y../../x/y
a/b/c/a/b/ c/ c/
a/a/b/ ../ ../
x/y a/b/ ../../x/y ../../x/y
/a/b (empty)/a/b /a/b
/a/b (null) /a/b /a/b
a/b (empty)a/b a/b
a/b (null) a/b a/b
But if both of the path and the prefix are the same, or the returned
relative path should be the current directory, the outputs of both
functions are different. Function relative_path returns ./, while
function path_relative returns empty string.
path prefix output of path_relative output of relative_path
= === ===
/a/b/ /a/b/ (empty) ./
a/b/ a/b/ (empty) ./
(empty) (null) (empty) ./
(empty) (empty)(empty) ./
But not panic, the callers of path_relative can handle such cases, or
never encounter this issue at all. E.g.
* In function quote_path_relative, if the output of path_relative is
empty, append ./ to it, like:
if (!out-len)
strbuf_addstr(out, ./);
* Another caller is write_name_quoted_relative, which is only used by
builtin/ls-files.c. git-ls-files only show files, so path of files
will never be identical with the prefix of a directory.
The following differences show that path_relative can not handle
extra slashes properly.
path prefix output of path_relative output of relative_path
= === ===
/a//b//c/ //a/b//../../../../a//b//c/ c/
a/b//ca//b ../b//c c
And if prefix has no trailing slash, path_relative can not work properly
either. But since prefix always has a trailing slash, so it's not a
problem.
path prefix output of path_relative output of relative_path
= === ===
/a/b/c/ /a/b b/c/ c/
/a/b /a/b b./
/a/b/ /a/b b/ ./
/a/a/b/ ../../a ../
a/b/c/a/bb/c/ c/
a/b/ a/bb/ ./
a a/b../a ../
x/y a/b/ ../x/y ../../x/y
a/c a/bc../c
/a/ /a/b (empty) ../
(empty) /a/b ../../ ./
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Jiang Xin worldhello@gmail.com
---
builtin/ls-files.c | 7 +--
quote.c| 55 ++
2 files changed, 7 insertions(+), 55 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 3a410c..d100 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -48,8 +48,11 @@ static const char *tag_resolve_undo = ;
static void write_name(const char* name, size_t len)
{
- write_name_quoted_relative(name, len, prefix, prefix_len, stdout,
- line_terminator);
+ /* Since prefix_len is ignored in write_name_quoted_relative, we
+* should turn off prefix here in case of running git ls-files
+* with --full-name option */
+ write_name_quoted_relative(name, len, prefix_len ? prefix : NULL,
+ prefix_len, stdout, line_terminator);
}
static void show_dir_entry(const char *tag, struct dir_entry *ent)
diff --git a/quote.c b/quote.c
index 91122..64ff3 100644
--- a/quote.c
+++ b/quote.c
@@ -312,75 +312,24 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,
fputc(terminator, fp);
}
-static const char *path_relative(const char *in, int len,
-struct strbuf *sb, const char *prefix,
-int prefix_len);
-
void write_name_quoted_relative(const char *name, size_t len,
[PATCH v15 05/16] Refactor write_name_quoted_relative, remove unused params
After substitute path_relative() in quote.c with relative_path() from
path.c, parameters (such as len and prefix_len) are obsolete in function
write_name_quoted_relative(). Remove unused parameters from
write_name_quoted_relative() and related functions.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/ls-files.c | 14 +++---
quote.c| 3 +--
quote.h| 3 +--
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index c7d2b..af6cd 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -46,13 +46,13 @@ static const char *tag_modified = ;
static const char *tag_skip_worktree = ;
static const char *tag_resolve_undo = ;
-static void write_name(const char* name, size_t len)
+static void write_name(const char *name)
{
- /* Since prefix_len is ignored in write_name_quoted_relative, we
+ /* Since prefix_len won't pass to write_name_quoted_relative, we
* should turn off prefix here in case of running git ls-files
* with --full-name option */
- write_name_quoted_relative(name, len, prefix_len ? prefix : NULL,
- prefix_len, stdout, line_terminator);
+ write_name_quoted_relative(name, prefix_len ? prefix : NULL,
+ stdout, line_terminator);
}
static void show_dir_entry(const char *tag, struct dir_entry *ent)
@@ -66,7 +66,7 @@ static void show_dir_entry(const char *tag, struct dir_entry
*ent)
return;
fputs(tag, stdout);
- write_name(ent-name, ent-len);
+ write_name(ent-name);
}
static void show_other_files(struct dir_struct *dir)
@@ -166,7 +166,7 @@ static void show_ce_entry(const char *tag, struct
cache_entry *ce)
find_unique_abbrev(ce-sha1,abbrev),
ce_stage(ce));
}
- write_name(ce-name, ce_namelen(ce));
+ write_name(ce-name);
if (debug_mode) {
struct stat_data *sd = ce-ce_stat_data;
@@ -201,7 +201,7 @@ static void show_ru_info(void)
printf(%s%06o %s %d\t, tag_resolve_undo, ui-mode[i],
find_unique_abbrev(ui-sha1[i], abbrev),
i + 1);
- write_name(path, len);
+ write_name(path);
}
}
}
diff --git a/quote.c b/quote.c
index ebb8..5c880 100644
--- a/quote.c
+++ b/quote.c
@@ -312,8 +312,7 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,
fputc(terminator, fp);
}
-void write_name_quoted_relative(const char *name, size_t len,
- const char *prefix, size_t prefix_len,
+void write_name_quoted_relative(const char *name, const char *prefix,
FILE *fp, int terminator)
{
struct strbuf sb = STRBUF_INIT;
diff --git a/quote.h b/quote.h
index 5610159..ed110 100644
--- a/quote.h
+++ b/quote.h
@@ -60,8 +60,7 @@ extern void quote_two_c_style(struct strbuf *, const char *,
const char *, int);
extern void write_name_quoted(const char *name, FILE *, int terminator);
extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,
const char *name, FILE *, int terminator);
-extern void write_name_quoted_relative(const char *name, size_t len,
- const char *prefix, size_t prefix_len,
+extern void write_name_quoted_relative(const char *name, const char *prefix,
FILE *fp, int terminator);
/* quote path as relative to the given prefix */
--
1.8.3.1.756.g2e9b71f
--
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 v15 01/16] test: add test cases for relative_path
Add subcommand relative_path in test-path-utils, and add test cases
in t0060.
Johannes tested this commit on Windows, and found that some relative_path
tests should be skipped on Windows. This is because the bash on Windows
rewrites arguments of regular Windows programs, such as git and the
test helpers, if the arguments look like absolute POSIX paths. As a
consequence, the actual tests performed are not what the tests scripts
expect.
The tests that need *not* be skipped are those where the two paths passed
to 'test-path-utils relative_path' have the same prefix and the result is
expected to be a relative path. This is because the rewriting changes
/a/b to D:/Src/MSysGit/a/b, and when both inputs are extended the same
way, this just cancels out in the relative path computation.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t0060-path-utils.sh | 37 +
test-path-utils.c | 25 +
2 files changed, 62 insertions(+)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 09a42..72e89 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -12,6 +12,11 @@ norm_path() {
test \\$(test-path-utils normalize_path_copy '$1')\ = '$2'
}
+relative_path() {
+ test_expect_success $4 relative path: $1 $2 = $3 \
+ test \\$(test-path-utils relative_path '$1' '$2')\ = '$3'
+}
+
# On Windows, we are using MSYS's bash, which mangles the paths.
# Absolute paths are anchored at the MSYS installation directory,
# which means that the path / accounts for this many characters:
@@ -183,4 +188,36 @@ test_expect_success SYMLINKS 'real path works on symlinks'
'
test $sym = $(test-path-utils real_path $dir2/syml)
'
+relative_path /a/b/c/ /a/b/ c/
+relative_path /a/b/c/ /a/bc/
+relative_path /a//b//c///a/b// c/ POSIX
+relative_path /a/b /a/b.
+relative_path /a/b//a/b.
+relative_path /a /a/b/a POSIX
+relative_path //a/b/ / POSIX
+relative_path /a/c /a/b/ /a/cPOSIX
+relative_path /a/c /a/b/a/cPOSIX
+relative_path /x/y /a/b/ /x/yPOSIX
+relative_path /a/b empty /a/bPOSIX
+relative_path /a/b null/a/bPOSIX
+relative_path a/b/c/ a/b/c/
+relative_path a/b/c/ a/b c/
+relative_path a/b//c a//bc
+relative_path a/b/ a/b/.
+relative_path a/b/ a/b .
+relative_path aa/b a # TODO: should be: ..
+relative_path x/y a/b x/y # TODO: should be: ../../x/y
+relative_path a/c a/b a/c # TODO: should be: ../c
+relative_path a/b empty a/b
+relative_path a/b nulla/b
+relative_path empty/a/b(empty)
+relative_path emptyempty (empty)
+relative_path emptynull(empty)
+relative_path null empty (null)
+relative_path null null(null)
+
+test_expect_failure 'relative path: null /a/b = segfault' '
+ test-path-utils relative_path null /a/b
+'
+
test_done
diff --git a/test-path-utils.c b/test-path-utils.c
index 0092cb..8a6d2 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item
*item, void *unused)
return 1;
}
+static void normalize_argv_string(const char **var, const char *input)
+{
+ if (!strcmp(input, null))
+ *var = NULL;
+ else if (!strcmp(input, empty))
+ *var = ;
+ else
+ *var = input;
+
+ if (*var (**var == '' || **var == '('))
+ die(Bad value: %s\n, input);
+}
+
int main(int argc, char **argv)
{
if (argc == 3 !strcmp(argv[1], normalize_path_copy)) {
@@ -103,6 +116,18 @@ int main(int argc, char **argv)
return 0;
}
+ if (argc == 4 !strcmp(argv[1], relative_path)) {
+ const char *in, *prefix, *rel;
+ normalize_argv_string(in, argv[2]);
+ normalize_argv_string(prefix, argv[3]);
+ rel = relative_path(in, prefix);
+ if (!rel)
+ puts((null));
+ else
+ puts(strlen(rel) 0 ? rel : (empty));
+ return 0;
+ }
+
fprintf(stderr, %s: unknown function name: %s\n, argv[0],
argv[1] ? argv[1] : (there was none));
return 1;
--
1.8.3.1.756.g2e9b71f
--
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 v15 07/16] git-clean: add support for -i/--interactive
Show what would be done and the user must confirm before actually
cleaning.
Would remove ...
Would remove ...
Would remove ...
Remove [y/n]?
Press y to start cleaning, and press n if you want to abort.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-clean.txt | 10 ++--
builtin/clean.c | 57 +
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..186e34 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
SYNOPSIS
[verse]
-'git clean' [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--] path...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--] path...
DESCRIPTION
---
@@ -34,7 +34,13 @@ OPTIONS
-f::
--force::
If the Git configuration variable clean.requireForce is not set
- to false, 'git clean' will refuse to run unless given -f or -n.
+ to false, 'git clean' will refuse to run unless given -f, -n or
+ -i.
+
+-i::
+--interactive::
+ Show what would be done and the user must confirm before actually
+ cleaning.
-n::
--dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 77ec1..698fb 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,10 +15,11 @@
#include quote.h
static int force = -1; /* unset */
+static int interactive;
static struct string_list del_list = STRING_LIST_INIT_DUP;
static const char *const builtin_clean_usage[] = {
- N_(git clean [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
+ N_(git clean [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
NULL
};
@@ -143,6 +144,50 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
return ret;
}
+static void interactive_main_loop(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+ const char *qname;
+
+ while (del_list.nr) {
+ putchar('\n');
+ for_each_string_list_item(item, del_list) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ printf(_(msg_would_remove), qname);
+ }
+ putchar('\n');
+
+ printf(_(Remove [y/n]? ));
+ if (strbuf_getline(confirm, stdin, '\n') != EOF) {
+ strbuf_trim(confirm);
+ } else {
+ /* Ctrl-D is the same as quit */
+ string_list_clear(del_list, 0);
+ putchar('\n');
+ printf_ln(Bye.);
+ break;
+ }
+
+ if (confirm.len) {
+ if (!strncasecmp(confirm.buf, yes, confirm.len)) {
+ break;
+ } else if (!strncasecmp(confirm.buf, no, confirm.len)
||
+ !strncasecmp(confirm.buf, quit,
confirm.len)) {
+ string_list_clear(del_list, 0);
+ printf_ln(Bye.);
+ break;
+ } else {
+ continue;
+ }
+ }
+ }
+
+ strbuf_release(buf);
+ strbuf_release(confirm);
+}
+
int cmd_clean(int argc, const char **argv, const char *prefix)
{
int i, res;
@@ -162,6 +207,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
OPT__QUIET(quiet, N_(do not print names of files removed)),
OPT__DRY_RUN(dry_run, N_(dry run)),
OPT__FORCE(force, N_(force)),
+ OPT_BOOL('i', interactive, interactive, N_(interactive
cleaning)),
OPT_BOOLEAN('d', NULL, remove_directories,
N_(remove whole directories)),
{ OPTION_CALLBACK, 'e', exclude, exclude_list, N_(pattern),
@@ -188,12 +234,12 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (ignored ignored_only)
die(_(-x and -X cannot be used together));
- if (!dry_run !force) {
+ if (!interactive !dry_run !force) {
if (config_set)
- die(_(clean.requireForce set to true and neither -n
nor -f given;
+ die(_(clean.requireForce set to true and neither -i,
-n nor -f given;
refusing to clean));
else
- die(_(clean.requireForce defaults to true and neither
-n nor -f given;
+ die(_(clean.requireForce defaults to true and neither
-i, -n nor -f given;
[PATCH v15 04/16] Refactor quote_path_relative, remove unused params
After substitute path_relative() in quote.c with relative_path() from
path.c, parameters (such as len and prefix_len) are obsolete in function
quote_path_relative(). Remove unused parameters and change the order of
parameters for quote_path_relative() function.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c| 18 +-
builtin/grep.c | 5 ++---
builtin/ls-files.c | 2 +-
quote.c| 7 ++-
quote.h| 4 ++--
wt-status.c| 17 -
6 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..f77f95 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -56,7 +56,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if ((force_flag REMOVE_DIR_KEEP_NESTED_GIT)
!resolve_gitlink_ref(path-buf, HEAD,
submodule_head)) {
if (!quiet) {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
printf(dry_run ? _(msg_would_skip_git_dir) :
_(msg_skip_git_dir),
quoted.buf);
}
@@ -70,7 +70,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
/* an empty dir could be removed even if it is unreadble */
res = dry_run ? 0 : rmdir(path-buf);
if (res) {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
}
@@ -94,7 +94,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if (remove_dirs(path, prefix, force_flag, dry_run,
quiet, gone))
ret = 1;
if (gone) {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
string_list_append(dels, quoted.buf);
} else
*dir_gone = 0;
@@ -102,10 +102,10 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
} else {
res = dry_run ? 0 : unlink(path-buf);
if (!res) {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
string_list_append(dels, quoted.buf);
} else {
- quote_path_relative(path-buf,
strlen(path-buf), quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
ret = 1;
@@ -127,7 +127,7 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
if (!res)
*dir_gone = 1;
else {
- quote_path_relative(path-buf, strlen(path-buf),
quoted, prefix);
+ quote_path_relative(path-buf, prefix, quoted);
warning(_(msg_warn_remove_failed), quoted.buf);
*dir_gone = 0;
ret = 1;
@@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (remove_dirs(directory, prefix, rm_flags,
dry_run, quiet, gone))
errors++;
if (gone !quiet) {
- qname =
quote_path_relative(directory.buf, directory.len, buf, prefix);
+ qname =
quote_path_relative(directory.buf, prefix, buf);
printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
}
}
@@ -272,11 +272,11 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
continue;
res = dry_run ? 0 : unlink(ent-name);
if (res) {
- qname = quote_path_relative(ent-name, -1,
buf, prefix);
+ qname = quote_path_relative(ent-name, prefix,
buf);
warning(_(msg_warn_remove_failed), qname);
errors++;
[PATCH v15 06/16] git-clean: refactor git-clean into two phases
Before introducing interactive git-clean, refactor git-clean operations
into two phases:
* hold cleaning items in del_list,
* and remove them in a separate loop at the end.
We will introduce interactive git-clean between the two phases. The
interactive git-clean will show what would be done and must confirm
before do real cleaning.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 64 -
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index f77f95..77ec1 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,6 +15,7 @@
#include quote.h
static int force = -1; /* unset */
+static struct string_list del_list = STRING_LIST_INIT_DUP;
static const char *const builtin_clean_usage[] = {
N_(git clean [-d] [-f] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
@@ -148,12 +149,13 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
- struct strbuf directory = STRBUF_INIT;
+ struct strbuf abs_path = STRBUF_INIT;
struct dir_struct dir;
static const char **pathspec;
struct strbuf buf = STRBUF_INIT;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct exclude_list *el;
+ struct string_list_item *item;
const char *qname;
char *seen = NULL;
struct option options[] = {
@@ -223,6 +225,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
int matches = 0;
struct cache_entry *ce;
struct stat st;
+ const char *rel;
/*
* Remove the '/' at the end that directory
@@ -242,13 +245,8 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
continue; /* Yup, this one exists unmerged */
}
- /*
-* we might have removed this as part of earlier
-* recursive directory removal, so lstat() here could
-* fail with ENOENT.
-*/
if (lstat(ent-name, st))
- continue;
+ die_errno(Cannot lstat '%s', ent-name);
if (pathspec) {
memset(seen, 0, argc 0 ? argc : 1);
@@ -257,33 +255,61 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
}
if (S_ISDIR(st.st_mode)) {
- strbuf_addstr(directory, ent-name);
if (remove_directories || (matches == MATCHED_EXACTLY))
{
- if (remove_dirs(directory, prefix, rm_flags,
dry_run, quiet, gone))
- errors++;
- if (gone !quiet) {
- qname =
quote_path_relative(directory.buf, prefix, buf);
- printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
- }
+ rel = relative_path(ent-name, prefix, buf);
+ string_list_append(del_list, rel);
}
- strbuf_reset(directory);
} else {
if (pathspec !matches)
continue;
- res = dry_run ? 0 : unlink(ent-name);
+ rel = relative_path(ent-name, prefix, buf);
+ string_list_append(del_list, rel);
+ }
+ }
+
+ /* TODO: do interactive git-clean here, which will modify del_list */
+
+ for_each_string_list_item(item, del_list) {
+ struct stat st;
+
+ if (prefix)
+ strbuf_addstr(abs_path, prefix);
+
+ strbuf_addstr(abs_path, item-string);
+
+ /*
+* we might have removed this as part of earlier
+* recursive directory removal, so lstat() here could
+* fail with ENOENT.
+*/
+ if (lstat(abs_path.buf, st))
+ continue;
+
+ if (S_ISDIR(st.st_mode)) {
+ if (remove_dirs(abs_path, prefix, rm_flags, dry_run,
quiet, gone))
+ errors++;
+ if (gone !quiet) {
+ qname = quote_path_relative(item-string, NULL,
buf);
+ printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
+ }
+ } else {
+ res = dry_run ? 0 :
[PATCH v15 15/16] test: add t7301 for git-clean--interactive
Add test cases for git-clean--interactive. Signed-off-by: Jiang Xin worldhello@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7301-clean-interactive.sh | 439 +++ 1 file changed, 439 insertions(+) create mode 100755 t/t7301-clean-interactive.sh diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh new file mode 100755 index 0..4e605 --- /dev/null +++ b/t/t7301-clean-interactive.sh @@ -0,0 +1,439 @@ +#!/bin/sh + +test_description='git clean -i basic tests' + +. ./test-lib.sh + +test_expect_success 'setup' ' + + mkdir -p src + touch src/part1.c Makefile + echo build .gitignore + echo \*.o .gitignore + git add . + git commit -m setup + touch src/part2.c README + git add . + +' + +test_expect_success 'git clean -i (clean)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo c | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test -f docs/manual.txt + test ! -f src/part3.c + test ! -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -i (quit)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo q | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -i (Ctrl+D)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + echo \04 | git clean -i + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter all)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo *; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo part3.* *.out; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test -f a.out + test ! -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (filter patterns 2)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo f; echo * !*.out; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test -f docs/manual.txt + test -f src/part3.c + test -f src/part3.h + test -f src/part4.c + test -f src/part4.h + test -f obj.o + test -f build/lib.so + +' + +test_expect_success 'git clean -id (select - all)' ' + + mkdir -p build docs + touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \ + docs/manual.txt obj.o build/lib.so + (echo s; echo *; echo; echo c) | \ + git clean -id + test -f Makefile + test -f README + test -f src/part1.c + test -f src/part2.c + test ! -f a.out + test ! -f docs/manual.txt + test ! -f src/part3.c + test ! -f src/part3.h + test ! -f src/part4.c + test ! -f src/part4.h + test -f obj.o + test -f build/lib.so + +' +
[PATCH v15 11/16] git-clean: add filter by pattern interactive action
Add a new action for interactive git-clean: filter by pattern. When the
user chooses this action, user can input space-separated patterns (the
same syntax as gitignore), and each clean candidate that matches with
one of the patterns will be excluded from cleaning. When the user feels
it's OK, presses ENTER and backs to the confirmation dialog.
Signed-off-by: Jiang Xin worldhello@gmail.com
Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 68 +
1 file changed, 68 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index df887..36369 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -614,6 +614,72 @@ static int clean_cmd(void)
return MENU_RETURN_NO_LOOP;
}
+static int filter_by_patterns_cmd(void)
+{
+ struct dir_struct dir;
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf **ignore_list;
+ struct string_list_item *item;
+ struct exclude_list *el;
+ int changed = -1, i;
+
+ for (;;) {
+ if (!del_list.nr)
+ break;
+
+ if (changed)
+ pretty_print_dels();
+
+ clean_print_color(CLEAN_COLOR_PROMPT);
+ printf(_(Input ignore patterns ));
+ clean_print_color(CLEAN_COLOR_RESET);
+ if (strbuf_getline(confirm, stdin, '\n') != EOF)
+ strbuf_trim(confirm);
+ else
+ putchar('\n');
+
+ /* quit filter_by_pattern mode if press ENTER or Ctrl-D */
+ if (!confirm.len)
+ break;
+
+ memset(dir, 0, sizeof(dir));
+ el = add_exclude_list(dir, EXC_CMDL, manual exclude);
+ ignore_list = strbuf_split_max(confirm, ' ', 0);
+
+ for (i = 0; ignore_list[i]; i++) {
+ strbuf_trim(ignore_list[i]);
+ if (!ignore_list[i]-len)
+ continue;
+
+ add_exclude(ignore_list[i]-buf, , 0, el, -(i+1));
+ }
+
+ changed = 0;
+ for_each_string_list_item(item, del_list) {
+ int dtype = DT_UNKNOWN;
+
+ if (is_excluded(dir, item-string, dtype)) {
+ *item-string = '\0';
+ changed++;
+ }
+ }
+
+ if (changed) {
+ string_list_remove_empty_items(del_list, 0);
+ } else {
+ clean_print_color(CLEAN_COLOR_ERROR);
+ printf_ln(_(WARNING: Cannot find items matched by:
%s), confirm.buf);
+ clean_print_color(CLEAN_COLOR_RESET);
+ }
+
+ strbuf_list_free(ignore_list);
+ clear_directory(dir);
+ }
+
+ strbuf_release(confirm);
+ return 0;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -626,6 +692,7 @@ static int help_cmd(void)
clean_print_color(CLEAN_COLOR_HELP);
printf_ln(_(
clean - start cleaning\n
+ filter by pattern - exclude items from deletion\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -641,6 +708,7 @@ static void interactive_main_loop(void)
struct menu_stuff menu_stuff;
struct menu_item menus[] = {
{'c', clean, 0, clean_cmd},
+ {'f', filter by pattern, 0,
filter_by_patterns_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g2e9b71f
--
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 v15 16/16] test: run testcases with POSIX absolute paths on Windows
Some test cases are skipped on Windows by marking with POSIX prereq.
This is because arguments look like absolute paths (such as /a/b)
for regular Windows programs (*.exe executables, no bash scripts)
are changed to Windows paths (like C:/msysgit/a/b).
There is no cygpath nor equivalent on msysGit, but it is easy to
write one. New subcommand mingw_path is added in test-path-utils,
so that we can get the expected absolute paths on Windows. E.g.
COMMAND LINELinux output Windows output
== ===
test-path-utils mingw_path // C:/msysgit
test-path-utils mingw_path /a/b//a/b/ C:/msysgit/a/b/
With this utility, most skipped test cases in t0060 can be turned on
to be tested correctly on Windows.
Signed-off-by: Jiang Xin worldhello@gmail.com
---
t/t0060-path-utils.sh | 44 +++-
test-path-utils.c | 5 +
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 76c779..3a48d 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -8,13 +8,15 @@ test_description='Test various path utilities'
. ./test-lib.sh
norm_path() {
+ expected=$(test-path-utils mingw_path $2)
test_expect_success $3 normalize path: $1 = $2 \
- test \\$(test-path-utils normalize_path_copy '$1')\ = '$2'
+ test \\$(test-path-utils normalize_path_copy '$1')\ = '$expected'
}
relative_path() {
+ expected=$(test-path-utils mingw_path $3)
test_expect_success $4 relative path: $1 $2 = $3 \
- test \\$(test-path-utils relative_path '$1' '$2')\ = '$3'
+ test \\$(test-path-utils relative_path '$1' '$2')\ = '$expected'
}
# On Windows, we are using MSYS's bash, which mangles the paths.
@@ -39,8 +41,8 @@ ancestor() {
test \\$actual\ = '$expected'
}
-# Absolute path tests must be skipped on Windows because due to path mangling
-# the test program never sees a POSIX-style absolute path
+# Some absolute path tests should be skipped on Windows due to path mangling
+# on POSIX-style absolute paths
case $(uname -s) in
*MINGW*)
;;
@@ -73,30 +75,30 @@ norm_path d1/s1//../s2/../../d2 d2
norm_path d1/.../d2 d1/.../d2
norm_path d1/..././../d2 d1/d2
-norm_path / / POSIX
+norm_path / /
norm_path // / POSIX
norm_path /// / POSIX
-norm_path /. / POSIX
+norm_path /. /
norm_path /./ / POSIX
norm_path /./.. ++failed++ POSIX
-norm_path /../. ++failed++ POSIX
+norm_path /../. ++failed++
norm_path /./../.// ++failed++ POSIX
norm_path /dir/.. / POSIX
norm_path /dir/sub/../.. / POSIX
norm_path /dir/sub/../../.. ++failed++ POSIX
-norm_path /dir /dir POSIX
-norm_path /dir// /dir/ POSIX
-norm_path /./dir /dir POSIX
-norm_path /dir/. /dir/ POSIX
-norm_path /dir///./ /dir/ POSIX
-norm_path /dir//sub/.. /dir/ POSIX
-norm_path /dir/sub/../ /dir/ POSIX
+norm_path /dir /dir
+norm_path /dir// /dir/
+norm_path /./dir /dir
+norm_path /dir/. /dir/
+norm_path /dir///./ /dir/
+norm_path /dir//sub/.. /dir/
+norm_path /dir/sub/../ /dir/
norm_path //dir/sub/../. /dir/ POSIX
-norm_path /dir/s1/../s2/ /dir/s2/ POSIX
-norm_path /d1/s1///s2/..//../s3/ /d1/s3/ POSIX
-norm_path /d1/s1//../s2/../../d2 /d2 POSIX
-norm_path /d1/.../d2 /d1/.../d2 POSIX
-norm_path /d1/..././../d2 /d1/d2 POSIX
+norm_path /dir/s1/../s2/ /dir/s2/
+norm_path /d1/s1///s2/..//../s3/ /d1/s3/
+norm_path /d1/s1//../s2/../../d2 /d2
+norm_path /d1/.../d2 /d1/.../d2
+norm_path /d1/..././../d2 /d1/d2
ancestor / / -1
ancestor /foo / 0
@@ -198,8 +200,8 @@ relative_path / /a/b/ ../../
relative_path /a/c /a/b/ ../c
relative_path /a/c /a/b../c
relative_path /x/y /a/b/ ../../x/y
-relative_path /a/b empty /a/bPOSIX
-relative_path /a/b null/a/bPOSIX
+relative_path /a/b empty /a/b
+relative_path /a/b null/a/b
relative_path a/b/c/ a/b/c/
relative_path a/b/c/ a/b c/
relative_path a/b//c a//bc
diff --git a/test-path-utils.c b/test-path-utils.c
index 1bf473..bb975 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -116,6 +116,11 @@ int main(int argc, char **argv)
return 0;
}
+ if (argc == 3 !strcmp(argv[1], mingw_path)) {
+ puts(argv[2]);
+ return 0;
+ }
+
if (argc == 4 !strcmp(argv[1], relative_path)) {
struct strbuf sb = STRBUF_INIT;
const char *in, *prefix, *rel;
--
1.8.3.1.756.g2e9b71f
--
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 v15 08/16] git-clean: show items of del_list in columns
When there are lots of items to be cleaned, it is hard to see them all
in one screen. Show them in columns will solve this problem.
Signed-off-by: Jiang Xin worldhello@gmail.com
Comments-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 4
builtin/clean.c | 49 +++-
2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e203..c415f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -959,6 +959,10 @@ column.branch::
Specify whether to output branch listing in `git branch` in columns.
See `column.ui` for details.
+column.clean::
+ Specify the layout when list items in `git clean -i`, which always
+ shows files and directories in columns. See `column.ui` for details.
+
column.status::
Specify whether to output untracked files in `git status` in columns.
See `column.ui` for details.
diff --git a/builtin/clean.c b/builtin/clean.c
index 698fb..75cc6 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,10 +13,12 @@
#include refs.h
#include string-list.h
#include quote.h
+#include column.h
static int force = -1; /* unset */
static int interactive;
static struct string_list del_list = STRING_LIST_INIT_DUP;
+static unsigned int colopts;
static const char *const builtin_clean_usage[] = {
N_(git clean [-d] [-f] [-i] [-n] [-q] [-e pattern] [-x | -X] [--]
paths...),
@@ -31,8 +33,13 @@ static const char *msg_warn_remove_failed = N_(failed to
remove %s);
static int git_clean_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, clean.requireforce))
+ if (!prefixcmp(var, column.))
+ return git_column_config(var, value, clean, colopts);
+
+ if (!strcmp(var, clean.requireforce)) {
force = !git_config_bool(var, value);
+ return 0;
+ }
return git_default_config(var, value, cb);
}
@@ -144,21 +151,46 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
return ret;
}
-static void interactive_main_loop(void)
+static void pretty_print_dels(void)
{
- struct strbuf confirm = STRBUF_INIT;
- struct strbuf buf = STRBUF_INIT;
+ struct string_list list = STRING_LIST_INIT_DUP;
struct string_list_item *item;
+ struct strbuf buf = STRBUF_INIT;
const char *qname;
+ struct column_options copts;
+
+ for_each_string_list_item(item, del_list) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ string_list_append(list, qname);
+ }
+
+ /*
+* always enable column display, we only consult column.*
+* about layout strategy and stuff
+*/
+ colopts = (colopts ~COL_ENABLE_MASK) | COL_ENABLED;
+ memset(copts, 0, sizeof(copts));
+ copts.indent = ;
+ copts.padding = 2;
+ print_columns(list, colopts, copts);
+ putchar('\n');
+ strbuf_release(buf);
+ string_list_clear(list, 0);
+}
+
+static void interactive_main_loop(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
while (del_list.nr) {
putchar('\n');
- for_each_string_list_item(item, del_list) {
- qname = quote_path_relative(item-string, NULL, buf);
- printf(_(msg_would_remove), qname);
- }
+ printf_ln(Q_(Would remove the following item:,
+Would remove the following items:,
+del_list.nr));
putchar('\n');
+ pretty_print_dels();
+
printf(_(Remove [y/n]? ));
if (strbuf_getline(confirm, stdin, '\n') != EOF) {
strbuf_trim(confirm);
@@ -184,7 +216,6 @@ static void interactive_main_loop(void)
}
}
- strbuf_release(buf);
strbuf_release(confirm);
}
--
1.8.3.1.756.g2e9b71f
--
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 v15 13/16] git-clean: add ask each interactive action
Add a new action for interactive git-clean: ask each. It's just like
the rm -i command, that the user must confirm one by one for each
file or directory to be cleaned.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 36
1 file changed, 36 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index 643a5e..bf03a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -717,6 +717,40 @@ static int select_by_numbers_cmd(void)
return 0;
}
+static int ask_each_cmd(void)
+{
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+ const char *qname;
+ int changed = 0, eof = 0;
+
+ for_each_string_list_item(item, del_list) {
+ /* Ctrl-D should stop removing files */
+ if (!eof) {
+ qname = quote_path_relative(item-string, NULL, buf);
+ printf(_(remove %s? ), qname);
+ if (strbuf_getline(confirm, stdin, '\n') != EOF) {
+ strbuf_trim(confirm);
+ } else {
+ putchar('\n');
+ eof = 1;
+ }
+ }
+ if (!confirm.len || strncasecmp(confirm.buf, yes,
confirm.len)) {
+ *item-string = '\0';
+ changed++;
+ }
+ }
+
+ if (changed)
+ string_list_remove_empty_items(del_list, 0);
+
+ strbuf_release(buf);
+ strbuf_release(confirm);
+ return MENU_RETURN_NO_LOOP;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -731,6 +765,7 @@ static int help_cmd(void)
clean - start cleaning\n
filter by pattern - exclude items from deletion\n
select by numbers - select items to be deleted by
numbers\n
+ ask each- confirm each deletion (like \rm
-i\)\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -748,6 +783,7 @@ static void interactive_main_loop(void)
{'c', clean, 0, clean_cmd},
{'f', filter by pattern, 0,
filter_by_patterns_cmd},
{'s', select by numbers, 0,
select_by_numbers_cmd},
+ {'a', ask each, 0, ask_each_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g2e9b71f
--
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 v15 14/16] git-clean: add documentation for interactive git-clean
Add new section Interactive mode for documentation of interactive git-clean. Signed-off-by: Jiang Xin worldhello@gmail.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-clean.txt | 65 +++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 186e34..5bf76 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -39,8 +39,8 @@ OPTIONS -i:: --interactive:: - Show what would be done and the user must confirm before actually - cleaning. + Show what would be done and clean files interactively. See + ``Interactive mode'' for details. -n:: --dry-run:: @@ -69,6 +69,67 @@ OPTIONS Remove only files ignored by Git. This may be useful to rebuild everything from scratch, but keep manually created files. +Interactive mode + +When the command enters the interactive mode, it shows the +files and directories to be cleaned, and goes into its +interactive command loop. + +The command loop shows the list of subcommands available, and +gives a prompt What now . In general, when the prompt ends +with a single '', you can pick only one of the choices given +and type return, like this: + + +*** Commands *** +1: clean2: filter by pattern3: select by numbers +4: ask each 5: quit 6: help +What now 1 + + +You also could say `c` or `clean` above as long as the choice is unique. + +The main command loop has 6 subcommands. + +clean:: + + Start cleaning files and directories, and then quit. + +filter by pattern:: + + This shows the files and directories to be deleted and issues an + Input ignore patterns prompt. You can input space-seperated + patterns to exclude files and directories from deletion. + E.g. *.c *.h will excludes files end with .c and .h from + deletion. When you are satisfied with the filtered result, press + ENTER (empty) back to the main menu. + +select by numbers:: + + This shows the files and directories to be deleted and issues an + Select items to delete prompt. When the prompt ends with double + '' like this, you can make more than one selection, concatenated + with whitespace or comma. Also you can say ranges. E.g. 2-5 7,9 + to choose 2,3,4,5,7,9 from the list. If the second number in a + range is omitted, all remaining patches are taken. E.g. 7- to + choose 7,8,9 from the list. You can say '*' to choose everything. + Also when you are satisfied with the filtered result, press ENTER + (empty) back to the main menu. + +ask each:: + + This will start to clean, and you must confirm one by one in order + to delete items. Please note that this action is not as efficient + as the above two actions. + +quit:: + + This lets you quit without do cleaning. + +help:: + + Show brief usage of interactive git-clean. + SEE ALSO linkgit:gitignore[5] -- 1.8.3.1.756.g2e9b71f -- 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 v15 10/16] git-clean: use a git-add-interactive compatible UI
Rewrite menu using a new method `list_and_choose`, which is borrowed
from `git-add--interactive.perl`. We will use this framework to add
new actions for interactive git-clean later.
Please NOTE:
* Method `list_and_choose` return an array of integers, and
* it is up to you to free the allocated memory of the array.
* The array ends with EOF.
* If user pressed CTRL-D (i.e. EOF), no selection returned.
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 456
1 file changed, 427 insertions(+), 29 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index dfa99b..df887 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -50,6 +50,36 @@ enum color_clean {
CLEAN_COLOR_ERROR = 5,
};
+#define MENU_OPTS_SINGLETON01
+#define MENU_OPTS_IMMEDIATE02
+#define MENU_OPTS_LIST_ONLY04
+
+struct menu_opts {
+ const char *header;
+ const char *prompt;
+ int flags;
+};
+
+#define MENU_RETURN_NO_LOOP10
+
+struct menu_item {
+ char hotkey;
+ const char *title;
+ int selected;
+ int (*fn)();
+};
+
+enum menu_stuff_type {
+ MENU_STUFF_TYPE_STRING_LIST = 1,
+ MENU_STUFF_TYPE_MENU_ITEM
+};
+
+struct menu_stuff {
+ enum menu_stuff_type type;
+ int nr;
+ void *stuff;
+};
+
static int parse_clean_color_slot(const char *var)
{
if (!strcasecmp(var, reset))
@@ -240,54 +270,422 @@ static void pretty_print_dels(void)
copts.indent = ;
copts.padding = 2;
print_columns(list, colopts, copts);
- putchar('\n');
strbuf_release(buf);
string_list_clear(list, 0);
}
-static void interactive_main_loop(void)
+static void pretty_print_menus(struct string_list *menu_list)
+{
+ unsigned int local_colopts = 0;
+ struct column_options copts;
+
+ local_colopts = COL_ENABLED | COL_ROW;
+ memset(copts, 0, sizeof(copts));
+ copts.indent = ;
+ copts.padding = 2;
+ print_columns(menu_list, local_colopts, copts);
+}
+
+static void prompt_help_cmd(int singleton)
+{
+ clean_print_color(CLEAN_COLOR_HELP);
+ printf_ln(singleton ?
+ _(Prompt help:\n
+ 1 - select a numbered item\n
+ foo- select item based on unique prefix\n
+ - (empty) select nothing) :
+ _(Prompt help:\n
+ 1 - select a single item\n
+ 3-5- select a range of items\n
+ 2-3,6-9- select multiple ranges\n
+ foo- select item based on unique prefix\n
+ -... - unselect specified items\n
+ * - choose all items\n
+ - (empty) finish selecting));
+ clean_print_color(CLEAN_COLOR_RESET);
+}
+
+/*
+ * display menu stuff with number prefix and hotkey highlight
+ */
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+{
+ struct string_list menu_list = STRING_LIST_INIT_DUP;
+ struct strbuf menu = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct menu_item *menu_item;
+ struct string_list_item *string_list_item;
+ int i;
+
+ switch (stuff-type) {
+ default:
+ die(Bad type of menu_staff when print menu);
+ case MENU_STUFF_TYPE_MENU_ITEM:
+ menu_item = (struct menu_item *)stuff-stuff;
+ for (i = 0; i stuff-nr; i++, menu_item++) {
+ const char *p;
+ int highlighted = 0;
+
+ p = menu_item-title;
+ if ((*chosen)[i] 0)
+ (*chosen)[i] = menu_item-selected ? 1 : 0;
+ strbuf_addf(menu, %s%2d: , (*chosen)[i] ? * : ,
i+1);
+ for (; *p; p++) {
+ if (!highlighted *p == menu_item-hotkey) {
+ strbuf_addstr(menu,
clean_get_color(CLEAN_COLOR_PROMPT));
+ strbuf_addch(menu, *p);
+ strbuf_addstr(menu,
clean_get_color(CLEAN_COLOR_RESET));
+ highlighted = 1;
+ } else {
+ strbuf_addch(menu, *p);
+ }
+ }
+ string_list_append(menu_list, menu.buf);
+ strbuf_reset(menu);
+ }
+ break;
+ case MENU_STUFF_TYPE_STRING_LIST:
+ i = 0;
+ for_each_string_list_item(string_list_item, (struct string_list
*)stuff-stuff) {
+ if ((*chosen)[i] 0)
+
[PATCH] column doc: rewrite documentation for column.ui
The configuration option column.ui is very poorly documented, and it is unclear what the defaults are, and what option can be combined with what. Rewrite it by splitting up the options into three sections clearly showing how COL_ENABLED, COL_LAYOUT_MASK, and COL_DENSE work. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Documentation/config.txt | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1153585..5a10169 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -934,6 +934,9 @@ column.ui:: This variable consists of a list of tokens separated by spaces or commas: + +These three options control when the feature should be enabled +(defaults to 'never'): ++ -- `always`;; always show in columns @@ -941,19 +944,30 @@ column.ui:: never show in columns `auto`;; show in columns if the output is to the terminal +-- ++ +These three options control layout (defaults to 'column'). Setting +layout implies 'always' if none of 'always', 'never', or 'auto' are +specified. ++ +-- `column`;; - fill columns before rows (default) + fill columns before rows `row`;; fill rows before columns `plain`;; show in one column +-- ++ +Finally, these options can be combined with a layout option (defaults +to 'nodense'): ++ +-- `dense`;; make unequal size columns to utilize more space `nodense`;; make equal size columns -- -+ -This option defaults to 'never'. column.branch:: Specify whether to output branch listing in `git branch` in columns. -- 1.8.3.1.585.g9832cb9 -- 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 v15 12/16] git-clean: add select by numbers interactive action
Draw a multiple choice menu using `list_and_choose` to select items
to be deleted by numbers.
User can input:
* 1,5-7 : select 1,5,6,7 items to be deleted
* * : select all items to be deleted
* -*: unselect all, nothing will be deleted
*: (empty) finish selecting, and return back to main menu
Signed-off-by: Jiang Xin worldhello@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clean.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/builtin/clean.c b/builtin/clean.c
index 36369..643a5e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -680,6 +680,43 @@ static int filter_by_patterns_cmd(void)
return 0;
}
+static int select_by_numbers_cmd(void)
+{
+ struct menu_opts menu_opts;
+ struct menu_stuff menu_stuff;
+ struct string_list_item *items;
+ int *chosen;
+ int i, j;
+
+ menu_opts.header = NULL;
+ menu_opts.prompt = N_(Select items to delete);
+ menu_opts.flags = 0;
+
+ menu_stuff.type = MENU_STUFF_TYPE_STRING_LIST;
+ menu_stuff.stuff = del_list;
+ menu_stuff.nr = del_list.nr;
+
+ chosen = list_and_choose(menu_opts, menu_stuff);
+ items = del_list.items;
+ for (i = 0, j = 0; i del_list.nr; i++) {
+ if (i chosen[j]) {
+ *(items[i].string) = '\0';
+ } else if (i == chosen[j]) {
+ /* delete selected item */
+ j++;
+ continue;
+ } else {
+ /* end of chosen (chosen[j] == EOF), won't delete */
+ *(items[i].string) = '\0';
+ }
+ }
+
+ string_list_remove_empty_items(del_list, 0);
+
+ free(chosen);
+ return 0;
+}
+
static int quit_cmd(void)
{
string_list_clear(del_list, 0);
@@ -693,6 +730,7 @@ static int help_cmd(void)
printf_ln(_(
clean - start cleaning\n
filter by pattern - exclude items from deletion\n
+ select by numbers - select items to be deleted by
numbers\n
quit- stop cleaning\n
help- this screen\n
? - help for prompt selection
@@ -709,6 +747,7 @@ static void interactive_main_loop(void)
struct menu_item menus[] = {
{'c', clean, 0, clean_cmd},
{'f', filter by pattern, 0,
filter_by_patterns_cmd},
+ {'s', select by numbers, 0,
select_by_numbers_cmd},
{'q', quit, 0, quit_cmd},
{'h', help, 0, help_cmd},
};
--
1.8.3.1.756.g2e9b71f
--
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 v15 09/16] git-clean: add colors to interactive git-clean
Show header, help, error messages, and prompt in colors for interactive
git-clean. Re-use config variables, such as color.interactive and
color.interactive.slot for command `git-add--interactive`.
Signed-off-by: Jiang Xin worldhello@gmail.com
Comments-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/config.txt | 17 +--
builtin/clean.c | 73 +++-
2 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c415f..1b31f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -879,16 +879,17 @@ The values of these variables may be specified as in
color.branch.slot.
color.interactive::
When set to `always`, always use colors for interactive prompts
- and displays (such as those used by git-add --interactive).
- When false (or `never`), never. When set to `true` or `auto`, use
- colors only when the output is to the terminal. Defaults to false.
+ and displays (such as those used by git-add --interactive and
+ git-clean --interactive). When false (or `never`), never.
+ When set to `true` or `auto`, use colors only when the output is
+ to the terminal. Defaults to false.
color.interactive.slot::
- Use customized color for 'git add --interactive'
- output. `slot` may be `prompt`, `header`, `help` or `error`, for
- four distinct types of normal output from interactive
- commands. The values of these variables may be specified as
- in color.branch.slot.
+ Use customized color for 'git add --interactive' and 'git clean
+ --interactive' output. `slot` may be `prompt`, `header`, `help`
+ or `error`, for four distinct types of normal output from
+ interactive commands. The values of these variables may be
+ specified as in color.branch.slot.
color.pager::
A boolean to enable/disable colored output when the pager is in
diff --git a/builtin/clean.c b/builtin/clean.c
index 75cc6..dfa99b 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -14,6 +14,7 @@
#include string-list.h
#include quote.h
#include column.h
+#include color.h
static int force = -1; /* unset */
static int interactive;
@@ -31,16 +32,82 @@ static const char *msg_skip_git_dir = N_(Skipping
repository %s\n);
static const char *msg_would_skip_git_dir = N_(Would skip repository %s\n);
static const char *msg_warn_remove_failed = N_(failed to remove %s);
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+ GIT_COLOR_RESET,
+ GIT_COLOR_NORMAL, /* PLAIN */
+ GIT_COLOR_BOLD_BLUE,/* PROMPT */
+ GIT_COLOR_BOLD, /* HEADER */
+ GIT_COLOR_BOLD_RED, /* HELP */
+ GIT_COLOR_BOLD_RED, /* ERROR */
+};
+enum color_clean {
+ CLEAN_COLOR_RESET = 0,
+ CLEAN_COLOR_PLAIN = 1,
+ CLEAN_COLOR_PROMPT = 2,
+ CLEAN_COLOR_HEADER = 3,
+ CLEAN_COLOR_HELP = 4,
+ CLEAN_COLOR_ERROR = 5,
+};
+
+static int parse_clean_color_slot(const char *var)
+{
+ if (!strcasecmp(var, reset))
+ return CLEAN_COLOR_RESET;
+ if (!strcasecmp(var, plain))
+ return CLEAN_COLOR_PLAIN;
+ if (!strcasecmp(var, prompt))
+ return CLEAN_COLOR_PROMPT;
+ if (!strcasecmp(var, header))
+ return CLEAN_COLOR_HEADER;
+ if (!strcasecmp(var, help))
+ return CLEAN_COLOR_HELP;
+ if (!strcasecmp(var, error))
+ return CLEAN_COLOR_ERROR;
+ return -1;
+}
+
static int git_clean_config(const char *var, const char *value, void *cb)
{
if (!prefixcmp(var, column.))
return git_column_config(var, value, clean, colopts);
+ /* honors the color.interactive* config variables which also
+ applied in git-add--interactive and git-stash */
+ if (!strcmp(var, color.interactive)) {
+ clean_use_color = git_config_colorbool(var, value);
+ return 0;
+ }
+ if (!prefixcmp(var, color.interactive.)) {
+ int slot = parse_clean_color_slot(var +
+ strlen(color.interactive.));
+ if (slot 0)
+ return 0;
+ if (!value)
+ return config_error_nonbool(var);
+ color_parse(value, var, clean_colors[slot]);
+ return 0;
+ }
+
if (!strcmp(var, clean.requireforce)) {
force = !git_config_bool(var, value);
return 0;
}
- return git_default_config(var, value, cb);
+
+ /* inspect the color.ui config variable and others */
+ return git_color_default_config(var, value, cb);
+}
+
+static const char *clean_get_color(enum color_clean ix)
+{
+ if (want_color(clean_use_color))
+
[PATCH] .mailmap: Map H.Merijn Brand to H. Merijn Brand
This patch was created by searching for duplicates of email addresses
in the shortlog by
git shortlog -sne |awk '{ print $NF }' |sort |uniq -d
This will yield all email addresses, which are found multiple times within
the shortlog. We can assume that commiters having the same email address
are indeed the same person.
Signed-off-by: Stefan Beller stefanbel...@googlemail.com
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index 48d7acf..345cce6 100644
--- a/.mailmap
+++ b/.mailmap
@@ -24,6 +24,7 @@ Eric S. Raymond e...@thyrsus.com
Erik Faye-Lund kusmab...@gmail.com kusmab...@googlemail.com
Fredrik Kuivinen freku...@student.liu.se
Frédéric Heitzmann frederic.heitzm...@gmail.com
+H. Merijn Brand h.m.br...@xs4all.nl H.Merijn Brand h.m.br...@xs4all.nl
H. Peter Anvin h...@bonde.sc.orionmulti.com
H. Peter Anvin h...@tazenda.sc.orionmulti.com
H. Peter Anvin h...@trantor.hos.anvin.org
--
1.8.3.1.587.g803fe36
--
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] Do not ignore merge options in interactive rebase
Arnaud Fontaine ar...@debian.org writes:
Fix inconsistency where `--strategy` and/or `--strategy-option` can be
specified in git rebase, but with `--interactive` argument only there
were completely ignored.
Signed-off-by: Arnaud Fontaine ar...@debian.org
---
git-rebase--interactive.sh| 13 ++---
t/t3404-rebase-interactive.sh | 11 +++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..e558397 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -80,6 +80,13 @@ amend=$state_dir/amend
rewritten_list=$state_dir/rewritten-list
rewritten_pending=$state_dir/rewritten-pending
+strategy_args=
+if test -n $do_merge
+then
+ strategy_args=${strategy+--strategy=$strategy}
+ $(echo $strategy_opts | sed s/'--\([^']*\)'/-X\1/g)
+fi
The ${var+if_set_use_this} interpolates if $var is set (as opposed to
unset); this will cause it to have --strategy= in strategy_args
when $do_merge is set. If you ran t3421, you would have noticed
breakage.
You should use ${var:+if_set_to_nonempty_use_this} here.
stragety_opts is designed to be a valid shell scriptlet that can be
inserted as a part of eval'ed string. git-rebase.sh does this:
-X)
shift
strategy_opts=$strategy_opts $(git rev-parse --sq-quote
--$1)
do_merge=t
so that git-rebase--merge.sh can use it like so:
eval 'git-merge-$strategy' $strategy_opts '$cmt^ -- $hd $cmt'
without having to worry about quoting issues when future strategy
options have letters that need quoting, e.g.
$ git rebase -X foo=bar ' baz
git rev-parse --sq-quote turns it into '--foo=bar '\'' baz' and then
in the eval'ed string, it becomes a single argument given to the
git-merge-$strategy command, even though it may have spaces and
single quotes and other characters that may be tricky to quote
manually without mistakes.
So munging it manually with sloppy sed script is simply wrong.
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
@@ -239,7 +246,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $empty_args $ff $@
+ output git cherry-pick $strategy_args $empty_args $ff $@
}
pick_one_preserving_merges () {
@@ -341,7 +348,7 @@ pick_one_preserving_merges () {
# No point in merging the first parent, that's HEAD
new_parents=${new_parents# $first_parent}
if ! do_with_author output \
- git merge --no-ff ${strategy:+-s $strategy} -m \
+ git merge --no-ff $strategy_args -m \
$msg_content $new_parents
then
printf %s\n $msg_content
$GIT_DIR/MERGE_MSG
@@ -350,7 +357,7 @@ pick_one_preserving_merges () {
echo $sha1 $(git rev-parse HEAD^0)
$rewritten_list
;;
*)
- output git cherry-pick $@ ||
+ output git cherry-pick $strategy_args $@ ||
die_with_patch $sha1 Could not pick $sha1
;;
esac
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 79e8d3c..8b6a36f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects
core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'rebase -i with --strategy and -X' '
+ git checkout -b conflict-merge-use-theirs conflict-branch
+ git reset --hard HEAD^
+ echo five conflict
+ echo Z file1
+ git commit -a -m one file conflict
+ EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch
+ test $(git show conflict-branch:conflict) = $(cat conflict)
+ test $(cat file1) = Z
+'
+
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: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Am 25.06.2013 00:10, schrieb Junio C Hamano: Mark Levedahl mleved...@gmail.com writes: On 06/22/2013 03:38 PM, Ramsay Jones wrote: Also, apart from running the git test-suite, I have the Win32 l/stat functions disabled on all of my repos. In particular, I have core.filemode set to true. (At one point, I used to build git with NO_TRUSTABLE_FILEMODE reset so that I wouldn't have to remember to reset core.filemode by hand after a git-clone or git-init). I should also note that I run MinGW git on the same laptop and, using git.git as an example, it does not seem that much faster (if at all) than cygwin git. After applying your patch to master, I've had the test-suite running in a VM using WinXP + current cygwin (v1.7.x) for about 8 hours, no failures so far but it could take another day to complete. I never found any real speed up using the Win32 stat/lstat functions, and the lack of Posix compatibility breaking cross-platform projects, links, etc., made this function a mis-feature in my opinion for years. As I found no positive benefit from the Win32 lstat, I modified git for local use years ago to set core.filemode=true when cloning / initing to avoid as many issues as possible. So that's two votes to use the vanilla Cygwin stat/lstat, essentially reverting adbc0b6b (cygwin: Use native Win32 API for stat, 2008-09-30), which was added by Dmitry and Shawn while I was away. Let's wait and see if people give us more data points and decide. That'll be more productive if we let the list know ;-) Some context: This is about a patch by Ramsay that removes the schizophrenic lstat hack for Cygwin. Junio, can you please queue that patch in pu? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead
Jiang Xin worldhello@gmail.com writes: 2013/6/25 Junio C Hamano gits...@pobox.com: Jiang Xin worldhello@gmail.com writes: Since there is an enhanced version of relative_path() in path.c, remove duplicate counterpart path_relative() in quote.c. There is no nice comparison chart before and after like you had in patch 02/16? You mean drawing a table to compare output of path_relative and relative_path? I was interested in comparison between the behaviour the current callers of path_relative() gets, and the behaviour the same callers get from the consolidated helper function after your patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] Reroll patches against v1.8.3.1
Alexey Shumkin alex.crez...@gmail.com writes: 4. [PATCH v6 4/5] pretty: Add failing tests: --format output should honor logOutputEncoding iso8859-5 encoding reverted back to cp1251 encoding (as it was in v4 series) Thanks for a reroll, but why this change? The reason I asked you to avoid 8859-5 is because our test do not use that encoding and I do not want to add new dependency to people when they run test. cp1251 shares exactly the same issue, doesn't it? So in that sense, this change does not make anything better. That is why I asked you if the breakage you are trying to demonstrate about non-ASCII non-UTF8 encoding was specific to Cyrillic/Russian. I do not recall seeing your answer, but what is the right thing to do depend on it. - If the answer is yes, then we would need to add dependency either way, and 8859-5 is just as fine as cp1251. - If the breakage is not specific to Cyrillic, it should be reproducible using 8859-1 (latin-1), and our tests already depend on 8859-1, so we wouldn't be adding new dependencies on people. -- 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 09/16] documentation: add documentation for the bitmap format
On Tue, Jun 25, 2013 at 7:42 AM, Shawn Pearce spea...@spearce.org wrote: I very much hate seeing a file format that is supposed to be portable that supports both big-endian and little-endian encoding. Well, the bitmap index is not supposed to be portable, as it doesn't get sent over the wire in any situation. Regardless, the format is portable because it supports both encodings and clearly defines which one the current file is using. I think that's a good tradeoff! Such a specification forces everyone to implement two code paths to handle reading data from the file, on the off-chance they are on the wrong platform. Extra code paths have never been an issue in the JGitAbstractFactoryGeneratorOfGit, har har har. Ah. I'm such a funny guy when it comes to Java. Anyway, I designed this keeping JGit in mind. In this specific case, it doesn't force you to add any new code paths. The endianness changes only affect the serialization format of the bitmaps, which is not part of Git or JGit itself but of the Javaewah/libewok library. The interface for reading on that library has already been wisely abstracted on JGit (https://github.com/eclipse/jgit/blob/master/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackBitmapIndexV1.java#L133), so changing the byte order simply means changing the SimpleDataInput to a LE one. I agree this is not ideal, or elegant, but I'm having a hard time making an argument for sacrificing objective speed for the sake of subjective simplicity. What is wrong with picking one encoding and sticking to it? It prevents us from making this optimally fast on the machines where it needs to be. Regardless, I must admit I haven't generated numbers for this in a while (the BE-LE switch is one of the first optimizations I did). I'm going to try to re-implement full NWO loading and see how much slower it is, before I continue arguing for/against it. If I can get it within a reasonable margin (say, 15%) of the current implementation, I'd definitely be in favor of sticking to only NWO on the whole file. If it's slower than that, well, Git has never compromised on speed, and I don't think there's a point to be made for starting to do that now. + - BITMAP_OPT_HASH_CACHE (0x8) + If present, a hash cache for finding delta bases will be available + right after the header block in this index. See the following + section for details. + + 4-byte entry count (network byte order) + + The total count of entries (bitmapped commits) in this bitmap index. + + 20-byte checksum + + The SHA1 checksum of the pack this bitmap index belongs to. + + - An OPTIONAL delta cache follows the header. Some may find the name delta cache confusing as it does not cache deltas of objects. May I suggest path hash cache as an alternative name? Definitely, this is a typo. + The cache is formed by `n` 4-byte hashes in a row, where `n` is + the amount of objects in the indexed packfile. Note that this amount + is the **total number of objects** and is not related to the + number of commits that have been selected and indexed in the + bitmap index. + + The hashes are stored in Network Byte Order and they are the same + values generated by a normal revision walk during the `pack-objects` + phase. I find it interesting this is network byte order and not big-endian or little-endian based on the flag in the header. As stated before, the flag in the header only affects the Javaewah/libewok interface. Everything Git-related in the bitmap index is in NWO, like it is customary in Git. + The `n`nth hash in the cache is the name hash for the `n`th object + in the index for the indexed packfile. + + [RATIONALE]: + + The bitmap index allows us to skip the Counting Objects phase + during `pack-objects` and yield all the OIDs that would be reachable + (WANTS) when generating the pack. + + This optimization, however, means that we're adding objects to the + packfile straight from the packfile index, and hence we are lacking + path information for the objects that would normally be generated + during the Counting Objects phase. + + This path information for each object is hashed and used as a very + effective way to find good delta bases when compressing the packfile; + without these hashes, the resulting packfiles are much less optimal. + + By storing all the hashes in a cache together with the bitmapsin + the bitmap index, we can yield not only the SHA1 of all
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
On 25.06.13 21:23, Johannes Sixt wrote: Am 25.06.2013 00:10, schrieb Junio C Hamano: Mark Levedahl mleved...@gmail.com writes: On 06/22/2013 03:38 PM, Ramsay Jones wrote: Also, apart from running the git test-suite, I have the Win32 l/stat functions disabled on all of my repos. In particular, I have core.filemode set to true. (At one point, I used to build git with NO_TRUSTABLE_FILEMODE reset so that I wouldn't have to remember to reset core.filemode by hand after a git-clone or git-init). I should also note that I run MinGW git on the same laptop and, using git.git as an example, it does not seem that much faster (if at all) than cygwin git. After applying your patch to master, I've had the test-suite running in a VM using WinXP + current cygwin (v1.7.x) for about 8 hours, no failures so far but it could take another day to complete. I never found any real speed up using the Win32 stat/lstat functions, and the lack of Posix compatibility breaking cross-platform projects, links, etc., made this function a mis-feature in my opinion for years. As I found no positive benefit from the Win32 lstat, I modified git for local use years ago to set core.filemode=true when cloning / initing to avoid as many issues as possible. So that's two votes to use the vanilla Cygwin stat/lstat, essentially reverting adbc0b6b (cygwin: Use native Win32 API for stat, 2008-09-30), which was added by Dmitry and Shawn while I was away. Let's wait and see if people give us more data points and decide. That'll be more productive if we let the list know ;-) Some context: This is about a patch by Ramsay that removes the schizophrenic lstat hack for Cygwin. Junio, can you please queue that patch in pu? -- Hannes Here some benchmarks: Clone the linux kernel from August 2012 (which was in house) to the windows box, 2.3 GhZ Core duo. Run with and without core.filemode, which triggers cygwinfstricks (and is shorter to type) Numbers are typical hot cache, cold cache is 6..8 seconds real $ git --version git version 1.8.3.1.g6f17ca7 __ $ time git -c core.filemode=true status -uno # On branch master # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: include/linux/netfilter/xt_CONNMARK.h # modified: include/linux/netfilter/xt_DSCP.h # modified: include/linux/netfilter/xt_MARK.h # modified: include/linux/netfilter/xt_RATEEST.h # modified: include/linux/netfilter/xt_TCPMSS.h # modified: include/linux/netfilter_ipv4/ipt_ECN.h # modified: include/linux/netfilter_ipv4/ipt_TTL.h # modified: include/linux/netfilter_ipv6/ip6t_HL.h # modified: net/netfilter/xt_DSCP.c # modified: net/netfilter/xt_HL.c # modified: net/netfilter/xt_RATEEST.c # modified: net/netfilter/xt_TCPMSS.c # no changes added to commit (use git add and/or git commit -a) real0m2.313s user0m0.577s sys 0m1.765s -- tb@snoopy ~/projects/linux-2.6 $ time git -c core.filemode=false status -uno # On branch master # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # # modified: include/linux/netfilter/xt_CONNMARK.h # modified: include/linux/netfilter/xt_DSCP.h # modified: include/linux/netfilter/xt_MARK.h # modified: include/linux/netfilter/xt_RATEEST.h # modified: include/linux/netfilter/xt_TCPMSS.h # modified: include/linux/netfilter_ipv4/ipt_ECN.h # modified: include/linux/netfilter_ipv4/ipt_TTL.h # modified: include/linux/netfilter_ipv6/ip6t_HL.h # modified: net/netfilter/xt_DSCP.c # modified: net/netfilter/xt_HL.c # modified: net/netfilter/xt_RATEEST.c # modified: net/netfilter/xt_TCPMSS.c # no changes added to commit (use git add and/or git commit -a) real0m1.047s user0m0.311s sys 0m0.765s /Torsten -- 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] Do not ignore merge options in interactive rebase
Junio C Hamano gits...@pobox.com writes:
You should use ${var:+if_set_to_nonempty_use_this} here.
...
So munging it manually with sloppy sed script is simply wrong.
Perhaps something like this on top of your patch squashed in?
The eval magic lets the shell to split $strategy_opts back into
individual words (e.g. --subtree=My Project is a single word),
strip the leading --, and then uses rev-parse --sq-quote again
to turn them into -X 'subtree=My Project' (two words), which can
be inserted into a string later to be eval'ed.
git-rebase--interactive.sh | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e558397..ae2da7a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -83,8 +83,13 @@ rewritten_pending=$state_dir/rewritten-pending
strategy_args=
if test -n $do_merge
then
- strategy_args=${strategy+--strategy=$strategy}
- $(echo $strategy_opts | sed s/'--\([^']*\)'/-X\1/g)
+ strategy_args=${strategy:+--strategy=$strategy}
+ eval '
+ for strategy_opt in '$strategy_opts'
+ do
+ strategy_args=$strategy_args -X$(git rev-parse
--sq-quote ${strategy_opt#--})
+ done
+ '
fi
GIT_CHERRY_PICK_HELP=$resolvemsg
@@ -246,7 +251,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $strategy_args $empty_args $ff $@
+ output eval git cherry-pick $strategy_args $empty_args $ff $@
}
pick_one_preserving_merges () {
@@ -347,9 +352,8 @@ pick_one_preserving_merges () {
msg_content=$(commit_message $sha1)
# No point in merging the first parent, that's HEAD
new_parents=${new_parents# $first_parent}
- if ! do_with_author output \
- git merge --no-ff $strategy_args -m \
- $msg_content $new_parents
+ if ! do_with_author output eval \
+ 'git merge --no-ff $strategy_args -m $msg_content
$new_parents'
then
printf %s\n $msg_content
$GIT_DIR/MERGE_MSG
die_with_patch $sha1 Error redoing merge $sha1
@@ -357,7 +361,7 @@ pick_one_preserving_merges () {
echo $sha1 $(git rev-parse HEAD^0)
$rewritten_list
;;
*)
- output git cherry-pick $strategy_args $@ ||
+ output eval git cherry-pick $strategy_args $@ ||
die_with_patch $sha1 Could not pick $sha1
;;
esac
--
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] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Mon Jun 17 07:27:20 -0700 2013:
Thomas Rast tr...@inf.ethz.ch writes:
I'm not sure it's worth arguing about whether the fixup! fixup! is a
symptom of some underlying problem, and changing rebase is only tapering
over the symptom; or whether it's actually a useful distinction.
If they are about the same complexity, then my instict tells me that
it is a better design not to strip on the writing side.
Thank you for the discussion. Sorry I have joined recently.
I agree that it is better to preserve information as long as feasible.
If we are going to strip it, it may as well be later. That is Thomas's
rearrange_squash patch, which I will send again.
The next question is, do we go all the way and respect the nested
fixup!s in rearrange_squash? I understand the case for it, though it's
hardly compelling to me in practice. :-) That would be more complicated
than Thomas's patch. But I'm happy to try it if someone gives me a
nudge. If not, at least the information is preserved in case someone
wants to do this later.
Regarding patches, I tried to follow the SubmittingPatches guidelines,
but I was confused about how to include a commit in an existing thread.
I think I was mislead by git-format-patch(1), When a patch is part of
an ongoing discussion..., which says to remove most header fields.
So if I don't want to break the discussion, should I append the unedited
format-patch output to my message after scissors, or should I send it
as a whole new message with --in-reply-to? Or something else? I'll try
the first.
Andrew
---8---
From 99023bff23f18a341441d6b7c447d9630a11b489 Mon Sep 17 00:00:00 2001
From: Andrew Pimlott and...@pimlott.net
Date: Fri, 14 Jun 2013 10:33:16 -0700
Subject: [PATCH 1/4] rebase -i: handle fixup! fixup! in --autosquash
In rebase -i --autosquash, ignore all fixup! or squash! after the
first. Handy in case a git commit --fixup/--squash referred to an earlier
fixup/squash instead of the original commit, for example with :/msg.
Signed-off-by: Andrew Pimlott and...@pimlott.net
---
Documentation/git-rebase.txt |4 +++-
git-rebase--interactive.sh | 13 ++-
t/t3415-rebase-autosquash.sh | 49 ++
3 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c84854a..6b2e1c8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -389,7 +389,9 @@ squash/fixup series.
the same ..., automatically modify the todo list of rebase -i
so that the commit marked for squashing comes right after the
commit to be modified, and change the action of the moved
- commit from `pick` to `squash` (or `fixup`).
+ commit from `pick` to `squash` (or `fixup`). Ignores subsequent
+ fixup! or squash! after the first, in case you referred to an
+ earlier fixup/squash with `git commit --fixup/--squash`.
+
This option is only valid when the '--interactive' option is used.
+
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..54ed4c3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -689,7 +689,18 @@ rearrange_squash () {
case $message in
squash! *|fixup! *)
action=${message%%!*}
- rest=${message#*! }
+ rest=$message
+ # ignore any squash! or fixup! after the first
+ while : ; do
+ case $rest in
+ squash! *|fixup! *)
+ rest=${rest#*! }
+ ;;
+ *)
+ break
+ ;;
+ esac
+ done
echo $sha1 $action $rest
# if it's a single word, try to resolve to a full sha1
and
# emit a second copy. This allows us to match on both
message
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index a1e86c4..1a3f40a 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -193,4 +193,53 @@ test_expect_success 'use commit --squash' '
test_auto_commit_flags squash 2
'
+test_auto_fixup_fixup () {
+ git reset --hard base
+ echo 1 file1
+ git add -u
+ test_tick
+ git commit -m $1! first
+ echo 2 file1
+ git add -u
+ test_tick
+ git commit -m $1! $2! first
+ git tag final-$1-$2
+ test_tick
+ git rebase --autosquash -i HEAD
+ git log --oneline actual
+ test_pause
+ if [ $1 = fixup ]; then
+ test_line_count = 3 actual
+ elif [ $1 = squash ]; then
+
Re: git diff returns fatal error with core.safecrlf is set to true.
+++ b/diff.c
@@ -2647,6 +2647,10 @@ static int diff_populate_gitlink(struct diff_filespec
*s, int size_only)
int diff_populate_filespec(struct diff_filespec *s, int size_only)
{
int err = 0;
+ enum safe_crlf crlf_warn = (safe_crlf != SAFE_CRLF_FAIL
+ ? safe_crlf
+ : SAFE_CRLF_WARN);
Thanks,
Does it makes sense to write it the other way around?
enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
? SAFE_CRLF_WARN
: safe_crlf);
--
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 09/16] documentation: add documentation for the bitmap format
Vicent Martí tan...@gmail.com writes: + There is a bitmap for each Git object type, stored in the following + order: + + - Commits + - Trees + - Blobs + - Tags + + In each bitmap, the `n`th bit is set to true if the `n`th object + in the packfile index is of that type. + + The obvious consequence is that the XOR of all 4 bitmaps will result + in a full set (all bits sets), and the AND of all 4 bitmaps will + result in an empty bitmap (no bits set). Instead of XOR did you mean OR here? Nope, I think XOR makes it more obvious: if the same bit is set on two bitmaps, it would be cleared when XORed together, and hence all the bits wouldn't be set. An OR would hide this case. What case are you talking about? The n-th object must be one of these four types and can never be of more than one type at the same time, so a natural expectation from the reader is If you OR them together, you will get the same set. If you say If you XOR them, that forces the reader to wonder when these bitmaps ever can overlap at the same bit position. To sum it up: I'd like to see this format be strictly in Network Byte Order, Good. I've been wondering what you meant by cannot be mmap-ed from the very beginning. We mmapped the index for a long time, and it is defined in terms of network byte order. Of course, pack .idx files are in network byte order, too, and we mmap them without problems. It seems that it primarily came from your fear that using network byte order may be unnecessarily hard to perform well, and it would be a good thing to do to try to do so first instead of punting from the beginning. and I'm going to try to make it run fast enough in that encoding. Hmph. Is it an option to start from what JGit does, so that people can use both JGit and your code on the same repository? And then if you do not succeed, after trying to optimize in-core processing using that on-disk format to make it fast enough, start thinking about tweaking the on-disk format? 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: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Johannes Sixt j...@kdbg.org writes: Some context: This is about a patch by Ramsay that removes the schizophrenic lstat hack for Cygwin. Junio, can you please queue that patch in pu? Sure. 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 03/16] pack-objects: use a faster hash table
Vicent Marti tan...@gmail.com writes:
This commit brings `khash.h`, a header only hash table implementation
that while remaining rather simple (uses quadratic probing and a
standard hashing scheme) and self-contained, offers a significant
performance improvement in both insertion and lookup times.
`khash` is a generic hash table implementation that can be 'templated'
for any given type while maintaining good performance by using preprocessor
macros. This specific version has been modified to define by default a
`khash_sha1` type, a map of SHA1s (const unsigned char[20]) to void *
pointers.
When replacing the old hash table implementation in `pack-objects` with
the khash_sha1 table, the insertion time is greatly reduced:
kh_put_sha1 :: 284.011ms
add_object_entry_1 : 36.06ms
hashcmp :: 24.045ms
This reduction of more than 50% in the insertion and lookup times,
although nice, is not particularly noticeable for normal `pack-objects`
operation: `pack-objects` performs massive batch insertions and
relatively few lookups, so `khash` doesn't get a chance to shine here.
The big win here, however, is in the massively reduced amount of hash
collisions (as you can see from the huge reduction of time spent in
`hashcmp` after the change). These greatly improved lookup times
will result critical once we implement the writing algorithm for bitmap
indxes in a later patch of this series.
Is that reduction in collisions purely because it uses quadratic
probing, or is there some other magic trick involved? Is the same also
applicable to the other users of the big object hash table? (I assume
Peff has already tried applying it there, but I'm still curious...)
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 02/16] sha1_file: refactor into `find_pack_object_pos`
Vicent Marti tan...@gmail.com writes:
if (use_lookup) {
- int pos = sha1_entry_pos(index, stride, 0,
- lo, hi, p-num_objects, sha1);
- if (pos 0)
- return 0;
- return nth_packed_object_offset(p, pos);
+ return sha1_entry_pos(index, stride, 0, lo, hi, p-num_objects,
sha1);
}
Our house style prefers not having the braces in a single-line conditional.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 10/16] pack-objects: use bitmaps when packing objects
Vicent Marti tan...@gmail.com writes:
diff --git a/Makefile b/Makefile
index e03c773..0f2e72b 100644
--- a/Makefile
+++ b/Makefile
@@ -703,6 +703,7 @@ LIB_H += notes.h
LIB_H += object.h
LIB_H += pack-revindex.h
LIB_H += pack.h
+LIB_H += pack-bitmap.h
LIB_H += parse-options.h
LIB_H += patch-ids.h
LIB_H += pathspec.h
@@ -838,6 +839,7 @@ LIB_OBJS += notes.o
LIB_OBJS += notes-cache.o
LIB_OBJS += notes-merge.o
LIB_OBJS += object.o
+LIB_OBJS += pack-bitmap.o
LIB_OBJS += pack-check.o
LIB_OBJS += pack-revindex.o
LIB_OBJS += pack-write.o
What does this apply on? When starting with the series from
origin/master, git-am fails, and 'git am -3' tells me I don't have the
necessary blobs (from the 'index' line above).
Not that it's super hard to fix this up as long as it's in the Makefile
only, but still.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 09/16] documentation: add documentation for the bitmap format
Vicent Marti tan...@gmail.com writes:
This is the technical documentation and design rationale for the new
Bitmap v2 on-disk format.
Hrmpf, that's what I get for reading the series in order...
+ The folowing flags are supported:
^^
typos marked by ^
+ By storing all the hashes in a cache together with the bitmapsin
^^
+ The obvious consequence is that the XOR of all 4 bitmaps will
result
+ in a full set (all bits sets), and the AND of all 4 bitmaps will
^
+ - 1-byte XOR-offset
+ The xor offset used to compress this bitmap. For an
entry
+ in position `x`, a XOR offset of `y` means that the
actual
+ bitmap representing for this commit is composed by
XORing the
+ bitmap for this entry with the bitmap in entry `x-y`
(i.e.
+ the bitmap `y` entries before this one).
+
+ Note that this compression can be recursive. In order to
+ XOR this entry with a previous one, the previous entry
needs
+ to be decompressed first, and so on.
+
+ The hard-limit for this offset is 160 (an entry can
only be
+ xor'ed against one of the 160 entries preceding it).
This
+ number is always positivea, and hence entries are
always xor'ed
^
+ with **previous** bitmaps, not bitmaps that will come
afterwards
+ in the index.
Clever. Why 160 though?
+ - 2 bytes of RESERVED data (used right now for better packing).
What do they mean?
+ With an index at the end of the file, we can load only this index in
memory,
+ allowing for very efficient access to all the available bitmaps lazily (we
+ have their offsets in the mmaped file).
Is there anything preventing you from mmap()ing the index also?
+== Appendix A: Serialization format for an EWAH bitmap
+
+Ewah bitmaps are serialized in the protocol as the JAVAEWAH
+library, making them backwards compatible with the JGit
+implementation:
+
+ - 4-byte number of bits of the resulting UNCOMPRESSED bitmap
+
+ - 4-byte number of words of the COMPRESSED bitmap, when stored
+
+ - N x 8-byte words, as specified by the previous field
+
+ This is the actual content of the compressed bitmap.
+
+ - 4-byte position of the current RLW for the compressed
+ bitmap
+
+Note that the byte order for this serialization is not defined by
+default. The byte order for all the content in a serialized EWAH
+bitmap can be known by the byte order flags in the header of the
+bitmap index file.
Please document the RLW format here.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 08/16] ewah: compressed bitmap implementation
Vicent Marti tan...@gmail.com writes:
The library is re-licensed under the GPLv2 with the permission of Daniel
Lemire, the original author.
This says GPLv2, but the license blurbs all say or (at your option)
any later version. IANAL, does this cause any problems? If so, can
they be GPLv2-only instead?
Makefile |6 +
ewah/bitmap.c | 229 +
ewah/ewah_bitmap.c | 703
ewah/ewah_io.c | 199 +++
ewah/ewah_rlw.c| 124 +
ewah/ewok.h| 194 +++
ewah/ewok_rlw.h| 114 +
Can we have a Documentation/technical/api-ewah.txt?
(Maybe if you insert all the comments I ask for in the below, it's not
necessary, but it would still be nice to have some central place where
the formats are documented.)
[...]
+struct ewah_bitmap *bitmap_to_ewah(struct bitmap *bitmap)
+{
+ struct ewah_bitmap *ewah = ewah_new();
+ size_t i, running_empty_words = 0;
+ eword_t last_word = 0;
+
+ for (i = 0; i bitmap-word_alloc; ++i) {
+ if (bitmap-words[i] == 0) {
+ running_empty_words++;
+ continue;
+ }
+
+ if (last_word != 0) {
+ ewah_add(ewah, last_word);
+ }
There are a lot of noisy braces -- like in this instance -- if you
apply the git style to the files in ewah/. I assume we'll give the
directory its own style, so that it should always use braces even on
one-line blocks.
[...]
+ ewah_add(ewah, last_word);
+ return ewah;
+}
+
+struct bitmap *ewah_to_bitmap(struct ewah_bitmap *ewah)
+{
+ struct bitmap *bitmap = bitmap_new();
+ struct ewah_iterator it;
+ eword_t blowup;
+ size_t i = 0;
+
+ ewah_iterator_init(it, ewah);
+
+ while (ewah_iterator_next(blowup, it)) {
+ if (i = bitmap-word_alloc) {
+ bitmap-word_alloc *= 1.5;
Any reason that this uses a scale factor of 1.5, while the bitmap_set
operation above uses 2?
+ bitmap-words = ewah_realloc(
+ bitmap-words, bitmap-word_alloc *
sizeof(eword_t));
+ }
[...]
+
+void bitmap_each_bit(struct bitmap *self, ewah_callback callback, void *data)
+{
[...]
+ for (offset = 0; offset BITS_IN_WORD; ++offset) {
+ if ((word offset) == 0)
+ break;
+
+ offset += __builtin_ctzll(word offset);
Here and in the rest, you use __builtin_* within the code. This needs
to be either in a separate helper that reimplements the function in
terms of C if it is not available (i.e. you don't use GCC).
(Alternatively, the whole series could be conditional on some
HAVE_GCC_BUILTINS macro. I'd think that would be a bad tradeoff
though.)
+ callback(pos + offset, data);
+ }
+ pos += BITS_IN_WORD;
+ }
+ }
+}
[...]
diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
[...]
+void ewah_free(struct ewah_bitmap *bitmap)
+{
+ if (bitmap-alloc_size)
+ free(bitmap-buffer);
+
+ free(bitmap);
+}
Maybe first if (!bitmap) return, so that it behaves like other free()s?
diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
[...]
+int ewah_serialize_native(struct ewah_bitmap *self, int fd)
+{
+ uint32_t write32;
+ size_t to_write = self-buffer_size * 8;
+
+ /* 32 bit -- bit size fr the map */
You cutpasted the typo (for) throughout the file :-)
[...]
+ /** 32 bit -- number of compressed 64-bit words */
+ write32 = (uint32_t)self-buffer_size;
+ if (write(fd, write32, 4) != 4)
+ return -1;
+
+ if (write(fd, self-buffer, to_write) != to_write)
+ return -1;
Shouldn't you use our neat write_in_full() and read_in_full() helpers,
throughout the file?
[...]
diff --git a/ewah/ewok.h b/ewah/ewok.h
[...]
+#ifndef __EWOK_BITMAP_C__
+#define __EWOK_BITMAP_C__
_H_?
+#ifndef ewah_malloc
+#define ewah_malloc malloc
+#endif
+#ifndef ewah_realloc
+#define ewah_realloc realloc
+#endif
+#ifndef ewah_calloc
+#define ewah_calloc calloc
+#endif
I see you later #define them to the corresponding x*alloc version in
pack-bitmap.h. Good.
+
+typedef uint64_t eword_t;
I assume this isn't ifdef'd to help 32bit platforms because the on-disk
format depends on it?
+#define BITS_IN_WORD (sizeof(eword_t) * 8)
+
+struct ewah_bitmap {
+ eword_t *buffer;
+ size_t buffer_size;
+ size_t alloc_size;
+ size_t bit_size;
+ eword_t *rlw;
+};
+
+typedef void (*ewah_callback)(size_t pos, void *);
+
+struct ewah_bitmap *ewah_pool_new(void);
+void ewah_pool_free(struct ewah_bitmap *bitmap);
How do the pool versions differ from the non-pool versions below? I
would have expected
Re: [PATCH] rebase -i: fixup fixup! fixup!
Andrew Pimlott and...@pimlott.net writes: Just reponding for the procedual part for now. So if I don't want to break the discussion, should I append the unedited format-patch output to my message after scissors, or should I send it as a whole new message with --in-reply-to? Or something else? I'll try the first. Which is fine, and you are almost there, but you do not want (1) From 99023b... that is not part of the message (it is a delimiter between multiple patches when/in case a file contains more than one); (2) From: Andrew... that is the same as the e-mail header in the message I am responding to; (3) Date: ... which is older than the e-mail header in the message I am responding to---the latter is the date people actually saw this patch on the mailing list, so it is preferrable to use it than the timestamp in your repository. So in this case, I'd expect to see, after the -- 8 -- line, only Subject: line, a blank and the log message. ---8--- From 99023bff23f18a341441d6b7c447d9630a11b489 Mon Sep 17 00:00:00 2001 From: Andrew Pimlott and...@pimlott.net Date: Fri, 14 Jun 2013 10:33:16 -0700 Subject: [PATCH 1/4] rebase -i: handle fixup! fixup! in --autosquash In rebase -i --autosquash, ignore all fixup! or squash! after the -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] Speed up Counting Objects with bitmap data
Vicent Marti tan...@gmail.com writes:
Like with every other patch that offers performance improvements,
sample benchmarks are provided (spoiler: they are pretty fucking
cool).
Great stuff.
I read the first half, and skimmed the second half. See the individual
replies for comments.
However:
Documentation/technical/bitmap-format.txt | 235
Makefile | 11 +
builtin.h |1 +
builtin/pack-objects.c| 362 +++-
builtin/pack-objects.h| 33 ++
builtin/rev-list.c| 35 +-
builtin/write-bitmap.c| 256 +
cache.h |5 +
ewah/bitmap.c | 229
ewah/ewah_bitmap.c| 703
ewah/ewah_io.c| 199 +++
ewah/ewah_rlw.c | 124 +
ewah/ewok.h | 194 +++
ewah/ewok_rlw.h | 114
git-compat-util.h | 28 +
git-repack.sh | 10 +-
git.c |1 +
khash.h | 329 +++
list-objects.c|1 +
pack-bitmap-write.c | 520 ++
pack-bitmap.c | 855
+
pack-bitmap.h | 64 +++
pack-write.c |2 +
revision.c|5 +
revision.h|2 +
sha1_file.c | 57 +-
It's pretty hard to miss that there isn't a single test in the entire
series. It seems that the features you add depend on pack.usebitmaps,
and since the tests run with empty config (unless of course they set
their own) your feature is completely untested -- unless I'm missing
something.
I imagine the tests would be of the format
test_expect_success 'do stuff without bitmaps' '
git ... expect
'
test_expect_success 'do stuff with bitmaps' '
test_config pack.usebitmaps true
# do something to ensure that we have bitmaps
git ... actual
test_cmp expect actual
'
or some such.
For bonus points, you could also add some light performance tests in
t/perf/, just to show off ;-)
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 11/16] rev-list: add bitmap mode to speed up lists
Vicent Marti tan...@gmail.com writes:
Calling `git rev-list --use-bitmaps [committish]` is the equivalent
of `git rev-list --objects`, but the rev list is performed based on
a bitmap result instead of using a manual counting objects phase.
Why would we ever want to not --use-bitmaps, once it actually works?
I.e., shouldn't this be the default if pack.usebitmaps is set (or
possibly even core.usebitmaps for these things)?
These are some example timings for `torvalds/linux`:
$ time ../git/git rev-list --objects master /dev/null
real0m25.567s
user0m25.148s
sys 0m0.384s
$ time ../git/git rev-list --use-bitmaps master /dev/null
real0m0.393s
user0m0.356s
sys 0m0.036s
I see your badass numbers, and raise you a critical issue:
$ time git rev-list --use-bitmaps --count --left-right origin/pu...origin/next
Segmentation fault
real0m0.408s
user0m0.383s
sys 0m0.022s
It actually seems to be related solely to having negated commits in the
walk:
thomas@linux-k42r:~/g(next u+65)$ time git rev-list --use-bitmaps --count
origin/pu
32315
real0m0.041s
user0m0.034s
sys 0m0.006s
thomas@linux-k42r:~/g(next u+65)$ time git rev-list --use-bitmaps --count
origin/pu ^origin/next
Segmentation fault
real0m0.460s
user0m0.214s
sys 0m0.244s
I also can't help noticing that the time spent generating the segfault
would have sufficed to generate the answer the old way as well:
$ time git rev-list --count --left-right origin/pu...origin/next
189 125
real0m0.409s
user0m0.386s
sys 0m0.022s
Can we use the same trick to speed up merge base computation and then
--left-right? The latter is a component of __git_ps1 and can get
somewhat slow in some cases, so it would be nice to make it really fast,
too.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] rebase -i: fixup fixup! fixup!
Andrew Pimlott and...@pimlott.net writes:
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c84854a..6b2e1c8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -389,7 +389,9 @@ squash/fixup series.
the same ..., automatically modify the todo list of rebase -i
so that the commit marked for squashing comes right after the
commit to be modified, and change the action of the moved
- commit from `pick` to `squash` (or `fixup`).
+ commit from `pick` to `squash` (or `fixup`). Ignores subsequent
+ fixup! or squash! after the first, in case you referred to an
+ earlier fixup/squash with `git commit --fixup/--squash`.
+
This option is only valid when the '--interactive' option is used.
+
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..54ed4c3 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -689,7 +689,18 @@ rearrange_squash () {
case $message in
squash! *|fixup! *)
action=${message%%!*}
- rest=${message#*! }
+ rest=$message
+ # ignore any squash! or fixup! after the first
+ while : ; do
Style:
while :
do
+ case $rest in
+ squash! *|fixup! *)
+ rest=${rest#*! }
+ ;;
+ *)
+ break
+ ;;
+ esac
+ done
echo $sha1 $action $rest
# if it's a single word, try to resolve to a full sha1
and
# emit a second copy. This allows us to match on both
message
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index a1e86c4..1a3f40a 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -193,4 +193,53 @@ test_expect_success 'use commit --squash' '
test_auto_commit_flags squash 2
'
+test_auto_fixup_fixup () {
+ git reset --hard base
+ echo 1 file1
+ git add -u
+ test_tick
+ git commit -m $1! first
+ echo 2 file1
+ git add -u
+ test_tick
+ git commit -m $1! $2! first
+ git tag final-$1-$2
+ test_tick
+ git rebase --autosquash -i HEAD
+ git log --oneline actual
+ test_pause
This patch obviously hasn't been tested. It breaks without -v.
+ if [ $1 = fixup ]; then
+ test_line_count = 3 actual
+ elif [ $1 = squash ]; then
+ test_line_count = 4 actual
+ else
+ false
+ fi
Style
if test $1 = fixup
then
...
elif test $1 = squash
then
...
(you got the idea).
+ git diff --exit-code final-$1-$2
+ test 2 = $(git cat-file blob HEAD^:file1)
+ if [ $1 = fixup ]; then
+ test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
+ elif [ $1 = squash ]; then
+ test 3 = $(git cat-file commit HEAD^ | grep first | wc -l)
+ else
+ false
+ fi
+}
+
+test_expect_success 'fixup! fixup!' '
+ test_auto_fixup_fixup fixup fixup
+'
+
+test_expect_success 'fixup! squash!' '
+ test_auto_fixup_fixup fixup squash
+'
+
+test_expect_success 'squash! squash!' '
+ test_auto_fixup_fixup squash squash
+'
This does not seem to pass for me.
+test_expect_success 'squash! fixup!' '
+ test_auto_fixup_fixup squash fixup
+'
+
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] rebase -i: fixup fixup! fixup!
Andrew Pimlott and...@pimlott.net writes: I agree that it is better to preserve information as long as feasible. If we are going to strip it, it may as well be later. That is Thomas's rearrange_squash patch, which I will send again. Thanks. The next question is, do we go all the way and respect the nested fixup!s in rearrange_squash? I understand the case for it, though it's hardly compelling to me in practice. :-) That would be more complicated than Thomas's patch. But I'm happy to try it if someone gives me a nudge. If not, at least the information is preserved in case someone wants to do this later. I think it is fine not to be too smart, as long as we do not lose information that would help the user to compensate. After all, autosquash will give the user an opportunity to eyeball the result of automatic rearrangement. If the user did this: git commit -m original git commit --fixup original ;# obviously fixing the first one git commit --fixup '!fixup original' ;# explicitly fixing the second git commit --fixup original ;# may want to fix the first one and then git rebase --autosquash gave him this: pick d78c915 original fixup 0c6388e original fixup d15b556 !fixup original fixup 1e39bcd original it may not be what the user originally intended, but I think it is OK. As long as !fixup original message is kept in the buffer, the user can notice and rearrange, e.g. pick d78c915 original fixup 0c6388e original fixup 1e39bcd original fixup d15b556 !fixup original if the user really wants to. -- 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] rebase -i: fixup fixup! fixup!
Junio C Hamano gits...@pobox.com writes: I guess I made typoes in the examples that made then unusable... I think it is fine not to be too smart, as long as we do not lose information that would help the user to compensate. After all, autosquash will give the user an opportunity to eyeball the result of automatic rearrangement. If the user did this: git commit -m original git commit --fixup original ;# obviously fixing the first one git commit --fixup '!fixup original' ;# explicitly fixing the second git commit --fixup original ;# may want to fix the first one and then git rebase --autosquash gave him this: (the result of automatic rearrangement should read like this) pick d78c915 original fixup 0c6388e !fixup original fixup d15b556 !fixup !fixup original fixup 1e39bcd !fixup original it may not be what the user originally intended, but I think it is OK. As long as !fixup original message is kept in the buffer, the user can notice and rearrange, e.g. (and the manual rearrangement should read like this) pick d78c915 original fixup 0c6388e !fixup original fixup 1e39bcd !fixup original fixup d15b556 !fixup !fixup original if the user really wants to. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jun 2013, #08; Tue, 25)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* bp/remote-mw-tests (2013-06-14) 1 commit
(merged to 'next' on 2013-06-17 at 2891373)
+ git-remote-mediawiki: remove hardcoded version number in the test suite
Code cleanup.
* cm/remote-mediawiki-perlcritique (2013-06-14) 31 commits
(merged to 'next' on 2013-06-17 at a41a924)
+ git-remote-mediawiki: make error message more precise
+ git-remote-mediawiki: add a perlcritic rule in Makefile
+ git-remote-mediawiki: add a .perlcriticrc file
+ git-remote-mediawiki: clearly rewrite double dereference
+ git-remote-mediawiki: fix a typo (mediwiki instead of mediawiki)
+ git-remote-mediawiki: put non-trivial numeric values in constants.
+ git-remote-mediawiki: don't use quotes for empty strings
+ git-remote-mediawiki: replace unless statements with negated if
statements
+ git-remote-mediawiki: brace file handles for print for more clarity
+ git-remote-mediawiki: modify strings for a better coding-style
+ git-remote-mediawiki: put long code into a subroutine
+ git-remote-mediawiki: remove import of unused open2
+ git-remote-mediawiki: check return value of open
+ git-remote-mediawiki: assign a variable as undef and make proper indentation
+ git-remote-mediawiki: rename a variable ($last) which has the name of a
keyword
+ git-remote-mediawiki: remove unused variable $entry
+ git-remote-mediawiki: turn double-negated expressions into simple expressions
+ git-remote-mediawiki: change the name of a variable
+ git-remote-mediawiki: add newline in the end of die() error messages
+ git-remote-mediawiki: change style in a regexp
+ git-remote-mediawiki: change style in a regexp
+ git-remote-mediawiki: change separator of some regexps
+ git-remote-mediawiki: change the behaviour of a split
+ git-remote-mediawiki: remove useless regexp modifier (m)
+ git-remote-mediawiki: rewrite unclear line of instructions
+ git-remote-mediawiki: change syntax of map calls
+ git-remote-mediawiki: move a variable declaration at the top of the code
+ git-remote-mediawiki: always end a subroutine with a return
+ git-remote-mediawiki: replace :utf8 by :encoding(UTF-8)
+ git-remote-mediawiki: move use warnings; before any instruction
+ git-remote-mediawiki: make a regexp clearer
Code cleanup.
* dk/maint-t5150-dirname (2013-06-17) 1 commit
(merged to 'next' on 2013-06-20 at 7441301)
+ tests: allow sha1's as part of the path
Fix a test script.
* dk/version-gen-gitdir (2013-06-17) 1 commit
(merged to 'next' on 2013-06-20 at c9c687e)
+ GIT-VERSION-GEN: support non-standard $GIT_DIR path
Allow packaging a tarball in a working tree with $GIT_DIR set elsewhere.
* fc/sequencer-plug-leak (2013-06-06) 2 commits
(merged to 'next' on 2013-06-20 at 3c94075)
+ sequencer: avoid leaking message buffer when refusing to create an empty
commit
+ sequencer: remove useless indentation
Plug a small leak in an error codepath.
* fg/submodule-fixup (2013-06-17) 1 commit
(merged to 'next' on 2013-06-20 at 64d74b4)
+ git-submodule.sh: remove duplicate call to set_rev_name
Code cleanup.
* jh/libify-note-handling (2013-06-12) 3 commits
(merged to 'next' on 2013-06-20 at 7dac8b6)
+ Move create_notes_commit() from notes-merge.c into notes-utils.c
+ Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
+ finish_copy_notes_for_rewrite(): Let caller provide commit message
Make it possible to call into copy-notes API from the sequencer code.
* jk/apache-test-for-2.4 (2013-06-14) 4 commits
(merged to 'next' on 2013-06-17 at 290e72e)
+ t/lib-httpd/apache.conf: configure an MPM module for apache 2.4
+ t/lib-httpd/apache.conf: load compat access module in apache 2.4
+ t/lib-httpd/apache.conf: load extra auth modules in apache 2.4
+ t/lib-httpd/apache.conf: do not use LockFile in apache = 2.4
Allow httpd tests to run with Apache 2.4.
* jk/doc-build-move-infordir-def (2013-06-17) 2 commits
(merged to 'next' on 2013-06-20 at 81e56a8)
+ Documentation/Makefile: move infodir to be with other '*dir's
+ Documentation/Makefile: fix spaces around assignments
Makefile cleanup.
* jk/mergetool-lib-refactor (2013-06-17) 1 commit
(merged to 'next' on 2013-06-20 at 7ce98c0)
+ mergetool--lib: refactor {diff,merge}_cmd logic
Code cleanup.
* jk/unpack-entry-fallback-to-another (2013-06-14) 2 commits
(merged to 'next' on 2013-06-17 at 89e0eab)
+ unpack_entry: do not die when we fail to apply a delta
+ t5303: drop count=1 from corruption dd
Follow-up to an earlier fix.
* mm/rm-coalesce-errors (2013-06-12) 2 commits
(merged to 'next' on 2013-06-20 at c70340c)
+ rm: introduce
Re: [PATCH 09/16] documentation: add documentation for the bitmap format
On Tue, Jun 25, 2013 at 11:17 PM, Junio C Hamano gits...@pobox.com wrote: What case are you talking about? The n-th object must be one of these four types and can never be of more than one type at the same time, so a natural expectation from the reader is If you OR them together, you will get the same set. If you say If you XOR them, that forces the reader to wonder when these bitmaps ever can overlap at the same bit position. I guess this is just wording. I don't particularly care about the distinction, but I'll change it to OR. To sum it up: I'd like to see this format be strictly in Network Byte Order, Good. I've been wondering what you meant by cannot be mmap-ed from the very beginning. We mmapped the index for a long time, and it is defined in terms of network byte order. Of course, pack .idx files are in network byte order, too, and we mmap them without problems. It seems that it primarily came from your fear that using network byte order may be unnecessarily hard to perform well, and it would be a good thing to do to try to do so first instead of punting from the beginning. It cannot be mmapped not particularly because of endianness issues, but because the original format is not indexed and requires a full parse of the whole index before it can be accessed programatically. The wrong endianness just increases the parse time. and I'm going to try to make it run fast enough in that encoding. Hmph. Is it an option to start from what JGit does, so that people can use both JGit and your code on the same repository? And then if you do not succeed, after trying to optimize in-core processing using that on-disk format to make it fast enough, start thinking about tweaking the on-disk format? I'm afraid this is not an option. I have an old patchset that implements JGit v1 bitmap loading (and in fact that's how I initially developed these series -- by loading the bitmaps from JGit for debugging), but I discarded it because it simply doesn't pan out in production. ~3 seconds time to spawn `upload-pack` is not an option for us. I did not develop a tweaked on-disk format out of boredom. I could dig up the patch if you're particularly interested in backwards compatibility, but since it was several times slower than the current iteration, I have no interest (time, actually) to maintain it, brush it up, and so on. I have already offered myself to port the v2 format to JGit as soon as it's settled. It sounds like a better investment of all our times. Following up on Shawn's comments, I removed the little-endian support from the on-disk format and implemented lazy loading of the bitmaps to make up for it. The result is decent (slowed down from 250ms to 300ms) and it lets us keep the whole format as NWO on disk. I think it's a good tradeback. The relevant commits are available on my fork of Git (I'll be sending v2 of the patchset once I finish tackling the other reviews): https://github.com/vmg/git/commit/d6cdd4329a547580bbc0143764c726c48b887271 https://github.com/vmg/git/commit/d8ec342fee87425e05c0db1e1630db8424612c71 As it stands right now, the only two changes from v1 of the on-disk format are: - There is an index at the end. This is a good idea. - The bitmaps are sorted in packfile-index order, not in packfile order. This is a good idea. As always, all your feedback is appreciated, but please keep in mind I have strict performance concerns. German kisses, vmg -- 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] Add depth to submodule update
On Tue, Jun 25, 2013 at 12:49:25AM +0200, Fredrik Gustafsson wrote: Used only when a clone is initialized. This is useful when the submodule(s) are huge and you're not really interested in anything but the latest commit. Signed-off-by: Fredrik Gustafsson iv...@iveqy.com I this is a valid use case. But this option only makes sense when a submodule is newly cloned so I am not sure whether submodule update is the correct place. Let me think about this a little more. Since we do not have any extra command that initiates the clone this is probably the only place we can put this option. But at the moment it does not feel completely right. Apart from that the code looks good. If the user does a checkout of a revision that was not fetched submodule update will error out the same way as if someone forgot to push his submodule changes. So that should not be a problem. Cheers Heiko -- 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] column doc: rewrite documentation for column.ui
Ramkumar Ramachandra artag...@gmail.com writes: The configuration option column.ui is very poorly documented, and it is unclear what the defaults are, and what option can be combined with what. Rewrite it by splitting up the options into three sections clearly showing how COL_ENABLED, COL_LAYOUT_MASK, and COL_DENSE work. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Thanks. One micronit (read: if you think the suggested change is a good idea, I could just locally amend it) is that it would read better and also easier to maintain to say These options control without three. Both 'man' and 'html' output looked good. Documentation/config.txt | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1153585..5a10169 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -934,6 +934,9 @@ column.ui:: This variable consists of a list of tokens separated by spaces or commas: + +These three options control when the feature should be enabled +(defaults to 'never'): ++ -- `always`;; always show in columns @@ -941,19 +944,30 @@ column.ui:: never show in columns `auto`;; show in columns if the output is to the terminal +-- ++ +These three options control layout (defaults to 'column'). Setting +layout implies 'always' if none of 'always', 'never', or 'auto' are +specified. ++ +-- `column`;; - fill columns before rows (default) + fill columns before rows `row`;; fill rows before columns `plain`;; show in one column +-- ++ +Finally, these options can be combined with a layout option (defaults +to 'nodense'): ++ +-- `dense`;; make unequal size columns to utilize more space `nodense`;; make equal size columns -- -+ -This option defaults to 'never'. column.branch:: Specify whether to output branch listing in `git branch` in columns. -- 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 09/16] documentation: add documentation for the bitmap format
On Tue, Jun 25, 2013 at 5:58 PM, Thomas Rast tr...@inf.ethz.ch wrote: This is the technical documentation and design rationale for the new Bitmap v2 on-disk format. Hrmpf, that's what I get for reading the series in order... + The folowing flags are supported: ^^ typos marked by ^ + By storing all the hashes in a cache together with the bitmapsin ^^ + The obvious consequence is that the XOR of all 4 bitmaps will result + in a full set (all bits sets), and the AND of all 4 bitmaps will ^ + - 1-byte XOR-offset + The xor offset used to compress this bitmap. For an entry + in position `x`, a XOR offset of `y` means that the actual + bitmap representing for this commit is composed by XORing the + bitmap for this entry with the bitmap in entry `x-y` (i.e. + the bitmap `y` entries before this one). + + Note that this compression can be recursive. In order to + XOR this entry with a previous one, the previous entry needs + to be decompressed first, and so on. + + The hard-limit for this offset is 160 (an entry can only be + xor'ed against one of the 160 entries preceding it). This + number is always positivea, and hence entries are always xor'ed ^ + with **previous** bitmaps, not bitmaps that will come afterwards + in the index. Clever. Why 160 though? JGit implementation detail. It's the equivalent of the delta-window in `pack-objects` for example. HINT HINT: in practice, JGit only looks 16 positions behind to find deltas, and we do the same. So the practical limit is 16. harhar + - 2 bytes of RESERVED data (used right now for better packing). What do they mean? + With an index at the end of the file, we can load only this index in memory, + allowing for very efficient access to all the available bitmaps lazily (we + have their offsets in the mmaped file). Is there anything preventing you from mmap()ing the index also? Yeah, this format allows you to easily do a SHA1 bsearch with custom step to lookup entries on the bitmap index, except for the fact that the index is not sorted by SHA1, so you'd need a linear search instead. :) I decided against it because during most complex invocations of `pack-objects`, we perform a couple thousand commit lookups to see if they have a bitmap in the index, so it makes a lot of sense to load the index tightly in a hash table before hand (which takes very little time, to be fair). We more-than-make up for the loading time by having much much faster lookups. I felt it was the right tradeoff (JGit does the same, but in their case, because they cannot mmap. :p) +== Appendix A: Serialization format for an EWAH bitmap + +Ewah bitmaps are serialized in the protocol as the JAVAEWAH +library, making them backwards compatible with the JGit +implementation: + + - 4-byte number of bits of the resulting UNCOMPRESSED bitmap + + - 4-byte number of words of the COMPRESSED bitmap, when stored + + - N x 8-byte words, as specified by the previous field + + This is the actual content of the compressed bitmap. + + - 4-byte position of the current RLW for the compressed + bitmap + +Note that the byte order for this serialization is not defined by +default. The byte order for all the content in a serialized EWAH +bitmap can be known by the byte order flags in the header of the +bitmap index file. Please document the RLW format here. Har har. I was going to comment on your review of the Ewah patchset, but might as well do it here: the only thing I know about Ewah bitmaps is that they work. And I know this because I did extensive fuzz testing of my C port. Unfortunately, the original Java code I ported from has 0 comments, so any documentation here would have to be reverse-engineered. Personally, I'd lean towards considering Ewah an external dependency (black box); the headers for the library are commented accordingly, clearly explaining the interfaces while hiding implementation details. Of course, you're welcome to help me reverse engineer the implementation, but I'm not sure this would be of much value. It'd be better to make sure it passes the extensive test suite of the Java version, and assume that Mr Lemire designed a sound format for the bitmaps. Swiss kisses, vmg -- 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
Call For Papers: Venice, Italy --- September 28-30, 2013 (Indexed in ISI ELSEVIER, SCOPUS, ACM, PubMed, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS) git@vger.kernel.org
Call For Papers and Special Sessions September 28-30, 2013, Venice, Italy http://www.europment.org The 2013 Intern. Conf. on Systems, Control, and Informatics The 2013 Intern. Conf. on Electronics, Signal Processing and Communication Systems The 2013 Intern. Conf. on Applied Mathematics and Computational Methods The 2013 Intern. Conf. on Mechanics, Fluids, Heat, Elasticity and Electromagnetic Fields The 2013 Intern. Conf. on Education and Modern Educational Technologies The 2013 Intern. Conf. on Environment, Energy, Ecosystems and Development The 2013 Intern. Conf. on Business Administration, Marketing and Economics The 2013 Intern. Conf. on Biology, Medical Physics, Medical Chemistry, Biochemistry and Biomedical Engineering http://www.europment.org Paper submission deadline (strictly): July 15, 2013 (without any extension) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - All the Accepted Papers will be published in our: (a) ISI/SCOPUS Book (Hard-Copy) (Indexed in ISI ELSEVIER, SCOPUS, ACM, PubMed, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS) (b) ISI/SCOPUS CD-ROM Proceedings (Indexed in ISI ELSEVIER, SCOPUS, ACM, PubMed, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS) (c) Journal (SCOPUS, AMS, Elsevier, Zentrablat, ACM etc... indexed) (d) E-Library with free access (more than 10 visits per month) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - The Proceedings of the Conference with all the accepted and registered papers of the conferences will be sent for indexing to: ISI (Thomson Reuters), ELSEVIER, SCOPUS, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS - American Chemical Society, EI Compendex, Engineering Village, DoPP, GEOBASE, Biobase, TIB|UB - German National Library of Science and Technology, American Mathematical Society (AMS), Inspec - The IET, Ulrich's International Periodicals Directory. The conference will feature tutorials, technical paper presentations, workshops and distinguished keynote speeches Registration fees: 490 EUR Organizing Committee General Chairs - Professor Charles A. Long Professor Emeritus University of Wisconsin Stevens Point, Wisconsin, USA Professor Nikos E. Mastorakis Military Institutes of University Education (ASEI) Hellenic Naval Academy Sector of Electrical Engineering and Computer Science Piraeus, Greece -also with- Technical University of Sofia 1000 Sofia, Bulgaria Professor Valeri Mladenov Technical University of Sofia 1000 Sofia, Bulgaria Senior Program Chair Professor Philippe Dondon ENSEIRB Rue A Schweitzer 33400 Talence France Program Chairs - Professor Filippo Neri Dipartimento di Informatica e Sistemistica University of Naples Federico II Naples, Italy Professor Hamid Reza Karimi Department of Engineering Faculty of Engineering and Science University of Agder, N-4898 Grimstad Norway Professor Sandra Sendra Instituto de Inv. para la Gestión Integrada de Zonas Costeras (IGIC) Universidad Politécnica de Valencia, Spain Tutorials Chair -- Professor Pradip Majumdar Department of Mechanical Engineering Northern Illinois University Dekalb, Illinois, USA Special Session Chair Professor Pavel Varacha Tomas Bata University in Zlin Faculty of Applied Informatics Department of Informatics and Artificial Intelligence Zlin, Czech Republic Workshops Chair -- Professor Ryszard S. Choras Institute of Telecommunications University of Technology Life Sciences Bydgoszcz, Poland Local Organizing Chair - Assistant Professor Klimis Ntalianis, Technological Educat. Inst. of Athens (TEI), Athens, Greece Publication Chair Professor Gen Qi Xu Department of Mathematics Tianjin University Tianjin, China Publicity Committee -- Professor Reinhard Neck Department of Economics Klagenfurt University Klagenfurt, Austria Professor Myriam Lazard Institut Superieur d' Ingenierie de la Conception Saint Die, France International Liaisons, France Professor Ka-Lok Ng Department of Bioinformatics Asia University Taichung, Taiwan Professor Olga Martin Applied Sciences Faculty Politehnica University of Bucharest Romania Professor Vincenzo Niola Departement of Mechanical Engineering for Energetics University of Naples Federico II Naples, Italy Professor Eduardo Mario Dias Electrical Energy and Automation Engineering Department Escola Politecnica da Universidade de Sao Paulo Brazil Steering Committee --- Professor Aida Bulucea, University of Craiova, Romania Professor Dana Simian, Univ. of Sibiu, Sibiu, Romania Professor Zoran Bojkovic, Univ. of Belgrade, Serbia Professor Metin Demiralp, Istanbul Technical University, Turkey Professor Imre Rudas, Obuda University, Budapest, Hungary Program Committee Prof. Lotfi Zadeh (IEEE Fellow,University of Berkeley, USA) Prof.
Re: [PATCH 03/16] pack-objects: use a faster hash table
Vicent Marti tan...@gmail.com writes:
@@ -901,19 +896,19 @@ static int no_try_delta(const char *path)
return 0;
}
-static int add_object_entry(const unsigned char *sha1, enum object_type type,
- const char *name, int exclude)
+static int add_object_entry_1(const unsigned char *sha1, enum object_type
type,
+ uint32_t hash, int exclude, struct packed_git
*found_pack,
+ off_t found_offset)
{
struct object_entry *entry;
- struct packed_git *p, *found_pack = NULL;
- off_t found_offset = 0;
- int ix;
- unsigned hash = name_hash(name);
+ struct packed_git *p;
+ khiter_t ix;
+ int hash_ret;
- ix = nr_objects ? locate_object_entry_hash(sha1) : -1;
- if (ix = 0) {
+ ix = kh_put_sha1(packed_objects, sha1, hash_ret);
+ if (hash_ret == 0) {
if (exclude) {
- entry = objects + object_ix[ix] - 1;
+ entry = kh_value(packed_objects, ix);
if (!entry-preferred_base)
nr_result--;
entry-preferred_base = 1;
After this, the function returns. The original did not add to the
table the object name we are looking at, but the new code first adds
it to the table with the unconditional kh_put_sha1() above. Is a
call to kh_del_sha1() missing here ...
@@ -921,38 +916,42 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
return 0;
}
- if (!exclude local has_loose_object_nonlocal(sha1))
+ if (!exclude local has_loose_object_nonlocal(sha1)) {
+ kh_del_sha1(packed_objects, ix);
return 0;
... like this one, which seems to compensate for ahh, after all we
realize we do not want to add this one to the table?
@@ -966,19 +965,30 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
entry-in_pack_offset = found_offset;
}
- if (object_ix_hashsz * 3 = nr_objects * 4)
- rehash_objects();
- else
- object_ix[-1 - ix] = nr_objects;
+ kh_value(packed_objects, ix) = entry;
+ kh_key(packed_objects, ix) = entry-idx.sha1;
+ objects[nr_objects++] = entry;
display_progress(progress_state, nr_objects);
- if (name no_try_delta(name))
- entry-no_try_delta = 1;
-
return 1;
}
+static int add_object_entry(const unsigned char *sha1, enum object_type type,
+ const char *name, int exclude)
+{
+ if (add_object_entry_1(sha1, type, name_hash(name), exclude, NULL, 0)) {
+ struct object_entry *entry = objects[nr_objects - 1];
+
+ if (name no_try_delta(name))
+ entry-no_try_delta = 1;
+
+ return 1;
+ }
+
+ return 0;
+}
It is somewhat unclear what we are getting from the split of the
main part of this function into *_1(), other than the *_1() function
now has a very deep indentation inside if (!found_pack), which is
always true because the caller always passes NULL to found_pack.
Perhaps this is an unrelated refactoring that is needed for later
steps and does not have anything to do with the use of new hash
function?
--
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 08/16] ewah: compressed bitmap implementation
Junio C Hamano gits...@pobox.com writes: Vicent Marti tan...@gmail.com writes: The library is re-licensed under the GPLv2 with the permission of Daniel Lemire, the original author. The source code for the C version can be found on GitHub: https://github.com/vmg/libewok The original Java implementation can also be found on GitHub: https://github.com/lemire/javaewah --- Please make sure that all patches are properly signed off. Makefile |6 + ewah/bitmap.c | 229 + ewah/ewah_bitmap.c | 703 ewah/ewah_io.c | 199 +++ ewah/ewah_rlw.c| 124 + ewah/ewok.h| 194 +++ ewah/ewok_rlw.h| 114 + This is lovely. A few comments after an initial quick scan-through. - The code and the headers are well commented, which is good. - What's __builtin_popcountll() doing there in a presumably generic codepath? - Two variants of bitmap are given different and easy to understand type names (vanilla one is bitmap, the clever one is ewah_bitmap), but at many places, a pointer to ewah_bitmap is simply called bitmap or bitmap_i without ewah anywhere, which waas confusing to read. Especially, the NAND operation for bitmap takes two bitmaps, while OR takes one bitmap and ewah_bitmap. That is fine as long as the combination is convenient for callers, but I wished the ewah variables be called with ewah somewhere in their names. - I compile with -Werror -Wdeclaration-after-statement; some places seem to trigger it. - Some extern declarations in *.c sources were irritating; shouldn't they be declared in *.h file and included? - There are some instances of if (condition) stmt; on a single line; looked irritating. - bool is not a C type we use (and not a particularly good type in C++, either). One more. - Use of unnecessary float (e.g. oldval *= 1.5) were moderately annoying. That is it for now. I am looking forward to read through the users of the library ;-) Thanks for working on 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: [PATCH 13/16] repack: consider bitmaps when performing repacks
Vicent Marti tan...@gmail.com writes: @@ -156,6 +156,11 @@ do fullbases=$fullbases pack-$name chmod a-w $PACKTMP-$name.pack chmod a-w $PACKTMP-$name.idx + + test -f $PACKTMP-$name.bitmap + chmod a-w $PACKTMP-$name.bitmap + mv -f $PACKTMP-$name.bitmap $PACKDIR/pack-$name.bitmap If we see a temporary bitmap but somehow failed to move it to the final name, should we _ignore_ that error, or should we die, like the next two lines do? mv -f $PACKTMP-$name.pack $PACKDIR/pack-$name.pack mv -f $PACKTMP-$name.idx $PACKDIR/pack-$name.idx || exit -- 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] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Tue Jun 25 14:45:07 -0700 2013: After all, autosquash will give the user an opportunity to eyeball the result of automatic rearrangement. If the user did this: git commit -m original git commit --fixup original ;# obviously fixing the first one git commit --fixup '!fixup original' ;# explicitly fixing the second git commit --fixup original ;# may want to fix the first one and then git rebase --autosquash gave him this: pick d78c915 original fixup 0c6388e original fixup d15b556 !fixup original fixup 1e39bcd original I assume you mean: pick d78c915 original fixup 0c6388e fixup! original fixup d15b556 fixup! fixup! original fixup 1e39bcd !fixup! original The current master code tries to keep the original commit message intact. I assume you would preserve that behavior, so you would want to see fixup! fixup! it may not be what the user originally intended, but I think it is OK. As long as !fixup original message is kept in the buffer, the user can notice and rearrange, e.g. Thomas's patch didn't do this: fixup! or squash! after the first is simply discarded, so you see: pick d78c915 original fixup 0c6388e fixup! original fixup d15b556 fixup! original fixup 1e39bcd !fixup! original But it will be a simple change to keep all the fixup!s and squash!s. I will do this (and try to make up for the carelessness of my previous patch). Andrew -- 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 10/16] pack-objects: use bitmaps when packing objects
Vicent Marti tan...@gmail.com writes:
@@ -83,6 +84,9 @@ static struct progress *progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static int bitmap_support;
+static int use_bitmap_index;
OK.
@@ -2131,6 +2135,10 @@ static int git_pack_config(const char *k, const char
*v, void *cb)
cache_max_small_delta_size = git_config_int(k, v);
return 0;
}
+ if (!strcmp(k, pack.usebitmaps)) {
+ bitmap_support = git_config_bool(k, v);
+ return 0;
+ }
Hmph, so bitmap_support, not use_bitmap_index, keeps track of the
user request? Somewhat confusing.
if (!strcmp(k, pack.threads)) {
delta_search_threads = git_config_int(k, v);
if (delta_search_threads 0)
@@ -2366,8 +2374,24 @@ static void get_object_list(int ac, const char **av)
die(bad revision '%s', line);
}
+ if (use_bitmap_index) {
+ uint32_t size_hint;
+
+ if (!prepare_bitmap_walk(revs, size_hint)) {
+ khint_t new_hash_size = (size_hint * (1.0 /
__ac_HASH_UPPER)) + 0.5;
What is __ac_HASH_UPPER? That is a very unusual name for a variable
or a constant. Also it is mildly annoying to see unnecessary use of
float like this.
+ kh_resize_sha1(packed_objects, new_hash_size);
+
+ nr_alloc = (size_hint + 63) ~63;
+ objects = xrealloc(objects, nr_alloc * sizeof(struct
object_entry *));
+
+ traverse_bitmap_commit_list(add_object_entry_1);
+ return;
+ }
+ }
+
if (prepare_revision_walk(revs))
die(revision walk setup failed);
+
mark_edges_uninteresting(revs.commits, revs, show_edge);
traverse_commit_list(revs, show_commit, show_object, NULL);
@@ -2495,6 +2519,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(pack compression level)),
OPT_SET_INT(0, keep-true-parents, grafts_replace_parents,
N_(do not hide commits by grafts), 0),
+ OPT_BOOL(0, bitmaps, bitmap_support,
+ N_(enable support for bitmap optimizations)),
Please match this with the name of configuration variable, i.e. --use-bitmaps
OPT_END(),
};
@@ -2561,6 +2587,11 @@ int cmd_pack_objects(int argc, const char **argv,
const char *prefix)
if (keep_unreachable unpack_unreachable)
die(--keep-unreachable and --unpack-unreachable are
incompatible.);
+ if (bitmap_support) {
+ if (use_internal_rev_list pack_to_stdout)
+ use_bitmap_index = 1;
OK, so only when some internal condition is met, the user request to
use bitmap is honored and the deision is kept in use_bitmap_index.
It may be easier to read if you get rid of bitmap_support, set
user_bitmap_index directly from the command line and config, and did
this here instead:
if (!(use_internal_rev_list pack_to_stdout))
use_bitmap_index = 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/16] pack-objects: use a faster hash table
On Wed, Jun 26, 2013 at 12:48 AM, Junio C Hamano gits...@pobox.com wrote:
After this, the function returns. The original did not add to the
table the object name we are looking at, but the new code first adds
it to the table with the unconditional kh_put_sha1() above. Is a
call to kh_del_sha1() missing here ...
No, this is not the case. That's the return case for when *the object
was found because it already existed in the hash table* (hence we
access it if we're excluding it, to tag it as excluded). We don't want
to remove it from the hash table because we're not the ones we
inserted it.
We only call `kh_del_sha1` in the cases where:
1. The object wasn't found.
2. We inserted its key on the hash table.
3. We later learnt that we don't really want to pack this object.
@@ -921,38 +916,42 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
return 0;
}
- if (!exclude local has_loose_object_nonlocal(sha1))
+ if (!exclude local has_loose_object_nonlocal(sha1)) {
+ kh_del_sha1(packed_objects, ix);
return 0;
... like this one, which seems to compensate for ahh, after all we
realize we do not want to add this one to the table?
@@ -966,19 +965,30 @@ static int add_object_entry(const unsigned char *sha1,
enum object_type type,
entry-in_pack_offset = found_offset;
}
- if (object_ix_hashsz * 3 = nr_objects * 4)
- rehash_objects();
- else
- object_ix[-1 - ix] = nr_objects;
+ kh_value(packed_objects, ix) = entry;
+ kh_key(packed_objects, ix) = entry-idx.sha1;
+ objects[nr_objects++] = entry;
display_progress(progress_state, nr_objects);
- if (name no_try_delta(name))
- entry-no_try_delta = 1;
-
return 1;
}
+static int add_object_entry(const unsigned char *sha1, enum object_type
type,
+ const char *name, int exclude)
+{
+ if (add_object_entry_1(sha1, type, name_hash(name), exclude, NULL, 0))
{
+ struct object_entry *entry = objects[nr_objects - 1];
+
+ if (name no_try_delta(name))
+ entry-no_try_delta = 1;
+
+ return 1;
+ }
+
+ return 0;
+}
It is somewhat unclear what we are getting from the split of the
main part of this function into *_1(), other than the *_1() function
now has a very deep indentation inside if (!found_pack), which is
always true because the caller always passes NULL to found_pack.
Perhaps this is an unrelated refactoring that is needed for later
steps and does not have anything to do with the use of new hash
function?
Yes, apologies for not making this clear. By refactoring into `_1`,
you can see how `traverse_bitmap_commit_list` can use the `_1` version
directly as a callback, to insert objects straight into the packing
list without looking them up. This is very efficient because we can
pass the whole API straight from the bitmap code:
1. The SHA1: we find it by simply looking up the `nth` sha1 on the
pack index (if we are yielding bit `n`)
2. The object type: we find it because we have type indexes that let
us know the type of any given bit in the bitmap by and-ing it with the
index.
3. The hash for its name: we can look it up from the name hash cache
in the new bitmap format.
4. Exclude flag: we never exclude when working with bitmaps
5. found_pack: all the bitmapped objects come from the same pack!
6. found_offset: we find it by simply looking up the `nth` offset on
the pack index (if we are yielding bit `n`)
Boom! We filled the callback just from the data in a bitmap. Ain't that nice?
Let me amend the commit message.
--
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 10/16] pack-objects: use bitmaps when packing objects
On Wed, Jun 26, 2013 at 1:06 AM, Junio C Hamano gits...@pobox.com wrote:
@@ -83,6 +84,9 @@ static struct progress *progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static int bitmap_support;
+static int use_bitmap_index;
OK.
@@ -2131,6 +2135,10 @@ static int git_pack_config(const char *k, const char
*v, void *cb)
cache_max_small_delta_size = git_config_int(k, v);
return 0;
}
+ if (!strcmp(k, pack.usebitmaps)) {
+ bitmap_support = git_config_bool(k, v);
+ return 0;
+ }
Hmph, so bitmap_support, not use_bitmap_index, keeps track of the
user request? Somewhat confusing.
if (!strcmp(k, pack.threads)) {
delta_search_threads = git_config_int(k, v);
if (delta_search_threads 0)
@@ -2366,8 +2374,24 @@ static void get_object_list(int ac, const char **av)
die(bad revision '%s', line);
}
+ if (use_bitmap_index) {
+ uint32_t size_hint;
+
+ if (!prepare_bitmap_walk(revs, size_hint)) {
+ khint_t new_hash_size = (size_hint * (1.0 /
__ac_HASH_UPPER)) + 0.5;
What is __ac_HASH_UPPER? That is a very unusual name for a variable
or a constant. Also it is mildly annoying to see unnecessary use of
float like this.
See the updated patch at:
https://github.com/vmg/git/blob/vmg/bitmaps-master/builtin/pack-objects.c#L2422
+ kh_resize_sha1(packed_objects, new_hash_size);
+
+ nr_alloc = (size_hint + 63) ~63;
+ objects = xrealloc(objects, nr_alloc * sizeof(struct
object_entry *));
+
+ traverse_bitmap_commit_list(add_object_entry_1);
+ return;
+ }
+ }
+
if (prepare_revision_walk(revs))
die(revision walk setup failed);
+
mark_edges_uninteresting(revs.commits, revs, show_edge);
traverse_commit_list(revs, show_commit, show_object, NULL);
@@ -2495,6 +2519,8 @@ int cmd_pack_objects(int argc, const char **argv,
const char *prefix)
N_(pack compression level)),
OPT_SET_INT(0, keep-true-parents, grafts_replace_parents,
N_(do not hide commits by grafts), 0),
+ OPT_BOOL(0, bitmaps, bitmap_support,
+ N_(enable support for bitmap optimizations)),
Please match this with the name of configuration variable, i.e. --use-bitmaps
OPT_END(),
};
@@ -2561,6 +2587,11 @@ int cmd_pack_objects(int argc, const char **argv,
const char *prefix)
if (keep_unreachable unpack_unreachable)
die(--keep-unreachable and --unpack-unreachable are
incompatible.);
+ if (bitmap_support) {
+ if (use_internal_rev_list pack_to_stdout)
+ use_bitmap_index = 1;
OK, so only when some internal condition is met, the user request to
use bitmap is honored and the deision is kept in use_bitmap_index.
It may be easier to read if you get rid of bitmap_support, set
user_bitmap_index directly from the command line and config, and did
this here instead:
if (!(use_internal_rev_list pack_to_stdout))
use_bitmap_index = 0;
Yeah, I'm not particularly happy with the way these flags are
implemented. I'll update 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: [PATCH 13/16] repack: consider bitmaps when performing repacks
On Wed, Jun 26, 2013 at 1:00 AM, Junio C Hamano gits...@pobox.com wrote: @@ -156,6 +156,11 @@ do fullbases=$fullbases pack-$name chmod a-w $PACKTMP-$name.pack chmod a-w $PACKTMP-$name.idx + + test -f $PACKTMP-$name.bitmap + chmod a-w $PACKTMP-$name.bitmap + mv -f $PACKTMP-$name.bitmap $PACKDIR/pack-$name.bitmap If we see a temporary bitmap but somehow failed to move it to the final name, should we _ignore_ that error, or should we die, like the next two lines do? I obviously decided against dying (as you can see on the patch, har har), because the bitmap is not required for the proper operation of the Git repository, unlike the packfile and the index. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: fixup fixup! fixup!
Excerpts from Junio C Hamano's message of Tue Jun 25 14:33:18 -0700 2013: Andrew Pimlott and...@pimlott.net writes: Just reponding for the procedual part for now. So if I don't want to break the discussion, should I append the unedited format-patch output to my message after scissors, or should I send it as a whole new message with --in-reply-to? Or something else? I'll try the first. Which is fine, and you are almost there, but you do not want (1) From 99023b... that is not part of the message (it is a delimiter between multiple patches when/in case a file contains more than one); (2) From: Andrew... that is the same as the e-mail header in the message I am responding to; (3) Date: ... which is older than the e-mail header in the message I am responding to---the latter is the date people actually saw this patch on the mailing list, so it is preferrable to use it than the timestamp in your repository. So in this case, I'd expect to see, after the -- 8 -- line, only Subject: line, a blank and the log message. Thank you. It was not clear to me even after several doc readings what git-mailinfo would look for where. I think I assumed that the idea was to transmit the original commit perfectly, and I stubbornly failed to give up that assumption even when it clearly didn't fit. Everything makes more sense with the understanding that the receiver will pull together non-patch metadata in the way that makes sense from his point of view (and that a different commit will come back via fetch). I will take a whack at clarifying the docs if I have time. Andrew -- 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 11/16] rev-list: add bitmap mode to speed up lists
I'm afraid I cannot reproduce the segfault locally (assuming you're
performing the rev-list on the git/git repository). Could you please
send me more information, and a core dump if possible?
On Tue, Jun 25, 2013 at 6:22 PM, Thomas Rast tr...@inf.ethz.ch wrote:
Vicent Marti tan...@gmail.com writes:
Calling `git rev-list --use-bitmaps [committish]` is the equivalent
of `git rev-list --objects`, but the rev list is performed based on
a bitmap result instead of using a manual counting objects phase.
Why would we ever want to not --use-bitmaps, once it actually works?
I.e., shouldn't this be the default if pack.usebitmaps is set (or
possibly even core.usebitmaps for these things)?
These are some example timings for `torvalds/linux`:
$ time ../git/git rev-list --objects master /dev/null
real0m25.567s
user0m25.148s
sys 0m0.384s
$ time ../git/git rev-list --use-bitmaps master /dev/null
real0m0.393s
user0m0.356s
sys 0m0.036s
I see your badass numbers, and raise you a critical issue:
$ time git rev-list --use-bitmaps --count --left-right
origin/pu...origin/next
Segmentation fault
real0m0.408s
user0m0.383s
sys 0m0.022s
It actually seems to be related solely to having negated commits in the
walk:
thomas@linux-k42r:~/g(next u+65)$ time git rev-list --use-bitmaps --count
origin/pu
32315
real0m0.041s
user0m0.034s
sys 0m0.006s
thomas@linux-k42r:~/g(next u+65)$ time git rev-list --use-bitmaps --count
origin/pu ^origin/next
Segmentation fault
real0m0.460s
user0m0.214s
sys 0m0.244s
I also can't help noticing that the time spent generating the segfault
would have sufficed to generate the answer the old way as well:
$ time git rev-list --count --left-right origin/pu...origin/next
189 125
real0m0.409s
user0m0.386s
sys 0m0.022s
Can we use the same trick to speed up merge base computation and then
--left-right? The latter is a component of __git_ps1 and can get
somewhat slow in some cases, so it would be nice to make it really fast,
too.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 03/16] pack-objects: use a faster hash table
On Tue, Jun 25, 2013 at 04:03:22PM +0200, Thomas Rast wrote: The big win here, however, is in the massively reduced amount of hash collisions (as you can see from the huge reduction of time spent in `hashcmp` after the change). These greatly improved lookup times will result critical once we implement the writing algorithm for bitmap indxes in a later patch of this series. Is that reduction in collisions purely because it uses quadratic probing, or is there some other magic trick involved? Is the same also applicable to the other users of the big object hash table? (I assume Peff has already tried applying it there, but I'm still curious...) I haven't done any actual timings yet. The general code is quite similar to our object.c hash table, with the exception that it does quadratic probing. I did try quadratic probing on our object.c hash once and didn't see much improvement (similarly, Junio tried cuckoo hashing, but the numbers were not that exciting). It's possible that the hash table in pack-objects did not behave as well as the one in object.c. It looks like we grow it when the table is 3/4 full, which is a little high (we grow at 1/2 in object.c). Quadratic probing should help when the hash table is close to full, so it would probably help. However, I also note that khash keeps its hash tables only half full, so that may be the real source of the performance improvement. So I suspect two things (but as I said, haven't verified): 1. You could speed up pack-objects just by keeping the table half full rather than 3/4 full. 2. You would see little to no speedup by moving object.c to khash, as it is adding only quadratic probing. With quadratic probing, you could potentially tweak the kh_put_* to resize less aggressively (say, 2/3) and save some memory without loss of performance. -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 v2 0/5] git-prompt: cleaning and improvement
Changes since the first version [1]: - During refactorization in 2/5, mention what the lack of \[...\] can affect. - New patch 3/5 that removes redundant tests (noticed by SZEDER Gábor [2]) - Rephrasing in 5/5: - rephrasing suggested by SZEDER Gábor [3] - rephrase some parts that mention only the current branch name being displayed in the prompt by stating that the repository status is displayed. - explain the color codes in the ZSH precmd example (there was an explanation that applied only for the Bash example, but it was not clear that it did not apply to ZSH). Following Gábor's example, I merged his patches series [4] to mine and published at: https://github.com/erdavila/git.git bash-prompt-speedup-and-color-refactorization Both Gábor's and my series were rebased/applied on top of the current master 9832cb9d4d. Thanks, Eduardo [1] http://thread.gmane.org/gmane.comp.version-control.git/228566 [2] http://article.gmane.org/gmane.comp.version-control.git/228705 [3] http://article.gmane.org/gmane.comp.version-control.git/228707 [4] http://thread.gmane.org/gmane.comp.version-control.git/228851 Eduardo R. D'Avila (5): t9903: add tests for git-prompt pcmode git-prompt.sh: refactor colored prompt code t9903: remove redundant tests git-prompt.sh: do not print duplicate clean color code git-prompt.sh: add missing information in comments contrib/completion/git-prompt.sh | 111 -- t/t9903-bash-prompt.sh | 141 +++ 2 files changed, 182 insertions(+), 70 deletions(-) -- 1.8.3.1.590.g42a98dd -- 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 1/5] t9903: add tests for git-prompt pcmode
git-prompt.sh lacks tests for PROMPT_COMMAND mode.
Add tests for:
* pcmode prompt without colors
* pcmode prompt with colors for bash
* pcmode prompt with colors for zsh
Having these tests enables an upcoming refactor in
a safe way.
Signed-off-by: Eduardo R. D'Avila erdav...@gmail.com
---
254 0 t/t9903-bash-prompt.sh
t/t9903-bash-prompt.sh | 254 +
1 file changed, 254 insertions(+)
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 15521cc..6a88778 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -10,6 +10,10 @@ test_description='test git-specific bash prompt functions'
. $GIT_BUILD_DIR/contrib/completion/git-prompt.sh
actual=$TRASH_DIRECTORY/actual
+c_red='\\[\\e[31m\\]'
+c_green='\\[\\e[32m\\]'
+c_lblue='\\[\\e[1;34m\\]'
+c_clear='\\[\\e[0m\\]'
test_expect_success 'setup for prompt tests' '
mkdir -p subdir/subsubdir
@@ -535,4 +539,254 @@ test_expect_success 'prompt - format string starting with
dash' '
test_cmp expected $actual
'
+test_expect_success 'prompt - pc mode' '
+ printf BEFORE: (master):AFTER expected
+ printf expected_output
+ (
+ __git_ps1 BEFORE: :AFTER $actual
+ test_cmp expected_output $actual
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - branch name' '
+ printf BEFORE: (${c_green}master${c_clear}${c_clear}):AFTER expected
+ (
+ GIT_PS1_SHOWCOLORHINTS=y
+ __git_ps1 BEFORE: :AFTER $actual
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - detached head' '
+ printf BEFORE: (${c_red}(%s...)${c_clear}${c_clear}):AFTER $(git log
-1 --format=%h b1^) expected
+ git checkout b1^
+ test_when_finished git checkout master
+ (
+ GIT_PS1_SHOWCOLORHINTS=y
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - dirty status indicator -
dirty worktree' '
+ printf BEFORE: (${c_green}master${c_clear} ${c_red}*${c_clear}):AFTER
expected
+ echo dirty file
+ test_when_finished git reset --hard
+ (
+ GIT_PS1_SHOWDIRTYSTATE=y
+ GIT_PS1_SHOWCOLORHINTS=y
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - dirty status indicator -
dirty index' '
+ printf BEFORE: (${c_green}master${c_clear}
${c_green}+${c_clear}):AFTER expected
+ echo dirty file
+ test_when_finished git reset --hard
+ git add -u
+ (
+ GIT_PS1_SHOWDIRTYSTATE=y
+ GIT_PS1_SHOWCOLORHINTS=y
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - dirty status indicator -
dirty index and worktree' '
+ printf BEFORE: (${c_green}master${c_clear}
${c_red}*${c_green}+${c_clear}):AFTER expected
+ echo dirty index file
+ test_when_finished git reset --hard
+ git add -u
+ echo dirty worktree file
+ (
+ GIT_PS1_SHOWCOLORHINTS=y
+ GIT_PS1_SHOWDIRTYSTATE=y
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - dirty status indicator -
before root commit' '
+ printf BEFORE: (${c_green}master${c_clear}
${c_green}#${c_clear}):AFTER expected
+ (
+ GIT_PS1_SHOWDIRTYSTATE=y
+ GIT_PS1_SHOWCOLORHINTS=y
+ cd otherrepo
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - inside .git directory' '
+ printf BEFORE: (${c_green}GIT_DIR!${c_clear}${c_clear}):AFTER
expected
+ echo dirty file
+ test_when_finished git reset --hard
+ (
+ GIT_PS1_SHOWDIRTYSTATE=y
+ GIT_PS1_SHOWCOLORHINTS=y
+ cd .git
+ __git_ps1 BEFORE: :AFTER
+ printf %s $PS1 $actual
+ )
+ test_cmp expected $actual
+'
+
+test_expect_success 'prompt - bash color pc mode - stash status indicator' '
+ printf BEFORE: (${c_green}master${c_clear}
${c_lblue}\$${c_clear}):AFTER expected
+ echo 2 file
+ git stash
+ test_when_finished git stash drop
+ (
+ GIT_PS1_SHOWSTASHSTATE=y
+ GIT_PS1_SHOWCOLORHINTS=y
+
[PATCH v2 2/5] git-prompt.sh: refactor colored prompt code
__git_ps1_colorize_gitstring() sets color codes and
builds the prompt gitstring. It has duplicated code
to handle color codes for bash and zsh shells.
__git_ps1() also has duplicated logic to build the
prompt gitstring.
Remove duplication of logic to build gitstring in
__git_ps1_colorize_gitstring() and __git_ps1().
Leave in __git_ps1_colorize_gitstring() only logic
to set color codes.
Signed-off-by: Eduardo R. D'Avila erdav...@gmail.com
---
26 59 contrib/completion/git-prompt.sh
contrib/completion/git-prompt.sh | 85
1 file changed, 26 insertions(+), 59 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 07a6218..fdedb45 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -225,8 +225,8 @@ __git_ps1_show_upstream ()
}
# Helper function that is meant to be called from __git_ps1. It
-# builds up a gitstring injecting color codes into the appropriate
-# places.
+# injects color codes into the appropriate gitstring variables used
+# to build a gitstring.
__git_ps1_colorize_gitstring ()
{
if [[ -n ${ZSH_VERSION-} ]]; then
@@ -234,74 +234,40 @@ __git_ps1_colorize_gitstring ()
local c_green='%F{green}'
local c_lblue='%F{blue}'
local c_clear='%f'
- local bad_color=$c_red
- local ok_color=$c_green
- local branch_color=$c_clear
- local flags_color=$c_lblue
- local branchstring=$c${b##refs/heads/}
-
- if [ $detached = no ]; then
- branch_color=$ok_color
- else
- branch_color=$bad_color
- fi
-
- gitstring=$branch_color$branchstring$c_clear
-
- if [ -n $w$i$s$u$r$p ]; then
- gitstring=$gitstring$z
- fi
- if [ $w = * ]; then
- gitstring=$gitstring$bad_color$w
- fi
- if [ -n $i ]; then
- gitstring=$gitstring$ok_color$i
- fi
- if [ -n $s ]; then
- gitstring=$gitstring$flags_color$s
- fi
- if [ -n $u ]; then
- gitstring=$gitstring$bad_color$u
- fi
- gitstring=$gitstring$c_clear$r$p
- return
+ else
+ # Using \[ and \] around colors is necessary to prevent
+ # issues with command line editing/browsing/completion!
+ local c_red='\[\e[31m\]'
+ local c_green='\[\e[32m\]'
+ local c_lblue='\[\e[1;34m\]'
+ local c_clear='\[\e[0m\]'
fi
- local c_red='\e[31m'
- local c_green='\e[32m'
- local c_lblue='\e[1;34m'
- local c_clear='\e[0m'
local bad_color=$c_red
local ok_color=$c_green
- local branch_color=$c_clear
local flags_color=$c_lblue
- local branchstring=$c${b##refs/heads/}
+ local branch_color=
if [ $detached = no ]; then
branch_color=$ok_color
else
branch_color=$bad_color
fi
+ c=$branch_color$c
+ b=$b$c_clear
- # Setting gitstring directly with \[ and \] around colors
- # is necessary to prevent wrapping issues!
- gitstring=\[$branch_color\]$branchstring\[$c_clear\]
-
- if [ -n $w$i$s$u$r$p ]; then
- gitstring=$gitstring$z
- fi
if [ $w = * ]; then
- gitstring=$gitstring\[$bad_color\]$w
+ w=$bad_color$w
fi
if [ -n $i ]; then
- gitstring=$gitstring\[$ok_color\]$i
+ i=$ok_color$i
fi
if [ -n $s ]; then
- gitstring=$gitstring\[$flags_color\]$s
+ s=$flags_color$s
fi
if [ -n $u ]; then
- gitstring=$gitstring\[$bad_color\]$u
+ u=$bad_color$u
fi
- gitstring=$gitstring\[$c_clear\]$r$p
+ r=$c_clear$r
}
# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
@@ -443,19 +409,20 @@ __git_ps1 ()
fi
local z=${GIT_PS1_STATESEPARATOR- }
+
+ # NO color option unless in PROMPT_COMMAND mode
+ if [ $pcmode = yes ] [ -n ${GIT_PS1_SHOWCOLORHINTS-} ];
then
+ __git_ps1_colorize_gitstring
+ fi
+
local f=$w$i$s$u
+ local gitstring=$c${b##refs/heads/}${f:+$z$f}$r$p
+
if [ $pcmode = yes ]; then
- local gitstring=
- if [ -n ${GIT_PS1_SHOWCOLORHINTS-} ]; then
- __git_ps1_colorize_gitstring
- else
- gitstring=$c${b##refs/heads/}${f:+$z$f}$r$p
- fi
[PATCH v2 3/5] t9903: remove redundant tests
After refactoring __git_ps1_colorize_gitstring, codepaths for bash and zsh
became mostly common and tests for bash and zsh became redundant.
Remove tests for zsh. Keep one minimal test that stress the difference
in codepaths for bash and zsh.
Suggested-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Eduardo R. D'Avila erdav...@gmail.com
---
1 114 t/t9903-bash-prompt.sh
t/t9903-bash-prompt.sh | 115 +
1 file changed, 1 insertion(+), 114 deletions(-)
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 6a88778..f250dfc 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -665,7 +665,7 @@ test_expect_success 'prompt - bash color pc mode -
untracked files status indica
test_cmp expected $actual
'
-test_expect_success 'prompt - zsh color pc mode - branch name' '
+test_expect_success 'prompt - zsh color pc mode' '
printf BEFORE: (%%F{green}master%%f%%f):AFTER expected
(
ZSH_VERSION=5.0.0
@@ -676,117 +676,4 @@ test_expect_success 'prompt - zsh color pc mode - branch
name' '
test_cmp expected $actual
'
-test_expect_success 'prompt - zsh color pc mode - detached head' '
- printf BEFORE: (%%F{red}(%s...)%%f%%f):AFTER $(git log -1
--format=%h b1^) expected
- git checkout b1^
- test_when_finished git checkout master
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWCOLORHINTS=y
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - dirty status indicator -
dirty worktree' '
- printf BEFORE: (%%F{green}master%%f %%F{red}*%%f):AFTER expected
- echo dirty file
- test_when_finished git reset --hard
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWDIRTYSTATE=y
- GIT_PS1_SHOWCOLORHINTS=y
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - dirty status indicator -
dirty index' '
- printf BEFORE: (%%F{green}master%%f %%F{green}+%%f):AFTER expected
- echo dirty file
- test_when_finished git reset --hard
- git add -u
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWDIRTYSTATE=y
- GIT_PS1_SHOWCOLORHINTS=y
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - dirty status indicator -
dirty index and worktree' '
- printf BEFORE: (%%F{green}master%%f %%F{red}*%%F{green}+%%f):AFTER
expected
- echo dirty index file
- test_when_finished git reset --hard
- git add -u
- echo dirty worktree file
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWCOLORHINTS=y
- GIT_PS1_SHOWDIRTYSTATE=y
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - dirty status indicator -
before root commit' '
- printf BEFORE: (%%F{green}master%%f %%F{green}#%%f):AFTER expected
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWDIRTYSTATE=y
- GIT_PS1_SHOWCOLORHINTS=y
- cd otherrepo
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - inside .git directory' '
- printf BEFORE: (%%F{green}GIT_DIR!%%f%%f):AFTER expected
- echo dirty file
- test_when_finished git reset --hard
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWDIRTYSTATE=y
- GIT_PS1_SHOWCOLORHINTS=y
- cd .git
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - stash status indicator' '
- printf BEFORE: (%%F{green}master%%f %%F{blue}$%%f):AFTER expected
- echo 2 file
- git stash
- test_when_finished git stash drop
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWSTASHSTATE=y
- GIT_PS1_SHOWCOLORHINTS=y
- __git_ps1 BEFORE: :AFTER
- printf %s $PS1 $actual
- )
- test_cmp expected $actual
-'
-
-test_expect_success 'prompt - zsh color pc mode - untracked files status
indicator' '
- printf BEFORE: (%%F{green}master%%f %%F{red}%%f):AFTER expected
- (
- ZSH_VERSION=5.0.0
- GIT_PS1_SHOWUNTRACKEDFILES=y
- GIT_PS1_SHOWCOLORHINTS=y
- __git_ps1
[PATCH v2 4/5] git-prompt.sh: do not print duplicate clean color code
Do not print a duplicate clean color code when there
is no other indicators other than the current branch
in colored prompt.
Acked-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Eduardo R. D'Avila erdav...@gmail.com
---
1 1 contrib/completion/git-prompt.sh
4 4 t/t9903-bash-prompt.sh
contrib/completion/git-prompt.sh | 2 +-
t/t9903-bash-prompt.sh | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index fdedb45..545518a 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -253,8 +253,8 @@ __git_ps1_colorize_gitstring ()
branch_color=$bad_color
fi
c=$branch_color$c
- b=$b$c_clear
+ z=$c_clear$z
if [ $w = * ]; then
w=$bad_color$w
fi
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index f250dfc..5cd138e 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -551,7 +551,7 @@ test_expect_success 'prompt - pc mode' '
'
test_expect_success 'prompt - bash color pc mode - branch name' '
- printf BEFORE: (${c_green}master${c_clear}${c_clear}):AFTER expected
+ printf BEFORE: (${c_green}master${c_clear}):AFTER expected
(
GIT_PS1_SHOWCOLORHINTS=y
__git_ps1 BEFORE: :AFTER $actual
@@ -561,7 +561,7 @@ test_expect_success 'prompt - bash color pc mode - branch
name' '
'
test_expect_success 'prompt - bash color pc mode - detached head' '
- printf BEFORE: (${c_red}(%s...)${c_clear}${c_clear}):AFTER $(git log
-1 --format=%h b1^) expected
+ printf BEFORE: (${c_red}(%s...)${c_clear}):AFTER $(git log -1
--format=%h b1^) expected
git checkout b1^
test_when_finished git checkout master
(
@@ -627,7 +627,7 @@ test_expect_success 'prompt - bash color pc mode - dirty
status indicator - befo
'
test_expect_success 'prompt - bash color pc mode - inside .git directory' '
- printf BEFORE: (${c_green}GIT_DIR!${c_clear}${c_clear}):AFTER
expected
+ printf BEFORE: (${c_green}GIT_DIR!${c_clear}):AFTER expected
echo dirty file
test_when_finished git reset --hard
(
@@ -666,7 +666,7 @@ test_expect_success 'prompt - bash color pc mode -
untracked files status indica
'
test_expect_success 'prompt - zsh color pc mode' '
- printf BEFORE: (%%F{green}master%%f%%f):AFTER expected
+ printf BEFORE: (%%F{green}master%%f):AFTER expected
(
ZSH_VERSION=5.0.0
GIT_PS1_SHOWCOLORHINTS=y
--
1.8.3.1.590.g42a98dd
--
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 5/5] git-prompt.sh: add missing information in comments
Mention that the command below is needed for prompt
in ZSH with PS1:
setopt PROMPT_SUBST
Rephrase some parts that mention only the current branch name
being displayed in the prompt. Replace it by stating that
the repository status is displayed.
Make it clear that colored prompt is only available
in PROMPT_COMMAND/precmd mode.
With-suggestions-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Eduardo R. D'Avila erdav...@gmail.com
---
15 11 contrib/completion/git-prompt.sh
contrib/completion/git-prompt.sh | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 545518a..b3f39e8 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -3,7 +3,7 @@
# Copyright (C) 2006,2007 Shawn O. Pearce spea...@spearce.org
# Distributed under the GNU General Public License, version 2.0.
#
-# This script allows you to see the current branch in your prompt.
+# This script allows you to see repository status in your prompt.
#
# To enable:
#
@@ -13,24 +13,27 @@
#3a) Change your PS1 to call __git_ps1 as
#command-substitution:
#Bash: PS1='[\u@\h \W$(__git_ps1 (%s))]\$ '
-#ZSH: PS1='[%n@%m %c$(__git_ps1 (%s))]\$ '
+#ZSH: setopt PROMPT_SUBST ; PS1='[%n@%m %c$(__git_ps1 (%s))]\$ '
#the optional argument will be used as format string.
-#3b) Alternatively, if you are using bash, __git_ps1 can be
-#used for PROMPT_COMMAND with two parameters, pre and
+#3b) Alternatively, __git_ps1 can be used for PROMPT_COMMAND in
+#Bash or for precmd() in ZSH with two parameters, pre and
#post, which are strings you would put in $PS1 before
#and after the status string generated by the git-prompt
#machinery. e.g.
#Bash: PROMPT_COMMAND='__git_ps1 \u@\h:\w \\\$ '
+# will show username, at-sign, host, colon, cwd, then
+# various status string, followed by dollar and SP, as
+# your prompt.
#ZSH: precmd () { __git_ps1 %n :%~$ |%s }
-#will show username, at-sign, host, colon, cwd, then
-#various status string, followed by dollar and SP, as
-#your prompt.
+# will show username, pipe, then various status string,
+# followed by colon, cwd, dollar and SP, as your prompt.
#Optionally, you can supply a third argument with a printf
#format string to finetune the output of the branch status
#
-# The argument to __git_ps1 will be displayed only if you are currently
-# in a git repository. The %s token will be the name of the current
-# branch.
+# The repository status will be displayed only if you are currently in a
+# git repository. The %s token is the placeholder for the shown status.
+#
+# The prompt status always includes the current branch name.
#
# In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value,
# unstaged (*) and staged (+) changes will be shown next to the branch
@@ -78,7 +81,8 @@
#
# If you would like a colored hint about the current dirty state, set
# GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
-# the colored output of git status -sb.
+# the colored output of git status -sb and are available only when
+# using __git_ps1 for PROMPT_COMMAND or precmd.
# __gitdir accepts 0 or 1 arguments (i.e., location)
# returns location of .git repo
--
1.8.3.1.590.g42a98dd
--
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 03/16] pack-objects: use a faster hash table
On Tue, Jun 25, 2013 at 10:14:17PM -0400, Jeff King wrote:
So I suspect two things (but as I said, haven't verified):
1. You could speed up pack-objects just by keeping the table half full
rather than 3/4 full.
I wasn't able to show any measurable speedup with this. I tried to make
as specific a measurement as I could, by adding a counting only option
like this:
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index fc12df8..a0438d0 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2452,6 +2452,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
const char *rp_av[6];
int rp_ac = 0;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
+ int counting_only = 0;
struct option pack_objects_options[] = {
OPT_SET_INT('q', quiet, progress,
N_(do not show progress meter), 0),
@@ -2515,6 +2516,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(pack compression level)),
OPT_SET_INT(0, keep-true-parents, grafts_replace_parents,
N_(do not hide commits by grafts), 0),
+ OPT_BOOL(0, counting-only, counting_only,
+N_(exit after counting objects phase)),
OPT_END(),
};
@@ -2600,6 +2603,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
for_each_ref(add_ref_tag, NULL);
stop_progress(progress_state);
+ if (counting_only)
+ return 0;
if (non_empty !nr_result)
return 0;
if (nr_result)
and even doing the whole object traversal ahead of time to just focus on
the object-entry hash, like this:
git rev-list --objects --all objects.out
time git pack-objects --counting-only --stdout objects.out
Tweaking the hash size didn't have any effect, but using Vicent's khash
patch actually made it about 5% slower. So I wonder if I'm even
measuring the right thing. Vicent, how did you get the timings you
showed in the commit message?
-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 09/16] documentation: add documentation for the bitmap format
On Tue, Jun 25, 2013 at 09:33:11PM +0200, Vicent Martí wrote: One way we side-stepped the size inflation problem in JGit was to only use the bitmap index information when sending data on the wire to a client. Here delta reuse plays a significant factor in building the pack, and we don't have to be as accurate on matching deltas. During the equivalent of `git repack` bitmaps are not used, allowing the traditional graph enumeration algorithm to generate path hash information. OH BOY HERE WE GO. This is worth its own thread, lots to discuss here. I think peff will have a patchset regarding this to upstream soon, we'll get back to it later. We do the same thing (only use bitmaps during on-the-wire fetches). But there a few problems with assuming delta reuse. For us (GitHub), the foremost one is that we pack many forks of a repository together into a single packfile. That means when you clone torvalds/linux, an object you want may be stored in the on-disk pack with a delta against an object that you are not going to get. So we have to throw out that delta and find a new one. I'm dealing with that by adding an option to respect islands during packing, where an island is a set of common objects (we split it by fork, since we expect those objects to be fetched together, but you could use other criteria). The rule is that an object cannot delta against another object that is not in all of its islands. So everybody can delta against shared history, but objects in your fork can only delta against other objects in the fork. You are guaranteed to be able to reuse such deltas during a full clone of a fork, and the on-disk pack size does not suffer all that much (because there is usually a good alternate delta base within your reachable history). So with that series, we can get good reuse for clones. But there are still two cases worth considering: 1. When you fetch a subset of the commits, git marks only the edges as preferred bases, and does not walk the full object graph down to the roots. So any object you want that is delta'd against something older will not get reused. If you have reachability bitmaps, I don't think there is any reason that we cannot use the entire object graph (starting at the have tips, of course) as preferred bases. 2. The server is not necessarily fully packed. In an active repo, you may have a large base pack with bitmaps, with several recently pushed packs on top. You still need to delta the recently pushed objects against the base objects. I don't have measurements on how much the deltas suffer in those two cases. I know they suffered quite badly for clones without the name hashes in our alternates repos, but that part should go away with my patch series. -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 11/16] rev-list: add bitmap mode to speed up lists
On Tue, Jun 25, 2013 at 09:22:28AM -0700, Thomas Rast wrote: Vicent Marti tan...@gmail.com writes: Calling `git rev-list --use-bitmaps [committish]` is the equivalent of `git rev-list --objects`, but the rev list is performed based on a bitmap result instead of using a manual counting objects phase. Why would we ever want to not --use-bitmaps, once it actually works? I.e., shouldn't this be the default if pack.usebitmaps is set (or possibly even core.usebitmaps for these things)? If you are using bitmaps, you cannot produce the same output as --objects; the latter prints the path at which each object is found. In the JGit bitmap format, we have no information at all; in Vicent's v2, we have only a hash of that pathname. -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] documentation: add git transport security notice
The fact that the git transport does not do any authentication is
easily overlooked. For example, DNS poisoning may result in
fetching from somewhere that was not intended.
Add a brief security notice to the GIT URLS section
of the documentation stating that the git transport should be used
with caution on unsecured networks.
Signed-off-by: Fraser Tweedale fr...@frase.id.au
---
Documentation/urls.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 3ca122f..b58a647 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -11,6 +11,9 @@ and ftps can be used for fetching and rsync can be used for
fetching
and pushing, but these are inefficient and deprecated; do not use
them).
+The git transport does not do any authentication and should be used
+with caution on unsecured networks.
+
The following syntaxes may be used with them:
- ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/path/to/repo.git/
--
1.8.3.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
