git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Doan Tran Cong Danh <congdanhqx@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 0/9] Improve odd encoding integration
Date: Mon, 11 Nov 2019 10:22:55 +0900	[thread overview]
Message-ID: <xmqqlfsnjjf4.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <cover.1573205699.git.congdanhqx@gmail.com> (Doan Tran Cong Danh's message of "Fri, 8 Nov 2019 16:43:42 +0700")

Doan Tran Cong Danh <congdanhqx@gmail.com> writes:

> The series is shifting from fixing test that failed on musl based Linux to
> correct the internal working encoding and output encoding of git-am
> git-cherry-pick git-rebase and git-revert.
>
> Change from v4:
> - Update commit message per review
> - Add test for last 2 patches
> - Notice a breakage with git rebase --rebase-merges (see patch 7)
>

Re: Patch 3, it indeed is a bit sad that out test framework does not
let us "show" what exactly is being executed in the debug/verbose
mode when we use parameterized test helpers. We designed the test
helpers like test_expect_success and test_eval_ so that the eval'ed
block can access the outer variables safely, i.e. "$flag-$old-$new"
can and should be written inside dq for safety, but the variable
names would be all we see in the verbose log because of that.  But
that's not the end of the world ;-)

I do not have a strong opinion on Patch 9 (I agree we need to fall
back on something sensible and without any change, the system is
quite broken---I do not have a strong opinion on what the fallback
should be); I'd love to hear what sequencer folks think.

Overall this round looks quite good.

Thanks, will queue.




>
> Doan Tran Cong Danh (9):
>   t0028: eliminate non-standard usage of printf
>   configure.ac: define ICONV_OMITS_BOM if necessary
>   t3900: demonstrate git-rebase problem with multi encoding
>   sequencer: reencode to utf-8 before arrange rebase's todo list
>   sequencer: reencode revert/cherry-pick's todo list
>   sequencer: reencode squashing commit's message
>   sequencer: reencode old merge-commit message
>   sequencer: reencode commit message for am/rebase --show-current-patch
>   sequencer: fallback to sane label in making rebase todo list
>
>  configure.ac                     |  49 ++++++++++++++++++
>  sequencer.c                      |  32 ++++++++----
>  t/t0028-working-tree-encoding.sh |   4 +-
>  t/t3433-rebase-i18n.sh           |  84 +++++++++++++++++++++++++++++++
>  t/t3433/ISO8859-1.txt            | Bin 0 -> 15 bytes
>  t/t3433/eucJP.txt                | Bin 0 -> 68 bytes
>  t/t3900-i18n-commit.sh           |  37 ++++++++++++++
>  7 files changed, 195 insertions(+), 11 deletions(-)
>  create mode 100755 t/t3433-rebase-i18n.sh
>  create mode 100644 t/t3433/ISO8859-1.txt
>  create mode 100644 t/t3433/eucJP.txt
>
> Range-diff against v4:
>  1:  daa0c27d28 =  1:  b3d6c4e720 t0028: eliminate non-standard usage of printf
>  2:  c50964f413 !  2:  fe63a6bc44 configure.ac: define ICONV_OMITS_BOM if necessary
>     @@ Commit message
>      
>              make ICONV_OMITS_BOM=Yes
>      
>     -    However, typing the flag all the time is cumbersome and error-prone.
>     +    However, configure script wasn't taught to detect those systems.
>      
>     -    Add a check into configure script to detect this flag automatically.
>     +    Teach configure to do so.
>      
>          Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>
>      
>  3:  47888f236c !  3:  30f15075c4 t3900: demonstrate git-rebase problem with multi encoding
>     @@ t/t3900-i18n-commit.sh: test_commit_autosquash_flags eucJP fixup
>      +	new=$3
>      +	msg=$4
>      +	test_expect_failure "commit --$flag into $old from $new" '
>     -+		git checkout -b '$flag-$old-$new' C0 &&
>     -+		git config i18n.commitencoding '$old' &&
>     -+		echo '$old' >>F &&
>     -+		git commit -a -F "$TEST_DIRECTORY/t3900/'$msg'" &&
>     ++		git checkout -b $flag-$old-$new C0 &&
>     ++		git config i18n.commitencoding $old &&
>     ++		echo $old >>F &&
>     ++		git commit -a -F "$TEST_DIRECTORY"/t3900/$msg &&
>      +		test_tick &&
>      +		echo intermediate stuff >>G &&
>      +		git add G &&
>      +		git commit -a -m "intermediate commit" &&
>      +		test_tick &&
>     -+		git config i18n.commitencoding '$new' &&
>     -+		echo '$new-$flag' >>F &&
>     -+		git commit -a --'$flag' HEAD^ &&
>     ++		git config i18n.commitencoding $new &&
>     ++		echo $new-$flag >>F &&
>     ++		git commit -a --$flag HEAD^ &&
>      +		git rebase --autosquash -i HEAD^^^ &&
>      +		git rev-list HEAD >actual &&
>      +		test_line_count = 3 actual
>  4:  42115f1e33 !  4:  17165b81e5 sequencer: reencode to utf-8 before arrange rebase's todo list
>     @@ Commit message
>          Thus, t3900::test_commit_autosquash_flags is failing on musl libc.
>      
>          Reencode to utf-8 before arranging rebase's todo list.
>     -    By doing this, we also remove a breakage introduced in the previous
>     -    commit.
>     +
>     +    By doing this, we also remove a breakage noticed by a test added in the
>     +    previous commit.
>      
>          Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>
>      
>     @@ t/t3900-i18n-commit.sh: test_commit_autosquash_multi_encoding () {
>       	msg=$4
>      -	test_expect_failure "commit --$flag into $old from $new" '
>      +	test_expect_success "commit --$flag into $old from $new" '
>     - 		git checkout -b '$flag-$old-$new' C0 &&
>     - 		git config i18n.commitencoding '$old' &&
>     - 		echo '$old' >>F &&
>     + 		git checkout -b $flag-$old-$new C0 &&
>     + 		git config i18n.commitencoding $old &&
>     + 		echo $old >>F &&
>  5:  5a871d7226 =  5:  40fa759492 sequencer: reencode revert/cherry-pick's todo list
>  6:  1c6194a598 !  6:  ed6cfab5d2 sequencer: reencode squashing commit's message
>     @@ sequencer.c: static int commit_staged_changes(struct repository *r,
>      
>       ## t/t3900-i18n-commit.sh ##
>      @@ t/t3900-i18n-commit.sh: test_commit_autosquash_multi_encoding () {
>     - 	old=$2
>     - 	new=$3
>     - 	msg=$4
>     -+	squash_msg=
>     -+	if test $flag = squash; then
>     -+		squash_msg='
>     -+		subject="squash! $(head -1 expect)" &&
>     -+		printf "\n%s\n" "$subject" >> expect &&
>     -+		'
>     -+	fi
>     - 	test_expect_success "commit --$flag into $old from $new" '
>     - 		git checkout -b '$flag-$old-$new' C0 &&
>     - 		git config i18n.commitencoding '$old' &&
>     -@@ t/t3900-i18n-commit.sh: test_commit_autosquash_multi_encoding () {
>     - 		git commit -a --'$flag' HEAD^ &&
>     + 		git commit -a --$flag HEAD^ &&
>       		git rebase --autosquash -i HEAD^^^ &&
>       		git rev-list HEAD >actual &&
>      -		test_line_count = 3 actual
>      +		test_line_count = 3 actual &&
>     -+		iconv -f '$old' -t utf-8 "$TEST_DIRECTORY/t3900/'$msg'" >expect &&
>     -+		'"$squash_msg"'
>     ++		iconv -f $old -t UTF-8 "$TEST_DIRECTORY"/t3900/$msg >expect &&
>     ++		if test $flag = squash; then
>     ++			subject="$(head -1 expect)" &&
>     ++			printf "\nsquash! %s\n" "$subject" >>expect
>     ++		fi &&
>      +		git cat-file commit HEAD^ >raw &&
>     -+		(sed "1,/^$/d" raw | iconv -f '$new' -t utf-8) >actual &&
>     ++		(sed "1,/^$/d" raw | iconv -f $new -t utf-8) >actual &&
>      +		test_cmp expect actual
>       	'
>       }
>  7:  95df3cdadf <  -:  ---------- sequencer: reencode old merge-commit message
>  8:  0606b2408d <  -:  ---------- sequencer: reencode commit message for am/rebase --show-current-patch
>  -:  ---------- >  7:  def9adf97e sequencer: reencode old merge-commit message
>  -:  ---------- >  8:  2e95ca57d2 sequencer: reencode commit message for am/rebase --show-current-patch
>  -:  ---------- >  9:  860dee65f4 sequencer: fallback to sane label in making rebase todo list

  parent reply	other threads:[~2019-11-11  1:23 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-31  9:26 [PATCH 0/3] Linux with musl libc improvement Doan Tran Cong Danh
2019-10-31  9:26 ` [PATCH 1/3] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-10-31 17:41   ` Jeff King
2019-11-01  1:33     ` Danh Doan
2019-10-31 19:50   ` brian m. carlson
2019-10-31  9:26 ` [PATCH 2/3] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-10-31 18:11   ` Jeff King
2019-10-31 20:02     ` brian m. carlson
2019-11-01  1:40     ` Danh Doan
2019-10-31  9:26 ` [PATCH 3/3] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-10-31 10:38   ` Johannes Schindelin
2019-10-31 19:26     ` Jeff King
2019-11-01  4:49       ` Danh Doan
2019-11-01  8:25 ` [PATCH v2 0/3] Linux with musl libc improvement Doan Tran Cong Danh
2019-11-01  8:25   ` [PATCH v2 1/3] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-11-01 16:54     ` Jeff King
2019-11-01  8:25   ` [PATCH v2 2/3] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-11-01 16:56     ` Jeff King
2019-11-02  0:43       ` Danh Doan
2019-11-01  8:25   ` [PATCH v2 3/3] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-11-01 16:59     ` Jeff King
2019-11-02  1:02       ` Danh Doan
2019-11-02 12:20         ` Danh Doan
2019-11-05  8:00         ` Jeff King
2019-11-06  1:30           ` Junio C Hamano
2019-11-06  4:03             ` Jeff King
2019-11-06 10:03               ` Danh Doan
2019-11-07  5:56                 ` Jeff King
2019-11-06  9:19 ` [PATCH v3 0/8] Correct internal working and output encoding Doan Tran Cong Danh
2019-11-06  9:19   ` [PATCH v3 1/8] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 2/8] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 3/8] t3900: demonstrate git-rebase problem with multi encoding Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 4/8] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 5/8] sequencer: reencode revert/cherry-pick's " Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 6/8] sequencer: reencode squashing commit's message Doan Tran Cong Danh
2019-11-06  9:20   ` [PATCH v3 7/8] sequencer: reencode old merge-commit message Doan Tran Cong Danh
2019-11-06 15:39     ` Eric Sunshine
2019-11-06  9:20   ` [PATCH v3 8/8] sequencer: reencode commit message for am/rebase --show-current-patch Doan Tran Cong Danh
2019-11-07  2:56 ` [PATCH v4 0/8] Correct internal working and output encoding Doan Tran Cong Danh
2019-11-07  2:56   ` [PATCH v4 1/8] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-11-07  2:56   ` [PATCH v4 2/8] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-11-07  6:18     ` Junio C Hamano
2019-11-07  2:56   ` [PATCH v4 3/8] t3900: demonstrate git-rebase problem with multi encoding Doan Tran Cong Danh
2019-11-07  6:02     ` Jeff King
2019-11-07  6:48       ` Danh Doan
2019-11-07  8:02         ` Jeff King
2019-11-07 10:51           ` Danh Doan
2019-11-11  8:22             ` Jeff King
2019-11-07  2:56   ` [PATCH v4 4/8] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-11-07  6:04     ` Jeff King
2019-11-07  2:56   ` [PATCH v4 5/8] sequencer: reencode revert/cherry-pick's " Doan Tran Cong Danh
2019-11-07  6:06     ` Jeff King
2019-11-07  2:56   ` [PATCH v4 6/8] sequencer: reencode squashing commit's message Doan Tran Cong Danh
2019-11-07  6:15     ` Jeff King
2019-11-07  2:56   ` [PATCH v4 7/8] sequencer: reencode old merge-commit message Doan Tran Cong Danh
2019-11-07  2:56   ` [PATCH v4 8/8] sequencer: reencode commit message for am/rebase --show-current-patch Doan Tran Cong Danh
2019-11-07  6:32     ` Jeff King
2019-11-07  7:48       ` Danh Doan
2019-11-07  8:03         ` Jeff King
2019-11-07 16:32           ` Danh Doan
2019-11-08  9:43 ` [PATCH v5 0/9] Improve odd encoding integration Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 1/9] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 2/9] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 3/9] t3900: demonstrate git-rebase problem with multi encoding Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 4/9] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 5/9] sequencer: reencode revert/cherry-pick's " Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 6/9] sequencer: reencode squashing commit's message Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 7/9] sequencer: reencode old merge-commit message Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 8/9] sequencer: reencode commit message for am/rebase --show-current-patch Doan Tran Cong Danh
2019-11-08  9:43   ` [PATCH v5 9/9] sequencer: fallback to sane label in making rebase todo list Doan Tran Cong Danh
2019-11-11  1:22   ` Junio C Hamano [this message]
2019-11-11  4:02   ` [PATCH v5 0/9] Improve odd encoding integration Junio C Hamano
2019-11-11  4:43     ` Danh Doan
2019-11-11  6:14     ` Junio C Hamano
2019-11-11  6:03 ` [PATCH v6 0/9] sequencer: handle other encoding better Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 1/9] t0028: eliminate non-standard usage of printf Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 2/9] configure.ac: define ICONV_OMITS_BOM if necessary Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 3/9] t3900: demonstrate git-rebase problem with multi encoding Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 4/9] sequencer: reencode to utf-8 before arrange rebase's todo list Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 5/9] sequencer: reencode revert/cherry-pick's " Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 6/9] sequencer: reencode squashing commit's message Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 7/9] sequencer: reencode old merge-commit message Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 8/9] sequencer: reencode commit message for am/rebase --show-current-patch Doan Tran Cong Danh
2019-11-11  6:03   ` [PATCH v6 9/9] sequencer: fallback to sane label in making rebase todo list Doan Tran Cong Danh
2019-11-11  8:39     ` Jeff King
2019-11-11 16:22       ` Phillip Wood
2019-11-11 18:26     ` Johannes Schindelin
2019-11-12  4:17       ` Junio C Hamano
2019-11-11  8:40   ` [PATCH v6 0/9] sequencer: handle other encoding better Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlfsnjjf4.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).