Re: [PATCH] sequencer.c: terminate the last line of author-script properly
Hi Akinori On 12/07/18 12:18, Akinori MUSHA wrote: It looks like write_author_script() intends to write out a file in Bourne shell syntax, but it doesn't put a closing single quote on the last line. This patch makes .git/rebase-merge/author-script actually parsable by sh(1) by adding a single quote and a linefeed to terminate the line properly. Signed-off-by: Akinori MUSHA --- sequencer.c | 1 + t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+) diff --git a/sequencer.c b/sequencer.c index 4034c0461..5f32b6df1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -651,6 +651,7 @@ static int write_author_script(const char *message) strbuf_addch(&buf, *(message++)); else strbuf_addf(&buf, "'%c'", *(message++)); + strbuf_addstr(&buf, "'\n"); res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1); The third parameter here means that write_message() will append a new line (you can check this by looking at the file that's created) so strictly speaking we only need to add "'" to the end of the message, alternatively it would be more obvious to keep adding "'\n" and change 1 to 0 in the call to write_message() Best Wishes Phillip strbuf_release(&buf); return res; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 352a52e59..345b103eb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' ' test_line_count = 6 actual ' +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' ' + test_when_finished "git rebase --abort ||:" && + git checkout master && + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i HEAD^ && + test -f .git/rebase-merge/author-script && + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && + eval "$(cat .git/rebase-merge/author-script)" && + test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" && + test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" && + test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE" +' + test_expect_success 'rebase -i with the exec command' ' git checkout master && (
[PATCH v5 2/7] t/t7510: check the validation of the new config gpg.format
Test setting gpg.format to both invalid and valid values. Signed-off-by: Henning Schild --- t/t7510-signed-commit.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 6e2015ed9..4e37ff8f1 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -227,4 +227,11 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' ' grep "gpg: Good signature" actual ' +test_expect_success GPG 'check config gpg.format values' ' + test_config gpg.format openpgp && + git commit -S --amend -m "success" && + test_config gpg.format OpEnPgP && + test_must_fail git commit -S --amend -m "fail" +' + test_done -- 2.16.4
[PATCH v5 7/7] gpg-interface t: extend the existing GPG tests with GPGSM
Add test cases to cover the new X509/gpgsm support. Most of them
resemble existing ones. They just switch the format to x509 and set the
signingkey when creating signatures. Validation of signatures does not
need any configuration of git, it does need gpgsm to be configured to
trust the key(-chain).
Several of the testcases build on top of existing gpg testcases.
The commit ships a self-signed key for commit...@example.com and
configures gpgsm to trust it.
Signed-off-by: Henning Schild
---
t/lib-gpg.sh | 28 -
t/lib-gpg/gpgsm-gen-key.in | 8 +++
t/lib-gpg/gpgsm_cert.p12 | Bin 0 -> 2652 bytes
t/t4202-log.sh | 37
t/t5534-push-signed.sh | 51 +
t/t7004-tag.sh | 13
t/t7030-verify-tag.sh | 33 +
7 files changed, 169 insertions(+), 1 deletion(-)
create mode 100644 t/lib-gpg/gpgsm-gen-key.in
create mode 100644 t/lib-gpg/gpgsm_cert.p12
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index a5d3b2cba..3fe02876c 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -38,7 +38,33 @@ then
"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
gpg --homedir "${GNUPGHOME}" /dev/null 2>&1 \
--sign -u commit...@example.com &&
- test_set_prereq GPG
+ test_set_prereq GPG &&
+ # Available key info:
+ # * see t/lib-gpg/gpgsm-gen-key.in
+ # To generate new certificate:
+ # * no passphrase
+ # gpgsm --homedir /tmp/gpghome/ \
+ # -o /tmp/gpgsm.crt.user \
+ # --generate-key \
+ # --batch t/lib-gpg/gpgsm-gen-key.in
+ # To import certificate:
+ # gpgsm --homedir /tmp/gpghome/ \
+ # --import /tmp/gpgsm.crt.user
+ # To export into a .p12 we can later import:
+ # gpgsm --homedir /tmp/gpghome/ \
+ # -o t/lib-gpg/gpgsm_cert.p12 \
+ # --export-secret-key-p12 "commit...@example.com"
+ echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
+ --passphrase-fd 0 --pinentry-mode loopback \
+ --import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+ gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+ | grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+ ${GNUPGHOME}/trustlist.txt &&
+ echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+ (gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+ echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+ -u commit...@example.com -o /dev/null --sign - 2>&1 &&
+ test_set_prereq GPGSM
;;
esac
fi
diff --git a/t/lib-gpg/gpgsm-gen-key.in b/t/lib-gpg/gpgsm-gen-key.in
new file mode 100644
index 0..a7fd87c06
--- /dev/null
+++ b/t/lib-gpg/gpgsm-gen-key.in
@@ -0,0 +1,8 @@
+Key-Type: RSA
+Key-Length: 2048
+Key-Usage: sign
+Serial: random
+Name-DN: CN=C O Mitter, O=Example, SN=C O, GN=Mitter
+Name-Email: commit...@example.com
+Not-Before: 1970-01-01 00:00:00
+Not-After: 3000-01-01 00:00:00
diff --git a/t/lib-gpg/gpgsm_cert.p12 b/t/lib-gpg/gpgsm_cert.p12
new file mode 100644
index
..94ffad0d31a3b6c4e849b29d960762e485ba7ea8
GIT binary patch
literal 2652
zcmY+Ec{mh|8pQ{*m?2^;S+ivrVaAfiQnHSnA|b+zofwQYAI3haA=!75ZX!!|$-Y*$
zWS5XVvW}3h?sM<`?)~F^&U?;zp7ZAqMS|U-rJ+NSVEkYxG8!9AJx2qf$s@s-fg~8i
zSqwpufKGo`;5-uW&RJwiO9MC)gTEUZ6fYR|?*&F0Fp3FCzs?3aZK`58prxe;gpq&(
zm?nam#=;<-Y>ihd?+EMByF}ef4%bKLwbO{e4U=0LG4jP<8x$`N#5#&@0as@!cSSK>
zh}oaTUi^XPMV(3;kKAjFB5Yq)zn_vnExr>`s&?M}i{A>>$j-{u*Jo)riK{fW!LXet
z)hkG3xgvIpDClY8=o4q?_bD%htwXG!_=;NxVjU=3#{TU0Q@=ZMc%6wes*Uy&CAPf=JB(a
zb<9VW4e)@R+qGEgfnewABCPf}mtJOYxL>*jtu9yKk?Fu3UWr~zJ#<*I(Ey>fhA6~_gUvg(8n<*TM
zdXo$0jMn2G$GcM484J8MUx*x=@XWq^uvCnI1jp>#Y_2Y<+T_*yX!uTA$vm7Ugs56O
z@!~>Y?E@FA@{5@ZDHD`TkEeHQ{M2n_Slp%By{89uVia3%>w?IZdvBBX$K*3UzCM7`
zxGZ-dbh^KlF#?RZaS4_^R|%@F5xX0j)vdE&#M1Ch^W+WzQjVH^m|vu9jE@aRtlbcP
z^NxHpt!%n59KJ!@-IHzjWw9l;)N}J^LLwLT#Xz!Tq4vWkHTO5#j_1_nvA>#u
zBX5jy&r_5h7PKFH26B8-1BMryQYu8HiW#k_Nq!qB4-5#YH|RDUL_&Ug
z+Y4DwCgmPTpSqu5lU#dxgcf>N8-aXsM}TDNd_~tW$92abZG)0q=Xzr)t;M39D`u~9
zdbi)dKxTXdE3OHN@sCmOGfdNEKC*BgS&ku$LNb1M>x6#MZ_37F<5gz)d_&6)%S0zP
z5_lFp_A5n(wMlQ+_2n%!z1tomK+~=DQ9MC|QDxZ1?2?A{(Dqi(*TE;q4g+&HH;vvX
zI%M?`5*^D+i5eC~tth)c%;)Q^n`$=Nt8=_jLdLg)*d^-BKJ2siLCaLDMJwSxfF|uO
zc}bc4oW*xw>!m!o6BG%Q_CG+$BZ1<8Bv8~@9Da5oV21zT1x7=A#-YtK0ImHWb?E+3
z$NK7MLMBq{?jPy^Nx&Yxu7#tyt@t3@=F07Bf;BHh5A|9FYRL9bIo$1a44s9^Wk41>
zq-0_p!?=F1wYc$wC8sgycQ~IHO0x4@BODYL?(uFu5qaXSm+YP$_!_RrSrzJ#*f(fy
zF)|i|!Ac}}R(rT~U3@SDll^wcX#c1XS5VcvP3@>e?Qfbr5MU*oxy@`fDnT0wr
zw!P*fjd#ErDfYvcz210jWuO
Re: [PATCH] sequencer.c: terminate the last line of author-script properly
Hi Junio On 12/07/18 18:22, Junio C Hamano wrote: "Akinori MUSHA" writes: It looks like write_author_script() intends to write out a file in Bourne shell syntax, but it doesn't put a closing single quote on the last line. s/closing single quote/& and the terminating newline/? This patch makes .git/rebase-merge/author-script actually parsable by sh(1) by adding a single quote and a linefeed to terminate the line properly. Sounds good. I wonder why this breakage was left unnoticed for a long time, though. It's not like writing and reading the author-script from C code was done first in the "rebase -i" and friends that are users of the sequencer machinery The only consumer of a faulty author script written by the sequencer is read_env_script() in sequencer.c which doesn't worry about checking that quotes are paired. The shell versions of rebase use the code in git-sh-setup.sh to create the author script which does write valid posix shell. (I think we had code to do so in "git am" that was rewritten in C first). The code in builtin/am.c doesn't try to write valid posix shell (if one assumes it is the only consumer of the author script then it doesn't need to) which results in simpler code, but external scripts cannot safely eval it anymore. Do we have a similar issue over there as well? If not, perhaps if we reused the existing code that was not broken, we wouldn't have seen this breakage on the sequencer side? Best Wishes Phillip Thanks. Signed-off-by: Akinori MUSHA --- sequencer.c | 1 + t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+) diff --git a/sequencer.c b/sequencer.c index 4034c0461..5f32b6df1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -651,6 +651,7 @@ static int write_author_script(const char *message) strbuf_addch(&buf, *(message++)); else strbuf_addf(&buf, "'%c'", *(message++)); + strbuf_addstr(&buf, "'\n"); res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1); strbuf_release(&buf); return res; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 352a52e59..345b103eb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' ' test_line_count = 6 actual ' +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' ' + test_when_finished "git rebase --abort ||:" && + git checkout master && + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i HEAD^ && + test -f .git/rebase-merge/author-script && + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && + eval "$(cat .git/rebase-merge/author-script)" && + test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" && + test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" && + test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE" +' + test_expect_success 'rebase -i with the exec command' ' git checkout master && ( -- 2.18.0
Re: [PATCH v4 2/7] t/t7510: check the validation of the new config gpg.format
Am Tue, 17 Jul 2018 14:31:56 -0700 schrieb Junio C Hamano : > Henning Schild writes: > > > Test setting gpg.format to both invalid and valid values. > > > > Signed-off-by: Henning Schild > > --- > > t/t7510-signed-commit.sh | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh > > index 6e2015ed9..7bdad570c 100755 > > --- a/t/t7510-signed-commit.sh > > +++ b/t/t7510-signed-commit.sh > > @@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature > > behaves like --show-signature' ' grep "gpg: Good signature" actual > > ' > > > > +test_expect_success GPG 'check config gpg.format values' ' > > + test_config gpg.format openpgp && > > + git commit -S --amend -m "success" && > > + test_config gpg.format OpEnPgP && > > + test_must_fail git commit -S --amend -m "fail" && > > This second one is a good demonstration that the value for this > variable is case sensitive. > > > + test_config gpg.format malformed && > > + test_must_fail git commit -S --amend -m "fail" > > And there is no longer a good reason to try another one. Let's drop > this last/third case. Done. Henning > > +' > > + > > test_done
Re: [PATCH v4 7/7] gpg-interface t: extend the existing GPG tests with GPGSM
Am Tue, 17 Jul 2018 14:31:36 -0700
schrieb Junio C Hamano :
> Henning Schild writes:
>
> > diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> > index a5d3b2cba..3fe02876c 100755
> > --- a/t/lib-gpg.sh
> > +++ b/t/lib-gpg.sh
> > @@ -38,7 +38,33 @@ then
> > "$TEST_DIRECTORY"/lib-gpg/ownertrust &&
> > gpg --homedir "${GNUPGHOME}" /dev/null
> > 2>&1 \ --sign -u commit...@example.com &&
> > - test_set_prereq GPG
> > + test_set_prereq GPG &&
>
> We do not mind making GPGSM dependent on GPG, hence this && is
> justified.
>
> > + # Available key info:
> > + # * see t/lib-gpg/gpgsm-gen-key.in
> > + # To generate new certificate:
> > + # * no passphrase
> > + # gpgsm --homedir /tmp/gpghome/ \
> > + # -o /tmp/gpgsm.crt.user \
> > + # --generate-key \
> > + # --batch t/lib-gpg/gpgsm-gen-key.in
> > + # To import certificate:
> > + # gpgsm --homedir /tmp/gpghome/ \
> > + # --import /tmp/gpgsm.crt.user
> > + # To export into a .p12 we can later import:
> > + # gpgsm --homedir /tmp/gpghome/ \
> > + # -o t/lib-gpg/gpgsm_cert.p12 \
> > + # --export-secret-key-p12
> > "commit...@example.com"
> > + echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
> > + --passphrase-fd 0 --pinentry-mode loopback
> > \
> > + --import
> > "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
> > + gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
> > + | grep fingerprint: | cut -d" " -f4 | tr
> > -d '\n' > \
> > + ${GNUPGHOME}/trustlist.txt &&
> > + echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
> > + (gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
> > + echo hello | gpgsm --homedir "${GNUPGHOME}"
> > >/dev/null \
> > + -u commit...@example.com -o /dev/null
> > --sign - 2>&1 &&
> > + test_set_prereq GPGSM
>
> And when any of the above fails, we refrain from setting GPGSM
> prereq. Otherwise we are prepared to perform tests with gpgsm
> and get the prereq.
>
> > diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> > index 25b1f8cc7..f57781e39 100755
> > --- a/t/t4202-log.sh
> > +++ b/t/t4202-log.sh
> > @@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed
> > branch' ' git commit -S -m signed_commit
> > '
> >
> > +test_expect_success GPGSM 'setup signed branch x509' '
> > + test_when_finished "git reset --hard && git checkout
> > master" &&
> > + git checkout -b signed-x509 master &&
> > + echo foo >foo &&
> > + git add foo &&
> > + test_config gpg.format x509 &&
> > + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> > + git commit -S -m signed_commit
> > +'
>
> OK.
>
> > +test_expect_success GPGSM 'log --graph --show-signature x509' '
> > + git log --graph --show-signature -n1 signed-x509 >actual &&
> > + grep "^| gpgsm: Signature made" actual &&
> > + grep "^| gpgsm: Good signature" actual
> > +'
>
> OK.
>
> > @@ -1581,6 +1597,29 @@ test_expect_success GPG 'log --graph
> > --show-signature for merged tag' ' grep "^| | gpg: Good signature"
> > actual '
> >
> > +test_expect_success GPGSM 'log --graph --show-signature for merged
> > tag x509' '
> > + test_when_finished "git reset --hard && git checkout
> > master" &&
> > + test_config gpg.format x509 &&
> > + test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> > + git checkout -b plain-x509 master &&
> > + echo aaa >bar &&
> > + git add bar &&
> > + git commit -m bar_commit &&
> > + git checkout -b tagged-x509 master &&
> > + echo bbb >baz &&
> > + git add baz &&
> > + git commit -m baz_commit &&
> > + git tag -s -m signed_tag_msg signed_tag_x509 &&
> > + git checkout plain-x509 &&
> > + git merge --no-ff -m msg signed_tag_x509 &&
> > + git log --graph --show-signature -n1 plain-x509 >actual &&
> > + grep "^|\\\ merged tag" actual &&
> > + grep "^| | gpgsm: Signature made" actual &&
> > + grep "^| | gpgsm: Good signature" actual &&
> > + git config --unset gpg.format &&
> > + git config --unset user.signingkey
>
> You are using test_config early enough in this test; doesn't that
> take care of the last two steps for you, even when an earlier step
> failed? If that is the case, then remove the last two line (and &&
> at the end of the line before).
Right, dropped those two lines and &&
> > diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> > index 1cea758f7..a3a12bd05 100755
> > --- a/t/t5534-push-signed.sh
> > +++ b/t/t5534-push-signed.sh
> > @@ -218,4 +218,56 @@ test_expect_success GPG 'fail without key and
> > heed user.signingkey' ' test_cmp expect dst/push-cert-status
> > '
> >
> > +test_expect_success GPGSM 'fail without key and heed
> > user.
Re: [PATCH 2/2] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
On 7/17/2018 6:49 PM, Stefan Beller wrote:
Signed-off-by: Stefan Beller
---
builtin/replace.c | 3 ++-
refs.c| 9 -
refs.h| 2 +-
replace-object.c | 3 ++-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index ef22d724bbc..5f34659071f 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -39,7 +39,8 @@ struct show_data {
enum replace_format format;
};
-static int show_reference(const char *refname, const struct object_id *oid,
+static int show_reference(struct repository *r, const char *refname,
+ const struct object_id *oid,
int flag, void *cb_data)
{
struct show_data *data = cb_data;
diff --git a/refs.c b/refs.c
index 2513f77acb3..5700cd4683f 100644
--- a/refs.c
+++ b/refs.c
@@ -1478,12 +1478,11 @@ int refs_for_each_fullref_in(struct ref_store *refs,
const char *prefix,
return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
}
-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
*cb_data)
{
- return do_for_each_ref(get_main_ref_store(r),
- git_replace_ref_base, fn,
- strlen(git_replace_ref_base),
- DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
+ return do_for_each_repo_ref(r, git_replace_ref_base, fn,
+ strlen(git_replace_ref_base),
+ DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 80eec8bbc68..a0a18223a14 100644
--- a/refs.h
+++ b/refs.h
@@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn,
void *cb_data,
int for_each_tag_ref(each_ref_fn fn, void *cb_data);
int for_each_branch_ref(each_ref_fn fn, void *cb_data);
int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
+int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
*cb_data);
int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
const char *prefix, void *cb_data);
diff --git a/replace-object.c b/replace-object.c
index 801b5c16789..01a5a59a35a 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -6,7 +6,8 @@
#include "repository.h"
#include "commit.h"
-static int register_replace_ref(const char *refname,
+static int register_replace_ref(struct repository *r,
+ const char *refname,
const struct object_id *oid,
int flag, void *cb_data)
{
Overall, I think this is the right approach. The only problem is that
you're missing a few 'the_repository' to 'r' replacements in the bodies
of show_reference and register_replace_ref.
Thanks,
-Stolee
sed: command garbled: rGIT-PERL-HEADER
I'm trying to build Git 2.18 on Solaris 11.3 x86_64.
$ gmake V=1
rm -f git-add--interactive git-add--interactive+ && \
sed -e '1{' \
-e 's|#!.*perl|#!/usr/bin/perl|' \
-e 'rGIT-PERL-HEADER' \
-e 'G' \
-e '}' \
-e 's/@@GIT_VERSION@@/2.18.0/g' \
git-add--interactive.perl >git-add--interactive+ && \
chmod +x git-add--interactive+ && \
mv git-add--interactive+ git-add--interactive
sed: command garbled: rGIT-PERL-HEADER
gmake: *** [git-add--interactive] Error 2
And:
$ perl --version
This is perl 5, version 12, subversion 5 (v5.12.5) built for i86pc-solaris-64int
(with 7 registered patches, see perl -V for more detail)
Any ideas how to fix it?
Thanks in advance.
Re: [PATCH 0/4] Use oid_object_info() instead of read_object_file()
Hi Olga,
On Fri, 13 Jul 2018, Оля Тележная wrote:
> 2018-07-09 11:27 GMT+03:00 Оля Тележная :
> > Hello everyone,
> > This is my new attempt to start using oid_object_info_extended() in
> > ref-filter. You could look at previous one [1] [2] but it is not
> > necessary.
> >
> > The goal (still) is to improve performance by avoiding calling expensive
> > functions when we don't need the information they provide
> > or when we could get it by using a cheaper function.
> >
> > This patch is a middle step. In the end, I want to add new atoms
> > ("objectsize:disk" and "deltabase") and reuse ref-filter logic in
> > cat-file command.
> >
> > I also know about problems with memory leaks in ref-filter: that would
> > be my next task that I will work on. Since I did not generate any new
> > leaks in this patch (just use existing ones), I decided to put this
> > part on a review and fix leaks as a separate task.
>
> UPDATES since v1:
> add init to eaten variable (thanks to Szeder Gabor, Johannes Schindelin)
> improve second commit message (thanks to Junio C Hamano)
> add static keyword (thanks to Ramsay Jones)
>
> >
> > Thank you!
> >
> > [1] https://github.com/git/git/pull/493
Could you please populate the description of that PR so that SubmitGit
picks it up as cover letter?
Thanks,
Johannes
> > [2]
> > https://public-inbox.org/git/010201637254c969-a346030e-0b75-41ad-8ef3-2ac7e04ba4fb-000...@eu-west-1.amazonses.com/
>
Re: Git 2.18: RUNTIME_PREFIX... is it working?
Hi, [dropping Perry's non-working email address again] On Sat, 14 Jul 2018, brian m. carlson wrote: > On Tue, Jul 10, 2018 at 10:03:10AM -0400, Jeff King wrote: > > My point is that aside from RUNTIME_PREFIX, we don't need /proc. So > > somebody who currently builds Git with a static path like > > "/usr/libexec/git-core" and runs it inside a chroot will be just fine as > > long as /usr/libexec/git-core is available at that name inside the > > chroot. But if the build starts relying on RUNTIME_PREFIX, it's going to > > regress their case. > > I will say that at cPanel, we have a configuration where end users can > end up inside a mount namespace without /proc (depending on the > preferences of the administrator). However, it's easy enough for us to > simply build without RUNTIME_PREFIX if necessary. > > If we turn it on by default, it would be nice if we documented (maybe in > the Makefile) that it requires /proc on Linux for the benefit of other > people who might be in a similar situation. Is there *really* no other way on Linux to figure out the absolute path of the current executable than to open a pseudo file in the `/proc` file system? Ciao, Johannes
Re: [PATCH 00/16] Consolidate reachability logic
Hi Peff, On Mon, 16 Jul 2018, Jeff King wrote: > On Mon, Jul 16, 2018 at 02:54:38PM +0100, Ramsay Jones wrote: > > > On 16/07/18 14:00, Derrick Stolee via GitGitGadget wrote: > > > There are many places in Git that use a commit walk to determine > > > reachability between commits and/or refs. A lot of this logic is > > > duplicated. > > [snip] ... > > > > This is not your problem, but I find these GitGitGadget > > submissions somewhat annoying. This series has been spewed > > all over my in-box in, what I assume, is commit date order. > > > > So, patches #4,5 dated 19/06, then #1,2,3 dated 25/06, > > then #15 dated 28/06, then #6,7 dated 12/07, then #8-16 > > dated 13/07, then 00/16 dated today. > > > > No I don't use a threaded display (I hate it), be even with > > that turned on, the patches still appear in the above order > > under the cover letter (but at least all together). > > Yeah, they're out of order in mutt's threaded display. And the > back-dating means there's a much higher chance of them getting blocked > as spam (e.g., some of the dates are from weeks ago). > > git-send-email uses the current time minus an offset, and then > monotonically increases for each patch: > > $time = time - scalar $#files; > ... > my $date = format_2822_time($time++); > > which seems to work pretty well in practice. It does mean the original > dates are lost. The committer date is not interesting at all (there will > be a new committer via "git am" anyway). The original author date is > potentially of interest, but could be included as an in-body header. > AFAIK send-email doesn't have such an option, though, and people are > fine with date-of-sending becoming the new author date. > > +cc Johannes as the GitGitGadget author Thanks for dumping even more work on my shoulders. I wanted to help with that insane process we have here, but in a more collaborative manner. This time I fixed it, but please do keep in mind that the decision to use the email transport for something it *was not designed for* (it was designed for humans talking to humans) is the culprit here. Next time, I will ask you to jump in, instead of putting the onus on me. I mean, seriously, what is this? "You can use *any* mail program to work with the Git mailing list, *any* mailer. As long as it is mutt. And as long as you spend hours and hours on tooling that oh BTW nobody else can use." Hopefully GitGitGadget will make this situation better. And hopefully not on the expense of my sanity. Ciao, Dscho
Re: [PATCH 00/16] Consolidate reachability logic
Hi, On Mon, 16 Jul 2018, Derrick Stolee wrote: > On 7/16/2018 2:44 PM, Eric Sunshine wrote: > > On Mon, Jul 16, 2018 at 1:27 PM Stefan Beller wrote: > > > Another pain point of the Gadget is that CC's in the cover letter > > > do not work as I would imagine. The line > > > > > > CC: sbel...@google.com > > > > > > did not put that email into the cc field. > > gitgitgadget recognizes case-sensitive "Cc:" only[1]. > > > > [1]: > > https://github.com/gitgitgadget/gitgitgadget/blob/c4805370f59532aa438283431b8ea7d4484c530f/lib/patch-series.ts#L188 > > Thanks for everyone's patience while we improve gitgitgadget (and - in this > case - I learn how to use it). And let's please stop pretending that this GitGitGadget project is somebody else's problem. It is our best shot at addressing the *constant* pain point that is the code contribution process of Git. In other words: if you see something that you don't like about GitGitGadget, get your butts off the ground and contribute a fix. The code contribution process of GitGitGadget is very easy: open a PR at https://github.com/gitgitgadget/gitgitgadget Ciao, Johannes
Re: [PATCH 00/16] Consolidate reachability logic
Hi Eric & Peff, On Mon, 16 Jul 2018, Eric Sunshine wrote: > On Mon, Jul 16, 2018 at 2:56 PM Jeff King wrote: > > On Mon, Jul 16, 2018 at 02:40:21PM -0400, Eric Sunshine wrote: > > > On Mon, Jul 16, 2018 at 12:18 PM Jeff King wrote: > > > > git-send-email uses the current time minus an offset, and then > > > > monotonically increases for each patch: > > > > > > Junio pointed this out to gitgitgadget developers in [1], which led to > > > an issue being opened[2]. That issue was merged today. > > > > > > [1]: > > > https://public-inbox.org/git/xmqq7em7gg3j@gitster-ct.c.googlers.com/ > > > [2]: https://github.com/gitgitgadget/gitgitgadget/pull/15 > > > > I was going to say "oh good, fixed", but it looks like it just merged > > adding that line to the TODO list. :) > > Erm, right. I actually knew a couple days ago that that issue was just > a change to the TODO list but forgot that important tidbit when I > wrote the above "was merged today". Anyhow, at least it's on the > radar. It is always nice to get such active contributions. Seriously again, do feel free to jump in and contribute improvements to GitGitGadget. We have a very time-consuming (read: time wasting) code contribution process, and it is an untenable situation, and GitGitGadget was designed to be able to address this huge problem. But I can't do it alone. And neither should you pretend that this is my problem alone. This problem is as much your problem as it is mine. (Whether you realize it or not.) Ciao, Dscho
[ANNOUNCE] Git Rev News edition 41
Hi everyone, The 41th edition of Git Rev News is now published: https://git.github.io/rev_news/2018/07/18/edition-41/ Thanks a lot to the contributors: Luca Milanesio and Derrick Stolee! Enjoy, Christian, Jakub, Markus and Gabriel.
Re: [PATCH] gc: do not warn about too many loose objects
On Tue, Jul 17 2018, Jeff King wrote:
> On Tue, Jul 17, 2018 at 10:59:50AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> That doesn't make sense to me. Just because git itself is happily
>> ignoring the exit code I don't think that should mean there shouldn't be
>> a meaningful exit code. Why don't you just ignore the exit code in the
>> repo tool as well?
>>
>> Now e.g. automation I have to see if git-gc ---auto is having issues
>> can't just be 'git gc --auto || echo fail @ {hostname}' across a fleet
>> of servers, but will need to start caring if stderr was emitted to.
>
> If you're daemonizing gc, though, there are a number of cases where the
> exit code is not propagated. If you really care about the outcome,
> you're probably better off either:
In theory a lot of the other stuff can fail, but in practice I've only
seen it get stuck due to running out of disk space (easily detected in
other ways), and because of having too many loose objects (e.g. due to,
but not limited to
https://public-inbox.org/git/87fu6bmr0j@evledraar.gmail.com/).
> - doing synchronous gc's, which will still return a meaningful code
> after Jonathan's patches
(Reply to this and "we've found at GitHub..." later in your mail)
I'm targeting clients (dev machines, laptops, staging boxes) which have
clones of repos, some put in place by automation, some manually cloned
by users in ~.
So it's much easier to rely on shipping a /etc/gitconfig than setting
gc.auto=0 and having some system-wide job that goes and hunts for local
git repos to manually gc.
It's also a big advantage that it's driven by user activity, because
it's an implicit guard of only_do_this_if_the_user_is_active_here()
before "git-gc" on a fleet of machines, which when some of those get
their disk space from shared NFS resources cuts down on overall I/O.
> - inspecting the log yourself. I know that comes close to the un-unixy
> stderr thing. But in this case, the presence of a non-empty log is
> literally the on-disk bit for "the previous run failed". And doing
> so catches all of the daemonized cases, even the "first one" that
> you'd miss by paying attention to the immediate exit code.
>
> This will treat the zero-exit-code "warning" case as an error. I'm
> not against propagating the true original error code, if somebody
> wants to work on it. But I think Jonathan's patches are a strict
> improvement over the current situation, and a patch to propagate
> could come on top.
Yeah, I can check gc.log, if others are of a different opinion about
this #3 case at the end of the day I don't mind if it returns 0. It just
doesn't make any conceptual sense to me.
>> I think you're conflating two things here in a way that (if I'm reading
>> this correctly) produces a pretty bad regression for users.
>>
>> a) If we have something in the gc.log we keep yelling at users until we
>> reach the gc.logExpiry, this was the subject of my thread back in
>> January. This sucks, and should be fixed somehow.
>>
>> Maybe we should only emit the warning once, e.g. creating a
>> .git/gc.log.wasemitted marker and as long as its ctime is later than
>> the mtime on .git/gc.log we don't emit it again).
>>
>> But that also creates the UX problem that it's easy to miss this
>> message in the middle of some huge "fetch" output. Perhaps we should
>> just start emitting this as part of "git status" or something (or
>> solve the root cause, as Peff notes...).
>
> I kind of like that "git status" suggestion. Of course many users run
> "git status" more often than "git commit", so it may actually spam them
> more!
Maybe piggy-packing on the advice facility ...
>> b) We *also* use this presence of a gc.log as a marker for "we failed
>> too recently, let's not try again until after a day".
>>
>> The semantics of b) are very useful, and just because they're tied up
>> with a) right now, let's not throw out b) just because we're trying to
>> solve a).
>
> Yeah. In general my concern was the handling of (b), which I think this
> last crop of patches is fine with. As far as showing the message
> repeatedly or not, I don't have a strong opinion (it could even be
> configurable, once it's split from the "marker" behavior).
>
>> Right now one thing we do right is we always try to look at the actual
>> state of the repo with too_many_packs() and too_many_loose_objects().
>>
>> We don't assume the state of your repo hasn't drastically changed
>> recently. That means that we do the right thing and gc when the repo
>> needs it, not just because we GC'd recently enough.
>>
>> With these proposed semantics we'd skip a needed GC (potentially for
>> days, depending on the default) just because we recently ran one.
>
> Yeah, I agree that deferring repeated gc's based on time is going to run
> into pathological corner cases. OTOH, what we've found at GitHub is that
> "gc --auto" is quite insufficient for scheduling maintenance
Re: [RFC] push: add documentation on push v2
On 7/17/2018 7:25 PM, Stefan Beller wrote:
On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote:
Signed-off-by: Brandon Williams
---
Since introducing protocol v2 and enabling fetch I've been thinking
about what its inverse 'push' would look like. After talking with a
number of people I have a longish list of things that could be done to
improve push and I think I've been able to distill the core features we
want in push v2.
It would be nice to know which things you want to improve.
Hopefully we can also get others to chime in with things they don't like
about the existing protocol. What pain points exist, and what can we do
to improve at the transport layer before considering new functionality?
Thankfully (due to the capability system) most of the
other features/improvements can be added later with ease.
What I've got now is a rough design for a more flexible push, more
flexible because it allows for the server to do what it wants with the
refs that are pushed and has the ability to communicate back what was
done to the client. The main motivation for this is to work around
issues when working with Gerrit and other code-review systems where you
need to have Change-Ids in the commit messages (now the server can just
insert them for you and send back new commits) and you need to push to
magic refs to get around various limitations (now a Gerrit server should
be able to communicate that pushing to 'master' doesn't update master
but instead creates a refs/changes/ ref).
Well Gerrit is our main motivation, but this allows for other workflows as well.
For example Facebook uses hg internally and they have a
"rebase-on-the-server-after-push" workflow IIRC as pushing to a single repo
brings up quite some contention. The protocol outlined below would allow
for such a workflow as well? (This might be an easier sell to the Git
community as most are not quite familiar with Gerrit)
I'm also curious how this "change commits on push" would be helpful to
other scenarios.
Since I'm not familiar with Gerrit: what is preventing you from having a
commit hook that inserts (or requests) a Change-Id when not present? How
can the server identify the Change-Id automatically when it isn't present?
Before actually moving to write any code I'm hoping to get some feedback
on if we think this is an acceptable base design for push (other
features like atomic-push, signed-push, etc can be added as
capabilities), so any comments are appreciated.
Documentation/technical/protocol-v2.txt | 76 +
1 file changed, 76 insertions(+)
diff --git a/Documentation/technical/protocol-v2.txt
b/Documentation/technical/protocol-v2.txt
index 49bda76d23..16c1ce60dd 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -403,6 +403,82 @@ header.
2 - progress messages
3 - fatal error message just before stream aborts
+ push
+~~
+
+`push` is the command used to push ref-updates and a packfile to a remote
+server in v2.
+
+Additional features not supported in the base command will be advertised
+as the value of the command in the capability advertisement in the form
+of a space separated list of features: "= "
+
+The format of a push request is as follows:
+
+request = *section
+section = (ref-updates | packfile)
This reads as if a request consists of sections, which
each can be a "ref-updates" or a packfile, no order given,
such that multiple ref-update sections mixed with packfiles
are possible.
I would assume we'd only want to allow for ref-updates
followed by the packfile.
Given the example above for "rebase-on-push" though
it is better to first send the packfile (as that is assumed to
take longer) and then send the ref updates, such that the
rebasing could be faster and has no bottleneck.
+ (delim-pkt | flush-pkt)
+
+ref-updates = PKT-LINE("ref-updates" LF)
+ *PKT-Line(update/force-update LF)
+
+update = txn_id SP action SP refname SP old_oid SP new_oid
+force-update = txn_id SP "force" SP action SP refname SP new_oid
So we insert "force" after the transaction id if we want to force it.
When adding the atomic capability later we could imagine another insert here
1 atomic create refs/heads/new-ref <0-hash>
1 atomic delete refs/heads/old-ref <0-hash>
which would look like a "rename" that we could also add instead.
The transaction numbers are an interesting concept, how do you
envision them to be used? In the example I put them both in the same
transaction to demonstrate the "atomic-ness", but one could also
imagine different transactions numbers per ref (i.e. exactly one
ref per txn_id) to have a better understanding of what the server did
to each individual ref.
+action = ("create" | "delete" | "update")
+txn_id = 1*DIGIT
+
+packfile = PKT-LINE("packfile" LF)
+ *PKT-LINE(*%x00-ff)
+
+ref-updates section
+ *
[PATCH] TO-SQUASH: replace the_repository with arbitrary r
Signed-off-by: Derrick Stolee
---
This should just be squashed into PATCH 2.
builtin/replace.c | 5 ++---
replace-object.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 5f34659071..d0b1cdb061 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -57,9 +57,8 @@ static int show_reference(struct repository *r, const char
*refname,
if (get_oid(refname, &object))
return error("Failed to resolve '%s' as a valid
ref.", refname);
- obj_type = oid_object_info(the_repository, &object,
- NULL);
- repl_type = oid_object_info(the_repository, oid, NULL);
+ obj_type = oid_object_info(r, &object, NULL);
+ repl_type = oid_object_info(r, oid, NULL);
printf("%s (%s) -> %s (%s)\n", refname,
type_name(obj_type),
oid_to_hex(oid), type_name(repl_type));
diff --git a/replace-object.c b/replace-object.c
index 01a5a59a35..017f02f8ef 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -26,7 +26,7 @@ static int register_replace_ref(struct repository *r,
oidcpy(&repl_obj->replacement, oid);
/* Register new object */
- if (oidmap_put(the_repository->objects->replace_map, repl_obj))
+ if (oidmap_put(r->objects->replace_map, repl_obj))
die("duplicate replace ref: %s", refname);
return 0;
--
2.18.0.118.gd4f65b8d14
Re: [PATCH] sequencer.c: terminate the last line of author-scriptproperly
On 18/07/18 10:45, Phillip Wood wrote: Hi Junio On 12/07/18 18:22, Junio C Hamano wrote: "Akinori MUSHA" writes: It looks like write_author_script() intends to write out a file in Bourne shell syntax, but it doesn't put a closing single quote on the last line. s/closing single quote/& and the terminating newline/? This patch makes .git/rebase-merge/author-script actually parsable by sh(1) by adding a single quote and a linefeed to terminate the line properly. Sounds good. I wonder why this breakage was left unnoticed for a long time, though. It's not like writing and reading the author-script from C code was done first in the "rebase -i" and friends that are users of the sequencer machinery The only consumer of a faulty author script written by the sequencer is read_env_script() in sequencer.c which doesn't worry about checking that quotes are paired. That's not quite true anymore, recently another consumer read_author_ident() was added which uses sq_dequote() instead of custom code. Looking more closely at write_author_script() the quoting of single quotes is buggy they are escaped as \\' instead of \'. The code in read_env_script() expects the buggy quoting so it works. I just tried the code path that uses sq_dequote() by doing rebase --root with an author containing a single quote and that worked but I'm not sure why. The shell versions of rebase use the code in git-sh-setup.sh to create the author script which does write valid posix shell. (I think we had code to do so in "git am" that was rewritten in C first). The code in builtin/am.c doesn't try to write valid posix shell (if one assumes it is the only consumer of the author script then it doesn't need to) Though it will break if git gets upgraded while am is stopped for a conflict resolution. Given that the code has been around for a while that may not be much of a concern now which results in simpler code, but external scripts cannot safely eval it anymore. Do we have a similar issue over there as well? If not, perhaps if we reused the existing code that was not broken, we wouldn't have seen this breakage on the sequencer side? Best Wishes Phillip Thanks. Signed-off-by: Akinori MUSHA --- sequencer.c | 1 + t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+) diff --git a/sequencer.c b/sequencer.c index 4034c0461..5f32b6df1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -651,6 +651,7 @@ static int write_author_script(const char *message) strbuf_addch(&buf, *(message++)); else strbuf_addf(&buf, "'%c'", *(message++)) >>> + strbuf_addstr(&buf, "'\n"); res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1); strbuf_release(&buf); return res; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 352a52e59..345b103eb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' ' test_line_count = 6 actual ' +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' ' + test_when_finished "git rebase --abort ||:" && + git checkout master && + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i HEAD^ && + test -f .git/rebase-merge/author-script && + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && + eval "$(cat .git/rebase-merge/author-script)" && + test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" && + test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" && + test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE" +' + test_expect_success 'rebase -i with the exec command' ' git checkout master && ( -- 2.18.0
Re: [PATCH] sequencer.c: terminate the last line of author-script properly
Hi Akinori On 12/07/18 12:18, Akinori MUSHA wrote: It looks like write_author_script() intends to write out a file in Bourne shell syntax, but it doesn't put a closing single quote on the last line. This patch makes .git/rebase-merge/author-script actually parsable by sh(1) by adding a single quote and a linefeed to terminate the line properly. Signed-off-by: Akinori MUSHA --- sequencer.c | 1 + t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+) diff --git a/sequencer.c b/sequencer.c index 4034c0461..5f32b6df1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -651,6 +651,7 @@ static int write_author_script(const char *message) strbuf_addch(&buf, *(message++)); else strbuf_addf(&buf, "'%c'", *(message++)); + strbuf_addstr(&buf, "'\n"); res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1); strbuf_release(&buf); return res; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 352a52e59..345b103eb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' ' test_line_count = 6 actual ' +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' ' + test_when_finished "git rebase --abort ||:" && + git checkout master && + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i HEAD^ && + test -f .git/rebase-merge/author-script && + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && + eval "$(cat .git/rebase-merge/author-script)" && + test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" && + test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" && + test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE" +' Have you checked that this test fails without your fix being applied? I just ran sh -c 'eval "$(cat .git/rebase-merge/author-script)"; echo "$GIT_AUTHOR_NAME"' while a rebase was stopped for an edit and it worked despite the fact that there is a missing quote at the end of the GIT_AUTHOR_DATE in the author script file. Best Wishes Phillip test_expect_success 'rebase -i with the exec command' ' git checkout master && (
Re: [PATCH] sequencer.c: terminate the last line of author-scriptproperly
On 18/07/18 14:50, Phillip Wood wrote: Hi Akinori On 12/07/18 12:18, Akinori MUSHA wrote: It looks like write_author_script() intends to write out a file in Bourne shell syntax, but it doesn't put a closing single quote on the last line. This patch makes .git/rebase-merge/author-script actually parsable by sh(1) by adding a single quote and a linefeed to terminate the line properly. Signed-off-by: Akinori MUSHA --- sequencer.c | 1 + t/t3404-rebase-interactive.sh | 13 + 2 files changed, 14 insertions(+) diff --git a/sequencer.c b/sequencer.c index 4034c0461..5f32b6df1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -651,6 +651,7 @@ static int write_author_script(const char *message) strbuf_addch(&buf, *(message++)); else strbuf_addf(&buf, "'%c'", *(message++)); + strbuf_addstr(&buf, "'\n"); res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1); strbuf_release(&buf); return res; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 352a52e59..345b103eb 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' ' test_line_count = 6 actual ' +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' ' + test_when_finished "git rebase --abort ||:" && + git checkout master && + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i HEAD^ && + test -f .git/rebase-merge/author-script && + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && + eval "$(cat .git/rebase-merge/author-script)" && + test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" && + test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" && + test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE" +' Have you checked that this test fails without your fix being applied? I just ran sh -c 'eval "$(cat .git/rebase-merge/author-script)"; echo "$GIT_AUTHOR_NAME"' while a rebase was stopped for an edit and it worked despite the fact that there is a missing quote at the end of the GIT_AUTHOR_DATE in the author script file. Best Wishes Phillip Please ignore that, I messed up, there was a closing "'" as I was using the shell version by a mistake. Sorry for the noise Phillip test_expect_success 'rebase -i with the exec command' ' git checkout master && (
Re: [PATCH 6/6] submodule--helper: introduce new update-module-mode helper
On 7/12/2018 3:47 PM, Stefan Beller wrote: + fprintf(stdout, submodule_strategy_to_string(&update_strategy)); This line is causing build failures on 'pu' for certain setups: 2018-07-18T14:53:03.6857987Z builtin/submodule--helper.c: In function ‘module_update_module_mode’: 2018-07-18T14:53:03.6874886Z builtin/submodule--helper.c:1504:2: error: format not a string literal and no format arguments [-Werror=format-security] 2018-07-18T14:53:03.6890814Z fprintf(stdout, submodule_strategy_to_string(&update_strategy)); 2018-07-18T14:53:03.6905905Z ^ Thanks, -Stolee
Re: [PATCH 00/16] Consolidate reachability logic
On Wed, Jul 18, 2018 at 2:30 PM Johannes Schindelin wrote: > > Hi, > > On Mon, 16 Jul 2018, Derrick Stolee wrote: > > > On 7/16/2018 2:44 PM, Eric Sunshine wrote: > > > On Mon, Jul 16, 2018 at 1:27 PM Stefan Beller wrote: > > > > Another pain point of the Gadget is that CC's in the cover letter > > > > do not work as I would imagine. The line > > > > > > > > CC: sbel...@google.com > > > > > > > > did not put that email into the cc field. > > > gitgitgadget recognizes case-sensitive "Cc:" only[1]. > > > > > > [1]: > > > https://github.com/gitgitgadget/gitgitgadget/blob/c4805370f59532aa438283431b8ea7d4484c530f/lib/patch-series.ts#L188 > > > > Thanks for everyone's patience while we improve gitgitgadget (and - in this > > case - I learn how to use it). > > And let's please stop pretending that this GitGitGadget project is > somebody else's problem. > > It is our best shot at addressing the *constant* pain point that is the code > contribution process of Git. > > In other words: if you see something that you don't like about > GitGitGadget, get your butts off the ground and contribute a fix. Thank you for the frank words. I will choose to not review any mails coming from GitGitGadget. -- Duy
[PATCH 1/8] refs.c: migrate internal ref iteration to pass thru repository argument
From: Stefan Beller
In 60ce76d3581 (refs: add repository argument to for_each_replace_ref,
2018-04-11) and 0d296c57aec (refs: allow for_each_replace_ref to handle
arbitrary repositories, 2018-04-11), for_each_replace_ref learned how
to iterate over refs by a given arbitrary repository.
New attempts in the object store conversion have shown that it is useful
to have the repository handle available that the refs iteration is
currently iterating over.
To achieve this goal we will need to add a repository argument to
each_ref_fn in refs.h. However as many callers rely on the signature
such a patch would be too large.
So convert the internals of the ref subsystem first to pass through a
repository argument without exposing the change to the user. Assume
the_repository for the passed through repository, although it is not
used anywhere yet.
Signed-off-by: Stefan Beller
Signed-off-by: Derrick Stolee
---
refs.c | 39 +--
refs.h | 10 ++
refs/iterator.c | 6 +++---
refs/refs-internal.h | 5 +++--
4 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index fcfd3171e..2513f77ac 100644
--- a/refs.c
+++ b/refs.c
@@ -1390,17 +1390,50 @@ struct ref_iterator *refs_ref_iterator_begin(
* non-zero value, stop the iteration and return that value;
* otherwise, return 0.
*/
+static int do_for_each_repo_ref(struct repository *r, const char *prefix,
+ each_repo_ref_fn fn, int trim, int flags,
+ void *cb_data)
+{
+ struct ref_iterator *iter;
+ struct ref_store *refs = get_main_ref_store(r);
+
+ if (!refs)
+ return 0;
+
+ iter = refs_ref_iterator_begin(refs, prefix, trim, flags);
+
+ return do_for_each_repo_ref_iterator(r, iter, fn, cb_data);
+}
+
+struct do_for_each_ref_help {
+ each_ref_fn *fn;
+ void *cb_data;
+};
+
+static int do_for_each_ref_helper(struct repository *r,
+ const char *refname,
+ const struct object_id *oid,
+ int flags,
+ void *cb_data)
+{
+ struct do_for_each_ref_help *hp = cb_data;
+
+ return hp->fn(refname, oid, flags, hp->cb_data);
+}
+
static int do_for_each_ref(struct ref_store *refs, const char *prefix,
each_ref_fn fn, int trim, int flags, void *cb_data)
{
struct ref_iterator *iter;
+ struct do_for_each_ref_help hp = { fn, cb_data };
if (!refs)
return 0;
iter = refs_ref_iterator_begin(refs, prefix, trim, flags);
- return do_for_each_ref_iterator(iter, fn, cb_data);
+ return do_for_each_repo_ref_iterator(the_repository, iter,
+ do_for_each_ref_helper, &hp);
}
int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -2029,10 +2062,12 @@ cleanup:
int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
{
struct ref_iterator *iter;
+ struct do_for_each_ref_help hp = { fn, cb_data };
iter = refs->be->reflog_iterator_begin(refs);
- return do_for_each_ref_iterator(iter, fn, cb_data);
+ return do_for_each_repo_ref_iterator(the_repository, iter,
+do_for_each_ref_helper, &hp);
}
int for_each_reflog(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index cc2fb4c68..80eec8bbc 100644
--- a/refs.h
+++ b/refs.h
@@ -274,6 +274,16 @@ struct ref_transaction;
typedef int each_ref_fn(const char *refname,
const struct object_id *oid, int flags, void *cb_data);
+/*
+ * The same as each_ref_fn, but also with a repository argument that
+ * contains the repository associated with the callback.
+ */
+typedef int each_repo_ref_fn(struct repository *r,
+const char *refname,
+const struct object_id *oid,
+int flags,
+void *cb_data);
+
/*
* The following functions invoke the specified callback function for
* each reference indicated. If the function ever returns a nonzero
diff --git a/refs/iterator.c b/refs/iterator.c
index 2ac91ac34..629e00a12 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -407,15 +407,15 @@ struct ref_iterator *prefix_ref_iterator_begin(struct
ref_iterator *iter0,
struct ref_iterator *current_ref_iter = NULL;
-int do_for_each_ref_iterator(struct ref_iterator *iter,
-each_ref_fn fn, void *cb_data)
+int do_for_each_repo_ref_iterator(struct repository *r, struct ref_iterator
*iter,
+ each_repo_ref_fn fn, void *cb_data)
{
int retval = 0, ok;
struct ref_iterator *old_ref_iter = current_ref_iter;
current_ref_iter = iter;
while ((o
[PATCH 0/8] Clarify commit-graph and grafts/replace/shallow incompatibilities
One unresolved issue with the commit-graph feature is that it can cause issues when combined with replace objects, commit grafts, or shallow clones. These are not 100% incompatible, as one could be reasonably successful writing a commit-graph after replacing some objects and not have issues. The problems happen when commits that are already in the commit-graph file are replaced, or when git is run with the `--no-replace-objects` option; this can cause incorrect parents or incorrect generation numbers. Similar things occur with commit grafts and shallow clones, especially when running `git fetch --unshallow` in a shallow repo. Instead of trying (and probably failing) to make these features work together, default to making the commit-graph feature unavailable in these situations. Create a new method 'commit_graph_compatible(r)' that checks if the repository 'r' has any of these features enabled. I will send a follow-up patch that shows how I tested these interactions by computing the commit-graph on every 'git commit'. This approach works for most cases, but I found one nagging test case that was causing problems. This led to the commit "commit-graph: close_commit_graph before shallow walk" and is the patch I am least confident about. Please take a close look at that one and suggest alternatives. This approach is very different from the previous RFC on the subject [1]. While building this series, I got some test failures in the non-the_repository tests. These issues are related to missing references to an arbitrary repository (instead of the_repository) and some uninitialized values in the tests. Stefan already sent a patch to address this [2], and I've included those commits (along with a small tweak [3]). These are only included for convenience. Thanks, -Stolee [1] https://public-inbox.org/git/20180531174024.124488-1-dsto...@microsoft.com/ [RFC PATCH 0/6] Fix commit-graph/graft/replace/shallow combo [2] https://public-inbox.org/git/20180717224935.96397-1-sbel...@google.com/T/#t [PATCH 0/2] RFC ref store to repository migration [3] https://public-inbox.org/git/20180717224935.96397-1-sbel...@google.com/T/#m966eac85fd58c66523654ddaf0bec72877d3295a [PATCH] TO-SQUASH: replace the_repository with arbitrary r Based-On: jt/commit-graph-per-object-store Cc: jonathanta...@google.com Cc: sbel...@google.com Derrick Stolee (6): commit-graph: update design document test-repository: properly init repo commit-graph: not compatible with replace objects commit-graph: not compatible with grafts commit-graph: not compatible with uninitialized repo commit-graph: close_commit_graph before shallow walk Stefan Beller (2): refs.c: migrate internal ref iteration to pass thru repository argument refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback Documentation/technical/commit-graph.txt | 18 ++-- builtin/replace.c| 8 ++-- commit-graph.c | 34 -- commit-graph.h | 1 + commit.c | 2 +- commit.h | 1 + refs.c | 48 +--- refs.h | 12 - refs/iterator.c | 6 +-- refs/refs-internal.h | 5 +- replace-object.c | 7 +-- replace-object.h | 2 + t/helper/test-repository.c | 10 +++- t/t5318-commit-graph.sh | 58 upload-pack.c| 2 + 15 files changed, 184 insertions(+), 30 deletions(-) base-commit: dade47c06cf849b0ca180a8e6383b55ea6f75812 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-11%2Fderrickstolee%2Fshallow%2Fupstream-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-11/derrickstolee/shallow/upstream-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/11 -- gitgitgadget
[PATCH 3/8] commit-graph: update design document
From: Derrick Stolee As it exists right now, the commit-graph feature may provide inconsistent results when combined with commit grafts, replace objects, and shallow clones. Update the design document to discuss why these interactions are difficult to reconcile and how we will avoid errors by preventing updates to and reads from the commit-graph file when these other features exist. Signed-off-by: Derrick Stolee --- Documentation/technical/commit-graph.txt | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Documentation/technical/commit-graph.txt b/Documentation/technical/commit-graph.txt index c664acbd7..18948f11a 100644 --- a/Documentation/technical/commit-graph.txt +++ b/Documentation/technical/commit-graph.txt @@ -112,12 +112,24 @@ Design Details - The file format includes parameters for the object ID hash function, so a future change of hash algorithm does not require a change in format. +- Commit grafts and replace objects can change the shape of the commit + history. These can also be enabled/disabled on the fly using + `--no-replace-objects`. This leads to difficultly storing both possible + interpretations of a commit id, especially when computing generation + numbers. The commit-graph will not be read or written when + replace-objects or grafts are present. + +- Shallow clones create grafts of commits by dropping their parents. This + leads the commit-graph to think those commits have generation number 1. + If and when those commits are made unshallow, those generation numbers + become invalid. Since shallow clones are intended to restrict the commit + history to a very small set of commits, the commit-graph feature is less + helpful for these clones, anyway. The commit-graph will not be read or + written when shallow commits are present. + Future Work --- -- The commit graph feature currently does not honor commit grafts. This can - be remedied by duplicating or refactoring the current graft logic. - - After computing and storing generation numbers, we must make graph walks aware of generation numbers to gain the performance benefits they enable. This will mostly be accomplished by swapping a commit-date-ordered -- gitgitgadget
[PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
From: Stefan Beller
Signed-off-by: Stefan Beller
Signed-off-by: Derrick Stolee
---
builtin/replace.c | 8
refs.c| 9 -
refs.h| 2 +-
replace-object.c | 5 +++--
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index ef22d724b..d0b1cdb06 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -39,7 +39,8 @@ struct show_data {
enum replace_format format;
};
-static int show_reference(const char *refname, const struct object_id *oid,
+static int show_reference(struct repository *r, const char *refname,
+ const struct object_id *oid,
int flag, void *cb_data)
{
struct show_data *data = cb_data;
@@ -56,9 +57,8 @@ static int show_reference(const char *refname, const struct
object_id *oid,
if (get_oid(refname, &object))
return error("Failed to resolve '%s' as a valid
ref.", refname);
- obj_type = oid_object_info(the_repository, &object,
- NULL);
- repl_type = oid_object_info(the_repository, oid, NULL);
+ obj_type = oid_object_info(r, &object, NULL);
+ repl_type = oid_object_info(r, oid, NULL);
printf("%s (%s) -> %s (%s)\n", refname,
type_name(obj_type),
oid_to_hex(oid), type_name(repl_type));
diff --git a/refs.c b/refs.c
index 2513f77ac..5700cd468 100644
--- a/refs.c
+++ b/refs.c
@@ -1478,12 +1478,11 @@ int refs_for_each_fullref_in(struct ref_store *refs,
const char *prefix,
return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
}
-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
*cb_data)
{
- return do_for_each_ref(get_main_ref_store(r),
- git_replace_ref_base, fn,
- strlen(git_replace_ref_base),
- DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
+ return do_for_each_repo_ref(r, git_replace_ref_base, fn,
+ strlen(git_replace_ref_base),
+ DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 80eec8bbc..a0a18223a 100644
--- a/refs.h
+++ b/refs.h
@@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn,
void *cb_data,
int for_each_tag_ref(each_ref_fn fn, void *cb_data);
int for_each_branch_ref(each_ref_fn fn, void *cb_data);
int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
+int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
*cb_data);
int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
const char *prefix, void *cb_data);
diff --git a/replace-object.c b/replace-object.c
index 801b5c167..017f02f8e 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -6,7 +6,8 @@
#include "repository.h"
#include "commit.h"
-static int register_replace_ref(const char *refname,
+static int register_replace_ref(struct repository *r,
+ const char *refname,
const struct object_id *oid,
int flag, void *cb_data)
{
@@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,
oidcpy(&repl_obj->replacement, oid);
/* Register new object */
- if (oidmap_put(the_repository->objects->replace_map, repl_obj))
+ if (oidmap_put(r->objects->replace_map, repl_obj))
die("duplicate replace ref: %s", refname);
return 0;
--
gitgitgadget
[PATCH 4/8] test-repository: properly init repo
From: Derrick Stolee
Signed-off-by: Derrick Stolee
---
t/helper/test-repository.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index 2762ca656..6a84a53ef 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -15,7 +15,10 @@ static void test_parse_commit_in_graph(const char *gitdir,
const char *worktree,
struct commit *c;
struct commit_list *parent;
- repo_init(&r, gitdir, worktree);
+ setup_git_env(gitdir);
+
+ if (repo_init(&r, gitdir, worktree))
+ die("Couldn't init repo");
c = lookup_commit(&r, commit_oid);
@@ -38,7 +41,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
struct commit *c;
struct tree *tree;
- repo_init(&r, gitdir, worktree);
+ setup_git_env(gitdir);
+
+ if (repo_init(&r, gitdir, worktree))
+ die("Couldn't init repo");
c = lookup_commit(&r, commit_oid);
--
gitgitgadget
[PATCH 6/8] commit-graph: not compatible with grafts
From: Derrick Stolee
Augment commit_graph_compatible(r) to return false when the given
repository r has commit grafts or is a shallow clone. Test that in these
situations we ignore existing commit-graph files and we do not write new
commit-graph files.
Signed-off-by: Derrick Stolee
---
commit-graph.c | 6 ++
commit.c| 2 +-
commit.h| 1 +
t/t5318-commit-graph.sh | 36
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index 711099858..5097c7c12 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -60,6 +60,12 @@ static struct commit_graph *alloc_commit_graph(void)
static int commit_graph_compatible(struct repository *r)
{
+ prepare_commit_graft(r);
+ if (r->parsed_objects && r->parsed_objects->grafts_nr)
+ return 0;
+ if (is_repository_shallow(r))
+ return 0;
+
prepare_replace_object(r);
if (hashmap_get_size(&r->objects->replace_map->map))
return 0;
diff --git a/commit.c b/commit.c
index 39b80bd21..609adaf8a 100644
--- a/commit.c
+++ b/commit.c
@@ -209,7 +209,7 @@ static int read_graft_file(struct repository *r, const char
*graft_file)
return 0;
}
-static void prepare_commit_graft(struct repository *r)
+void prepare_commit_graft(struct repository *r)
{
char *graft_file;
diff --git a/commit.h b/commit.h
index da0db36eb..5459e279f 100644
--- a/commit.h
+++ b/commit.h
@@ -202,6 +202,7 @@ typedef int (*each_commit_graft_fn)(const struct
commit_graft *, void *);
struct commit_graft *read_graft_line(struct strbuf *line);
int register_commit_graft(struct repository *r, struct commit_graft *, int);
+void prepare_commit_graft(struct repository *r);
struct commit_graft *lookup_commit_graft(struct repository *r, const struct
object_id *oid);
extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit
*rev2);
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index c90626f5d..1d9f49af5 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -281,6 +281,42 @@ test_expect_success 'replace-objects invalidates
commit-graph' '
)
'
+test_expect_success 'commit grafts invalidate commit-graph' '
+ cd "$TRASH_DIRECTORY" &&
+ test_when_finished rm -rf graft &&
+ git clone full graft &&
+ (
+ cd graft &&
+ git commit-graph write --reachable &&
+ test_path_is_file .git/objects/info/commit-graph &&
+ git replace --graft HEAD~1 HEAD~3 &&
+ git -c core.commitGraph=false log >expect &&
+ git -c core.commitGraph=true log >actual &&
+ test_cmp expect actual &&
+ git commit-graph write --reachable &&
+ git -c core.commitGraph=false --no-replace-objects log >expect
&&
+ git -c core.commitGraph=true --no-replace-objects log >actual &&
+ test_cmp expect actual &&
+ rm -rf .git/objects/info/commit-graph &&
+ git commit-graph write --reachable &&
+ test_path_is_missing .git/objects/info/commit-graph
+ )
+'
+
+test_expect_success 'replace-objects invalidates commit-graph' '
+ cd "$TRASH_DIRECTORY" &&
+ test_when_finished rm -rf shallow &&
+ git clone --depth 2 "file://$TRASH_DIRECTORY/full" shallow &&
+ (
+ cd shallow &&
+ git commit-graph write --reachable &&
+ test_path_is_missing .git/objects/info/commit-graph &&
+ git fetch origin --unshallow &&
+ git commit-graph write --reachable &&
+ test_path_is_file .git/objects/info/commit-graph
+ )
+'
+
# the verify tests below expect the commit-graph to contain
# exactly the commits reachable from the commits/8 branch.
# If the file changes the set of commits in the list, then the
--
gitgitgadget
[PATCH 5/8] commit-graph: not compatible with replace objects
From: Derrick Stolee
Create new method commit_graph_compatible(r) to check if a given
repository r is compatible with the commit-graph feature. Fill the
method with a check to see if replace-objects exist. Test this
interaction succeeds, including ignoring an existing commit-graph and
failing to write a new commit-graph.
Signed-off-by: Derrick Stolee
---
commit-graph.c | 17 +
replace-object.c| 2 +-
replace-object.h| 2 ++
t/t5318-commit-graph.sh | 22 ++
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index b0a55ad12..711099858 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -13,6 +13,8 @@
#include "commit-graph.h"
#include "object-store.h"
#include "alloc.h"
+#include "hashmap.h"
+#include "replace-object.h"
#define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
#define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -56,6 +58,15 @@ static struct commit_graph *alloc_commit_graph(void)
return g;
}
+static int commit_graph_compatible(struct repository *r)
+{
+ prepare_replace_object(r);
+ if (hashmap_get_size(&r->objects->replace_map->map))
+ return 0;
+
+ return 1;
+}
+
struct commit_graph *load_commit_graph_one(const char *graph_file)
{
void *graph_map;
@@ -223,6 +234,9 @@ static int prepare_commit_graph(struct repository *r)
*/
return 0;
+ if (!commit_graph_compatible(r))
+ return 0;
+
obj_dir = r->objects->objectdir;
prepare_commit_graph_one(r, obj_dir);
prepare_alt_odb(r);
@@ -693,6 +707,9 @@ void write_commit_graph(const char *obj_dir,
int num_extra_edges;
struct commit_list *parent;
+ if (!commit_graph_compatible(the_repository))
+ return;
+
oids.nr = 0;
oids.alloc = approximate_object_count() / 4;
diff --git a/replace-object.c b/replace-object.c
index 017f02f8e..4d2d84cf4 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -32,7 +32,7 @@ static int register_replace_ref(struct repository *r,
return 0;
}
-static void prepare_replace_object(struct repository *r)
+void prepare_replace_object(struct repository *r)
{
if (r->objects->replace_map)
return;
diff --git a/replace-object.h b/replace-object.h
index f996de3d6..c7a99fc35 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -10,6 +10,8 @@ struct replace_object {
struct object_id replacement;
};
+void prepare_replace_object(struct repository *r);
+
/*
* This internal function is only declared here for the benefit of
* lookup_replace_object(). Please do not call it directly.
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 4f17d7701..c90626f5d 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -259,6 +259,28 @@ test_expect_success 'check that gc computes commit-graph' '
test_cmp commit-graph-after-gc $objdir/info/commit-graph
'
+test_expect_success 'replace-objects invalidates commit-graph' '
+ cd "$TRASH_DIRECTORY" &&
+ test_when_finished rm -rf replace &&
+ git clone full replace &&
+ (
+ cd replace &&
+ git commit-graph write --reachable &&
+ test_path_is_file .git/objects/info/commit-graph &&
+ git replace HEAD~1 HEAD~2 &&
+ git -c core.commitGraph=false log >expect &&
+ git -c core.commitGraph=true log >actual &&
+ test_cmp expect actual &&
+ git commit-graph write --reachable &&
+ git -c core.commitGraph=false --no-replace-objects log >expect
&&
+ git -c core.commitGraph=true --no-replace-objects log >actual &&
+ test_cmp expect actual &&
+ rm -rf .git/objects/info/commit-graph &&
+ git commit-graph write --reachable &&
+ test_path_is_missing .git/objects/info/commit-graph
+ )
+'
+
# the verify tests below expect the commit-graph to contain
# exactly the commits reachable from the commits/8 branch.
# If the file changes the set of commits in the list, then the
--
gitgitgadget
[PATCH 7/8] commit-graph: not compatible with uninitialized repo
From: Derrick Stolee
Signed-off-by: Derrick Stolee
---
commit-graph.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/commit-graph.c b/commit-graph.c
index 5097c7c12..233958e10 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -60,6 +60,9 @@ static struct commit_graph *alloc_commit_graph(void)
static int commit_graph_compatible(struct repository *r)
{
+ if (!r->gitdir)
+ return 0;
+
prepare_commit_graft(r);
if (r->parsed_objects && r->parsed_objects->grafts_nr)
return 0;
--
gitgitgadget
[PATCH 8/8] commit-graph: close_commit_graph before shallow walk
From: Derrick Stolee
Make close_commit_graph() work for arbitrary repositories.
Call close_commit_graph() when about to start a rev-list walk that
includes shallow commits. This is necessary in code paths that "fake"
shallow commits for the sake of fetch. Specifically, test 351 in
t5500-fetch-pack.sh runs
git fetch --shallow-exclude one origin
with a file-based transfer. When the "remote" has a commit-graph, we do
not prevent the commit-graph from being loaded, but then the commits are
intended to be dynamically transferred into shallow commits during
get_shallow_commits_by_rev_list(). By closing the commit-graph before
this call, we prevent this interaction.
Signed-off-by: Derrick Stolee
---
commit-graph.c | 8
commit-graph.h | 1 +
upload-pack.c | 2 ++
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 233958e10..237d4e7d1 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -256,10 +256,10 @@ static int prepare_commit_graph(struct repository *r)
return !!r->objects->commit_graph;
}
-static void close_commit_graph(void)
+void close_commit_graph(struct repository *r)
{
- free_commit_graph(the_repository->objects->commit_graph);
- the_repository->objects->commit_graph = NULL;
+ free_commit_graph(r->objects->commit_graph);
+ r->objects->commit_graph = NULL;
}
static int bsearch_graph(struct commit_graph *g, struct object_id *oid,
uint32_t *pos)
@@ -871,7 +871,7 @@ void write_commit_graph(const char *obj_dir,
write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);
write_graph_chunk_large_edges(f, commits.list, commits.nr);
- close_commit_graph();
+ close_commit_graph(the_repository);
finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC);
commit_lock_file(&lk);
diff --git a/commit-graph.h b/commit-graph.h
index 76e098934..13d736cdd 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -59,6 +59,7 @@ void write_commit_graph(const char *obj_dir,
int verify_commit_graph(struct repository *r, struct commit_graph *g);
+void close_commit_graph(struct repository *);
void free_commit_graph(struct commit_graph *);
#endif
diff --git a/upload-pack.c b/upload-pack.c
index 4ca052d0b..52ad6c8e5 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -24,6 +24,7 @@
#include "quote.h"
#include "upload-pack.h"
#include "serve.h"
+#include "commit-graph.h"
/* Remember to update object flag allocation in object.h */
#define THEY_HAVE (1u << 11)
@@ -739,6 +740,7 @@ static void deepen_by_rev_list(int ac, const char **av,
{
struct commit_list *result;
+ close_commit_graph(the_repository);
result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW);
send_shallow(result);
free_commit_list(result);
--
gitgitgadget
[PATCH] DO-NOT-MERGE: write and read commit-graph always
This commit is not intended to be merged, but is only to create a test
environment to see what works with the commit-graph feature and what
does not. The tests that fail are primarily related to corrupting the
object store to remove a commit from visibility and testing that
rev-list fails -- except it doesn't when there is a commit-graph that
prevents parsing from the object database. The following tests will fail
with this commit, but are not "real" bugs:
t0410-partial-clone.sh, Test 9
t5307-pack-missing-commit.sh, Tests 3-4
t6011-rev-list-with-bad-commit.sh, Test 4
The following test fails because the repo has ambiguous merge-bases, and
the commit-graph changes the walk order so we select a different one.
This alters the resulting merge from the expected result.
t6024-recursive-merge.sh, Test 4
The tests above are made to pass by deleting the commit-graph file
before the necessary steps.
Signed-off-by: Derrick Stolee
---
builtin/commit.c| 2 ++
builtin/gc.c| 3 +--
commit-graph.c | 11 ---
t/t0410-partial-clone.sh| 1 +
t/t5307-pack-missing-commit.sh | 2 ++
t/t6011-rev-list-with-bad-commit.sh | 1 +
t/t6024-recursive-merge.sh | 1 +
7 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 158e3f843a..acc31252a9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -33,6 +33,7 @@
#include "sequencer.h"
#include "mailmap.h"
#include "help.h"
+#include "commit-graph.h"
static const char * const builtin_commit_usage[] = {
N_("git commit [] [--] ..."),
@@ -1652,6 +1653,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
"not exceeded, and then \"git reset HEAD\" to recover."));
rerere(0);
+ write_commit_graph_reachable(get_object_directory(), 1);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
if (amend && !no_post_rewrite) {
diff --git a/builtin/gc.c b/builtin/gc.c
index e103f0f85d..60ab773087 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -41,7 +41,7 @@ static int aggressive_depth = 50;
static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
+static int gc_write_commit_graph = 1;
static int detach_auto = 1;
static timestamp_t gc_log_expire_time;
static const char *gc_log_expire = "1.day.ago";
@@ -131,7 +131,6 @@ static void gc_config(void)
git_config_get_int("gc.aggressivedepth", &aggressive_depth);
git_config_get_int("gc.auto", &gc_auto_threshold);
git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
- git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
git_config_get_bool("gc.autodetach", &detach_auto);
git_config_get_expiry("gc.pruneexpire", &prune_expire);
git_config_get_expiry("gc.worktreepruneexpire",
&prune_worktrees_expire);
diff --git a/commit-graph.c b/commit-graph.c
index 237d4e7d1b..ed0d27c12e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -227,22 +227,11 @@ static int prepare_commit_graph(struct repository *r)
{
struct alternate_object_database *alt;
char *obj_dir;
- int config_value;
if (r->objects->commit_graph_attempted)
return !!r->objects->commit_graph;
r->objects->commit_graph_attempted = 1;
- if (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
- !config_value)
- /*
-* This repository is not configured to use commit graphs, so
-* do not load one. (But report commit_graph_attempted anyway
-* so that commit graph loading is not attempted again for this
-* repository.)
-*/
- return 0;
-
if (!commit_graph_compatible(r))
return 0;
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 4984ca583d..c235672b03 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -181,6 +181,7 @@ test_expect_success 'rev-list stops traversal at missing
and promised commit' '
git -C repo config core.repositoryformatversion 1 &&
git -C repo config extensions.partialclone "arbitrary string" &&
+ rm -rf repo/.git/objects/info/commit-graph &&
git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
grep $(git -C repo rev-parse bar) out &&
! grep $FOO out
diff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh
index ae52a1882d..0bb54ae227 100755
--- a/t/t5307-pack-missing-commit.sh
+++ b/t/t5307-pack-missing-commit.sh
@@ -24,10 +24,12 @@ test_expect_success 'check corruption' '
'
test_expect_success 'rev-list notices corruption (1)' '
+ rm -rf .git/objects/info/commit-graph &&
[RFC PATCH] sequencer: fix quoting in write_author_script
From: Phillip Wood
Single quotes should be escaped as \' not \\'. Note that this only
affects authors that contain a single quote and then only external
scripts that read the author script and users whose git is upgraded from
the shell version of rebase -i while rebase was stopped. This is because
the parsing in read_env_script() expected the broken version and for
some reason sq_dequote() called by read_author_ident() seems to handle
the broken quoting correctly.
Ideally write_author_script() would be rewritten to use
split_ident_line() and sq_quote_buf() but this commit just fixes the
immediate bug.
Signed-off-by: Phillip Wood
---
This is untested, unfortuantely I don't have really have time to write a test or
follow this up at the moment, if someone else want to run with it then please
do.
sequencer.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5354d4d51e..0b78d1f100 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -638,21 +638,21 @@ static int write_author_script(const char *message)
else if (*message != '\'')
strbuf_addch(&buf, *(message++));
else
- strbuf_addf(&buf, "'%c'", *(message++));
+ strbuf_addf(&buf, "'\\%c'", *(message++));
strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
while (*message && *message != '\n' && *message != '\r')
if (skip_prefix(message, "> ", &message))
break;
else if (*message != '\'')
strbuf_addch(&buf, *(message++));
else
- strbuf_addf(&buf, "'%c'", *(message++));
+ strbuf_addf(&buf, "'\\%c'", *(message++));
strbuf_addstr(&buf, "'\nGIT_AUTHOR_DATE='@");
while (*message && *message != '\n' && *message != '\r')
if (*message != '\'')
strbuf_addch(&buf, *(message++));
else
- strbuf_addf(&buf, "'%c'", *(message++));
+ strbuf_addf(&buf, "'\\%c'", *(message++));
res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1);
strbuf_release(&buf);
return res;
@@ -666,13 +666,21 @@ static int read_env_script(struct argv_array *env)
{
struct strbuf script = STRBUF_INIT;
int i, count = 0;
- char *p, *p2;
+ const char *p2;
+ char *p;
if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
return -1;
for (p = script.buf; *p; p++)
- if (skip_prefix(p, "'''", (const char **)&p2))
+ /*
+* write_author_script() used to escape "'" incorrectly as
+* "'''" rather than "'\\''" so we check for the correct
+* version the incorrect version in case git was upgraded while
+* rebase was stopped.
+*/
+ if (skip_prefix(p, "'\\''", &p2) ||
+ skip_prefix(p, "'''", &p2))
strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
else if (*p == '\'')
strbuf_splice(&script, p-- - script.buf, 1, "", 0);
--
2.18.0
Re: [PATCH 06/16] upload-pack: generalize commit date cutoff
On 7/16/2018 3:38 PM, Stefan Beller wrote: On Mon, Jul 16, 2018 at 6:00 AM Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The ok_to_give_up() method uses the commit date as a cutoff to avoid walking the entire reachble set of commits. Before moving the reachable() method to commit-reach.c, pull out the dependence on the global constant 'oldest_have' with a 'min_commit_date' parameter. 'oldest_have' seems to be used in only one method after that (function got_oid); but as that function is called many times we either have to make it a function-global or pass around as a parameter, we'll defer that to later. There is a lot of global state involved in this negotiation code, and it lives between negotiation rounds when the transfer is stateful. Tread carefully! I did not attempt to reduce the global state at all. Thanks, -Stolee
[PATCH v2 07/23] builtin/replace.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/replace.c | 74 +++
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 2d7abf7a43..c77b325aa1 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -54,7 +54,7 @@ static int show_reference(const char *refname, const struct
object_id *oid,
enum object_type obj_type, repl_type;
if (get_oid(refname, &object))
- return error("failed to resolve '%s' as a valid
ref", refname);
+ return error(_("failed to resolve '%s' as a
valid ref"), refname);
obj_type = oid_object_info(the_repository, &object,
NULL);
@@ -83,8 +83,8 @@ static int list_replace_refs(const char *pattern, const char
*format)
else if (!strcmp(format, "long"))
data.format = REPLACE_FORMAT_LONG;
else
- return error("invalid replace format '%s'\n"
-"valid formats are 'short', 'medium' and 'long'",
+ return error(_("invalid replace format '%s'\n"
+ "valid formats are 'short', 'medium' and
'long'"),
format);
for_each_replace_ref(the_repository, show_reference, (void *)&data);
@@ -118,7 +118,7 @@ static int for_each_replace_name(const char **argv,
each_replace_name_fn fn)
full_hex = ref.buf + base_len;
if (read_ref(ref.buf, &oid)) {
- error("replace ref '%s' not found", full_hex);
+ error(_("replace ref '%s' not found"), full_hex);
had_error = 1;
continue;
}
@@ -134,7 +134,7 @@ static int delete_replace_ref(const char *name, const char
*ref,
{
if (delete_ref(NULL, ref, oid, 0))
return 1;
- printf_ln("Deleted replace ref '%s'", name);
+ printf_ln(_("Deleted replace ref '%s'"), name);
return 0;
}
@@ -146,12 +146,12 @@ static int check_ref_valid(struct object_id *object,
strbuf_reset(ref);
strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
if (check_refname_format(ref->buf, 0))
- return error("'%s' is not a valid ref name", ref->buf);
+ return error(_("'%s' is not a valid ref name"), ref->buf);
if (read_ref(ref->buf, prev))
oidclr(prev);
else if (!force)
- return error("replace ref '%s' already exists", ref->buf);
+ return error(_("replace ref '%s' already exists"), ref->buf);
return 0;
}
@@ -171,10 +171,10 @@ static int replace_object_oid(const char *object_ref,
obj_type = oid_object_info(the_repository, object, NULL);
repl_type = oid_object_info(the_repository, repl, NULL);
if (!force && obj_type != repl_type)
- return error("Objects must be of the same type.\n"
-"'%s' points to a replaced object of type '%s'\n"
-"while '%s' points to a replacement object of "
-"type '%s'.",
+ return error(_("Objects must be of the same type.\n"
+ "'%s' points to a replaced object of type '%s'\n"
+ "while '%s' points to a replacement object of "
+ "type '%s'."),
object_ref, type_name(obj_type),
replace_ref, type_name(repl_type));
@@ -200,10 +200,10 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
struct object_id object, repl;
if (get_oid(object_ref, &object))
- return error("failed to resolve '%s' as a valid ref",
+ return error(_("failed to resolve '%s' as a valid ref"),
object_ref);
if (get_oid(replace_ref, &repl))
- return error("failed to resolve '%s' as a valid ref",
+ return error(_("failed to resolve '%s' as a valid ref"),
replace_ref);
return replace_object_oid(object_ref, &object, replace_ref, &repl,
force);
@@ -222,7 +222,7 @@ static int export_object(const struct object_id *oid, enum
object_type type,
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (fd < 0)
- return error_errno("unable to open %s for writing", filename);
+ return error_errno(_("unable to open %s for writing"),
filename);
argv_array_push(&cmd.args, "--no-replace-objects");
argv_array_push(&cmd.args, "cat-file");
@@ -235,7 +235,7 @@ static int export_object(const struct object_id *oid, enum
object_type type,
cmd.out = fd;
[PATCH v2 08/23] commit-graph.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
commit-graph.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index b63a1fc85e..c8d521923c 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -77,28 +77,28 @@ struct commit_graph *load_commit_graph_one(const char
*graph_file)
if (graph_size < GRAPH_MIN_SIZE) {
close(fd);
- die("graph file %s is too small", graph_file);
+ die(_("graph file %s is too small"), graph_file);
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
data = (const unsigned char *)graph_map;
graph_signature = get_be32(data);
if (graph_signature != GRAPH_SIGNATURE) {
- error("graph signature %X does not match signature %X",
+ error(_("graph signature %X does not match signature %X"),
graph_signature, GRAPH_SIGNATURE);
goto cleanup_fail;
}
graph_version = *(unsigned char*)(data + 4);
if (graph_version != GRAPH_VERSION) {
- error("graph version %X does not match version %X",
+ error(_("graph version %X does not match version %X"),
graph_version, GRAPH_VERSION);
goto cleanup_fail;
}
hash_version = *(unsigned char*)(data + 5);
if (hash_version != GRAPH_OID_VERSION) {
- error("hash version %X does not match version %X",
+ error(_("hash version %X does not match version %X"),
hash_version, GRAPH_OID_VERSION);
goto cleanup_fail;
}
@@ -122,7 +122,7 @@ struct commit_graph *load_commit_graph_one(const char
*graph_file)
chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;
if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
- error("improper chunk offset %08x%08x",
(uint32_t)(chunk_offset >> 32),
+ error(_("improper chunk offset %08x%08x"),
(uint32_t)(chunk_offset >> 32),
(uint32_t)chunk_offset);
goto cleanup_fail;
}
@@ -158,7 +158,7 @@ struct commit_graph *load_commit_graph_one(const char
*graph_file)
}
if (chunk_repeated) {
- error("chunk id %08x appears multiple times", chunk_id);
+ error(_("chunk id %08x appears multiple times"),
chunk_id);
goto cleanup_fail;
}
@@ -244,7 +244,7 @@ static struct commit_list **insert_parent_or_die(struct
commit_graph *g,
hashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);
c = lookup_commit(&oid);
if (!c)
- die("could not find commit %s", oid_to_hex(&oid));
+ die(_("could not find commit %s"), oid_to_hex(&oid));
c->graph_pos = pos;
return &commit_list_insert(c, pptr)->next;
}
@@ -537,7 +537,7 @@ static int add_packed_commits(const struct object_id *oid,
oi.typep = &type;
if (packed_object_info(the_repository, pack, offset, &oi) < 0)
- die("unable to get type of object %s", oid_to_hex(oid));
+ die(_("unable to get type of object %s"), oid_to_hex(oid));
if (type != OBJ_COMMIT)
return 0;
@@ -683,9 +683,9 @@ void write_commit_graph(const char *obj_dir,
strbuf_addstr(&packname, pack_indexes[i]);
p = add_packed_git(packname.buf, packname.len, 1);
if (!p)
- die("error adding pack %s", packname.buf);
+ die(_("error adding pack %s"), packname.buf);
if (open_pack_index(p))
- die("error opening index for %s", packname.buf);
+ die(_("error opening index for %s"),
packname.buf);
for_each_object_in_pack(p, add_packed_commits, &oids);
close_pack(p);
}
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 02/23] archive-tar.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive-tar.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index b6f58ddf38..68e72d9176 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -121,7 +121,7 @@ static int stream_blocked(const struct object_id *oid)
st = open_istream(oid, &type, &sz, NULL);
if (!st)
- return error("cannot stream blob %s", oid_to_hex(oid));
+ return error(_("cannot stream blob %s"), oid_to_hex(oid));
for (;;) {
readlen = read_istream(st, buf, sizeof(buf));
if (readlen <= 0)
@@ -256,7 +256,7 @@ static int write_tar_entry(struct archiver_args *args,
*header.typeflag = TYPEFLAG_REG;
mode = (mode | ((mode & 0100) ? 0777 : 0666)) & ~tar_umask;
} else {
- return error("unsupported file mode: 0%o (SHA1: %s)",
+ return error(_("unsupported file mode: 0%o (SHA1: %s)"),
mode, oid_to_hex(oid));
}
if (pathlen > sizeof(header.name)) {
@@ -283,7 +283,7 @@ static int write_tar_entry(struct archiver_args *args,
enum object_type type;
buffer = object_file_to_archive(args, path, oid, old_mode,
&type, &size);
if (!buffer)
- return error("cannot read %s", oid_to_hex(oid));
+ return error(_("cannot read %s"), oid_to_hex(oid));
} else {
buffer = NULL;
size = 0;
@@ -454,17 +454,17 @@ static int write_tar_filter_archive(const struct archiver
*ar,
filter.in = -1;
if (start_command(&filter) < 0)
- die_errno("unable to start '%s' filter", argv[0]);
+ die_errno(_("unable to start '%s' filter"), argv[0]);
close(1);
if (dup2(filter.in, 1) < 0)
- die_errno("unable to redirect descriptor");
+ die_errno(_("unable to redirect descriptor"));
close(filter.in);
r = write_tar_archive(ar, args);
close(1);
if (finish_command(&filter) != 0)
- die("'%s' filter reported error", argv[0]);
+ die(_("'%s' filter reported error"), argv[0]);
strbuf_release(&cmd);
return r;
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 11/23] convert.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
convert.c | 38 --
t/t0021-conversion.sh | 2 +-
2 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/convert.c b/convert.c
index f47e60022e..e53911d4f8 100644
--- a/convert.c
+++ b/convert.c
@@ -190,7 +190,7 @@ static enum eol output_eol(enum crlf_action crlf_action)
/* fall through */
return text_eol_is_crlf() ? EOL_CRLF : EOL_LF;
}
- warning("illegal crlf_action %d", (int)crlf_action);
+ warning(_("illegal crlf_action %d"), (int)crlf_action);
return core_eol;
}
@@ -207,7 +207,7 @@ static void check_global_conv_flags_eol(const char *path,
enum crlf_action crlf_
else if (conv_flags & CONV_EOL_RNDTRP_WARN)
warning(_("CRLF will be replaced by LF in %s.\n"
"The file will have its original line"
- " endings in your working directory."), path);
+ " endings in your working directory"), path);
} else if (old_stats->lonelf && !new_stats->lonelf ) {
/*
* CRLFs would be added by checkout
@@ -217,7 +217,7 @@ static void check_global_conv_flags_eol(const char *path,
enum crlf_action crlf_
else if (conv_flags & CONV_EOL_RNDTRP_WARN)
warning(_("LF will be replaced by CRLF in %s.\n"
"The file will have its original line"
- " endings in your working directory."), path);
+ " endings in your working directory"), path);
}
}
@@ -492,7 +492,7 @@ static int encode_to_worktree(const char *path, const char
*src, size_t src_len,
dst = reencode_string_len(src, src_len, enc, default_encoding,
&dst_len);
if (!dst) {
- error("failed to encode '%s' from %s to %s",
+ error(_("failed to encode '%s' from %s to %s"),
path, default_encoding, enc);
return 0;
}
@@ -670,7 +670,8 @@ static int filter_buffer_or_fd(int in, int out, void *data)
if (start_command(&child_process)) {
strbuf_release(&cmd);
- return error("cannot fork to run external filter '%s'",
params->cmd);
+ return error(_("cannot fork to run external filter '%s'"),
+params->cmd);
}
sigchain_push(SIGPIPE, SIG_IGN);
@@ -689,13 +690,14 @@ static int filter_buffer_or_fd(int in, int out, void
*data)
if (close(child_process.in))
write_err = 1;
if (write_err)
- error("cannot feed the input to external filter '%s'",
params->cmd);
+ error(_("cannot feed the input to external filter '%s'"),
+ params->cmd);
sigchain_pop(SIGPIPE);
status = finish_command(&child_process);
if (status)
- error("external filter '%s' failed %d", params->cmd, status);
+ error(_("external filter '%s' failed %d"), params->cmd, status);
strbuf_release(&cmd);
return (write_err || status);
@@ -730,13 +732,13 @@ static int apply_single_file_filter(const char *path,
const char *src, size_t le
return 0; /* error was already reported */
if (strbuf_read(&nbuf, async.out, len) < 0) {
- err = error("read from external filter '%s' failed", cmd);
+ err = error(_("read from external filter '%s' failed"), cmd);
}
if (close(async.out)) {
- err = error("read from external filter '%s' failed", cmd);
+ err = error(_("read from external filter '%s' failed"), cmd);
}
if (finish_async(&async)) {
- err = error("external filter '%s' failed", cmd);
+ err = error(_("external filter '%s' failed"), cmd);
}
if (!err) {
@@ -790,7 +792,7 @@ static void handle_filter_error(const struct strbuf
*filter_status,
* Something went wrong with the protocol filter.
* Force shutdown and restart if another blob requires
filtering.
*/
- error("external filter '%s' failed", entry->subprocess.cmd);
+ error(_("external filter '%s' failed"), entry->subprocess.cmd);
subprocess_stop(&subprocess_map, &entry->subprocess);
free(entry);
}
@@ -838,7 +840,7 @@ static int apply_multi_file_filter(const char *path, const
char *src, size_t len
else if (wanted_capability & CAP_SMUDGE)
filter_type = "smudge";
else
- die("unexpected filter type");
+ die(_("unexpected filter type"));
sigchain_push(SIGPIPE, SIG_IGN);
@@ -849,7 +851,7 @@ static int apply_multi_fil
[PATCH v2 14/23] exec-cmd.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
exec-cmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/exec-cmd.c b/exec-cmd.c
index 02d31ee897..4f81f44310 100644
--- a/exec-cmd.c
+++ b/exec-cmd.c
@@ -358,7 +358,7 @@ int execl_git_cmd(const char *cmd, ...)
}
va_end(param);
if (MAX_ARGS <= argc)
- return error("too many args to run %s", cmd);
+ return error(_("too many args to run %s"), cmd);
argv[argc] = NULL;
return execv_git_cmd(argv);
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 15/23] object.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
object.c| 10 +-
t/t1450-fsck.sh | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/object.c b/object.c
index 10d167825e..477e686da7 100644
--- a/object.c
+++ b/object.c
@@ -49,7 +49,7 @@ int type_from_string_gently(const char *str, ssize_t len, int
gentle)
if (gentle)
return -1;
- die("invalid object type \"%s\"", str);
+ die(_("invalid object type \"%s\""), str);
}
/*
@@ -169,7 +169,7 @@ void *object_as_type(struct object *obj, enum object_type
type, int quiet)
}
else {
if (!quiet)
- error("object %s is a %s, not a %s",
+ error(_("object %s is a %s, not a %s"),
oid_to_hex(&obj->oid),
type_name(obj->type), type_name(type));
return NULL;
@@ -229,7 +229,7 @@ struct object *parse_object_buffer(const struct object_id
*oid, enum object_type
obj = &tag->object;
}
} else {
- warning("object %s has unknown type id %d", oid_to_hex(oid),
type);
+ warning(_("object %s has unknown type id %d"), oid_to_hex(oid),
type);
obj = NULL;
}
return obj;
@@ -262,7 +262,7 @@ struct object *parse_object(const struct object_id *oid)
(!obj && has_object_file(oid) &&
oid_object_info(the_repository, oid, NULL) == OBJ_BLOB)) {
if (check_object_signature(repl, NULL, 0, NULL) < 0) {
- error("sha1 mismatch %s", oid_to_hex(oid));
+ error(_("sha1 mismatch %s"), oid_to_hex(oid));
return NULL;
}
parse_blob_buffer(lookup_blob(oid), NULL, 0);
@@ -273,7 +273,7 @@ struct object *parse_object(const struct object_id *oid)
if (buffer) {
if (check_object_signature(repl, buffer, size, type_name(type))
< 0) {
free(buffer);
- error("sha1 mismatch %s", oid_to_hex(repl));
+ error(_("sha1 mismatch %s"), oid_to_hex(repl));
return NULL;
}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 91fd71444d..7b7602ddb4 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -372,7 +372,7 @@ test_expect_success 'rev-list --verify-objects with bad
sha1' '
test_might_fail git rev-list --verify-objects refs/heads/bogus
>/dev/null 2>out &&
cat out &&
- grep -q "error: sha1 mismatch 63ff"
out
+ test_i18ngrep -q "error: sha1 mismatch
63ff" out
'
test_expect_success 'force fsck to ignore double author' '
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 12/23] dir.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
dir.c| 6 +++---
t/t3005-ls-files-relative.sh | 4 ++--
t/t7400-submodule-basic.sh | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index e1a2e1cffb..7c6e7a0a37 100644
--- a/dir.c
+++ b/dir.c
@@ -560,7 +560,7 @@ int report_path_error(const char *ps_matched,
if (found_dup)
continue;
- error("pathspec '%s' did not match any file(s) known to git",
+ error(_("pathspec '%s' did not match any file(s) known to git"),
pathspec->items[num].original);
errors++;
}
@@ -949,7 +949,7 @@ static void add_excludes_from_file_1(struct dir_struct
*dir, const char *fname,
dir->unmanaged_exclude_files++;
el = add_exclude_list(dir, EXC_FILE, fname);
if (add_excludes(fname, "", 0, el, NULL, oid_stat) < 0)
- die("cannot use %s as an exclude file", fname);
+ die(_("cannot use %s as an exclude file"), fname);
}
void add_excludes_from_file(struct dir_struct *dir, const char *fname)
@@ -3028,7 +3028,7 @@ static void connect_wt_gitdir_in_nested(const char
*sub_worktree,
return;
if (repo_read_index(&subrepo) < 0)
- die("index file corrupt in repo %s", subrepo.gitdir);
+ die(_("index file corrupt in repo %s"), subrepo.gitdir);
for (i = 0; i < subrepo.index->cache_nr; i++) {
const struct cache_entry *ce = subrepo.index->cache[i];
diff --git a/t/t3005-ls-files-relative.sh b/t/t3005-ls-files-relative.sh
index 341fad54ce..209b4c7cd8 100755
--- a/t/t3005-ls-files-relative.sh
+++ b/t/t3005-ls-files-relative.sh
@@ -50,7 +50,7 @@ test_expect_success 'ls-files -c' '
ls ../x* >expect.out &&
test_must_fail git ls-files -c --error-unmatch ../[xy]*
>actual.out 2>actual.err &&
test_cmp expect.out actual.out &&
- test_cmp expect.err actual.err
+ test_i18ncmp expect.err actual.err
)
'
@@ -65,7 +65,7 @@ test_expect_success 'ls-files -o' '
ls ../y* >expect.out &&
test_must_fail git ls-files -o --error-unmatch ../[xy]*
>actual.out 2>actual.err &&
test_cmp expect.out actual.out &&
- test_cmp expect.err actual.err
+ test_i18ncmp expect.err actual.err
)
'
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 812db137b8..d6853e94be 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -377,7 +377,7 @@ test_expect_success 'init should register submodule url in
.git/config' '
test_failure_with_unknown_submodule () {
test_must_fail git submodule $1 no-such-submodule 2>output.err &&
- grep "^error: .*no-such-submodule" output.err
+ test_i18ngrep "^error: .*no-such-submodule" output.err
}
test_expect_success 'init should fail with unknown submodule' '
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 17/23] refs.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
refs.c| 40 +--
t/t1400-update-ref.sh | 20 +++---
t/t1404-update-ref-errors.sh | 4 +--
t/t3210-pack-refs.sh | 2 +-
t/t3310-notes-merge-manual-resolve.sh | 6 ++--
t/t5505-remote.sh | 2 +-
6 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/refs.c b/refs.c
index a033910353..becb78e441 100644
--- a/refs.c
+++ b/refs.c
@@ -188,7 +188,7 @@ int ref_resolves_to_object(const char *refname,
if (flags & REF_ISBROKEN)
return 0;
if (!has_sha1_file(oid->hash)) {
- error("%s does not point to a valid object!", refname);
+ error(_("%s does not point to a valid object!"), refname);
return 0;
}
return 1;
@@ -567,9 +567,9 @@ int expand_ref(const char *str, int len, struct object_id
*oid, char **ref)
if (!warn_ambiguous_refs)
break;
} else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf,
"HEAD")) {
- warning("ignoring dangling symref %s", fullref.buf);
+ warning(_("ignoring dangling symref %s"), fullref.buf);
} else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
- warning("ignoring broken ref %s", fullref.buf);
+ warning(_("ignoring broken ref %s"), fullref.buf);
}
}
strbuf_release(&fullref);
@@ -673,7 +673,7 @@ static int write_pseudoref(const char *pseudoref, const
struct object_id *oid,
fd = hold_lock_file_for_update_timeout(&lock, filename, 0,
get_files_ref_lock_timeout_ms());
if (fd < 0) {
- strbuf_addf(err, "could not open '%s' for writing: %s",
+ strbuf_addf(err, _("could not open '%s' for writing: %s"),
filename, strerror(errno));
goto done;
}
@@ -683,18 +683,18 @@ static int write_pseudoref(const char *pseudoref, const
struct object_id *oid,
if (read_ref(pseudoref, &actual_old_oid)) {
if (!is_null_oid(old_oid)) {
- strbuf_addf(err, "could not read ref '%s'",
+ strbuf_addf(err, _("could not read ref '%s'"),
pseudoref);
rollback_lock_file(&lock);
goto done;
}
} else if (is_null_oid(old_oid)) {
- strbuf_addf(err, "ref '%s' already exists",
+ strbuf_addf(err, _("ref '%s' already exists"),
pseudoref);
rollback_lock_file(&lock);
goto done;
} else if (oidcmp(&actual_old_oid, old_oid)) {
- strbuf_addf(err, "unexpected object ID when writing
'%s'",
+ strbuf_addf(err, _("unexpected object ID when writing
'%s'"),
pseudoref);
rollback_lock_file(&lock);
goto done;
@@ -702,7 +702,7 @@ static int write_pseudoref(const char *pseudoref, const
struct object_id *oid,
}
if (write_in_full(fd, buf.buf, buf.len) < 0) {
- strbuf_addf(err, "could not write to '%s'", filename);
+ strbuf_addf(err, _("could not write to '%s'"), filename);
rollback_lock_file(&lock);
goto done;
}
@@ -734,9 +734,9 @@ static int delete_pseudoref(const char *pseudoref, const
struct object_id *old_o
return -1;
}
if (read_ref(pseudoref, &actual_old_oid))
- die("could not read ref '%s'", pseudoref);
+ die(_("could not read ref '%s'"), pseudoref);
if (oidcmp(&actual_old_oid, old_oid)) {
- error("unexpected object ID when deleting '%s'",
+ error(_("unexpected object ID when deleting '%s'"),
pseudoref);
rollback_lock_file(&lock);
return -1;
@@ -871,13 +871,13 @@ static int read_ref_at_ent(struct object_id *ooid, struct
object_id *noid,
if (!is_null_oid(&cb->ooid)) {
oidcpy(cb->oid, noid);
if (oidcmp(&cb->ooid, noid))
- warning("log for ref %s has gap after %s",
+ warning(_("log for ref %s has gap after %s"),
cb->refname, show_date(cb->date,
cb->tz, DATE_MODE(RFC2822)));
}
else if (cb->date == cb->at_time)
[PATCH v2 16/23] pkt-line.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pkt-line.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 941e41dfc1..04d10bbd03 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -101,7 +101,7 @@ int packet_flush_gently(int fd)
{
packet_trace("", 4, 1);
if (write_in_full(fd, "", 4) < 0)
- return error("flush packet write failed");
+ return error(_("flush packet write failed"));
return 0;
}
@@ -139,7 +139,7 @@ static void format_packet(struct strbuf *out, const char
*fmt, va_list args)
n = out->len - orig_len;
if (n > LARGE_PACKET_MAX)
- die("protocol error: impossibly long line");
+ die(_("protocol error: impossibly long line"));
set_packet_header(&out->buf[orig_len], n);
packet_trace(out->buf + orig_len + 4, n - 4, 1);
@@ -155,9 +155,9 @@ static int packet_write_fmt_1(int fd, int gently,
if (write_in_full(fd, buf.buf, buf.len) < 0) {
if (!gently) {
check_pipe(errno);
- die_errno("packet write with format failed");
+ die_errno(_("packet write with format failed"));
}
- return error("packet write with format failed");
+ return error(_("packet write with format failed"));
}
return 0;
@@ -189,21 +189,21 @@ static int packet_write_gently(const int fd_out, const
char *buf, size_t size)
size_t packet_size;
if (size > sizeof(packet_write_buffer) - 4)
- return error("packet write failed - data exceeds max packet
size");
+ return error(_("packet write failed - data exceeds max packet
size"));
packet_trace(buf, size, 1);
packet_size = size + 4;
set_packet_header(packet_write_buffer, packet_size);
memcpy(packet_write_buffer + 4, buf, size);
if (write_in_full(fd_out, packet_write_buffer, packet_size) < 0)
- return error("packet write failed");
+ return error(_("packet write failed"));
return 0;
}
void packet_write(int fd_out, const char *buf, size_t size)
{
if (packet_write_gently(fd_out, buf, size))
- die_errno("packet write failed");
+ die_errno(_("packet write failed"));
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
@@ -225,7 +225,7 @@ void packet_buf_write_len(struct strbuf *buf, const char
*data, size_t len)
n = buf->len - orig_len;
if (n > LARGE_PACKET_MAX)
- die("protocol error: impossibly long line");
+ die(_("protocol error: impossibly long line"));
set_packet_header(&buf->buf[orig_len], n);
packet_trace(data, len, 1);
@@ -288,7 +288,7 @@ static int get_packet_data(int fd, char **src_buf, size_t
*src_size,
} else {
ret = read_in_full(fd, dst, size);
if (ret < 0)
- die_errno("read error");
+ die_errno(_("read error"));
}
/* And complain if we didn't get enough bytes to satisfy the read. */
@@ -296,7 +296,7 @@ static int get_packet_data(int fd, char **src_buf, size_t
*src_size,
if (options & PACKET_READ_GENTLE_ON_EOF)
return -1;
- die("the remote end hung up unexpectedly");
+ die(_("the remote end hung up unexpectedly"));
}
return ret;
@@ -324,7 +324,7 @@ enum packet_read_status packet_read_with_status(int fd,
char **src_buffer,
len = packet_length(linelen);
if (len < 0) {
- die("protocol error: bad line length character: %.4s", linelen);
+ die(_("protocol error: bad line length character: %.4s"),
linelen);
} else if (!len) {
packet_trace("", 4, 0);
*pktlen = 0;
@@ -334,12 +334,12 @@ enum packet_read_status packet_read_with_status(int fd,
char **src_buffer,
*pktlen = 0;
return PACKET_READ_DELIM;
} else if (len < 4) {
- die("protocol error: bad line length %d", len);
+ die(_("protocol error: bad line length %d"), len);
}
len -= 4;
if ((unsigned)len >= size)
- die("protocol error: bad line length %d", len);
+ die(_("protocol error: bad line length %d"), len);
if (get_packet_data(fd, src_buffer, src_len, buffer, len, options) < 0)
{
*pktlen = -1;
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 00/23] Mark strings for translation
This series marks more strings for translation. It was done during the
last rc period and probably misses new strings now, but we can mark
more later.
Compared to v1 [1], this moves interesting changes back to patch one
and leave the boring _() to the remaining patches. I also cleaned up
the "die (" and "warning (" a bit. Interdiff
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a0388aaef..b77ba6ae82 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -408,7 +408,7 @@ static void parse_color_fields(const char *s)
}
if (next == EXPECT_COLOR)
- die (_("must end with a color"));
+ die(_("must end with a color"));
colorfield[colorfield_nr].hop = TIME_MAX;
string_list_clear(&l, 0);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c41d8b4c8..626868637a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1191,12 +1191,12 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
if (opts.track != BRANCH_TRACK_UNSPECIFIED && !opts.new_branch) {
const char *argv0 = argv[0];
if (!argc || !strcmp(argv0, "--"))
- die (_("--track needs a branch name"));
+ die(_("--track needs a branch name"));
skip_prefix(argv0, "refs/", &argv0);
skip_prefix(argv0, "remotes/", &argv0);
argv0 = strchr(argv0, '/');
if (!argv0 || !argv0[1])
- die (_("Missing branch name; try -b"));
+ die(_("missing branch name; try -b"));
opts.new_branch = argv0 + 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 9bcbb0c25c..d3276a10b2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1647,9 +1647,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
unlink(git_path_squash_msg());
if (commit_index_files())
- die (_("Repository has been updated, but unable to write\n"
-"new_index file. Check that disk is not full and quota
is\n"
-"not exceeded, and then \"git reset HEAD\" to recover."));
+ die(_("repository has been updated, but unable to write\n"
+ "new_index file. Check that disk is not full and quota
is\n"
+ "not exceeded, and then \"git reset HEAD\" to recover."));
rerere(0);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 73d12c1020..83c482581b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -240,7 +240,7 @@ static void export_blob(const struct object_id *oid)
} else {
buf = read_object_file(oid, &type, &size);
if (!buf)
- die ("Could not read blob %s", oid_to_hex(oid));
+ die("could not read blob %s", oid_to_hex(oid));
if (check_object_signature(oid, buf, size, type_name(type)) < 0)
die("sha1 mismatch in blob %s", oid_to_hex(oid));
object = parse_object_buffer(oid, type, size, buf, &eaten);
@@ -253,7 +253,7 @@ static void export_blob(const struct object_id *oid)
printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
if (size && fwrite(buf, size, 1, stdout) != 1)
- die_errno ("Could not write blob '%s'", oid_to_hex(oid));
+ die_errno("could not write blob '%s'", oid_to_hex(oid));
printf("\n");
show_progress();
@@ -560,14 +560,14 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev,
commit_buffer = get_commit_buffer(commit, NULL);
author = strstr(commit_buffer, "\nauthor ");
if (!author)
- die ("Could not find author in commit %s",
-oid_to_hex(&commit->object.oid));
+ die("could not find author in commit %s",
+ oid_to_hex(&commit->object.oid));
author++;
author_end = strchrnul(author, '\n');
committer = strstr(author_end, "\ncommitter ");
if (!committer)
- die ("Could not find committer in commit %s",
-oid_to_hex(&commit->object.oid));
+ die("could not find committer in commit %s",
+ oid_to_hex(&commit->object.oid));
committer++;
committer_end = strchrnul(committer, '\n');
message = strstr(committer_end, "\n\n");
@@ -688,7 +688,7 @@ static void handle_tag(const char *name, struct tag *tag)
buf = read_object_file(&tag->object.oid, &type, &size);
if (!buf)
- die ("Could not read tag %s", oid_to_hex(&tag->object.oid));
+ die("could not read tag %s", oid_to_hex(&tag->object.oid));
message = memmem(buf, size, "\n\n", 2);
if (message) {
message += 2;
@@ -725,18 +725,18 @@ s
[PATCH v2 03/23] archive-zip.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive-zip.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 48d843489c..7ad46d8854 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -309,11 +309,11 @@ static int write_zip_entry(struct archiver_args *args,
if (is_utf8(path))
flags |= ZIP_UTF8;
else
- warning("path is not valid UTF-8: %s", path);
+ warning(_("path is not valid UTF-8: %s"), path);
}
if (pathlen > 0x) {
- return error("path too long (%d chars, SHA1: %s): %s",
+ return error(_("path too long (%d chars, SHA1: %s): %s"),
(int)pathlen, oid_to_hex(oid), path);
}
@@ -340,7 +340,7 @@ static int write_zip_entry(struct archiver_args *args,
size > big_file_threshold) {
stream = open_istream(oid, &type, &size, NULL);
if (!stream)
- return error("cannot stream blob %s",
+ return error(_("cannot stream blob %s"),
oid_to_hex(oid));
flags |= ZIP_STREAM;
out = buffer = NULL;
@@ -348,7 +348,7 @@ static int write_zip_entry(struct archiver_args *args,
buffer = object_file_to_archive(args, path, oid, mode,
&type, &size);
if (!buffer)
- return error("cannot read %s",
+ return error(_("cannot read %s"),
oid_to_hex(oid));
crc = crc32(crc, buffer, size);
is_binary = entry_is_binary(path_without_prefix,
@@ -357,7 +357,7 @@ static int write_zip_entry(struct archiver_args *args,
}
compressed_size = (method == 0) ? size : 0;
} else {
- return error("unsupported file mode: 0%o (SHA1: %s)", mode,
+ return error(_("unsupported file mode: 0%o (SHA1: %s)"), mode,
oid_to_hex(oid));
}
@@ -466,7 +466,7 @@ static int write_zip_entry(struct archiver_args *args,
zstream.avail_in = readlen;
result = git_deflate(&zstream, 0);
if (result != Z_OK)
- die("deflate error (%d)", result);
+ die(_("deflate error (%d)"), result);
out_len = zstream.next_out - compressed;
if (out_len > 0) {
@@ -601,7 +601,7 @@ static void dos_time(timestamp_t *timestamp, int *dos_date,
int *dos_time)
struct tm *t;
if (date_overflows(*timestamp))
- die("timestamp too large for this system: %"PRItime,
+ die(_("timestamp too large for this system: %"PRItime),
*timestamp);
time = (time_t)*timestamp;
t = localtime(&time);
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 18/23] refspec.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
refspec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refspec.c b/refspec.c
index c66351743b..f529092fd6 100644
--- a/refspec.c
+++ b/refspec.c
@@ -134,7 +134,7 @@ void refspec_item_init_or_die(struct refspec_item *item,
const char *refspec,
int fetch)
{
if (!refspec_item_init(item, refspec, fetch))
- die("invalid refspec '%s'", refspec);
+ die(_("invalid refspec '%s'"), refspec);
}
void refspec_item_clear(struct refspec_item *item)
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 04/23] builtin/config.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/config.c | 48 +--
t/t1308-config-set.sh | 2 +-
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index abb3195fa9..3c26df6c48 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -110,7 +110,7 @@ static int option_parse_type(const struct option *opt,
const char *arg,
* --int' and '--type=bool
* --type=int'.
*/
- error("only one type at a time");
+ error(_("only one type at a time"));
usage_with_options(builtin_config_usage,
builtin_config_options);
}
@@ -161,9 +161,9 @@ static void check_argc(int argc, int min, int max) {
if (argc >= min && argc <= max)
return;
if (min == max)
- error("wrong number of arguments, should be %d", min);
+ error(_("wrong number of arguments, should be %d"), min);
else
- error("wrong number of arguments, should be from %d to %d",
+ error(_("wrong number of arguments, should be from %d to %d"),
min, max);
usage_with_options(builtin_config_usage, builtin_config_options);
}
@@ -297,7 +297,7 @@ static int get_value(const char *key_, const char *regex_)
key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
if (regcomp(key_regexp, key, REG_EXTENDED)) {
- error("invalid key pattern: %s", key_);
+ error(_("invalid key pattern: %s"), key_);
FREE_AND_NULL(key_regexp);
ret = CONFIG_INVALID_PATTERN;
goto free_strings;
@@ -317,7 +317,7 @@ static int get_value(const char *key_, const char *regex_)
regexp = (regex_t*)xmalloc(sizeof(regex_t));
if (regcomp(regexp, regex_, REG_EXTENDED)) {
- error("invalid pattern: %s", regex_);
+ error(_("invalid pattern: %s"), regex_);
FREE_AND_NULL(regexp);
ret = CONFIG_INVALID_PATTERN;
goto free_strings;
@@ -390,7 +390,7 @@ static char *normalize_value(const char *key, const char
*value)
if (type == TYPE_COLOR) {
char v[COLOR_MAXLEN];
if (git_config_color(v, key, value))
- die("cannot parse color '%s'", value);
+ die(_("cannot parse color '%s'"), value);
/*
* The contents of `v` now contain an ANSI escape
@@ -485,13 +485,13 @@ static int get_colorbool(const char *var, int print)
static void check_write(void)
{
if (!given_config_source.file && !startup_info->have_repository)
- die("not in a git directory");
+ die(_("not in a git directory"));
if (given_config_source.use_stdin)
- die("writing to stdin is not supported");
+ die(_("writing to stdin is not supported"));
if (given_config_source.blob)
- die("writing config blobs is not supported");
+ die(_("writing config blobs is not supported"));
}
struct urlmatch_current_candidate_value {
@@ -599,7 +599,7 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (use_global_config + use_system_config + use_local_config +
!!given_config_source.file + !!given_config_source.blob > 1) {
- error("only one config file at a time");
+ error(_("only one config file at a time"));
usage_with_options(builtin_config_usage,
builtin_config_options);
}
@@ -626,7 +626,7 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
* location; error out even if XDG_CONFIG_HOME
* is set and points at a sane location.
*/
- die("$HOME is not set");
+ die(_("$HOME is not set"));
if (access_or_warn(user_config, R_OK, 0) &&
xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
@@ -663,12 +663,12 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {
- error("--get-color and variable type are incoherent");
+ error(_("--get-color and variable type are incoherent"));
usage_with_options(builtin_config_usage,
builtin_config_options);
}
if (HAS_MULTI_BITS(actions)) {
- error("only one action at a time");
+ error(_("only one action at a time"));
usage_with_options(builtin_config_usage,
builtin_config_options);
}
if (actions == 0)
@@ -681,19 +681,19 @@ in
[PATCH v2 01/23] Update messages in preparation for i18n
Many messages will be marked for translation in the following
commits. This commit updates some of them to be more consistent and
reduce diff noise in those commits. Changes are
- keep the first letter of die(), error() and warning() in lowercase
- no full stop in die(), error() or warning() if it's single sentence
messages
- indentation
- some messages are turned to BUG(), or prefixed with "BUG:" and will
not be marked for i18n
- some messages are improved to give more information
- some messages are broken down by sentence to be i18n friendly
(on the same token, combine multiple warning() into one big string)
- the trailing \n is converted to printf_ln if possible, or deleted
if not redundant
- errno_errno() is used instead of explicit strerror()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive-zip.c | 2 +-
builtin/blame.c | 2 +-
builtin/checkout.c| 4 +--
builtin/commit.c | 6 ++--
builtin/config.c | 20 +++--
builtin/fast-export.c | 42 +--
builtin/fmt-merge-msg.c | 2 +-
builtin/grep.c| 10 +++
builtin/log.c | 6 ++--
builtin/merge.c | 2 +-
builtin/pack-objects.c| 18 ++--
builtin/replace.c | 32 ++---
builtin/rm.c | 2 +-
config.c | 12
connect.c | 33 +++--
convert.c | 6 ++--
diff.c| 4 +--
dir.c | 4 +--
pkt-line.c| 2 +-
reflog-walk.c | 4 +--
refs.c| 12
refspec.c | 2 +-
sequencer.c | 8 +++---
sha1-file.c | 8 +++---
t/t1400-update-ref.sh | 8 +++---
t/t3005-ls-files-relative.sh | 4 +--
t/t5801-remote-helpers.sh | 6 ++--
t/t7063-status-untracked-cache.sh | 2 +-
transport-helper.c| 48 +++
transport.c | 8 +++---
30 files changed, 163 insertions(+), 156 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 74f3fe9103..48d843489c 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -309,7 +309,7 @@ static int write_zip_entry(struct archiver_args *args,
if (is_utf8(path))
flags |= ZIP_UTF8;
else
- warning("Path is not valid UTF-8: %s", path);
+ warning("path is not valid UTF-8: %s", path);
}
if (pathlen > 0x) {
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a0388aaef..b77ba6ae82 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -408,7 +408,7 @@ static void parse_color_fields(const char *s)
}
if (next == EXPECT_COLOR)
- die (_("must end with a color"));
+ die(_("must end with a color"));
colorfield[colorfield_nr].hop = TIME_MAX;
string_list_clear(&l, 0);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c41d8b4c8..626868637a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1191,12 +1191,12 @@ int cmd_checkout(int argc, const char **argv, const
char *prefix)
if (opts.track != BRANCH_TRACK_UNSPECIFIED && !opts.new_branch) {
const char *argv0 = argv[0];
if (!argc || !strcmp(argv0, "--"))
- die (_("--track needs a branch name"));
+ die(_("--track needs a branch name"));
skip_prefix(argv0, "refs/", &argv0);
skip_prefix(argv0, "remotes/", &argv0);
argv0 = strchr(argv0, '/');
if (!argv0 || !argv0[1])
- die (_("Missing branch name; try -b"));
+ die(_("missing branch name; try -b"));
opts.new_branch = argv0 + 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 9bcbb0c25c..d3276a10b2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1647,9 +1647,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
unlink(git_path_squash_msg());
if (commit_index_files())
- die (_("Repository has been updated, but unable to write\n"
-"new_index file. Check that disk is not full and quota
is\n"
-"not exceeded, and then \"git reset HEAD\" to recover."));
+ die(_("repository has been updated, but unable to write\n"
+ "new_index file. Check that disk is not full and quota
is\n"
+ "not exceeded, and then \"git reset HEAD\" to recover."));
rerere(0);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
diff --git a/builtin/config.c b
[PATCH v2 19/23] replace-object.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
replace-object.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/replace-object.c b/replace-object.c
index 801b5c1678..ddc1546b8c 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -17,7 +17,7 @@ static int register_replace_ref(const char *refname,
if (get_oid_hex(hash, &repl_obj->original.oid)) {
free(repl_obj);
- warning("bad replace ref name: %s", refname);
+ warning(_("bad replace ref name: %s"), refname);
return 0;
}
@@ -26,7 +26,7 @@ static int register_replace_ref(const char *refname,
/* Register new object */
if (oidmap_put(the_repository->objects->replace_map, repl_obj))
- die("duplicate replace ref: %s", refname);
+ die(_("duplicate replace ref: %s"), refname);
return 0;
}
@@ -69,5 +69,5 @@ const struct object_id *do_lookup_replace_object(struct
repository *r,
return cur;
cur = &repl_obj->replacement;
}
- die("replace depth too high for object %s", oid_to_hex(oid));
+ die(_("replace depth too high for object %s"), oid_to_hex(oid));
}
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 06/23] builtin/pack-objects.c: mark more strings for translation
Most of these are straight forward. GETTEXT_POISON does catch the last
string in cmd_pack_objects(), but since this is --progress output, it's
not supposed to be machine-readable.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/pack-objects.c | 104 +
t/t5500-fetch-pack.sh | 2 +-
2 files changed, 54 insertions(+), 52 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7624f0205a..6b778b0a82 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -140,7 +140,7 @@ static void *get_delta(struct object_entry *entry)
buf = read_object_file(&entry->idx.oid, &type, &size);
if (!buf)
- die("unable to read %s", oid_to_hex(&entry->idx.oid));
+ die(_("unable to read %s"), oid_to_hex(&entry->idx.oid));
base_buf = read_object_file(&DELTA(entry)->idx.oid, &type,
&base_size);
if (!base_buf)
@@ -149,7 +149,7 @@ static void *get_delta(struct object_entry *entry)
delta_buf = diff_delta(base_buf, base_size,
buf, size, &delta_size, 0);
if (!delta_buf || delta_size != DELTA_SIZE(entry))
- die("delta size changed");
+ die(_("delta size changed"));
free(buf);
free(base_buf);
return delta_buf;
@@ -406,7 +406,7 @@ static off_t write_reuse_object(struct hashfile *f, struct
object_entry *entry,
datalen = revidx[1].offset - offset;
if (!pack_to_stdout && p->index_version > 1 &&
check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {
- error("bad packed object CRC for %s",
+ error(_("bad packed object CRC for %s"),
oid_to_hex(&entry->idx.oid));
unuse_pack(&w_curs);
return write_no_reuse_object(f, entry, limit, usable_delta);
@@ -417,7 +417,7 @@ static off_t write_reuse_object(struct hashfile *f, struct
object_entry *entry,
if (!pack_to_stdout && p->index_version == 1 &&
check_pack_inflate(p, &w_curs, offset, datalen, entry_size)) {
- error("corrupt packed object for %s",
+ error(_("corrupt packed object for %s"),
oid_to_hex(&entry->idx.oid));
unuse_pack(&w_curs);
return write_no_reuse_object(f, entry, limit, usable_delta);
@@ -548,7 +548,7 @@ static enum write_one_status write_one(struct hashfile *f,
*/
recursing = (e->idx.offset == 1);
if (recursing) {
- warning("recursive delta detected for object %s",
+ warning(_("recursive delta detected for object %s"),
oid_to_hex(&e->idx.oid));
return WRITE_ONE_RECURSIVE;
} else if (e->idx.offset || e->preferred_base) {
@@ -582,7 +582,7 @@ static enum write_one_status write_one(struct hashfile *f,
/* make sure off_t is sufficiently large not to wrap */
if (signed_add_overflows(*offset, size))
- die("pack too large for current definition of off_t");
+ die(_("pack too large for current definition of off_t"));
*offset += size;
return WRITE_ONE_WRITTEN;
}
@@ -748,7 +748,8 @@ static struct object_entry **compute_write_order(void)
}
if (wo_end != to_pack.nr_objects)
- die("ordered %u objects, expected %"PRIu32, wo_end,
to_pack.nr_objects);
+ die(_("ordered %u objects, expected %"PRIu32),
+ wo_end, to_pack.nr_objects);
return wo;
}
@@ -760,15 +761,15 @@ static off_t write_reused_pack(struct hashfile *f)
int fd;
if (!is_pack_valid(reuse_packfile))
- die("packfile is invalid: %s", reuse_packfile->pack_name);
+ die(_("packfile is invalid: %s"), reuse_packfile->pack_name);
fd = git_open(reuse_packfile->pack_name);
if (fd < 0)
- die_errno("unable to open packfile for reuse: %s",
+ die_errno(_("unable to open packfile for reuse: %s"),
reuse_packfile->pack_name);
if (lseek(fd, sizeof(struct pack_header), SEEK_SET) == -1)
- die_errno("unable to seek in reused packfile");
+ die_errno(_("unable to seek in reused packfile"));
if (reuse_packfile_offset < 0)
reuse_packfile_offset = reuse_packfile->pack_size -
the_hash_algo->rawsz;
@@ -779,7 +780,7 @@ static off_t write_reused_pack(struct hashfile *f)
int read_pack = xread(fd, buffer, sizeof(buffer));
if (read_pack <= 0)
- die_errno("unable to read from reused packfile");
+ die_errno(_("unable to read from reused packfile"));
if (read_pack > to_write)
read_pack = to_write;
@@ -882,7 +883,7 @@ static void write_pack_file(void)
[PATCH v2 10/23] connect.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
connect.c | 78 +++
t/t5570-git-daemon.sh | 6 ++--
2 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/connect.c b/connect.c
index 083cf804a7..70b97cfef6 100644
--- a/connect.c
+++ b/connect.c
@@ -78,7 +78,7 @@ int server_supports_v2(const char *c, int die_on_error)
}
if (die_on_error)
- die("server doesn't support '%s'", c);
+ die(_("server doesn't support '%s'"), c);
return 0;
}
@@ -100,7 +100,7 @@ int server_supports_feature(const char *c, const char
*feature,
}
if (die_on_error)
- die("server doesn't support feature '%s'", feature);
+ die(_("server doesn't support feature '%s'"), feature);
return 0;
}
@@ -111,7 +111,7 @@ static void process_capabilities_v2(struct packet_reader
*reader)
argv_array_push(&server_capabilities_v2, reader->line);
if (reader->status != PACKET_READ_FLUSH)
- die("expected flush after capabilities");
+ die(_("expected flush after capabilities"));
}
enum protocol_version discover_version(struct packet_reader *reader)
@@ -230,7 +230,7 @@ static int process_dummy_ref(const char *line)
static void check_no_capabilities(const char *line, int len)
{
if (strlen(line) != len)
- warning("ignoring capabilities after first line '%s'",
+ warning(_("ignoring capabilities after first line '%s'"),
line + strlen(line));
}
@@ -249,7 +249,7 @@ static int process_ref(const char *line, int len, struct
ref ***list,
if (extra_have && !strcmp(name, ".have")) {
oid_array_append(extra_have, &old_oid);
} else if (!strcmp(name, "capabilities^{}")) {
- die("protocol error: unexpected capabilities^{}");
+ die(_("protocol error: unexpected capabilities^{}"));
} else if (check_ref(name, flags)) {
struct ref *ref = alloc_ref(name);
oidcpy(&ref->old_oid, &old_oid);
@@ -270,9 +270,9 @@ static int process_shallow(const char *line, int len,
return 0;
if (get_oid_hex(arg, &old_oid))
- die("protocol error: expected shallow sha-1, got '%s'", arg);
+ die(_("protocol error: expected shallow sha-1, got '%s'"), arg);
if (!shallow_points)
- die("repository on the other end cannot be shallow");
+ die(_("repository on the other end cannot be shallow"));
oid_array_append(shallow_points, &old_oid);
check_no_capabilities(line, len);
return 1;
@@ -307,13 +307,13 @@ struct ref **get_remote_heads(struct packet_reader
*reader,
case PACKET_READ_NORMAL:
len = reader->pktlen;
if (len > 4 && skip_prefix(reader->line, "ERR ", &arg))
- die("remote error: %s", arg);
+ die(_("remote error: %s"), arg);
break;
case PACKET_READ_FLUSH:
state = EXPECTING_DONE;
break;
case PACKET_READ_DELIM:
- die("invalid packet");
+ die(_("invalid packet"));
}
switch (state) {
@@ -333,7 +333,7 @@ struct ref **get_remote_heads(struct packet_reader *reader,
case EXPECTING_SHALLOW:
if (process_shallow(reader->line, len, shallow_points))
break;
- die("protocol error: unexpected '%s'", reader->line);
+ die(_("protocol error: unexpected '%s'"), reader->line);
case EXPECTING_DONE:
break;
}
@@ -441,11 +441,11 @@ struct ref **get_remote_refs(int fd_out, struct
packet_reader *reader,
/* Process response from server */
while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
if (!process_ref_v2(reader->line, &list))
- die("invalid ls-refs response: %s", reader->line);
+ die(_("invalid ls-refs response: %s"), reader->line);
}
if (reader->status != PACKET_READ_FLUSH)
- die("expected flush after ref listing");
+ die(_("expected flush after ref listing"));
return list;
}
@@ -544,7 +544,7 @@ static enum protocol get_protocol(const char *name)
return PROTO_SSH;
if (!strcmp(name, "file"))
return PROTO_FILE;
- die("protocol '%s' is not supported", name);
+ die(_("protocol '%s' is not supported"), name);
}
static char *host_end(char **hoststart, int removebrackets)
@@ -595,7 +595,7 @@ static void enable_keepalive(int sockfd)
int ka = 1;
if (setsockopt
[PATCH v2 20/23] sequencer.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
sequencer.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 6b6162d194..f7c2f2422d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -716,7 +716,7 @@ static const char *read_author_ident(struct strbuf *buf)
/* dequote values and construct ident line in-place */
for (in = out = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
if (!skip_prefix(in, keys[i], (const char **)&in)) {
- warning("could not parse '%s' (looking for '%s'",
+ warning(_("could not parse '%s' (looking for '%s'"),
rebase_path_author_script(), keys[i]);
return NULL;
}
@@ -738,7 +738,7 @@ static const char *read_author_ident(struct strbuf *buf)
}
if (i < 3) {
- warning("could not parse '%s' (looking for '%s')",
+ warning(_("could not parse '%s' (looking for '%s')"),
rebase_path_author_script(), keys[i]);
return NULL;
}
@@ -1444,7 +1444,7 @@ static const char *command_to_string(const enum
todo_command command)
{
if (command < TODO_COMMENT)
return todo_command_info[command].str;
- die("unknown command: %d", command);
+ die(_("unknown command: %d"), command);
}
static char command_to_char(const enum todo_command command)
@@ -2600,15 +2600,17 @@ static int error_with_patch(struct commit *commit,
if (intend_to_amend())
return -1;
- fprintf(stderr, "You can amend the commit now, with\n"
- "\n"
- " git commit --amend %s\n"
- "\n"
- "Once you are satisfied with your changes, run\n"
- "\n"
- " git rebase --continue\n", gpg_sign_opt_quoted(opts));
+ fprintf(stderr,
+ _("You can amend the commit now, with\n"
+ "\n"
+ " git commit --amend %s\n"
+ "\n"
+ "Once you are satisfied with your changes, run\n"
+ "\n"
+ " git rebase --continue\n"),
+ gpg_sign_opt_quoted(opts));
} else if (exit_code)
- fprintf_ln(stderr, "Could not apply %s... %.*s",
+ fprintf_ln(stderr, _("Could not apply %s... %.*s"),
short_commit_name(commit), subject_len, subject);
return exit_code;
@@ -2719,7 +2721,7 @@ static int do_label(const char *name, int len)
struct object_id head_oid;
if (len == 1 && *name == '#')
- return error("illegal label name: '%.*s'", len, name);
+ return error(_("illegal label name: '%.*s'"), len, name);
strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
strbuf_addf(&msg, "rebase -i (label) '%.*s'", len, name);
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 05/23] builtin/grep.c: mark strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9774920999..58f941e951 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -489,7 +489,7 @@ static int grep_cache(struct grep_opt *opt, struct
repository *repo,
}
if (repo_read_index(repo) < 0)
- die("index file corrupt");
+ die(_("index file corrupt"));
for (nr = 0; nr < repo->index->cache_nr; nr++) {
const struct cache_entry *ce = repo->index->cache[nr];
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 09/23] config.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
config.c | 72 +++
t/t1305-config-include.sh | 2 +-
2 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/config.c b/config.c
index 6ba07989f1..58d076e833 100644
--- a/config.c
+++ b/config.c
@@ -116,12 +116,12 @@ static long config_buf_ftell(struct config_source *conf)
}
#define MAX_INCLUDE_DEPTH 10
-static const char include_depth_advice[] =
+static const char include_depth_advice[] = N_(
"exceeded maximum include depth (%d) while including\n"
" %s\n"
"from\n"
" %s\n"
-"Do you have circular includes?";
+"Do you have circular includes?");
static int handle_path_include(const char *path, struct config_include_data
*inc)
{
int ret = 0;
@@ -133,7 +133,7 @@ static int handle_path_include(const char *path, struct
config_include_data *inc
expanded = expand_user_path(path, 0);
if (!expanded)
- return error("could not expand include path '%s'", path);
+ return error(_("could not expand include path '%s'"), path);
path = expanded;
/*
@@ -144,7 +144,7 @@ static int handle_path_include(const char *path, struct
config_include_data *inc
char *slash;
if (!cf || !cf->path)
- return error("relative config includes must come from
files");
+ return error(_("relative config includes must come from
files"));
slash = find_last_dir_sep(cf->path);
if (slash)
@@ -155,7 +155,7 @@ static int handle_path_include(const char *path, struct
config_include_data *inc
if (!access_or_die(path, R_OK, 0)) {
if (++inc->depth > MAX_INCLUDE_DEPTH)
- die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
+ die(_(include_depth_advice), MAX_INCLUDE_DEPTH, path,
!cf ? "" :
cf->name ? cf->name :
"the command line");
@@ -342,13 +342,13 @@ static int git_config_parse_key_1(const char *key, char
**store_key, int *basele
if (last_dot == NULL || last_dot == key) {
if (!quiet)
- error("key does not contain a section: %s", key);
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
if (!quiet)
- error("key does not contain variable name: %s", key);
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -372,13 +372,13 @@ static int git_config_parse_key_1(const char *key, char
**store_key, int *basele
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
if (!quiet)
- error("invalid key: %s", key);
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
if (!quiet)
- error("invalid key (newline): %s", key);
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
if (store_key)
@@ -414,7 +414,7 @@ int git_config_parse_parameter(const char *text,
pair = strbuf_split_str(text, '=', 2);
if (!pair[0])
- return error("bogus config parameter: %s", text);
+ return error(_("bogus config parameter: %s"), text);
if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') {
strbuf_setlen(pair[0], pair[0]->len - 1);
@@ -426,7 +426,7 @@ int git_config_parse_parameter(const char *text,
strbuf_trim(pair[0]);
if (!pair[0]->len) {
strbuf_list_free(pair);
- return error("bogus config parameter: %s", text);
+ return error(_("bogus config parameter: %s"), text);
}
if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL)) {
@@ -461,7 +461,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
envw = xstrdup(env);
if (sq_dequote_to_argv(envw, &argv, &nr, &alloc) < 0) {
- ret = error("bogus format in %s", CONFIG_DATA_ENVIRONMENT);
+ ret = error(_("bogus format in %s"), CONFIG_DATA_ENVIRONMENT);
goto out;
}
@@ -1154,7 +1154,7 @@ static int git_default_core_config(const char *var, const
char *value)
else {
int abbrev = git_config_int(var, value);
if (abbrev < minimum_abbrev || abbrev > 40)
-
[PATCH v2 13/23] environment.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
environment.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/environment.c b/environment.c
index 2a6de2330b..d129c4adc5 100644
--- a/environment.c
+++ b/environment.c
@@ -147,7 +147,7 @@ static char *expand_namespace(const char *raw_namespace)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
if (check_refname_format(buf.buf, 0))
- die("bad git namespace path \"%s\"", raw_namespace);
+ die(_("bad git namespace path \"%s\""), raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
}
@@ -329,7 +329,7 @@ char *get_graft_file(void)
static void set_git_dir_1(const char *path)
{
if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
- die("could not set GIT_DIR to '%s'", path);
+ die(_("could not set GIT_DIR to '%s'"), path);
setup_git_env(path);
}
--
2.18.0.rc2.476.g39500d3211
[PATCH v2 21/23] sha1-file.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
sha1-file.c | 104 ++--
1 file changed, 52 insertions(+), 52 deletions(-)
diff --git a/sha1-file.c b/sha1-file.c
index ed7ac73fa9..92c27583db 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -378,8 +378,8 @@ static int alt_odb_usable(struct raw_object_store *o,
/* Detect cases where alternate disappeared */
if (!is_directory(path->buf)) {
- error("object directory %s does not exist; "
- "check .git/objects/info/alternates",
+ error(_("object directory %s does not exist; "
+ "check .git/objects/info/alternates"),
path->buf);
return 0;
}
@@ -429,7 +429,7 @@ static int link_alt_odb_entry(struct repository *r, const
char *entry,
strbuf_addstr(&pathbuf, entry);
if (strbuf_normalize_path(&pathbuf) < 0 && relative_base) {
- error("unable to normalize alternate object path: %s",
+ error(_("unable to normalize alternate object path: %s"),
pathbuf.buf);
strbuf_release(&pathbuf);
return -1;
@@ -500,14 +500,14 @@ static void link_alt_odb_entries(struct repository *r,
const char *alt,
return;
if (depth > 5) {
- error("%s: ignoring alternate object stores, nesting too deep.",
+ error(_("%s: ignoring alternate object stores, nesting too
deep"),
relative_base);
return;
}
strbuf_add_absolute_path(&objdirbuf, r->objects->objectdir);
if (strbuf_normalize_path(&objdirbuf) < 0)
- die("unable to normalize object directory: %s",
+ die(_("unable to normalize object directory: %s"),
objdirbuf.buf);
while (*alt) {
@@ -562,7 +562,7 @@ void add_to_alternates_file(const char *reference)
hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
out = fdopen_lock_file(&lock, "w");
if (!out)
- die_errno("unable to fdopen alternates lockfile");
+ die_errno(_("unable to fdopen alternates lockfile"));
in = fopen(alts, "r");
if (in) {
@@ -580,14 +580,14 @@ void add_to_alternates_file(const char *reference)
fclose(in);
}
else if (errno != ENOENT)
- die_errno("unable to read alternates file");
+ die_errno(_("unable to read alternates file"));
if (found) {
rollback_lock_file(&lock);
} else {
fprintf_or_die(out, "%s\n", reference);
if (commit_lock_file(&lock))
- die_errno("unable to move new alternates file into
place");
+ die_errno(_("unable to move new alternates file into
place"));
if (the_repository->objects->alt_odb_tail)
link_alt_odb_entries(the_repository, reference,
'\n', NULL, 0);
@@ -778,7 +778,7 @@ static void mmap_limit_check(size_t length)
limit = SIZE_MAX;
}
if (length > limit)
- die("attempting to mmap %"PRIuMAX" over limit %"PRIuMAX,
+ die(_("attempting to mmap %"PRIuMAX" over limit %"PRIuMAX),
(uintmax_t)length, (uintmax_t)limit);
}
@@ -803,7 +803,7 @@ void *xmmap(void *start, size_t length,
{
void *ret = xmmap_gently(start, length, prot, flags, fd, offset);
if (ret == MAP_FAILED)
- die_errno("mmap failed");
+ die_errno(_("mmap failed"));
return ret;
}
@@ -970,7 +970,7 @@ static void *map_sha1_file_1(struct repository *r, const
char *path,
*size = xsize_t(st.st_size);
if (!*size) {
/* mmap() is forbidden on empty files */
- error("object file %s is empty", path);
+ error(_("object file %s is empty"), path);
return NULL;
}
map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
@@ -1090,9 +1090,9 @@ static void *unpack_sha1_rest(git_zstream *stream, void
*buffer, unsigned long s
}
if (status < 0)
- error("corrupt loose object '%s'", sha1_to_hex(sha1));
+ error(_("corrupt loose object '%s'"), sha1_to_hex(sha1));
else if (stream->avail_in)
- error("garbage at end of loose object '%s'",
+ error(_("garbage at end of loose object '%s'"),
sha1_to_hex(sha1));
free(buf);
return NULL;
@@ -1134,7 +1134,7 @@ static int parse_sha1_header_extended(const char *hdr,
struct object_info *oi,
if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE
[PATCH v2 23/23] transport-helper.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
t/t5801-remote-helpers.sh | 8 ++--
transport-helper.c| 87 ---
2 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 88c7f158ef..e3bc53b0c7 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -126,7 +126,7 @@ test_expect_success 'forced push' '
test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC="" \
git clone "testgit::${PWD}/server" local2 2>error &&
- grep "this remote helper should implement refspec capability" error &&
+ test_i18ngrep "this remote helper should implement refspec capability"
error &&
compare_refs local2 HEAD server HEAD
'
@@ -134,7 +134,7 @@ test_expect_success 'pulling without refspecs' '
(cd local2 &&
git reset --hard &&
GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2>../error) &&
- grep "this remote helper should implement refspec capability" error &&
+ test_i18ngrep "this remote helper should implement refspec capability"
error &&
compare_refs local2 HEAD server HEAD
'
@@ -146,7 +146,7 @@ test_expect_success 'pushing without refspecs' '
GIT_REMOTE_TESTGIT_REFSPEC="" &&
export GIT_REMOTE_TESTGIT_REFSPEC &&
test_must_fail git push 2>../error) &&
- grep "remote-helper doesn.t support push; refspec needed" error
+ test_i18ngrep "remote-helper doesn.t support push; refspec needed" error
'
test_expect_success 'pulling without marks' '
@@ -246,7 +246,7 @@ test_expect_success 'proper failure checks for fetching' '
(cd local &&
test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2>error &&
cat error &&
- grep -q "error while running fast-import" error
+ test_i18ngrep -q "error while running fast-import" error
)
'
diff --git a/transport-helper.c b/transport-helper.c
index 9f487cc905..84a10661cc 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -48,7 +48,7 @@ static void sendline(struct helper_data *helper, struct
strbuf *buffer)
if (debug)
fprintf(stderr, "Debug: Remote helper: -> %s", buffer->buf);
if (write_in_full(helper->helper->in, buffer->buf, buffer->len) < 0)
- die_errno("full write to remote helper failed");
+ die_errno(_("full write to remote helper failed"));
}
static int recvline_fh(FILE *helper, struct strbuf *buffer)
@@ -77,7 +77,7 @@ static void write_constant(int fd, const char *str)
if (debug)
fprintf(stderr, "Debug: Remote helper: -> %s", str);
if (write_in_full(fd, str, strlen(str)) < 0)
- die_errno("full write to remote helper failed");
+ die_errno(_("full write to remote helper failed"));
}
static const char *remove_ext_force(const char *url)
@@ -129,7 +129,7 @@ static struct child_process *get_helper(struct transport
*transport)
code = start_command(helper);
if (code < 0 && errno == ENOENT)
- die("unable to find remote helper for '%s'", data->name);
+ die(_("unable to find remote helper for '%s'"), data->name);
else if (code != 0)
exit(code);
@@ -145,7 +145,7 @@ static struct child_process *get_helper(struct transport
*transport)
*/
duped = dup(helper->out);
if (duped < 0)
- die_errno("can't dup helper output fd");
+ die_errno(_("can't dup helper output fd"));
data->out = xfdopen(duped, "r");
write_constant(helper->in, "capabilities\n");
@@ -196,13 +196,13 @@ static struct child_process *get_helper(struct transport
*transport)
} else if (starts_with(capname, "no-private-update")) {
data->no_private_update = 1;
} else if (mandatory) {
- die("unknown mandatory capability %s; this remote "
- "helper probably needs newer version of Git",
+ die(_("unknown mandatory capability %s; this remote "
+ "helper probably needs newer version of Git"),
capname);
}
}
if (!data->rs.nr && (data->import || data->bidi_import ||
data->export)) {
- warning("this remote helper should implement refspec
capability");
+ warning(_("this remote helper should implement refspec
capability"));
}
strbuf_release(&buf);
if (debug)
@@ -269,7 +269,7 @@ static int strbuf_set_helper_option(struct helper_data
*data,
else if (!strcmp(buf->buf, "unsupported"))
ret = 1;
else {
- warning("%s unexpectedly said: '%s'", data->name, buf->buf);
+ warning(_("%s unexpectedly said: '%s'"), data->name, buf->buf);
ret = 1;
}
[PATCH v2 22/23] transport.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
transport.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/transport.c b/transport.c
index 9fcc33915a..516a83b7f6 100644
--- a/transport.c
+++ b/transport.c
@@ -139,7 +139,7 @@ static struct ref *get_refs_from_bundle(struct transport
*transport,
close(data->fd);
data->fd = read_bundle_header(transport->url, &data->header);
if (data->fd < 0)
- die("could not read bundle '%s'", transport->url);
+ die(_("could not read bundle '%s'"), transport->url);
for (i = 0; i < data->header.references.nr; i++) {
struct ref_list_entry *e = data->header.references.list + i;
struct ref *ref = alloc_ref(e->name);
@@ -654,7 +654,7 @@ static int git_transport_push(struct transport *transport,
struct ref *remote_re
switch (data->version) {
case protocol_v2:
- die("support for protocol v2 not implemented yet");
+ die(_("support for protocol v2 not implemented yet"));
break;
case protocol_v1:
case protocol_v0:
@@ -780,7 +780,7 @@ static enum protocol_allow_config
parse_protocol_config(const char *key,
else if (!strcasecmp(value, "user"))
return PROTOCOL_ALLOW_USER_ONLY;
- die("unknown value for config '%s': %s", key, value);
+ die(_("unknown value for config '%s': %s"), key, value);
}
static enum protocol_allow_config get_protocol_config(const char *type)
@@ -846,7 +846,7 @@ int is_transport_allowed(const char *type, int from_user)
void transport_check_allowed(const char *type)
{
if (!is_transport_allowed(type, -1))
- die("transport '%s' not allowed", type);
+ die(_("transport '%s' not allowed"), type);
}
static struct transport_vtable bundle_vtable = {
@@ -898,7 +898,7 @@ struct transport *transport_get(struct remote *remote,
const char *url)
if (helper) {
transport_helper_init(ret, helper);
} else if (starts_with(url, "rsync:")) {
- die("git-over-rsync is no longer supported");
+ die(_("git-over-rsync is no longer supported"));
} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url,
1)) {
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
transport_check_allowed("file");
@@ -1143,7 +1143,7 @@ int transport_push(struct transport *transport,
transport->push_options,
pretend)) {
oid_array_clear(&commits);
- die("failed to push all needed submodules!");
+ die(_("failed to push all needed submodules"));
}
oid_array_clear(&commits);
}
@@ -1265,7 +1265,7 @@ int transport_connect(struct transport *transport, const
char *name,
if (transport->vtable->connect)
return transport->vtable->connect(transport, name, exec, fd);
else
- die("operation not supported by protocol");
+ die(_("operation not supported by protocol"));
}
int transport_disconnect(struct transport *transport)
@@ -1347,7 +1347,7 @@ static void read_alternate_refs(const char *path,
if (get_oid_hex(line.buf, &oid) ||
line.buf[GIT_SHA1_HEXSZ] != ' ') {
- warning("invalid line while parsing alternate refs: %s",
+ warning(_("invalid line while parsing alternate refs:
%s"),
line.buf);
break;
}
--
2.18.0.rc2.476.g39500d3211
Re: [PATCH 08/16] test-reach: create new test tool for ref_newer
On 7/16/2018 7:00 PM, Jonathan Tan wrote:
To use the new test-tool, use 'test-tool reach ' and provide
input to stdin that describes the inputs to the method. Currently, we
only implement the ref_newer method, which requires two commits. Use
lines "A:" and "B:" for the two inputs. We will
expand this input later to accommodate methods that take lists of
commits.
It would be nice if "A" and "B" were "ancestor" and "descendant" (or
something like that) instead, so that I don't have to check which
direction the reach is calculated in.
Different methods will use different combinations. I do notice that I
forgot to list the method parameters as part of the test-tool output. It
should print "ref_newer(A,B)" so you know which input goes in which place.
+int cmd__reach(int ac, const char **av)
+{
+ struct object_id oid_A, oid_B;
+ struct strbuf buf = STRBUF_INIT;
+ struct repository *r = the_repository;
+
+ setup_git_directory();
+
+ if (ac < 2)
+ exit(1);
+
+
+ while (strbuf_getline(&buf, stdin) != EOF) {
+ struct object_id oid;
+ struct object *o;
+ struct commit *c;
+ if (buf.len < 3)
+ continue;
+
+ if (get_oid_committish(buf.buf + 2, &oid))
+ die("failed to resolve %s", buf.buf + 2);
You can also use skip_prefix() instead of using arithmetic to determine
the start of the OID.
+# Construct a grid-like commit graph with points (x,y)
+# with 1 <= x <= 10, 1 <= y <= 10, where (x,y) has
+# parents (x-1, y) and (x, y-1), keeping in mind that
+# we drop a parent if a coordinate is nonpositive.
+#
+# (10,10)
+#/ \
+# (10,9)(9,10)
+#/ \ / \
+#(10,8)(9,9) (8,10)
+# / \/ \ /\
+# ( continued...)
+# \ /\ / \/
+#(3,1) (2,2) (1,3)
+#\ /\ /
+# (2,1) (2,1)
+# \/
+# (1,1)
This is quite a good design, thanks.
+# We use branch 'comit-x-y' to refer to (x,y).
s/comit/commit/
+ git show-ref -s commit-7-7 | git commit-graph write --stdin-commits &&
+ mv .git/objects/info/commit-graph commit-graph-half &&
My understanding is that this writes for 7-7 and all its ancestors,
but...
+test_expect_success 'ref_newer:hit' '
+ cat >input <<-\EOF &&
+ A:commit-5-7
+ B:commit-2-3
+ EOF
+ printf "ref_newer:1\n" >expect &&
+ test_three_modes ref_newer
+'
+
+test_done
...both 5-7 and 2-3 are ancestors of 7-7, right? Which means that you
don't test the "half" commit graph here. (It's probably sufficient to
just adjust the numbers.)
Good point! Thanks. I'll just write the commit-graph starting at
commit-5-5 and that should satisfy the point of the "mixed-mode" tests.
-Stolee
Re: [PATCH 3/3] gc: do not return error for prior errors in daemonized mode
Jeff King writes: >> There's nothing for the calling program to act on on the basis of that >> error. Use status 0 consistently instead, to indicate that we decided >> not to run a gc (just like if no housekeeping was required). This >> way, repo and similar tools can get the benefit of the same behavior >> as tools like "git fetch" that ignore the exit status from gc --auto. > > I think this is a good change. > > In theory it might be useful to propagate the original exit code (_not_ > "did we see a warning or an error", but the true original exit code. But > as you note, it's not deterministic anyway (we'd miss that exit code on > the first run, or even any simultaneous runs that exit early due to lock > contention). So it's clear that callers can't really do anything robust > based on the exit code of a daemonized "gc --auto". > > I still think that "repo" should probably stop respecting the exit code. > But that's no excuse for Git not to have a sensible exit code in the > first place. I am not yet convinced that this last step to exit with 0 is a good change, even though I can understand that it would be more convenient, as there currently is no easy way for the calling script to tell two error cases apart. I think the "sensible exit code" you mention would be something like "1 for hard error, 2 for 'I am punting as I see there were previous errors---you may want to examine your repository'". If we did that from day one and documented that behaviour, nobody would have complained, but adopting that suddenly is of course a breaking change. Perhaps we should exit with 2 (not 0) in that "previous error" case by default, and then have a configuration knob to turn that 2 into 0 for those who cannot easily modify the calling script? That way, we by default will *not* break those who have been paying attention to zero-ness of the exit status, we allow those who want to treat this "prior error" case as if there were no error with just a knob, and then those who are willing to update their script can tell two cases by the exit status and act differently.
Re: [PATCH 13/16] test-reach: test can_all_from_reach_with_flags
On 7/16/2018 5:54 PM, Stefan Beller wrote: On Mon, Jul 16, 2018 at 6:00 AM Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The can_all_from_reach_with_flags method is used by ok_to_give_up in upload-pack.c to see if we have done enough negotiation during a fetch. This method is intentionally created to preserve state between calls to assist with stateful negotiation, such as over SSH. To make this method testable, add a new can_all_from_reach method that does the initial setup and final tear-down. Call the method from 'test-tool reach'. Since this is a many-to-many reachability query, add a new type of input to the 'test-tool reach' input format. Lines "Y:" create a list of commits to be the reachability targets from the commits in the 'X' list. In the context of fetch negotiation, the 'X' commits are the 'want' commits and the 'Y' commits are the 'have' commits. Makes sense. I shortly wondered if we want to s/Y/Z/ as I find X and Z more distinguishable than X/Y for reading/skimming. Thanks, Stefan +++ b/commit-reach.c @@ -593,3 +593,50 @@ int can_all_from_reach_with_flag(struct object_array *from, } return 1; } + +int can_all_from_reach(struct commit_list *from, struct commit_list *to, + int cutoff_by_min_date) We'll put this method (that is only used by tests so far) here to not clutter the test tool code too much, or do we see more benefits from the code here? If so, docs would be nice. We will use it later as we reduce duplicate walk implementations, but I can hint at that in the message. +++ b/t/t6600-test-reach.sh +test_expect_success 'can_all_from_reach:hit' ' [...] + Y:commit-7-3 + Y:commit-8-1 +test_expect_success 'can_all_from_reach:miss' ' [...] + Y:commit-8-5 It would be nice if the difference in the list could be easier to spot as a reader. (There is a lot of repetition). Maybe we can teach "test-tool reach" to ignore input lines starting with '#' such that we can annotate the last line in the miss case? Why do we omit 7-3 in the miss case? (might be nice for symmetry to keep around) The X-commit that fails to reach a Y-commit in this second case is commit-8-3. That commit can reach both commit-7-3 and commit-8-1, so both need to be removed. Thanks, -Stolee
Re: [RFC] push: add documentation on push v2
On Wed, Jul 18, 2018 at 6:31 AM Derrick Stolee wrote: > > On 7/17/2018 7:25 PM, Stefan Beller wrote: > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote: > >> Signed-off-by: Brandon Williams > >> --- > >> > >> Since introducing protocol v2 and enabling fetch I've been thinking > >> about what its inverse 'push' would look like. After talking with a > >> number of people I have a longish list of things that could be done to > >> improve push and I think I've been able to distill the core features we > >> want in push v2. > > It would be nice to know which things you want to improve. > > Hopefully we can also get others to chime in with things they don't like > about the existing protocol. What pain points exist, and what can we do > to improve at the transport layer before considering new functionality? Another thing that I realized last night was the possibility to chunk requests. The web of today is driven by lots of small http(s) requests. I know our server team fights with the internal tools all the time because the communication involved in git-fetch is usually a large http request (large packfile). So it would be nice to have the possibility of chunking the request. But I think that can be added as a capability? (Not sure how) > >> Thankfully (due to the capability system) most of the > >> other features/improvements can be added later with ease. > >> > >> What I've got now is a rough design for a more flexible push, more > >> flexible because it allows for the server to do what it wants with the > >> refs that are pushed and has the ability to communicate back what was > >> done to the client. The main motivation for this is to work around > >> issues when working with Gerrit and other code-review systems where you > >> need to have Change-Ids in the commit messages (now the server can just > >> insert them for you and send back new commits) and you need to push to > >> magic refs to get around various limitations (now a Gerrit server should > >> be able to communicate that pushing to 'master' doesn't update master > >> but instead creates a refs/changes/ ref). > > Well Gerrit is our main motivation, but this allows for other workflows as > > well. > > For example Facebook uses hg internally and they have a > > "rebase-on-the-server-after-push" workflow IIRC as pushing to a single repo > > brings up quite some contention. The protocol outlined below would allow > > for such a workflow as well? (This might be an easier sell to the Git > > community as most are not quite familiar with Gerrit) > > I'm also curious how this "change commits on push" would be helpful to > other scenarios. > > Since I'm not familiar with Gerrit: what is preventing you from having a > commit hook that inserts (or requests) a Change-Id when not present? That is how you do it normally. But what if you just get started or want to send a one-off to the server (I wanted to upload a git patch to our internal Gerrit once, and as my repository is configured to work with upstream Git which doesn't carry change ids, I ran into this problem. I had to manually add it to have the server accept it) > How > can the server identify the Change-Id automatically when it isn't present? The change id is just a randomly assigned id, which can be made up, but should stay consistent in further revisions. (Put another way: change ids solve the 'linear assignment problem' of range-diff at scale) So once the protocol support is in, the client would need to get some UX update to replace its commits just pushed with the answer from the server to work well with server side generated change ids. But as said I am not sure how much we want to discuss in that direction, but rather see if we could have other use cases: Instead of just rebasing to solve the contention problem server side, we could also offer a "coding helper as a service" - server. That would work similar as the change id workflow lines out above: You push to the server, the server performs some action (style formatting your code for example, linting) and you download it back and have it locallly again. I think that would be pretty cool actually. Thanks, Stefan
Re: [PATCH 00/16] Consolidate reachability logic
Duy Nguyen writes: >> In other words: if you see something that you don't like about >> GitGitGadget, get your butts off the ground and contribute a fix. > > Thank you for the frank words. I will choose to not review any mails > coming from GitGitGadget. I wouldn't say I will choose not to, but certainly I noticed that I'd backburner reading a series that are way out of order in my mailbox, no matter who authored them or how they were sent out, as they consume way more concentration-point out of my mind than they are often worth X-<. While there are easier-to-read and more nicely organized patch series, I'd deal with them first, consciously or not. No, fixing a tool that throws such a harder-to-read patch series in reader's mailbox is *not* something I'd spend my primary focus on, especially when many contributors are perfectly capable of sending reasonably formatted series without using such a tool under development. That won't stop those who want to improve the tool. But I'd wish those who want to make Git better spend their time on making Git, over making GitGitGadget, better.
Re: [PATCH v4 7/7] gpg-interface t: extend the existing GPG tests with GPGSM
Henning Schild writes: >> > + unset GIT_COMMITTER_EMAIL && >> > + git config user.email hasno...@nowhere.com && >> > + git config user.signingkey "" && >> > + test_must_fail git push --signed dst noop ff +noff && >> >> This is OK for a test that is known to be always at the end, but >> also forbids others to further update this script to add more tests >> at the end, as the standard setting of environment is blown away >> (the config is probably OK, but test_config to arrange them to be >> cleaned up would have been nicer), which is not very nice. I think >> it should be easy to fix it when it becomes necessary, but at the >> same time if it is easy to fix, then probably we shouldn't introduce >> a breakage in the first place, so I am on the fence. > > Switched to test_config, this is all coming from copying the previous > tests, which i left as is. I was specifically talking about the unsetting of GIT_COMMITTER_EMAIL, not the configuration variables. An easy way to try one step without the environment without affecting later tests would be to run the above in a subshell, perhaps like test_config user.email ... && test_config user.signingkey "" && ( safe_unset GIT_COMMITTER_EMAIL && test_must_fail git push --signed ... ) &&
Re: [PATCH 2/2] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
Hi Derrick, > Overall, I think this is the right approach. The only problem is that > you're missing a few 'the_repository' to 'r' replacements in the bodies > of show_reference and register_replace_ref. Thanks. Originally I had another approach, which was to convert all callbacks to take a repository argument, and only if the refs backend learned to propagate the correct repository the repo argument would be the specific repo, NULL otherwise. With that approach it was not possible to replace all occurrences of the_repository with 'r', and it would also be confusing. But as I pursued that approach at the time of writing this patch... I missed the conversions needed. Thanks for catching them (and fixing them in the reroll that you just sent out via gitgitgadget)! Thanks, Stefan
Re: [RFC] push: add documentation on push v2
On 07/17, Stefan Beller wrote:
> On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote:
> >
> > Signed-off-by: Brandon Williams
> > ---
> >
> > Since introducing protocol v2 and enabling fetch I've been thinking
> > about what its inverse 'push' would look like. After talking with a
> > number of people I have a longish list of things that could be done to
> > improve push and I think I've been able to distill the core features we
> > want in push v2.
>
> It would be nice to know which things you want to improve.
I mean this tackles the main point I want to improve. Others include:
rebase/merge-on-push, push symrefs, and then the inclusion of other
capabilities from v0 like atomic push, signed push, etc.
>
> > Thankfully (due to the capability system) most of the
> > other features/improvements can be added later with ease.
> >
> > What I've got now is a rough design for a more flexible push, more
> > flexible because it allows for the server to do what it wants with the
> > refs that are pushed and has the ability to communicate back what was
> > done to the client. The main motivation for this is to work around
> > issues when working with Gerrit and other code-review systems where you
> > need to have Change-Ids in the commit messages (now the server can just
> > insert them for you and send back new commits) and you need to push to
> > magic refs to get around various limitations (now a Gerrit server should
> > be able to communicate that pushing to 'master' doesn't update master
> > but instead creates a refs/changes/ ref).
>
> Well Gerrit is our main motivation, but this allows for other workflows as
> well.
> For example Facebook uses hg internally and they have a
> "rebase-on-the-server-after-push" workflow IIRC as pushing to a single repo
> brings up quite some contention. The protocol outlined below would allow
> for such a workflow as well? (This might be an easier sell to the Git
> community as most are not quite familiar with Gerrit)
Yes the idea would be such that we could easily add a "rebase" or
"merge" verb later for explicit user controlled workflows like that.
This proposal would already make it possible (although the server would
need to be configured as such) since the server can do what it wants
with the updates a client sends to it.
>
> > Before actually moving to write any code I'm hoping to get some feedback
> > on if we think this is an acceptable base design for push (other
> > features like atomic-push, signed-push, etc can be added as
> > capabilities), so any comments are appreciated.
> >
> > Documentation/technical/protocol-v2.txt | 76 +
> > 1 file changed, 76 insertions(+)
> >
> > diff --git a/Documentation/technical/protocol-v2.txt
> > b/Documentation/technical/protocol-v2.txt
> > index 49bda76d23..16c1ce60dd 100644
> > --- a/Documentation/technical/protocol-v2.txt
> > +++ b/Documentation/technical/protocol-v2.txt
> > @@ -403,6 +403,82 @@ header.
> > 2 - progress messages
> > 3 - fatal error message just before stream aborts
> >
> > + push
> > +~~
> > +
> > +`push` is the command used to push ref-updates and a packfile to a remote
> > +server in v2.
> > +
> > +Additional features not supported in the base command will be advertised
> > +as the value of the command in the capability advertisement in the form
> > +of a space separated list of features: "= "
> > +
> > +The format of a push request is as follows:
> > +
> > +request = *section
> > +section = (ref-updates | packfile)
>
> This reads as if a request consists of sections, which
> each can be a "ref-updates" or a packfile, no order given,
> such that multiple ref-update sections mixed with packfiles
> are possible.
>
> I would assume we'd only want to allow for ref-updates
> followed by the packfile.
>
> Given the example above for "rebase-on-push" though
> it is better to first send the packfile (as that is assumed to
> take longer) and then send the ref updates, such that the
> rebasing could be faster and has no bottleneck.
I don't really follow this logic. I don't think it would change
anything much considering the ref-updates section would usually be
much smaller than the packfile itself, course I don't have any data so
idk.
>
> > + (delim-pkt | flush-pkt)
>
>
>
> > +
> > +ref-updates = PKT-LINE("ref-updates" LF)
> > + *PKT-Line(update/force-update LF)
> > +
> > +update = txn_id SP action SP refname SP old_oid SP new_oid
> > +force-update = txn_id SP "force" SP action SP refname SP new_oid
>
> So we insert "force" after the transaction id if we want to force it.
> When adding the atomic capability later we could imagine another insert here
>
> 1 atomic create refs/heads/new-ref <0-hash>
> 1 atomic delete refs/heads/old-ref <0-hash>
>
> which would look like a "rename" that we could also add instead.
> The transaction numbers are an interesting concept, how do you
> en
Re: [RFC] push: add documentation on push v2
On 07/18, Derrick Stolee wrote: > On 7/17/2018 7:25 PM, Stefan Beller wrote: > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams wrote: > > > Signed-off-by: Brandon Williams > > > --- > > > > > > Since introducing protocol v2 and enabling fetch I've been thinking > > > about what its inverse 'push' would look like. After talking with a > > > number of people I have a longish list of things that could be done to > > > improve push and I think I've been able to distill the core features we > > > want in push v2. > > It would be nice to know which things you want to improve. > > Hopefully we can also get others to chime in with things they don't like > about the existing protocol. What pain points exist, and what can we do to > improve at the transport layer before considering new functionality? > > > > Thankfully (due to the capability system) most of the > > > other features/improvements can be added later with ease. > > > > > > What I've got now is a rough design for a more flexible push, more > > > flexible because it allows for the server to do what it wants with the > > > refs that are pushed and has the ability to communicate back what was > > > done to the client. The main motivation for this is to work around > > > issues when working with Gerrit and other code-review systems where you > > > need to have Change-Ids in the commit messages (now the server can just > > > insert them for you and send back new commits) and you need to push to > > > magic refs to get around various limitations (now a Gerrit server should > > > be able to communicate that pushing to 'master' doesn't update master > > > but instead creates a refs/changes/ ref). > > Well Gerrit is our main motivation, but this allows for other workflows as > > well. > > For example Facebook uses hg internally and they have a > > "rebase-on-the-server-after-push" workflow IIRC as pushing to a single repo > > brings up quite some contention. The protocol outlined below would allow > > for such a workflow as well? (This might be an easier sell to the Git > > community as most are not quite familiar with Gerrit) > > I'm also curious how this "change commits on push" would be helpful to other > scenarios. > > Since I'm not familiar with Gerrit: what is preventing you from having a > commit hook that inserts (or requests) a Change-Id when not present? How can > the server identify the Change-Id automatically when it isn't present? Right now all Gerrit users have a commit hook installed which inserts the Change-Id. The issue is that if you push to gerrit and you don't have Change-ids, the push fails and you're prompted to blindly run a command to install the commit-hook. So if we could just have the server handle this completely then the users of gerrit wouldn't ever need to have a hook installed in the first place. -- Brandon Williams
Re: [PATCH 00/16] Consolidate reachability logic
On 7/18/2018 1:01 PM, Junio C Hamano wrote: No, fixing a tool that throws such a harder-to-read patch series in reader's mailbox is *not* something I'd spend my primary focus on, especially when many contributors are perfectly capable of sending reasonably formatted series without using such a tool under development. That won't stop those who want to improve the tool. But I'd wish those who want to make Git better spend their time on making Git, over making GitGitGadget, better. I appreciate the feedback in how this series caused reviewer pain. Hopefully this date issue is now resolved. Any further feedback is welcome. I'm choosing to use and contribute to GitGitGadget not because I'm incapable of sending series myself, but because I _have_ had difficulty. Using the email submissions creates a friction that I'm willing to overcome, but we are probably missing out on contributors who are not willing to push through that friction. Perhaps having another way for new contributors to feel welcome is an indirect way to make Git better. Thanks, -Stolee
Re: Receiving console output from GIT 10mins after abort/termination?
On Tue, Jul 17, 2018 at 11:38 PM Frank Wolf wrote: > > Hi @ll, > > I hope I'm posting to the right group (not sure if it's Windows related) but > I've got > a weird problem using GIT: > > By accident I've tried to push a repository (containing an already > commited but not yet pushed submodule reference). > This fails immediately with an error of course BUT > > after 10 mins I get an output on the console though the command exited!? > (... $Received disconnect from : User session has timed out > idling after 600 ms) This sounds like it is better suited for https://github.com/git-for-windows/git/issues/new as the error message suggests it comes from some software bundled with GfW to make Git work on Windows. (Gits source code doesn't contain such an error message)
Re: [RFC] push: add documentation on push v2
On 07/18, Stefan Beller wrote: > On Wed, Jul 18, 2018 at 6:31 AM Derrick Stolee wrote: > > > > On 7/17/2018 7:25 PM, Stefan Beller wrote: > > > On Tue, Jul 17, 2018 at 2:09 PM Brandon Williams > > > wrote: > > >> Signed-off-by: Brandon Williams > > >> --- > > >> > > >> Since introducing protocol v2 and enabling fetch I've been thinking > > >> about what its inverse 'push' would look like. After talking with a > > >> number of people I have a longish list of things that could be done to > > >> improve push and I think I've been able to distill the core features we > > >> want in push v2. > > > It would be nice to know which things you want to improve. > > > > Hopefully we can also get others to chime in with things they don't like > > about the existing protocol. What pain points exist, and what can we do > > to improve at the transport layer before considering new functionality? > > Another thing that I realized last night was the possibility to chunk > requests. > The web of today is driven by lots of small http(s) requests. I know our > server > team fights with the internal tools all the time because the communication > involved in git-fetch is usually a large http request (large packfile). > So it would be nice to have the possibility of chunking the request. > But I think that can be added as a capability? (Not sure how) Fetch and push requests/responses are already "chunked" when using the http transport. So I'm not sure what you mean by adding a capability because the protocol doesn't care about which transport you're using. This is of course unless you're talking about a different "chunking" from what it means to chunk an http request/response. -- Brandon Williams
Re: [PATCH] sequencer.c: terminate the last line of author-script properly
Phillip Wood writes: >> (I think we had code to do so in "git am" >> that was rewritten in C first). > > The code in builtin/am.c doesn't try to write valid posix shell (if > one assumes it is the only consumer of the author script then it > doesn't need to) which results in simpler code, but external scripts > cannot safely eval it anymore. Are you sure about that? If so we probably should see if we can fix the writer, and better yet, if we can share code with the writer discussed here, as presumably we are fixing it in this thread. But I do not see how builtin/am.c::write_author_script() would produce something that would not eval correctly. sq_quote_buf() was introduced specifically to write correct string for shell's consumption.
Re: [RFC] push: add documentation on push v2
On Wed, Jul 18, 2018 at 7:13 PM Brandon Williams wrote: > > > > What I've got now is a rough design for a more flexible push, more > > > > flexible because it allows for the server to do what it wants with the > > > > refs that are pushed and has the ability to communicate back what was > > > > done to the client. The main motivation for this is to work around > > > > issues when working with Gerrit and other code-review systems where you > > > > need to have Change-Ids in the commit messages (now the server can just > > > > insert them for you and send back new commits) and you need to push to > > > > magic refs to get around various limitations (now a Gerrit server should > > > > be able to communicate that pushing to 'master' doesn't update master > > > > but instead creates a refs/changes/ ref). > > > Well Gerrit is our main motivation, but this allows for other workflows > > > as well. > > > For example Facebook uses hg internally and they have a > > > "rebase-on-the-server-after-push" workflow IIRC as pushing to a single > > > repo > > > brings up quite some contention. The protocol outlined below would allow > > > for such a workflow as well? (This might be an easier sell to the Git > > > community as most are not quite familiar with Gerrit) > > > > I'm also curious how this "change commits on push" would be helpful to other > > scenarios. > > > > Since I'm not familiar with Gerrit: what is preventing you from having a > > commit hook that inserts (or requests) a Change-Id when not present? How can > > the server identify the Change-Id automatically when it isn't present? > > Right now all Gerrit users have a commit hook installed which inserts > the Change-Id. The issue is that if you push to gerrit and you don't > have Change-ids, the push fails and you're prompted to blindly run a > command to install the commit-hook. So if we could just have the server > handle this completely then the users of gerrit wouldn't ever need to > have a hook installed in the first place. I don't trust the server side to rewrite commits for me. And this is basically rewriting history (e.g. I can push multiple commits to gerrit if I remember correctly; if they all don't have change-id, then the history must be rewritten for change-id to be inserted). Don't we already have "plans" to push config from server to client? There's also talk about configuring hooks with config file. These should make it possible to deal with change-id generation with minimum manual intervention. -- Duy
Re: [PATCH 3/3] gc: do not return error for prior errors in daemonized mode
On Wed, Jul 18, 2018 at 09:21:18AM -0700, Junio C Hamano wrote: > > I still think that "repo" should probably stop respecting the exit code. > > But that's no excuse for Git not to have a sensible exit code in the > > first place. > > I am not yet convinced that this last step to exit with 0 is a good > change, even though I can understand that it would be more > convenient, as there currently is no easy way for the calling script > to tell two error cases apart. > > I think the "sensible exit code" you mention would be something like > "1 for hard error, 2 for 'I am punting as I see there were previous > errors---you may want to examine your repository'". > > If we did that from day one and documented that behaviour, nobody > would have complained, but adopting that suddenly is of course a > breaking change. > > Perhaps we should exit with 2 (not 0) in that "previous error" case > by default, and then have a configuration knob to turn that 2 into 0 > for those who cannot easily modify the calling script? That way, we > by default will *not* break those who have been paying attention to > zero-ness of the exit status, we allow those who want to treat this > "prior error" case as if there were no error with just a knob, and > then those who are willing to update their script can tell two cases > by the exit status and act differently. I think we have been exiting non-zero with "previous errors" for some time with the daemonizing code. It was just spelled "-1" instead of "2". So just jumping right there does not mean any regression from the current state, I don't think (but it also does not fix existing scripts like "repo" that check the code). I agree the config you suggest would give people the tools to make that case work. But it somehow rubs me the wrong way. Can you imagine the perspective of a user who is told "oh, your script breaks? Just try setting this option to ignore error codes in this one particular situation". It feels like a weird hack, because it is. It's also still inconsistent in the daemonize case. The run that yields the error won't return a non-zero exit. But the next run will exit with "2". -Peff
Re: [PATCH] sequencer.c: terminate the last line of author-scriptproperly
Phillip Wood writes:
>> The only consumer of a faulty author script written by the sequencer
>> is read_env_script() in sequencer.c which doesn't worry about
>> checking that quotes are paired.
>
> That's not quite true anymore, recently another consumer
> read_author_ident() was added which uses sq_dequote() instead of
> custom code. Looking more closely at write_author_script() the quoting
> of single quotes is buggy they are escaped as \\' instead of \'.
That's embarrassing re-invention (instead of reuse) with additional
bug X-<. It seems that all of that blame to d87d48b2 ("sequencer:
learn about the special "fake root commit" handling", 2018-05-04).
We should fix both broken writer and readers that compensate for
breakage in the writer, I guess. Sigh...
Re: [PATCH] gc: do not warn about too many loose objects
On Wed, Jul 18, 2018 at 03:11:38PM +0200, Ævar Arnfjörð Bjarmason wrote: > > Yeah, I agree that deferring repeated gc's based on time is going to run > > into pathological corner cases. OTOH, what we've found at GitHub is that > > "gc --auto" is quite insufficient for scheduling maintenance anyway > > (e.g., if a machine gets pushes to 100 separate repositories in quick > > succession, you probably want to queue and throttle any associated gc). > > I'm sure you have better solutions for this at GitHub, but as an aside > it might be interesting to add some sort of gc flock + retry setting for > this use-case, i.e. even if you had 100 concurrent gc's due to > too_many_*(), they'd wait + retry until they could get the flock on a > given file. > > Then again this is probably too specific, and could be done with a > pre-auto-gc hook too.. Yeah, I think any multi-repo solution is getting way too specific for Git, and the best thing we can do is provide a hook. I agree you could probably do this today with a pre-auto-gc hook (if it skips gc it would just queue itself and return non-zero). Or even just make a mark in a database that says "there was some activity here". Since we have so much other infrastructure sitting between the user and Git anyway, we do that marking at a separate layer which is already talking to a database. ;) Anyway, I do agree with your general notion that this isn't the right approach for many situations, and auto-gc is a better solution for many cases. -Peff
Re: [PATCH 2/2] repack -ad: prune the list of shallow commits
On Tue, Jul 17, 2018 at 9:41 PM Jeff King wrote: > I slept on this to see if I could brainstorm any other ways. > > .. > > Sort of an aside to the patch under discussion, but I think it may make > sense for prune_shallow() to take a callback function for determining > whether a commit is reachable. > > I have an old patch that teaches git-prune to lazily do the reachability > check, since in many cases "git repack" will have just packed all of the > loose objects. But it just occurred to me that this patch is totally > broken with respect to prune_shallow(), because it would not set the > SEEN flag (I've literally been running with it for years, which goes to > show how often I use the shallow feature). > > And if we were to have repack do a prune_shallow(), it may want to use a > different method than traversing and marking each object SEEN. All of this sounds good. The only thing I'd like to do a bit differently is to pass the reachable commits in prune_shallow() as a commit-slab instead of taking a callback function. I'll refactor this code, move prune_shallow() to a separate command prune-shallow and do the locking thing. Johannes I could take over this bug too (since I introduced the shallow pruning and missed this part) if you are busy, but if you want it to be done fast, I can't help. I still have a list of things to go through. -- Duy
[PATCH] Documentation: fix --color option formatting
Add missing colon in two places to fix formatting of options.
Signed-off-by: Andrei Rybak
---
Done on top of maint.
The earliest this patch applies is on top of commit aebd23506e ("Merge
branch 'jk/ui-color-always-to-auto-maint' into
jk/ui-color-always-to-auto", 2017-10-04), one commit away from the
commit that added both of the affected lines: 0c88bf5050 (provide
--color option for all ref-filter users, 2017-10-03).
I grepped the Documentation folder, and haven't found any other
similar typos.
---
Documentation/git-for-each-ref.txt | 2 +-
Documentation/git-tag.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 085d177d97..901faef1bf 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -57,7 +57,7 @@ OPTIONS
`xx`; for example `%00` interpolates to `\0` (NUL),
`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
---color[=]:
+--color[=]::
Respect any colors specified in the `--format` option. The
`` field must be one of `always`, `never`, or `auto` (if
`` is absent, behave as if `always` was given).
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 87c4288ffc..92f9c12b87 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -115,7 +115,7 @@ options for details.
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
---color[=]:
+--color[=]::
Respect any colors specified in the `--format` option. The
`` field must be one of `always`, `never`, or `auto` (if
`` is absent, behave as if `always` was given).
--
2.18.0
Re: [PATCH] diff.c: offer config option to control ws handling in move detection
Stefan Beller writes:
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 143acd9417e..8da7fed4e22 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -294,8 +294,11 @@ dimmed_zebra::
>
> --color-moved-ws=::
> This configures how white spaces are ignored when performing the
> - move detection for `--color-moved`. These modes can be given
> - as a comma separated list:
> + move detection for `--color-moved`.
> +ifdef::git-diff[]
> + It can be set by the `diff.colorMovedWS` configuration setting.
> +endif::git-diff[]
The patch to diff.c::git_diff_ui_config() we see below does not seem
to make any effort to make sure that this new configuration applies
only to "git diff" and that other commands like "git log" that call
git_diff_ui_config() are not affected.
And I do not see a strong reason why "git log --color-moved" should
not honor this setting, either, so I am not quite sure why we want
this ifdef/endif pair to hide it from "git log --help".
Or am I totally misunderstanding the reason why we want ifdef/endif
here?
Puzzled...
> diff --git a/diff.c b/diff.c
> index f51f0ac32f4..9de917108d8 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -35,6 +35,7 @@ static int diff_rename_limit_default = 400;
> static int diff_suppress_blank_empty;
> static int diff_use_color_default = -1;
> static int diff_color_moved_default;
> +static int diff_color_moved_ws_default;
> static int diff_context_default = 3;
> static int diff_interhunk_context_default;
> static const char *diff_word_regex_cfg;
> @@ -332,6 +333,13 @@ int git_diff_ui_config(const char *var, const char
> *value, void *cb)
> diff_color_moved_default = cm;
> return 0;
> }
> + if (!strcmp(var, "diff.colormovedws")) {
> + int cm = parse_color_moved_ws(value);
> + if (cm < 0)
> + return -1;
> + diff_color_moved_ws_default = cm;
> + return 0;
> + }
> if (!strcmp(var, "diff.context")) {
> diff_context_default = git_config_int(var, value);
> if (diff_context_default < 0)
> @@ -4327,6 +4335,7 @@ void diff_setup(struct diff_options *options)
> }
>
> options->color_moved = diff_color_moved_default;
> + options->color_moved_ws_handling = diff_color_moved_ws_default;
> }
>
> void diff_setup_done(struct diff_options *options)
Re: [PATCH 2/2] repack -ad: prune the list of shallow commits
On Wed, Jul 18, 2018 at 07:31:40PM +0200, Duy Nguyen wrote: > > Sort of an aside to the patch under discussion, but I think it may make > > sense for prune_shallow() to take a callback function for determining > > whether a commit is reachable. > > > > I have an old patch that teaches git-prune to lazily do the reachability > > check, since in many cases "git repack" will have just packed all of the > > loose objects. But it just occurred to me that this patch is totally > > broken with respect to prune_shallow(), because it would not set the > > SEEN flag (I've literally been running with it for years, which goes to > > show how often I use the shallow feature). > > > > And if we were to have repack do a prune_shallow(), it may want to use a > > different method than traversing and marking each object SEEN. > > All of this sounds good. The only thing I'd like to do a bit > differently is to pass the reachable commits in prune_shallow() as a > commit-slab instead of taking a callback function. I'll refactor this > code, move prune_shallow() to a separate command prune-shallow and do > the locking thing. I think using a slab is much nicer than the current global-struct flags. But it's not as flexible as a callback, for two reasons: - in the lazy case, the caller might not even have loaded the slab yet. On the other hand, it might be sufficient to just be broad there, and just always pre-populate the slab when is_repository_shallow(), before we even call into prune_shallow(). If we have _any_ entries in the shallow file, we'd need to compute reachability. - it precludes any optimizations that compute partial reachability. Asking "is XYZ reachable" is a much easier question to answer than "show me the full reachability graph." At the least, it lets you stop the graph traversal early. And with generation numbers, it can even avoid traversing down unproductive segments of the graph. I think it's OK to switch to a slab for now if you don't want to do the callback thing. But I think a callback is probably long-term the right thing (and I actually think it may be _less_ work to implement, since then prune would probably be OK sticking with the global struct flags). -Peff
Re: [RFC] push: add documentation on push v2
On 07/18, Duy Nguyen wrote: > On Wed, Jul 18, 2018 at 7:13 PM Brandon Williams wrote: > > > > > What I've got now is a rough design for a more flexible push, more > > > > > flexible because it allows for the server to do what it wants with the > > > > > refs that are pushed and has the ability to communicate back what was > > > > > done to the client. The main motivation for this is to work around > > > > > issues when working with Gerrit and other code-review systems where > > > > > you > > > > > need to have Change-Ids in the commit messages (now the server can > > > > > just > > > > > insert them for you and send back new commits) and you need to push to > > > > > magic refs to get around various limitations (now a Gerrit server > > > > > should > > > > > be able to communicate that pushing to 'master' doesn't update master > > > > > but instead creates a refs/changes/ ref). > > > > Well Gerrit is our main motivation, but this allows for other workflows > > > > as well. > > > > For example Facebook uses hg internally and they have a > > > > "rebase-on-the-server-after-push" workflow IIRC as pushing to a single > > > > repo > > > > brings up quite some contention. The protocol outlined below would allow > > > > for such a workflow as well? (This might be an easier sell to the Git > > > > community as most are not quite familiar with Gerrit) > > > > > > I'm also curious how this "change commits on push" would be helpful to > > > other > > > scenarios. > > > > > > Since I'm not familiar with Gerrit: what is preventing you from having a > > > commit hook that inserts (or requests) a Change-Id when not present? How > > > can > > > the server identify the Change-Id automatically when it isn't present? > > > > Right now all Gerrit users have a commit hook installed which inserts > > the Change-Id. The issue is that if you push to gerrit and you don't > > have Change-ids, the push fails and you're prompted to blindly run a > > command to install the commit-hook. So if we could just have the server > > handle this completely then the users of gerrit wouldn't ever need to > > have a hook installed in the first place. > > I don't trust the server side to rewrite commits for me. And this is That's a fair point. Though I think there are a number of projects where this would be very beneficial for contributors. The main reason for wanting a feature like this is to make the UX easier for Gerrit users (Having server insert change ids as well as potentially getting rid of the weird HEAD:refs/for/master syntax you need to push for review). Also, if you don't control a server yourself, then who ever controls it can do what it wants with the objects/ref-updates you send them. Of course even if they rewrite history that doesn't mean your local copy needs to mimic those changes if you don't want them too. So even if we move forward with a design like this, there would need to be some config option to actually accept and apply the changes a server makes and sends back to you. This RFC doesn't actually address those sorts of UX implications because I expect those are things which can be added and tweaked at some point in the future. I'm just trying to build the foundation for such changes. > basically rewriting history (e.g. I can push multiple commits to > gerrit if I remember correctly; if they all don't have change-id, then > the history must be rewritten for change-id to be inserted). Don't we > already have "plans" to push config from server to client? There's I know there has been talk about this, but I don't know any of any current proposals or work being done in this area. > also talk about configuring hooks with config file. These should make > it possible to deal with change-id generation with minimum manual > intervention. -- Brandon Williams
Re: [PATCH 2/2] repack -ad: prune the list of shallow commits
On Wed, Jul 18, 2018 at 7:45 PM Jeff King wrote: > > On Wed, Jul 18, 2018 at 07:31:40PM +0200, Duy Nguyen wrote: > > > > Sort of an aside to the patch under discussion, but I think it may make > > > sense for prune_shallow() to take a callback function for determining > > > whether a commit is reachable. > > > > > > I have an old patch that teaches git-prune to lazily do the reachability > > > check, since in many cases "git repack" will have just packed all of the > > > loose objects. But it just occurred to me that this patch is totally > > > broken with respect to prune_shallow(), because it would not set the > > > SEEN flag (I've literally been running with it for years, which goes to > > > show how often I use the shallow feature). > > > > > > And if we were to have repack do a prune_shallow(), it may want to use a > > > different method than traversing and marking each object SEEN. > > > > All of this sounds good. The only thing I'd like to do a bit > > differently is to pass the reachable commits in prune_shallow() as a > > commit-slab instead of taking a callback function. I'll refactor this > > code, move prune_shallow() to a separate command prune-shallow and do > > the locking thing. > > I think using a slab is much nicer than the current global-struct flags. > But it's not as flexible as a callback, for two reasons: > > - in the lazy case, the caller might not even have loaded the slab > yet. On the other hand, it might be sufficient to just be broad > there, and just always pre-populate the slab when > is_repository_shallow(), before we even call into prune_shallow(). > If we have _any_ entries in the shallow file, we'd need to compute > reachability. > > - it precludes any optimizations that compute partial reachability. > Asking "is XYZ reachable" is a much easier question to answer than > "show me the full reachability graph." At the least, it lets you > stop the graph traversal early. And with generation numbers, it can > even avoid traversing down unproductive segments of the graph. Both good points. Callback it is then. -- Duy
Re: sed: command garbled: rGIT-PERL-HEADER
Jeffrey Walton writes:
> I'm trying to build Git 2.18 on Solaris 11.3 x86_64.
>
> $ gmake V=1
> rm -f git-add--interactive git-add--interactive+ && \
> sed -e '1{' \
> -e 's|#!.*perl|#!/usr/bin/perl|' \
> -e 'rGIT-PERL-HEADER' \
> -e 'G' \
> -e '}' \
> -e 's/@@GIT_VERSION@@/2.18.0/g' \
> git-add--interactive.perl >git-add--interactive+ && \
> chmod +x git-add--interactive+ && \
> mv git-add--interactive+ git-add--interactive
> sed: command garbled: rGIT-PERL-HEADER
> gmake: *** [git-add--interactive] Error 2
>
> And:
>
> $ perl --version
>
> This is perl 5, version 12, subversion 5 (v5.12.5) built for
> i86pc-solaris-64int
> (with 7 registered patches, see perl -V for more detail)
>
> Any ideas how to fix it?
The fix is prepared to become part of the next feature release (and
also some maintenance release before that happens). Please try the
attached patch.
Thanks.
-- >8 --
From: Alejandro R. Sedeño
Date: Mon, 25 Jun 2018 15:13:25 -0400
Subject: [PATCH] Makefile: tweak sed invocation
With GNU sed, the r command doesn't care if a space separates it and
the filename it reads from.
With SunOS sed, the space is required.
Signed-off-by: Alejandro R. Sedeño
Reviewed-by: Eric Sunshine
Signed-off-by: Junio C Hamano
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 2ba24035f5..50138e85eb 100644
--- a/Makefile
+++ b/Makefile
@@ -2086,7 +2086,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES
GIT-PERL-HEADER GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1{' \
-e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
- -e 'rGIT-PERL-HEADER' \
+ -e 'r GIT-PERL-HEADER' \
-e 'G' \
-e '}' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
--
2.18.0-129-ge3331758f1
Re: [PATCH] Documentation: fix --color option formatting
On Wed, Jul 18, 2018 at 07:37:48PM +0200, Andrei Rybak wrote: > Add missing colon in two places to fix formatting of options. Thanks for catching. > diff --git a/Documentation/git-for-each-ref.txt > b/Documentation/git-for-each-ref.txt > index 085d177d97..901faef1bf 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -57,7 +57,7 @@ OPTIONS > `xx`; for example `%00` interpolates to `\0` (NUL), > `%09` to `\t` (TAB) and `%0a` to `\n` (LF). > > ---color[=]: > +--color[=]:: > Respect any colors specified in the `--format` option. The > `` field must be one of `always`, `never`, or `auto` (if > `` is absent, behave as if `always` was given). This is obviously the right fix. I am guilty of not always building the documentation and eye-balling the output when I'm not specifically changing the formatting. I wonder if we could provide tooling to make that easier, by showing a diff between the text-formatted manpages before and after a series. I've manually hacked stuff up like that in the past, but there's often a lot of noise around date and version info in the footers. -Peff
Re: [PATCH 0/4] Use oid_object_info() instead of read_object_file()
Johannes Schindelin writes:
> Hi Olga,
>
> On Fri, 13 Jul 2018, Оля Тележная wrote:
>
>> 2018-07-09 11:27 GMT+03:00 Оля Тележная :
>> > Hello everyone,
>> > This is my new attempt to start using oid_object_info_extended() in
>> > ref-filter. You could look at previous one [1] [2] but it is not
>> > necessary.
>> >
>> > The goal (still) is to improve performance by avoiding calling expensive
>> > functions when we don't need the information they provide
>> > or when we could get it by using a cheaper function.
>> >
>> > This patch is a middle step. In the end, I want to add new atoms
>> > ("objectsize:disk" and "deltabase") and reuse ref-filter logic in
>> > cat-file command.
>> >
>> > I also know about problems with memory leaks in ref-filter: that would
>> > be my next task that I will work on. Since I did not generate any new
>> > leaks in this patch (just use existing ones), I decided to put this
>> > part on a review and fix leaks as a separate task.
>>
>> UPDATES since v1:
>> add init to eaten variable (thanks to Szeder Gabor, Johannes Schindelin)
>> improve second commit message (thanks to Junio C Hamano)
>> add static keyword (thanks to Ramsay Jones)
>>
>> >
>> > Thank you!
>> >
>> > [1] https://github.com/git/git/pull/493
>
> Could you please populate the description of that PR so that SubmitGit
> picks it up as cover letter?
Thanks for suggesting that. Yes, an updated version of a series,
even if it is a small one with just 4 or 5 patches, becomes much
easier to read with a well-written cover letter.
Re: [RFC] push: add documentation on push v2
On Wed, Jul 18, 2018 at 7:46 PM Brandon Williams wrote: > > On 07/18, Duy Nguyen wrote: > > On Wed, Jul 18, 2018 at 7:13 PM Brandon Williams wrote: > > > > > > What I've got now is a rough design for a more flexible push, more > > > > > > flexible because it allows for the server to do what it wants with > > > > > > the > > > > > > refs that are pushed and has the ability to communicate back what > > > > > > was > > > > > > done to the client. The main motivation for this is to work around > > > > > > issues when working with Gerrit and other code-review systems where > > > > > > you > > > > > > need to have Change-Ids in the commit messages (now the server can > > > > > > just > > > > > > insert them for you and send back new commits) and you need to push > > > > > > to > > > > > > magic refs to get around various limitations (now a Gerrit server > > > > > > should > > > > > > be able to communicate that pushing to 'master' doesn't update > > > > > > master > > > > > > but instead creates a refs/changes/ ref). > > > > > Well Gerrit is our main motivation, but this allows for other > > > > > workflows as well. > > > > > For example Facebook uses hg internally and they have a > > > > > "rebase-on-the-server-after-push" workflow IIRC as pushing to a > > > > > single repo > > > > > brings up quite some contention. The protocol outlined below would > > > > > allow > > > > > for such a workflow as well? (This might be an easier sell to the Git > > > > > community as most are not quite familiar with Gerrit) > > > > > > > > I'm also curious how this "change commits on push" would be helpful to > > > > other > > > > scenarios. > > > > > > > > Since I'm not familiar with Gerrit: what is preventing you from having a > > > > commit hook that inserts (or requests) a Change-Id when not present? > > > > How can > > > > the server identify the Change-Id automatically when it isn't present? > > > > > > Right now all Gerrit users have a commit hook installed which inserts > > > the Change-Id. The issue is that if you push to gerrit and you don't > > > have Change-ids, the push fails and you're prompted to blindly run a > > > command to install the commit-hook. So if we could just have the server > > > handle this completely then the users of gerrit wouldn't ever need to > > > have a hook installed in the first place. > > > > I don't trust the server side to rewrite commits for me. And this is > > That's a fair point. Though I think there are a number of projects > where this would be very beneficial for contributors. The main reason > for wanting a feature like this is to make the UX easier for Gerrit > users (Having server insert change ids as well as potentially getting > rid of the weird HEAD:refs/for/master syntax you need to push for > review). Also, if you don't control a server yourself, then who ever > controls it can do what it wants with the objects/ref-updates you send > them. Of course even if they rewrite history that doesn't mean your > local copy needs to mimic those changes if you don't want them too. So > even if we move forward with a design like this, there would need to be > some config option to actually accept and apply the changes a server > makes and sends back to you. This is the main pain point for me. I almost wrote a follow mail along the line of "having said that, if we can be transparent what the changes are or have some protection at client side against "dangerous" changes like tree/blob/commit-header replacement then it's probably ok". There's also other things like signing which will not work well with remote updates like this. I guess you can cross it out as "not supported" after consideration though. > This RFC doesn't actually address those > sorts of UX implications because I expect those are things which can be > added and tweaked at some point in the future. I'm just trying to build > the foundation for such changes. Speaking of UX, gerrit and this server-side ref-update. My experience with average gerrit users is they tend to stick to a very basic set of commands and if this is not handled well, you just replace the one-time pain of installing hooks with a new one that happens much more often, potentially more confusing too. > > basically rewriting history (e.g. I can push multiple commits to > > gerrit if I remember correctly; if they all don't have change-id, then > > the history must be rewritten for change-id to be inserted). Don't we > > already have "plans" to push config from server to client? There's > > I know there has been talk about this, but I don't know any of any > current proposals or work being done in this area. As far as I know, nobody has worked on it. You're welcome to start of course ;-) -- Duy
Re: [RFC] push: add documentation on push v2
> > Given the example above for "rebase-on-push" though > > it is better to first send the packfile (as that is assumed to > > take longer) and then send the ref updates, such that the > > rebasing could be faster and has no bottleneck. > > I don't really follow this logic. I don't think it would change > anything much considering the ref-updates section would usually be > much smaller than the packfile itself, course I don't have any data so > idk. The server would need to serialize all incoming requests and apply them in order. The receiving of the packfile and the response to the client are not part of the critical section that needs to happen serialized but can be spread out to threads. So for that use case it would make sense to allow sending the packfile first. > > > +update = txn_id SP action SP refname SP old_oid SP new_oid > > > +force-update = txn_id SP "force" SP action SP refname SP new_oid > > > > So we insert "force" after the transaction id if we want to force it. > > When adding the atomic capability later we could imagine another insert here > > > > 1 atomic create refs/heads/new-ref <0-hash> > > 1 atomic delete refs/heads/old-ref <0-hash> > > > > which would look like a "rename" that we could also add instead. > > The transaction numbers are an interesting concept, how do you > > envision them to be used? In the example I put them both in the same > > transaction to demonstrate the "atomic-ness", but one could also > > imagine different transactions numbers per ref (i.e. exactly one > > ref per txn_id) to have a better understanding of what the server did > > to each individual ref. > > I believe I outlined their use later. Basically if you give the server > free reign to do what it wants with the updates you send it, then you > need a way for the client to be able to map the result back to what it > requested. Since now i could push to "master" but instead of updating > master the server creates a refs/changes/1 ref and puts my changes there > instead of updating master. The client needs to know that the ref > update it requested to master is what caused the creation of the > refs/changes/1 ref. understood, the question was more related to how you envision what the client/server SHOULD be doing here, (and I think a one txn_id per ref is what SHOULD be done is how this is best to implement the thoughts above, also the client is ALLOWED to put many refs in one txn, or would we just disallow that already at this stage to not confuse the server?) > > > > > Are new capabilities attached to ref updates or transactions? > > Unlike the example above, stating "atomic" on each line, you could just > > say "transaction 1 should be atomic" in another line, that would address > > all refs in that transaction. > > I haven't thought through "atomic" so i have no idea what you'd want > that to look like. Yeah I have not really thought about them either, I just see two ways: (A) adding more keywords in each ref-update (like "force") or (B) adding new subsections somewhere where we talk about the capabilities instead. Depending on why way we want to go this might have impact on the design how to write the code. > > > + * Normal ref-updates require that the old value of a ref is > > > supplied so > > > + that the server can verify that the reference that is being > > > updated > > > + hasn't changed while the request was being processed. > > > > create/delete assume <00..00> for either old or new ? (We could also > > omit the second hash for create delete, which is more appealing to me) > > Well that depends, in the case of a create you want to ensure that no > ref with that name exists and would want it to fail if one already > existed. If you want to force it then you don't care if one existed or > not, you just want the ref to have a certain value. What I was trying to say is to have update = txn_id SP (modifier SP) action modifier = "force" | "atomic" action = (create | delete | update) create = "create" SP update = "update" SP SP delete = "delete" SP i.e. only one hash for the create and delete action. (I added the "atomic" modifier to demonstrate (A) from above, not needed here) > > > > > + * Forced ref-updates only include the new value of a ref as we > > > don't > > > + care what the old value was. > > > > How are you implementing force-with-lease then? > > Currently force-with-lease/force is implemented 100% on the client side, Uh? That would be bad. Reading 631b5ef219c (push --force-with-lease: tie it all together, 2013-07-08) I think that send-pack is done server-side? > this proposal extends these two to be implemented on the server as well. > non-forced variant are basically the "with-lease" case and "force" now > actually forces an update. I think we have 3 modes: (1) standard push, where both client and server check for a fast-forward (2) "force" that blindly overwrites the ref, but as that
Re: [PATCH] diff.c: offer config option to control ws handling in move detection
On Wed, Jul 18, 2018 at 10:45 AM Junio C Hamano wrote: > > Stefan Beller writes: > > > diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt > > index 143acd9417e..8da7fed4e22 100644 > > --- a/Documentation/diff-options.txt > > +++ b/Documentation/diff-options.txt > > @@ -294,8 +294,11 @@ dimmed_zebra:: > > > > --color-moved-ws=:: > > This configures how white spaces are ignored when performing the > > - move detection for `--color-moved`. These modes can be given > > - as a comma separated list: > > + move detection for `--color-moved`. > > +ifdef::git-diff[] > > + It can be set by the `diff.colorMovedWS` configuration setting. > > +endif::git-diff[] > > The patch to diff.c::git_diff_ui_config() we see below does not seem > to make any effort to make sure that this new configuration applies > only to "git diff" and that other commands like "git log" that call > git_diff_ui_config() are not affected. That is as intended. (We want to have it in git-log) > And I do not see a strong reason why "git log --color-moved" should > not honor this setting, either, so I am not quite sure why we want > this ifdef/endif pair to hide it from "git log --help". > > Or am I totally misunderstanding the reason why we want ifdef/endif > here? > > Puzzled... I am somewhat puzzled, too, by the use of ifdefs in Documentation/diff-options.txt, but I rationalized it as "man git diff" has all the details, whereas the other pages are a bit shorter and concise: $ git grep -A 2 "ifdef::git-diff" Documentation/diff-options.txt ifdef::git-diff[] This is the default. endif::git-diff[] ifdef::git-diff-core[] This is the default. endif::git-diff-core[] ifdef::git-diff[] It can be changed by the `color.ui` and `color.diff` configuration settings. ifdef::git-diff[] This can be used to override configuration settings. endif::git-diff[] ifdef::git-diff[] It can be changed by the `diff.colorMoved` configuration setting. endif::git-diff[] ifdef::git-diff[] It can be set by the `diff.colorMovedWS` configuration setting. endif::git-diff[] So I followed the style that was already there, specifically 61e89eaae88 (diff: document the new --color-moved setting, 2017-06-30) which followed the style of 6999c54029b (config.txt,diff-options.txt: porcelain vs. plumbing for color.diff, 2011-04-27) So I might have picked up a bad habit there or misunderstood the original? Stefan
Re: [RFC] push: add documentation on push v2
On Wed, Jul 18, 2018 at 8:08 PM Stefan Beller wrote: > P.S.: another feature that just came to mind is localisation of error > messages. > But that is also easy to do with capabilities (the client sends a capability > such as "preferred-i18n=DE" and the server may translate all its errors > if it can. > > That brings me to another point: do we assume all errors to be read > by humans? or do we want some markup things in there, too, similar to > EAGAIN? We do have several classes of errors (fatal, error, warning) so at least some machine-friendly code should be there. Perhaps just follow many protocols out there (http in particular) and separate error codes in groups. Then we can standardize some specific error codes later if we want too but by default, error(), warning() or die() when running in server context will have a fixed error code in each group. -- Duy
Re: [PATCH 3/3] gc: do not return error for prior errors in daemonized mode
Jeff King writes: >> Perhaps we should exit with 2 (not 0) in that "previous error" case >> by default, and then have a configuration knob to turn that 2 into 0 >> for those who cannot easily modify the calling script? That way, we >> by default will *not* break those who have been paying attention to >> zero-ness of the exit status, we allow those who want to treat this >> "prior error" case as if there were no error with just a knob, and >> then those who are willing to update their script can tell two cases >> by the exit status and act differently. > > I think we have been exiting non-zero with "previous errors" for some > time with the daemonizing code. It was just spelled "-1" instead of "2". > So just jumping right there does not mean any regression from the > current state, I don't think (but it also does not fix existing scripts > like "repo" that check the code). Correct. That is why I suggested that default setting. However. If we change it so that by default we exit with 0 but allow those who care to see 2 with a knob, that would be a regression to those who actually were acting on the non-zero exit, but they no longer have to scrape the log/warning output if they choose to set the knob, so the "convenience" factor might argue in favor of the "by default we return 0 but you can choose to get the 'sensible exit codes'" approach. I do not have a strong opinion either way wrt which one should be the default. > I agree the config you suggest would give people the tools to make that > case work. But it somehow rubs me the wrong way. Can you imagine the > perspective of a user who is told "oh, your script breaks? Just try > setting this option to ignore error codes in this one particular > situation". It feels like a weird hack, because it is. Well, the message is "your script is broken (and has always been when 'gc --auto' saw that the previous round failed), but now we have two ways to help you correct. An easier but broken way is to turn this knob to turn 2 to 0 to hide the issue under the rug; or you can notice the difference between $?==1 and $?==2 and act differently". "turn 2 to 0" is a weird hack, but they are also given the option of doing the right thing, so... > It's also still inconsistent in the daemonize case. The run that yields > the error won't return a non-zero exit. But the next run will exit with > "2". I do not see this particular "inconsistency" a problem. The run that first discovers the problem by definition cannot return with non-zero; not waiting until finding out the outcome is the whole point of running in the background and giving control back early.
Re: [RFC] push: add documentation on push v2
On 07/18, Stefan Beller wrote: > > > Given the example above for "rebase-on-push" though > > > it is better to first send the packfile (as that is assumed to > > > take longer) and then send the ref updates, such that the > > > rebasing could be faster and has no bottleneck. > > > > I don't really follow this logic. I don't think it would change > > anything much considering the ref-updates section would usually be > > much smaller than the packfile itself, course I don't have any data so > > idk. > > The server would need to serialize all incoming requests and apply > them in order. The receiving of the packfile and the response to the client > are not part of the critical section that needs to happen serialized but > can be spread out to threads. So for that use case it would make > sense to allow sending the packfile first. I would think that a server needs to read the whole request before any actions can be done, but maybe I need to think about this a bit more. > > > > > +update = txn_id SP action SP refname SP old_oid SP new_oid > > > > +force-update = txn_id SP "force" SP action SP refname SP new_oid > > > > > > So we insert "force" after the transaction id if we want to force it. > > > When adding the atomic capability later we could imagine another insert > > > here > > > > > > 1 atomic create refs/heads/new-ref <0-hash> > > > 1 atomic delete refs/heads/old-ref <0-hash> > > > > > > which would look like a "rename" that we could also add instead. > > > The transaction numbers are an interesting concept, how do you > > > envision them to be used? In the example I put them both in the same > > > transaction to demonstrate the "atomic-ness", but one could also > > > imagine different transactions numbers per ref (i.e. exactly one > > > ref per txn_id) to have a better understanding of what the server did > > > to each individual ref. > > > > I believe I outlined their use later. Basically if you give the server > > free reign to do what it wants with the updates you send it, then you > > need a way for the client to be able to map the result back to what it > > requested. Since now i could push to "master" but instead of updating > > master the server creates a refs/changes/1 ref and puts my changes there > > instead of updating master. The client needs to know that the ref > > update it requested to master is what caused the creation of the > > refs/changes/1 ref. > > understood, the question was more related to how you envision what > the client/server SHOULD be doing here, (and I think a one txn_id per > ref is what SHOULD be done is how this is best to implement the > thoughts above, also the client is ALLOWED to put many refs in one > txn, or would we just disallow that already at this stage to not confuse > the server?) Oh sorry for misunderstanding. Yeah, as of right now I think having a one-to-one relationship makes it easier to write/implement but I don't know if there are other workflows which would benefit from multiple per txn_id. > > > > > > > > > Are new capabilities attached to ref updates or transactions? > > > Unlike the example above, stating "atomic" on each line, you could just > > > say "transaction 1 should be atomic" in another line, that would address > > > all refs in that transaction. > > > > I haven't thought through "atomic" so i have no idea what you'd want > > that to look like. > > Yeah I have not really thought about them either, I just see two ways: > (A) adding more keywords in each ref-update (like "force") or > (B) adding new subsections somewhere where we talk about the capabilities > instead. > > Depending on why way we want to go this might have impact on the > design how to write the code. > > > > > + * Normal ref-updates require that the old value of a ref is > > > > supplied so > > > > + that the server can verify that the reference that is being > > > > updated > > > > + hasn't changed while the request was being processed. > > > > > > create/delete assume <00..00> for either old or new ? (We could also > > > omit the second hash for create delete, which is more appealing to me) > > > > Well that depends, in the case of a create you want to ensure that no > > ref with that name exists and would want it to fail if one already > > existed. If you want to force it then you don't care if one existed or > > not, you just want the ref to have a certain value. > > What I was trying to say is to have > > update = txn_id SP (modifier SP) action > modifier = "force" | "atomic" > action = (create | delete | update) > create = "create" SP > update = "update" SP SP > delete = "delete" SP > > i.e. only one hash for the create and delete action. > (I added the "atomic" modifier to demonstrate (A) from above, not needed here) I understood what you were asking, I was just pointing out the rationale for including the zero-id but i guess you're correct in that simply omitting it would work
Re: [PATCH] Documentation: fix --color option formatting
Andrei Rybak writes:
> Add missing colon in two places to fix formatting of options.
>
> Signed-off-by: Andrei Rybak
> ---
>
> Done on top of maint.
>
> The earliest this patch applies is on top of commit aebd23506e ("Merge
> branch 'jk/ui-color-always-to-auto-maint' into
> jk/ui-color-always-to-auto", 2017-10-04), one commit away from the
> commit that added both of the affected lines: 0c88bf5050 (provide
> --color option for all ref-filter users, 2017-10-03).
>
> I grepped the Documentation folder, and haven't found any other
> similar typos.
Thanks.
Re: [PATCH 9/9] diff.c: add white space mode to move detection that allows indent changes
On 2018-07-17 01:05, Stefan Beller wrote: > > This patch brings some challenges, related to the detection of blocks. > We need a white net the catch the possible moved lines, but then need to The s/white/wide/ was already suggested by Brandon Williams in previous iteration, but it seems this also needs s/the catch/to catch/ > narrow down to check if the blocks are still in tact. Consider this > example (ignoring block sizes): >
Re: [PATCH 9/9] diff.c: add white space mode to move detection that allows indent changes
On Wed, Jul 18, 2018 at 11:25 AM Andrei Rybak wrote: > > On 2018-07-17 01:05, Stefan Beller wrote: > > > > This patch brings some challenges, related to the detection of blocks. > > We need a white net the catch the possible moved lines, but then need to > > The s/white/wide/ was already suggested by Brandon Williams in previous > iteration, but it seems this also needs s/the catch/to catch/ > Both fixed locally, thanks! I'll resend this series later.
Re: [PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
"Stefan Beller via GitGitGadget" writes:
> From: Stefan Beller
>
> Signed-off-by: Stefan Beller
> Signed-off-by: Derrick Stolee
> ---
> builtin/replace.c | 8
> refs.c| 9 -
> refs.h| 2 +-
> replace-object.c | 5 +++--
> 4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/replace.c b/builtin/replace.c
> index ef22d724b..d0b1cdb06 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -39,7 +39,8 @@ struct show_data {
> enum replace_format format;
> };
>
> -static int show_reference(const char *refname, const struct object_id *oid,
> +static int show_reference(struct repository *r, const char *refname,
> + const struct object_id *oid,
> int flag, void *cb_data)
> {
> struct show_data *data = cb_data;
> @@ -56,9 +57,8 @@ static int show_reference(const char *refname, const struct
> object_id *oid,
> if (get_oid(refname, &object))
> return error("Failed to resolve '%s' as a valid
> ref.", refname);
>
> - obj_type = oid_object_info(the_repository, &object,
> -NULL);
> - repl_type = oid_object_info(the_repository, oid, NULL);
> + obj_type = oid_object_info(r, &object, NULL);
> + repl_type = oid_object_info(r, oid, NULL);
>
> printf("%s (%s) -> %s (%s)\n", refname,
> type_name(obj_type),
> oid_to_hex(oid), type_name(repl_type));
> diff --git a/refs.c b/refs.c
> index 2513f77ac..5700cd468 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1478,12 +1478,11 @@ int refs_for_each_fullref_in(struct ref_store *refs,
> const char *prefix,
> return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
> }
>
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
> *cb_data)
> {
> - return do_for_each_ref(get_main_ref_store(r),
> -git_replace_ref_base, fn,
> -strlen(git_replace_ref_base),
> -DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> + return do_for_each_repo_ref(r, git_replace_ref_base, fn,
> + strlen(git_replace_ref_base),
> + DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> }
>
> int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
> diff --git a/refs.h b/refs.h
> index 80eec8bbc..a0a18223a 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn
> fn, void *cb_data,
> int for_each_tag_ref(each_ref_fn fn, void *cb_data);
> int for_each_branch_ref(each_ref_fn fn, void *cb_data);
> int for_each_remote_ref(each_ref_fn fn, void *cb_data);
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void
> *cb_data);
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void
> *cb_data);
With a signature change like this, any change that introduces new
call to for_each_replace_ref() using eac_ref_fn() would get
compilation error, so this is minimally correct.
Two things that bothersome are that
- for_each_tag/branch/remote_ref() and for_each_replace_ref() now
work and look quite differently.
- existing users of for_each_replace_ref() who were all happy
working in the_repository have to pass it explicitly, even
thought they do not have any need to.
In this case, even if you introduced for_each_replace_ref_in_repo(),
making for_each_replace_ref() a thin wrapper that always uses
the_repository is a bit more cumbersome than just a simple macro.
But it *is* doable (you'd need to use a wrapping structure around
cb_data), and a developer who case about maintainability during API
transition would have taken pains to do so. A bit dissapointing.
> int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
> int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
>const char *prefix, void *cb_data);
> diff --git a/replace-object.c b/replace-object.c
> index 801b5c167..017f02f8e 100644
> --- a/replace-object.c
> +++ b/replace-object.c
> @@ -6,7 +6,8 @@
> #include "repository.h"
> #include "commit.h"
>
> -static int register_replace_ref(const char *refname,
> +static int register_replace_ref(struct repository *r,
> + const char *refname,
> const struct object_id *oid,
> int flag, void *cb_data)
> {
> @@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,
> oidcpy(&repl_obj->replacement, oid);
>
> /* Register new object */
> - if (oidmap_put(the_repository->objects->replace_map, repl_obj))
> + if (oidmap_put(r->objects->replace_map
Re: sed: command garbled: rGIT-PERL-HEADER
On Wed, Jul 18, 2018 at 1:49 PM, Junio C Hamano wrote:
> Jeffrey Walton writes:
> ...
> diff --git a/Makefile b/Makefile
> index 2ba24035f5..50138e85eb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2086,7 +2086,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES
> GIT-PERL-HEADER GIT-VERSION-FILE
> $(QUIET_GEN)$(RM) $@ $@+ && \
> sed -e '1{' \
> -e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
> - -e 'rGIT-PERL-HEADER' \
> + -e 'r GIT-PERL-HEADER' \
> -e 'G' \
> -e '}' \
> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
Thanks, that tested good.
Here's another one for FreeBSD 11 x86_64. LDLIBS includes -pthread.
$ cat ~/git-freebsd.txt
LINK git-credential-store
/usr/local/bin/ld: libgit.a(name-hash.o): undefined reference to
symbol 'pthread_create@@FBSD_1.0'
//lib/libthr.so.3: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
gmake: *** [Makefile:2329: git-credential-store] Error 1
Jeff
Re: [PATCH 3/3] gc: do not return error for prior errors in daemonized mode
On Wed, Jul 18, 2018 at 11:19:01AM -0700, Junio C Hamano wrote: > > It's also still inconsistent in the daemonize case. The run that yields > > the error won't return a non-zero exit. But the next run will exit with > > "2". > > I do not see this particular "inconsistency" a problem. The run > that first discovers the problem by definition cannot return with > non-zero; not waiting until finding out the outcome is the whole > point of running in the background and giving control back early. I guess I just see it as encouraging a non-robust flow. I can see somebody thinking they should care about "gc --auto" errors, because they can turn up latent repository corruption (because most operations try to only look at the objects they need to, and gc inherently considers every object). But doing so would give a very delayed notification for a repository that is handled infrequently. So finding out about a corruption that we detected might takes weeks or months. I dunno. If your primary motivation is not finding latent problems, but loudly complaining when gc does not make forward progress, I suppose it makes more sense. -Peff
Re: sed: command garbled: rGIT-PERL-HEADER
On Wed, Jul 18, 2018 at 2:47 PM, Jeffrey Walton wrote:
> On Wed, Jul 18, 2018 at 1:49 PM, Junio C Hamano wrote:
>> Jeffrey Walton writes:
>> ...
>> diff --git a/Makefile b/Makefile
>> index 2ba24035f5..50138e85eb 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2086,7 +2086,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES
>> GIT-PERL-HEADER GIT-VERSION-FILE
>> $(QUIET_GEN)$(RM) $@ $@+ && \
>> sed -e '1{' \
>> -e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
>> - -e 'rGIT-PERL-HEADER' \
>> + -e 'r GIT-PERL-HEADER' \
>> -e 'G' \
>> -e '}' \
>> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
>
> Thanks, that tested good.
>
> Here's another one for FreeBSD 11 x86_64. LDLIBS includes -pthread.
>
> $ cat ~/git-freebsd.txt
> LINK git-credential-store
> /usr/local/bin/ld: libgit.a(name-hash.o): undefined reference to
> symbol 'pthread_create@@FBSD_1.0'
> //lib/libthr.so.3: error adding symbols: DSO missing from command line
> collect2: error: ld returned 1 exit status
> gmake: *** [Makefile:2329: git-credential-store] Error 1
I was able to clear this one with sed (trailing space is important):
sed -e 's|$(LIB_FILE) |$(LIB_FILE) -lpthread |g' "$file.orig"
But I don't know how to fix the missing recipe:
$ gmake
gmake: *** No rule to make target 'git-daemon', needed by 'all'. Stop.
Jeff
Re: [PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
> > @@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn > > fn, void *cb_data, > > int for_each_tag_ref(each_ref_fn fn, void *cb_data); > > int for_each_branch_ref(each_ref_fn fn, void *cb_data); > > int for_each_remote_ref(each_ref_fn fn, void *cb_data); > > -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void > > *cb_data); > > +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void > > *cb_data); > > With a signature change like this, any change that introduces new > call to for_each_replace_ref() using eac_ref_fn() would get > compilation error, so this is minimally correct. > > Two things that bothersome are that > > - for_each_tag/branch/remote_ref() and for_each_replace_ref() now >work and look quite differently. Yes, this series is not finished; we need to convert/upgrade for_each_tag et al, too. > - existing users of for_each_replace_ref() who were all happy >working in the_repository have to pass it explicitly, even >thought they do not have any need to. All callbacks that are passed to for_each_replace_ref now operate on 'r' instead of the_repository, and that actually fixes a bug (commit message is lacking), but the cover letter hints at: While building this series, I got some test failures in the non-the_repository tests. These issues are related to missing references to an arbitrary repository (instead of the_repository) and some uninitialized values in the tests. Stefan already sent a patch to address this [2], and I've included those commits (along with a small tweak [3]). These are only included for convenience. > In this case, even if you introduced for_each_replace_ref_in_repo(), > making for_each_replace_ref() a thin wrapper that always uses > the_repository is a bit more cumbersome than just a simple macro. Yes, but such a callback would do the *wrong* subtle thing in some cases as you really want to work in the correct repository for e.g. looking up commits. > But it *is* doable (you'd need to use a wrapping structure around > cb_data), and a developer who case about maintainability during API > transition would have taken pains to do so. A bit dissapointing. My original patches were RFC-ish and a trade off as for the reflog only there is nothing in flight to care about. Given that we would want to upgrade all the ref callbacks, we have to take this route, I think. Thanks, Stefan
Re: [PATCH 00/16] Consolidate reachability logic
On Wed, Jul 18, 2018 at 02:23:11PM +0200, Johannes Schindelin wrote: > > Yeah, they're out of order in mutt's threaded display. And the > > back-dating means there's a much higher chance of them getting blocked > > as spam (e.g., some of the dates are from weeks ago). > > > > git-send-email uses the current time minus an offset, and then > > monotonically increases for each patch: > > > > $time = time - scalar $#files; > > ... > > my $date = format_2822_time($time++); > > > > which seems to work pretty well in practice. It does mean the original > > dates are lost. The committer date is not interesting at all (there will > > be a new committer via "git am" anyway). The original author date is > > potentially of interest, but could be included as an in-body header. > > AFAIK send-email doesn't have such an option, though, and people are > > fine with date-of-sending becoming the new author date. > > > > +cc Johannes as the GitGitGadget author > > Thanks for dumping even more work on my shoulders. Wow. Here's my perspective on what I wrote. Somebody pointed out an issue in the tool. I tried to add an additional data point (how other clients react, and that I've seen spam-related problems). And I tried to point to an existing solution in another tool, in case that was helpful. I thought cc-ing you would be a favor, since you obviously have an interest in the tool, and it is easy to miss discussions buried deep in a thread. So no, I didn't write the patch for you. But I tried to contribute positively to the process. And I got yelled at for it. That makes me a lot less inclined to try to help in the future. > Next time, I will ask you to jump in, instead of putting the onus on me. > > I mean, seriously, what is this? "You can use *any* mail program to work > with the Git mailing list, *any* mailer. As long as it is mutt. And as > long as you spend hours and hours on tooling that oh BTW nobody else can > use." The irony here is that I actually _did_ look at the GitGitGadget repository, and thought about making a patch to be helpful. But as it is written in a language I'm not all that familiar with, using tools that I don't normally use, I didn't want to spend hours and hours in order to make what was probably going to be a one-line patch in software that I don't use myself. -Peff
[PATCHv6 00/10] Reroll of sb/diff-color-move-more
v6: * fixed issues hinted at by Andrei, thanks! (range-diff below) * incorporates the new config option, sent separately previously. v5: This is a resend of sb/diff-color-move-more https://public-inbox.org/git/20180629001958.85143-1-sbel...@google.com/ that fixes an errornous squashing within the series; the end result is the same. range diff is below. (As the latest cooking email said this series is going to land in next soon, I hope this is not too late of a resend; otherwise just ignore it as the end result is the same) Thanks, Stefan Stefan Beller (10): xdiff/xdiff.h: remove unused flags xdiff/xdiffi.c: remove unneeded function declarations t4015: avoid git as a pipe input diff.c: do not pass diff options as keydata to hashmap diff.c: adjust hash function signature to match hashmap expectation diff.c: add a blocks mode for moved code detection diff.c: decouple white space treatment from move detection algorithm diff.c: factor advance_or_nullify out of mark_color_as_moved diff.c: add white space mode to move detection that allows indent changes diff.c: offer config option to control ws handling in move detection Documentation/config.txt | 5 + Documentation/diff-options.txt | 33 - diff.c | 262 + diff.h | 9 +- t/t4015-diff-whitespace.sh | 243 +- xdiff/xdiff.h | 8 - xdiff/xdiffi.c | 17 --- 7 files changed, 489 insertions(+), 88 deletions(-) 1: a512cd40cae ! 1: aabbc4e8aff diff.c: add white space mode to move detection that allows indent changes @@ -27,8 +27,8 @@ modes in the move detection. This patch brings some challenges, related to the detection of blocks. -We need a white net the catch the possible moved lines, but then need to -narrow down to check if the blocks are still in tact. Consider this +We need a wide net to catch the possible moved lines, but then need to +narrow down to check if the blocks are still intact. Consider this example (ignoring block sizes): - A @@ -254,7 +254,7 @@ + } + } + -+ for (i = 0; i next_line->wsd = pmb[i]->wsd; -: --- > 2: f80fbe78d9b diff.c: offer config option to control ws handling in move detection
