From patchwork Sun Nov 20 10:06:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13049937 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95165C4332F for ; Sun, 20 Nov 2022 10:08:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229640AbiKTKGV (ORCPT ); Sun, 20 Nov 2022 05:06:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59260 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229462AbiKTKGU (ORCPT ); Sun, 20 Nov 2022 05:06:20 -0500 Received: from mout.web.de (mout.web.de [212.227.15.14]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D898A5E9F9 for ; Sun, 20 Nov 2022 02:06:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=web.de; s=s29768273; t=1668938766; bh=64RcOQPsVobNySgLG/XWXDVwZQeJNovvPvrxXvUqpHE=; h=X-UI-Sender-Class:Date:Subject:From:To:Cc:References:In-Reply-To; b=j07104eew2WrMtWnneFW/phjzm8ilBS0O4Y0lWmh0tJHEMX1CKqOWFwEbW3jZ5MPG Xu4KzZ0r7lSGATv8/Id7KhAVUd1HDxqVVQ/DDqYrtmjn5HYFkBMrb4+lAYmTyPeBx/ XpDkfAcOfszfo07O3VbwDlL4pDmRCRCg3MfJZ0KCFWBwfUla59Do8jfDEf+jZ155bH 3p+p4ZfLe6siOLPUSl4mvFc5aLDdTt1WlLvm/riRfEZsjl1x+RBXLMhjsdQ0GhuQTw T5wdIaLiEOVc4C64n9h3RqUC3UA/B5dxDqLk9CbapMrKpjhrubR69K+Sxb6zz96Ljn 9DraqL7PTjwYw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.154.159]) by smtp.web.de (mrweb006 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MUUAG-1oWC9G0SC8-00QX5D; Sun, 20 Nov 2022 11:06:06 +0100 Message-ID: Date: Sun, 20 Nov 2022 11:06:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: [PATCH v2 1/3] t5317: stop losing return codes of git ls-files Content-Language: en-US From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: Git List Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Junio C Hamano , Taylor Blau , Christian Couder , Jeff King References: In-Reply-To: X-Provags-ID: V03:K1:3RITmMEObXF+9Px9NzX6oclkjuCP68eqcLB3WDW6Sd+CJ7/vLFV h0mKcBwoZX2U03vA5U6TWbUmWeo3fJuH8FEaUpBd7jTzpEd4IAGjq6gnn339hY9Wbb8YYZl DDqBXjFi8ugEaERjfidLzQk7J0VXM6ElWBSiaFeVqahIzcQlPjlRBV+35XGAo19U9/ntdkR FQDeomuCWwu+tV0kwzjXQ== UI-OutboundReport: notjunk:1;M01:P0:Z0RgREgxhls=;wBnjdc0R08xG9qHuUe/Bi5EzMoi cnZIkL+jhN9rUBzShQxqwrNJtnjzGugmK1A6ONfepRCynfRCuMh26f0Ucm466hKiH2IunPjeB WAAWIGhBo5uLf1ET6pkjJ4AuXE4RZP07IppXdKFBze3u3/ZxVZ2HeoQYRdXtQk4VgX7WV/D0z HsT4BcJFxHI9nwxuUClIUYbMVfpnmpNn3QHcO/cOXYZgpekypx34+J6/3K3ZCQjRzcPp8Gdvp Ig1HDrWw778XBDMfE3IX6fStt9t6pW7iqC4SA+J9MZ1d14D77u76IT8cm3YOdb9o0NDBoTwON kThd5I2jghow3EPriGW6rm0zprZTOk1wlv3KX1CGsfX4w5O+Bb5iq+9vGWxFX0j5jUBhN5/o6 F4DhGucahV4j8vnU2JWTYs9ylVeaiQRWad/T9f52TkJyowso79GpSVOmiwxjv1zqccnxgaNoM /no6BbnNB2OJv7zZ6pZ9AII6hHf4aTWEeakfHE02HVK098XNxHnTCBr1bOi5bGMMDg3QFTlDQ iZi9BBdOeLrqLnFzDdPjuWiuWxB479oE6vEFi5DvgQBdVyvc1fDEnGXRYH2xNnoS7CQo8LmHz 6FKYvr2kFLTIjCohepeU9Evaj8sb0Js0SzK+bEdTZWJiCmgMLQM/twYDqoHjmMgzOxG8ynq1p Gfp9sYZiYX5f4kFvt5EQK+/ZhKllXGayElfQFVueUCNaWTY19I6caT88GGo25mwRrP8bkATas K0nNPy+yvGqtUCoQr8EV+GSv2b4ZT6Xe6BBu8dcftUjLTLUJVf2gykFaZiaQPOPkZwvm0Evnd IqmNY2G30whgs1I1fpDHR4+dCDFfmSUld97vM+KKHun0K6OGcSzaM5jBY1AxlAcyFFDlrUOsH HAOOrjOigSj4LWPoWWsWyGEjZ8r7YXkIXeFiGc0u1e/dM0SjMnZRbGJaYpIq/7Hjc1nM3lC00 oo3+jg== Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org fb2d0db502 (test-lib-functions: add parsing helpers for ls-files and ls-tree, 2022-04-04) not only started to use helper functions, it also started to pipe the output of git ls-files into them directly, without using a temporary file. No explanation was given. This causes the return code of that git command to be ignored. Revert that part of the change, use temporary files and check the return code of git ls-files again. Suggested-by: Ævar Arnfjörð Bjarmason Signed-off-by: René Scharfe --- t/t5317-pack-objects-filter-objects.sh | 52 ++++++++++++++------------ 1 file changed, 28 insertions(+), 24 deletions(-) -- 2.38.1 diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh index bb633c9b09..82a22ecaa5 100755 --- a/t/t5317-pack-objects-filter-objects.sh +++ b/t/t5317-pack-objects-filter-objects.sh @@ -24,8 +24,9 @@ parse_verify_pack_blob_oid () { } test_expect_success 'verify blob count in normal packfile' ' - git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 | - test_parse_ls_files_stage_oids | + git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \ + >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r1 pack-objects --revs --stdout >all.pack <<-EOF && @@ -123,8 +124,8 @@ test_expect_success 'setup r2' ' ' test_expect_success 'verify blob count in normal packfile' ' - git -C r2 ls-files -s large.1000 large.10000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 large.10000 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r2 pack-objects --revs --stdout >all.pack <<-EOF && @@ -161,8 +162,8 @@ test_expect_success 'verify blob:limit=1000' ' ' test_expect_success 'verify blob:limit=1001' ' - git -C r2 ls-files -s large.1000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r2 pack-objects --revs --stdout --filter=blob:limit=1001 >filter.pack <<-EOF && @@ -179,8 +180,8 @@ test_expect_success 'verify blob:limit=1001' ' ' test_expect_success 'verify blob:limit=10001' ' - git -C r2 ls-files -s large.1000 large.10000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 large.10000 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r2 pack-objects --revs --stdout --filter=blob:limit=10001 >filter.pack <<-EOF && @@ -197,8 +198,8 @@ test_expect_success 'verify blob:limit=10001' ' ' test_expect_success 'verify blob:limit=1k' ' - git -C r2 ls-files -s large.1000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r2 pack-objects --revs --stdout --filter=blob:limit=1k >filter.pack <<-EOF && @@ -215,8 +216,8 @@ test_expect_success 'verify blob:limit=1k' ' ' test_expect_success 'verify explicitly specifying oversized blob in input' ' - git -C r2 ls-files -s large.1000 large.10000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 large.10000 >ls_files_result && + test_parse_ls_files_stage_oids expected && echo HEAD >objects && @@ -233,8 +234,8 @@ test_expect_success 'verify explicitly specifying oversized blob in input' ' ' test_expect_success 'verify blob:limit=1m' ' - git -C r2 ls-files -s large.1000 large.10000 | - test_parse_ls_files_stage_oids | + git -C r2 ls-files -s large.1000 large.10000 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r2 pack-objects --revs --stdout --filter=blob:limit=1m >filter.pack <<-EOF && @@ -289,8 +290,9 @@ test_expect_success 'setup r3' ' ' test_expect_success 'verify blob count in normal packfile' ' - git -C r3 ls-files -s sparse1 sparse2 dir1/sparse1 dir1/sparse2 | - test_parse_ls_files_stage_oids | + git -C r3 ls-files -s sparse1 sparse2 dir1/sparse1 dir1/sparse2 \ + >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r3 pack-objects --revs --stdout >all.pack <<-EOF && @@ -341,8 +343,9 @@ test_expect_success 'setup r4' ' ' test_expect_success 'verify blob count in normal packfile' ' - git -C r4 ls-files -s pattern sparse1 sparse2 dir1/sparse1 dir1/sparse2 | - test_parse_ls_files_stage_oids | + git -C r4 ls-files -s pattern sparse1 sparse2 dir1/sparse1 dir1/sparse2 \ + >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r4 pack-objects --revs --stdout >all.pack <<-EOF && @@ -359,8 +362,8 @@ test_expect_success 'verify blob count in normal packfile' ' ' test_expect_success 'verify sparse:oid=OID' ' - git -C r4 ls-files -s dir1/sparse1 dir1/sparse2 | - test_parse_ls_files_stage_oids | + git -C r4 ls-files -s dir1/sparse1 dir1/sparse2 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r4 ls-files -s pattern >staged && @@ -379,8 +382,8 @@ test_expect_success 'verify sparse:oid=OID' ' ' test_expect_success 'verify sparse:oid=oid-ish' ' - git -C r4 ls-files -s dir1/sparse1 dir1/sparse2 | - test_parse_ls_files_stage_oids | + git -C r4 ls-files -s dir1/sparse1 dir1/sparse2 >ls_files_result && + test_parse_ls_files_stage_oids expected && git -C r4 pack-objects --revs --stdout --filter=sparse:oid=main:pattern >filter.pack <<-EOF && @@ -400,8 +403,9 @@ test_expect_success 'verify sparse:oid=oid-ish' ' # This models previously omitted objects that we did not receive. test_expect_success 'setup r1 - delete loose blobs' ' - git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 | - test_parse_ls_files_stage_oids | + git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \ + >ls_files_result && + test_parse_ls_files_stage_oids expected && for id in `cat expected | sed "s|..|&/|"` From patchwork Sun Nov 20 10:07:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13049936 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F99CC433FE for ; Sun, 20 Nov 2022 10:08:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229642AbiKTKIA (ORCPT ); Sun, 20 Nov 2022 05:08:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59932 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229462AbiKTKH7 (ORCPT ); Sun, 20 Nov 2022 05:07:59 -0500 Received: from mout.web.de (mout.web.de [212.227.15.4]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02C5D711BD for ; Sun, 20 Nov 2022 02:07:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=web.de; s=s29768273; t=1668938864; bh=ss9oV8qrC1wV+87ts7pUNHi/Ib17sbePie+Y5nQZtYw=; h=X-UI-Sender-Class:Date:Subject:From:To:Cc:References:In-Reply-To; b=wKp9LcUnS/BglIMVG5jwWI06Y/Xc9T/Ql9p48d9MhBt608W7lQVpiQ/ZWfjQ3Mllx d+HymgdBrhvrwOrKibKQCmPCD1TMlbMkFSQufWa00UZkqqprYNuqx4vgsibKzTaGXh LT/U/RR0OvJ3upoazbTkHU6mWiNOMmz3vEdwkiUx61uRxLY4zd6wY1/JGrISuRvemO 2/8AyjBVov/bes/S3L9BytoUp9f3uuI6RslDU6uR1MikIv1V3/uyhtR+NWoQt3dOev CXyDOIXWWc4sIO/rwPE3zpuxhESEWZ23+EY/9o0enPcGxzCo6iYNUjftiDwcwVP8T3 mgrsqY4pICmuw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.154.159]) by smtp.web.de (mrweb005 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MQ8Wa-1oabya38U4-00M3TL; Sun, 20 Nov 2022 11:07:44 +0100 Message-ID: <0c6722e6-e6bc-0f7d-5f0a-1d242359e37d@web.de> Date: Sun, 20 Nov 2022 11:07:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: [PATCH v2 2/3] t5317: demonstrate failure to handle multiple --filter options Content-Language: en-US From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: Git List Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Junio C Hamano , Taylor Blau , Christian Couder , Jeff King References: In-Reply-To: X-Provags-ID: V03:K1:nXEAGIET0akLFuPNqaa5KAJ1qMAcvnoLMhUag7DgL5xVMFhpexq vPKGT1apgPlOqfWeYBzAiEUQzB3OHViK1SKvPZ2fG8fFlnxmiAbEMPAL88hpbl3cB/vxVDZ vHkc3zkbDBPuqYxS8VIZ4gTlIw5obRpaAZd+k6yWgmJ1k83AhUHP80m+tinXh98chNL7n7l LySi5fCRu0vhJMMJGPPWQ== UI-OutboundReport: notjunk:1;M01:P0:JebrDoyL0Hk=;QLt8IT5VucH4W0gT2SwDlwDqOq+ MhSDOkrUt3e/knI3ZloGzWzZm7FkN4kIBvssZ8/ewsu3bNk4MQo0jGsDDl6pUJbV54Lpw+If1 aRWnbghZo905leFeXIZW2PmBBm2cju78jIhwf9+WmPydx5kUnIQzIWJ0tSTmdqor/z91JIcm9 Hew/81VfIY2ZXvfRofX9A5bQNdkGLGqBgy2K9sQDBPq+gIJ5G0wSiiZWki+zIPgQQeeL9Tnj1 k3M3v0MDc1c9fFuMEiCgkwd+IEu3ZTcZtfOKd+YpUDqeouBEphZcjK4l/a+gPF09UR2fg7K1G wXGbjluuAtdnIG/APiowH80l3b/6wNwkPTUZGyWvawarB0GhtKvVSznk/+QgeMaPEbV/IzCMQ ry0CUmltRbv9IHx3Y62RtBKIhif70zHYhrqHLAgspKV+xPvON1BD2KWYd6NCM+X0j4949dBV1 37uG3XXIzzqfRvs+TA3tN3a1/T1/+qpwGUD1gbz9xYljQS10eNnEbn66V+Mwkd7783IOQyblh cNcYhVD5fyKIPkt7p6FGggshpZ2GAOuM86bXtWqIVfdJXq9UqQeuqKLA/EdlTHqtGvol+hHCZ qTaySabgt92XmUWMVGq9xY9mA8FFWvWkYkTKJjYinHbOTc67ZQ69xu2b0bWnFAUSuNrEdEBAg 4utvqvHWoErwYuuH6UFscSBP+7nlEeDYdZv/4OLZnJvSU0Sm1/v0jaO22eRmEj8GEZG59kpz/ a8/Dqm7h6lI73b5Evs7HPYpUGXJXmotZApjkvk4kn4EHinb8eDl698QORHmzs/tcfDDFe61Rz TCJSN/pjREtvjqrZPAiy3F00ufovQYV2oLZN+YXzvNhHak7CnQgyO0UxE+M95ww3KTMUfAshs mUGy5rD2kIDTBnVn6t27Sm9kXuvNENAKyXR1xTbH7DCIERAB0hbiEjXQsefSfQF9ARnSF8wwJ MVWS8w== Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org git pack-objects should accept multiple --filter options as documented in Documentation/rev-list-options.txt, but currently the last one wins. Show that using tests with multiple blob size limits. Signed-off-by: René Scharfe --- t/t5317-pack-objects-filter-objects.sh | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) -- 2.38.1 diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh index 82a22ecaa5..25faebaada 100755 --- a/t/t5317-pack-objects-filter-objects.sh +++ b/t/t5317-pack-objects-filter-objects.sh @@ -265,6 +265,44 @@ test_expect_success 'verify normal and blob:limit packfiles have same commits/tr test_cmp expected observed ' +test_expect_failure 'verify small limit and big limit results in small limit' ' + git -C r2 ls-files -s large.1000 >ls_files_result && + test_parse_ls_files_stage_oids expected && + + git -C r2 pack-objects --revs --stdout --filter=blob:limit=1001 \ + --filter=blob:limit=10001 >filter.pack <<-EOF && + HEAD + EOF + git -C r2 index-pack ../filter.pack && + + git -C r2 verify-pack -v ../filter.pack >verify_result && + grep blob verify_result | + parse_verify_pack_blob_oid | + sort >observed && + + test_cmp expected observed +' + +test_expect_success 'verify big limit and small limit results in small limit' ' + git -C r2 ls-files -s large.1000 >ls_files_result && + test_parse_ls_files_stage_oids expected && + + git -C r2 pack-objects --revs --stdout --filter=blob:limit=10001 \ + --filter=blob:limit=1001 >filter.pack <<-EOF && + HEAD + EOF + git -C r2 index-pack ../filter.pack && + + git -C r2 verify-pack -v ../filter.pack >verify_result && + grep blob verify_result | + parse_verify_pack_blob_oid | + sort >observed && + + test_cmp expected observed +' + # Test sparse:path= filter. # !!!! # NOTE: sparse:path filter support has been dropped for security reasons, From patchwork Sun Nov 20 10:13:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13049940 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D42DFC4332F for ; Sun, 20 Nov 2022 10:14:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229558AbiKTKOF (ORCPT ); Sun, 20 Nov 2022 05:14:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32886 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229462AbiKTKOD (ORCPT ); Sun, 20 Nov 2022 05:14:03 -0500 Received: from mout.web.de (mout.web.de [212.227.15.3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 51D2D1D336 for ; Sun, 20 Nov 2022 02:14:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=web.de; s=s29768273; t=1668939229; bh=HrXBCWqECipdEYR7iTKtuLzwh1wWaQmnyAR0UoLzaoY=; h=X-UI-Sender-Class:Date:Subject:From:To:Cc:References:In-Reply-To; b=Nb+PvNtzkef3zN05wUBz1PMxI/tERL/XJb4EumgXivFHYwAbLuqmYJAm2vUMIXNgP 2y/IP6Mg3PGLJTkCbcovRMbneOc/74e+pgmUhJxhizt3+nCgrFH/PZRVsgyaIdTuu3 ZgirJOfMWp1irGG40UZtgws5Dky/ZprTSIMlfSk/VTyavHVInVf9kgLMb8179UfgJt 1qfLl0A8Nuqm1BP+sgn9TLgrmA4Iu9hA4UMYtAlHDkUdNGnENWetG+D7XRAjSUE4FQ fiW3bIlK4X5Yq5s4xi0m28U+Q7ejrFskA8PzMgaHFxBRkeiuxVmJ3OVR2MaU05ni0l ToMuUQ0UpdDcg== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.154.159]) by smtp.web.de (mrweb005 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MLifk-1of1vI3ttg-00HdYk; Sun, 20 Nov 2022 11:13:48 +0100 Message-ID: Date: Sun, 20 Nov 2022 11:13:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: [PATCH v2 3/3] Revert "pack-objects: lazily set up "struct rev_info", don't leak" Content-Language: en-US From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: Git List Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Junio C Hamano , Taylor Blau , Christian Couder , Jeff King References: In-Reply-To: X-Provags-ID: V03:K1:Y1QjA4fZgxEmvqOcrAyrAd/3fqkrT1CyQZEnrOT1X+N6EmrIofR xouyE1mA4IJ02idzoVPGN4NNbSvzMLOw+iXWm38jVaxF5yPi5ANRHq34TDdKQVkqOQ0IC2G 1AuArAFoMeL+6dlXy5CBGRNEN0mrdqjZDvVyJPJ7YlzUtGp8UH+u4dNBtATEel/W419BA3y 2i+HBnmWdetxNp+9qqtTA== UI-OutboundReport: notjunk:1;M01:P0:jZiFHxPm4lo=;gq+tdJJZzDlbvPadWsJESQ98yfX aEDgykZcm0/2n95KPln6AfSEBRk1bsEgp7R8XLAw2AfpkZxRyKJJxy1OTMV0US8BJQm6JevZz b+m86fWt2LaBsgtElmTGDfEmGyRy3xqZ6l9apsIKf1L0jof5MpKeb4fu7FHv+8R7ET1HwN3nv 6wA3ZCMN7rGkheKhoIQ/0onySgtWjRCOvRCOkyB+9KTz2fDo3SZwLr9XLu3Z42CpqX8KwhrC9 X/oQwxsQ8bLsVuJegu2oI7m58JWqSsW1PFp4/OHOVQb6ZI57hnHSbHmHnmNdMRqAYIjgGLDwv LWwewH/9DiV4bxcu/KH0JH89HgN8iRtw432ai5xqLLsiSvIOmgE82Av+ULy23n5RQcYe3cPw4 ZzLLg6dhizStDaVhZrQVWKG5PvNL0EGuNguMrfKppd9NdtRQqWg0ooJcsj3Sspybwwlt7ijLU IwDEKaWuAQn42BcsnpJyO0vT0gsKegwkMW6NaMG2As6ZiUu6AG+s9tfgin1FxhQT5/SRspGte HsCi0ZX6/T38gwxD33krXmSpdp+pnST7MnfTA2OuuCVKIJ+6l9ygblF3MjfsaAXA7q58UQ0B7 3XQweGqiFdak4xAA8YzSKSARZEIn1PlyaYMpNU+h+2Gk5oG9oD0zihYyCF1hwmgMX8QllPIb+ MpZuQ8kbFG/IWPG2c1QVeEPrAyDkOvkIeNn0c1dxCuVGysHAXQTHU/6ITsHKRUZJlGgfn2tWM 923J+E/ci+4f6kUsJEJGPY8H/s0Z3zXdtnnr4RU6i0bS9wfkGBvqQ7UOhK/56+zyFZBYTJv3L 2icM6CQ5uwYda4QZk9ZaBJWwVdOL+USxoVuz9WYkJiLwwjD28eo6YYB0TOuFLAabQibfd859G 9Nvvb9aPlwYdVKix5blV0Y6XB9wiET12kPO9ZMmLH88ySqx93ok9zP31nw/Q0Os40GT25+u2H AZUqVw== Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This reverts commit 5cb28270a1ff94a0a23e67b479bbbec3bc993518. 5cb28270a1 (pack-objects: lazily set up "struct rev_info", don't leak, 2022-03-28) avoided leaking rev_info allocations in many cases by calling repo_init_revisions() only when the .filter member was actually needed, but then still leaking it. That was fixed later by 2108fe4a19 (revisions API users: add straightforward release_revisions(), 2022-04-13), making the reverted commit unnecessary. 5cb28270a1 broke support for multiple --filter options by calling repo_init_revisions() for each one, clobbering earlier state and leaking rev_info allocations. Reverting it fixes that regression. Luckily it only affects users that run pack-objects directly. Internal callers in bundle.c and upload-pack.c combine multiple filters into a single option using "+". 5cb28270a1 introduced OPT_PARSE_LIST_OBJECTS_FILTER_INIT. It relies on being able to faithfully convert function pointers to object pointers and back, which is undefined in C99. The standard mentions this ability as a common extension in its annex J (J.5.7 Function pointer casts), but avoiding that dependency is more portable. The macro hasn't been used since, OPT_PARSE_LIST_OBJECTS_FILTER is enough so far. So revert 5cb28270a1 fully to fix support for multiple --filter options and avoid reliance on undefined behavior. Its intent is better served by the release_revisions() call added by 2108fe4a19 alone -- we just need to do it unconditionally, mirroring the unconditional call of repo_init_revisions(). Helped-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: René Scharfe Signed-off-by: René Scharfe Signed-off-by: Ævar Arnfjörð Bjarmason --- builtin/pack-objects.c | 31 +++++--------------------- list-objects-filter-options.c | 4 ---- list-objects-filter-options.h | 24 +++----------------- t/t5317-pack-objects-filter-objects.sh | 2 +- 4 files changed, 10 insertions(+), 51 deletions(-) -- 2.38.1 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 573d0b20b7..3e74fbb0cd 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4149,21 +4149,6 @@ static int option_parse_cruft_expiration(const struct option *opt, return 0; } -struct po_filter_data { - unsigned have_revs:1; - struct rev_info revs; -}; - -static struct list_objects_filter_options *po_filter_revs_init(void *value) -{ - struct po_filter_data *data = value; - - repo_init_revisions(the_repository, &data->revs, NULL); - data->have_revs = 1; - - return &data->revs.filter; -} - int cmd_pack_objects(int argc, const char **argv, const char *prefix) { int use_internal_rev_list = 0; @@ -4174,7 +4159,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) int rev_list_index = 0; int stdin_packs = 0; struct string_list keep_pack_list = STRING_LIST_INIT_NODUP; - struct po_filter_data pfd = { .have_revs = 0 }; + struct rev_info revs; struct option pack_objects_options[] = { OPT_SET_INT('q', "quiet", &progress, @@ -4265,7 +4250,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) &write_bitmap_index, N_("write a bitmap index if possible"), WRITE_BITMAP_QUIET, PARSE_OPT_HIDDEN), - OPT_PARSE_LIST_OBJECTS_FILTER_INIT(&pfd, po_filter_revs_init), + OPT_PARSE_LIST_OBJECTS_FILTER(&revs.filter), OPT_CALLBACK_F(0, "missing", NULL, N_("action"), N_("handling for missing objects"), PARSE_OPT_NONEG, option_parse_missing_action), @@ -4284,6 +4269,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) read_replace_refs = 0; + repo_init_revisions(the_repository, &revs, NULL); + sparse = git_env_bool("GIT_TEST_PACK_SPARSE", -1); if (the_repository->gitdir) { prepare_repo_settings(the_repository); @@ -4385,7 +4372,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (!rev_list_all || !rev_list_reflog || !rev_list_index) unpack_unreachable_expiration = 0; - if (pfd.have_revs && pfd.revs.filter.choice) { + if (revs.filter.choice) { if (!pack_to_stdout) die(_("cannot use --filter without --stdout")); if (stdin_packs) @@ -4472,16 +4459,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) read_cruft_objects(); } else if (!use_internal_rev_list) { read_object_list_from_stdin(); - } else if (pfd.have_revs) { - get_object_list(&pfd.revs, rp.nr, rp.v); - release_revisions(&pfd.revs); } else { - struct rev_info revs; - - repo_init_revisions(the_repository, &revs, NULL); get_object_list(&revs, rp.nr, rp.v); - release_revisions(&revs); } + release_revisions(&revs); cleanup_preferred_base(); if (include_tag && nr_result) for_each_tag_ref(add_ref_tag, NULL); diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 5339660238..ee01bcd2cc 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -290,10 +290,6 @@ int opt_parse_list_objects_filter(const struct option *opt, const char *arg, int unset) { struct list_objects_filter_options *filter_options = opt->value; - opt_lof_init init = (opt_lof_init)opt->defval; - - if (init) - filter_options = init(opt->value); if (unset || !arg) list_objects_filter_set_no_filter(filter_options); diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h index 7eeadab2dd..64af2e29e2 100644 --- a/list-objects-filter-options.h +++ b/list-objects-filter-options.h @@ -107,31 +107,13 @@ void parse_list_objects_filter( struct list_objects_filter_options *filter_options, const char *arg); -/** - * The opt->value to opt_parse_list_objects_filter() is either a - * "struct list_objects_filter_option *" when using - * OPT_PARSE_LIST_OBJECTS_FILTER(). - * - * Or, if using no "struct option" field is used by the callback, - * except the "defval" which is expected to be an "opt_lof_init" - * function, which is called with the "opt->value" and must return a - * pointer to the ""struct list_objects_filter_option *" to be used. - * - * The OPT_PARSE_LIST_OBJECTS_FILTER_INIT() can be used e.g. the - * "struct list_objects_filter_option" is embedded in a "struct - * rev_info", which the "defval" could be tasked with lazily - * initializing. See cmd_pack_objects() for an example. - */ int opt_parse_list_objects_filter(const struct option *opt, const char *arg, int unset); -typedef struct list_objects_filter_options *(*opt_lof_init)(void *); -#define OPT_PARSE_LIST_OBJECTS_FILTER_INIT(fo, init) \ - { OPTION_CALLBACK, 0, "filter", (fo), N_("args"), \ - N_("object filtering"), 0, opt_parse_list_objects_filter, \ - (intptr_t)(init) } #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \ - OPT_PARSE_LIST_OBJECTS_FILTER_INIT((fo), NULL) + OPT_CALLBACK(0, "filter", fo, N_("args"), \ + N_("object filtering"), \ + opt_parse_list_objects_filter) /* * Translates abbreviated numbers in the filter's filter_spec into their diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh index 25faebaada..5b707d911b 100755 --- a/t/t5317-pack-objects-filter-objects.sh +++ b/t/t5317-pack-objects-filter-objects.sh @@ -265,7 +265,7 @@ test_expect_success 'verify normal and blob:limit packfiles have same commits/tr test_cmp expected observed ' -test_expect_failure 'verify small limit and big limit results in small limit' ' +test_expect_success 'verify small limit and big limit results in small limit' ' git -C r2 ls-files -s large.1000 >ls_files_result && test_parse_ls_files_stage_oids expected &&