From patchwork Tue Jun 21 13:59:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Oliver X-Patchwork-Id: 12889327 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 D33EAC43334 for ; Tue, 21 Jun 2022 13:59:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350531AbiFUN7x (ORCPT ); Tue, 21 Jun 2022 09:59:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50652 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229717AbiFUN7v (ORCPT ); Tue, 21 Jun 2022 09:59:51 -0400 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2065.outbound.protection.outlook.com [40.107.237.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7ABB513E08 for ; Tue, 21 Jun 2022 06:59:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TuXTvyN57pF9fjY9x7Wl3VRUG/NfNYqCqRPxS0IYnQ6i7fAC72DsMxEC/PNlSap01lSahq6RlOThxVsdceJwGKbl3RopOh+Lu2MbspdQK0T1BmuU2zq+l+KLD9o137UZqTN+pzkfDyRsSw4CKMJ3VOxU85RqdLiRmm8cfdLdGIe2167f1SkAuQ03sfbt+94Ll5WSUkzZZLHcuw2EoDseOuBtnFFhPCY2osHjwRmN4mR5qnvwVNDb2IAPtadY3IjjjZeT6x23LDCoyM/IRpzlvgQp25Y+8qTyaJzDW9HHM7EcwUooEXHXI2JF2xWyBpzgAUl2YVa/jVemPKfpunPLxQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=0ORM+eDn4ruRZ4hmh7PK3zhB68zKBiOnjuD7gqQ0ZC4=; b=DvM/ryaB6E8zjsd7kQSP6lN/tVq45AZ4l9pMq7GObq/WbJAD+Je2ZscsOU2fy2I6KCGQ+8KM4nf6bUUGJO2jqiz2NelMqPUnJrD45ttavcjE3EbvpXmVwu3L+cns136D0A9Oi4ESMjVu/Oe3sG/AWnDp4dc5d0a3ZIsSNsXoyJGV7RIecM713dtCJ8h+soroNXHZq9St1aH9R+zzoHc8O1B9XWJt3cU+Nvi+l+2/3MWqyYJMTE5NBcflemP6RhQywE2xdN4VxnEZIwzCwX/BAIrvL1XZhyxPijC+of+tTsV1Zsgm6rczD50LiJrjrISj3UnKUjylpvE928l30Z9AYQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=roku.com; dmarc=pass action=none header.from=roku.com; dkim=pass header.d=roku.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=roku.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=0ORM+eDn4ruRZ4hmh7PK3zhB68zKBiOnjuD7gqQ0ZC4=; b=Jk5a5vTkk1PqGZHEfGNiminAO6EXU2DnMY1OBv1Yx/kxoYjdPv+jtdQMRdtU43OLU1NB5J2VoNW6wJ6X6NgqBxgW72Td/GbigbW4vx9LMFIJXbDq7VMV2NjVMfNyoJ6uVNJtVSlozHY/eSCpTjovc8zEMkdBbjrtrryV7TimJsfZkMQi2wVY462fSQSxUS2c9bpPYaSkaGpVRzOoLa0gLtXQdwTFDBicgWJWRg1BywdjgSdwRUrp1/GLhlqONhBrifXaGE4hjTqdIR3EJA6GnLACeZm2Dyk91XSdyEuzghK4uegwZFlr9q7L1KiKZiZPd3Dw4i0jXGd9Cmr35g54KQ== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=roku.com; Received: from CO1PR01MB7225.prod.exchangelabs.com (2603:10b6:303:160::21) by BN7PR01MB3684.prod.exchangelabs.com (2603:10b6:406:88::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5353.20; Tue, 21 Jun 2022 13:59:45 +0000 Received: from CO1PR01MB7225.prod.exchangelabs.com ([fe80::1062:158b:2993:5535]) by CO1PR01MB7225.prod.exchangelabs.com ([fe80::1062:158b:2993:5535%6]) with mapi id 15.20.5353.022; Tue, 21 Jun 2022 13:59:45 +0000 Message-ID: <748f39a9-65aa-2110-cf92-7ddf81b5f507@roku.com> Date: Tue, 21 Jun 2022 14:59:39 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: [PATCH] mktree: do not check type of remote objects To: Junio C Hamano Cc: Jeff King , Taylor Blau , Derrick Stolee , git@vger.kernel.org, jonathantanmy@google.com References: <77035a0f-c42e-5cb3-f422-03fe81093adb@roku.com> <0067c46a-7bfd-db9c-5156-16f032814464@github.com> <797af8c8-229f-538b-d122-8ea48067cc19@roku.com> <574dc4a9-b3c7-1fd3-8c0e-39071117c7f0@github.com> <1566aed1-a38f-a9ca-241c-21b56d732328@roku.com> From: Richard Oliver In-Reply-To: X-ClientProxiedBy: LO4P123CA0270.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:194::23) To CO1PR01MB7225.prod.exchangelabs.com (2603:10b6:303:160::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: bdb28fe8-d95d-4cc8-aff5-08da538e4c18 X-MS-TrafficTypeDiagnostic: BN7PR01MB3684:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: J9WTBhrYvhdePh4hIJQesq5/wtwp4dxxKoi3WdryJGtzYg2t5gtOcbGqcSWc2WAMuYyvAZE+lXuE0Iixmf9qC5Uxyr2FfAFTf1CUThY8T2wncGGnm/NBwDruDDrDnEIVGEk3+ZKVKrpj65I2f5ipEnAjEVAHdORe/IMGuFwR2H5P22yx9R1K9n2Ssr4RLMY6Q3eJukQfAT0zefdFWP7RDDx3alIdM/232vZIglXzwajPi9u948BGnouQ9dYrJQLBX5vsSt/PjVzMImYI00PZHBP4wJzAbFup9dkjzENhdD5fJO79iaHmqDj/E/A39aKaw/CCF06yXjCj4FingcngE/7oBZ+m+1LQNPots4KtUIM0Sp9rPrWGuMuiRtACzBSTaCsXC7bX5NKxRpBc31FEI9a3opZWRy8MJ+UXRiMH0g4XiZ4iqvaX7KQzFAp7wSoJ0Q3xQA1+K/EeOP1CCsooJXpeSfUrkdSB6FLemJbxsBZuU4amDrMaSfb09RuOuJ/ckfm5QJ9o/7SxywUe8DYpIiY3ZbDwT3iGEq2+qQgF7DcxFOpJ3DAWLuSZ+TX1tILRyEPQehkRO+l3sYSspHwTBumu3u8wlRUyUgXuUpZZVqjF8Oqn6nBJHGdtYX4BJbDIZ+RP4wwxjpTF8lSo/Di3Z6KgFobXiYq0ThKahyRScBRekXVLQt/UWFCRidaj6Vvjm3fCexmRtzIFEEGPHeNN0w== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO1PR01MB7225.prod.exchangelabs.com;PTR:;CAT:NONE;SFS:(13230016)(39850400004)(136003)(346002)(376002)(396003)(366004)(66556008)(66476007)(4326008)(2616005)(66946007)(316002)(8676002)(41300700001)(36756003)(186003)(31686004)(83380400001)(8936002)(478600001)(6512007)(6666004)(5660300002)(86362001)(31696002)(53546011)(2906002)(26005)(6506007)(54906003)(38100700002)(6916009)(6486002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?q?CMSRkaCqmuwbWWTLuLf3h3XyTOo6?= =?utf-8?q?0eAM5Sjp6YNVF5l8zL5rrzJ6plJClVz4FgMA3n2XZ5l9hO3EE4/BCJQIdYvl/M9i4?= =?utf-8?q?tUyZai/Pm/9+szx8Y+jgt0DwZYs3rqKDlWX7Vb8FTScw+D+58qZwAgR8q5ozrnsMN?= =?utf-8?q?UNM8CkO98s6UbfZZY3k5V+olBZvwKHwkA1nrk9Rn0ERk61t8b2Kdh3cd5xYPtwJMe?= =?utf-8?q?rsFBRu9CmWbMMUtAayz8wY4EM+XYbzP0WQq/BOsEfdYDp/BuN8ATD7ZQ5Fz119oZS?= =?utf-8?q?TEcxkNcZNc5Z/kQR8DbB9d/Ej0uYoUNXrvDDk888Q9s5m3Ze/IGtB2/AKjYoGVsGG?= =?utf-8?q?OjZXMwny6hxQIZTZ/kturOD20lu43feFgXD5YGZChVGqbyvOCALKYEMrk+xq7Dk1w?= =?utf-8?q?suJWnGHJuo0iuZhImVIYrXa+cJWGMr65nZ5xXYf45OZPlymyeypcPgAVK/LCkIVLt?= =?utf-8?q?7U0e/iPD+z8L96/7OYfHaNcDpJM5BBgY4sINPmIQ4A3HJ0Iwb+MPwZsnJXxkV5gC+?= =?utf-8?q?ZBXwuZZMeORGyirD7lyqpxk99ZCSECGSYGolYdhreVoAHr4uA296RcS1e4+2H+jGP?= =?utf-8?q?BRRsb5aUXKBUaBqoPLuZtpV/Dp5OGtYah7is9m/u8DKuo73Yiv0dHEswJ/HHRv7i5?= =?utf-8?q?gYrBEPysD0IT8SbL+1hgKf6NaASiH9qN7LlI4HZV2eME5AZWC9ltgctgfAym9yv2j?= =?utf-8?q?OI0zad4IP+q9RPzG2QBpKhGA1i2Vj2HN45WC5jICr/Da529kbuiAY2zVXeO6lMhLL?= =?utf-8?q?jaZucEELEJQhZ3FcpgZTPqQ5sceY3JeugBi0Wn0S+BhezLmvfHSkAjoSHJWKoVHdq?= =?utf-8?q?G/qZS64M4F1mi6pKlu2KNc/HS8JzCO1E2KtEcpQJcDfmNC180NI5mnEBO2WKD8nh6?= =?utf-8?q?enkSJpXkMallxF6KIHqGcKdxIVD86MHY6l+VWfwer/EAwRO8c+iEQIt9xjctLedNZ?= =?utf-8?q?MpShUz7Xy+QOkQvJ8exKUNHkLltLrlEWHU2xZXpdKKHHEV/HZM2NC3VwN3JB0Dc5x?= =?utf-8?q?xyTcG+VpDuqGRdgZvSlpHc1mZQzKVNP3SNGTN17gXBTvFZcb/DKo6zxRwPv2nAemh?= =?utf-8?q?N6IyNygQ9emfGp4VjO2r68ZnEyhfPp+hx1ScPS/6pAwVgi9QgmbmmA5wZk5m0CdD0?= =?utf-8?q?cqEEw/gLLpPgcCKTEmbjJQXUFXv7lt4iShifilc6il8kJHreW+Fha8gu3/kPhgVay?= =?utf-8?q?wwg0ln2n33wFx/cIa2GE+LEEPM+owO8cU6y1Vsm4khMXySxWF7WrH2n19tu3RzquI?= =?utf-8?q?tYfal2ytcXn0feHO2CTCxBgH/bnjoeFnRivP7jU89x8WSHTREfoK64Lp2YS8RW+Yo?= =?utf-8?q?3/DW05GRolOtKQf18uR/Mz9nez6lH4Z/1AFBuBDpRrXdpoyeSNH0Incht4VM9ihNH?= =?utf-8?q?VAQnTOTf5/fejSmf19GqqVSgtPfSIN29Y1jjS2B9/ShGvw1RYaF3TZsEw+pa0HdwJ?= =?utf-8?q?KYLrcjqGqmqo41GZD9to6bXaxjYHkiUD6S7xKFgh0UjRVNizgispnZwhYDdtb297V?= =?utf-8?q?vWaCtGPP6uVTWWgzvM148xn4TcV37zkpGRHSR60S4XYWOrRUIQ1doCztb2V8izoBY?= =?utf-8?q?XrgLxI646ny2OYdlqH/G/38Ikt18D66/Gkbezw+5OpHuD011KaApGtlOG6ZsELk/I?= =?utf-8?q?rno/+jn1azR/78ubHXIhO+rklUYPAcIA=3D=3D?= X-OriginatorOrg: roku.com X-MS-Exchange-CrossTenant-Network-Message-Id: bdb28fe8-d95d-4cc8-aff5-08da538e4c18 X-MS-Exchange-CrossTenant-AuthSource: CO1PR01MB7225.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jun 2022 13:59:45.6306 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 00fc7e42-ad65-4c4c-ab54-848ba124a5b7 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: RlKr3nf47mogUzOJXn4YFaXTxnN7QnKBQZig5tHnZO1RqrVNNDNxfV2wHtn5BG4rZwByjPZOIklp1DWA1C3wnQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR01MB3684 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 16/06/2022 18:44, Junio C Hamano wrote: > This patch would be a good first cut as a starting point, but we > probably can do better by doing oid_object_info_extended() call with > OBJECT_INFO_SKIP_FETCH_OBJECT bit (and probably QUICK bit, too) set, > with the current code structure. > > And when we do so, the title would not match the purpose of the > change. The verification was disabled with "--missing" all along > and that is not what we are changing. What we will be fixing is the > wasteful implementation. > > mktree: do not check types of remote objects > > With 31c8221a (mktree: validate entry type in input, 2009-05-14), we > called the sha1_object_info() API to obtain the type information, > but allowed the call to silently fail when the object was missing > locally, so that we can sanity-check the types opportunistically > when the object did exist. > > The implementation is understandable because back then there was no > lazy/on-demand downloading of individual objects from the promisor > remotes that causes a long delay and materializes the object, hence > defeating the point of using "--missing". The design is hurting us > now. > > We could bypass the opportunistic type/mode consistency check > altogether when "--missing" is given, but instead, use the > oid_object_info_extended() API and tell it that we are only > interested in objects that locally exist and are immediately > available by passing OBJECT_INFO_SKIP_FETCH_OBJECT bit to it. That > way, we will still retain the cheap and opportunistic sanity check > for local objects. I've prepared a patch below as per your suggestion. As a side note, do you think we need to re-work some uses of the word 'missing' in the documentation? Some uses of the word, such as in mktree, predate the concept of promisor remotes. The partial-clone.txt documentation differentiates between missing "due to a partial clone or fetch" and missing "due to repository corruption". Would making such a distinction elsewhere be useful? Cheers, Richard -- >8 -- Subject: [PATCH] mktree: do not check type of remote objects With 31c8221a (mktree: validate entry type in input, 2009-05-14), we called the sha1_object_info() API to obtain the type information, but allowed the call to silently fail when the object was missing locally, so that we can sanity-check the types opportunistically when the object did exist. The implementation is understandable because back then there was no lazy/on-demand downloading of individual objects from the promisor remotes that causes a long delay and materializes the object, hence defeating the point of using "--missing". The design is hurting us now. We could bypass the opportunistic type/mode consistency check altogether when "--missing" is given, but instead, use the oid_object_info_extended() API and tell it that we are only interested in objects that locally exist and are immediately available by passing OBJECT_INFO_SKIP_FETCH_OBJECT bit to it. That way, we will still retain the cheap and opportunistic sanity check for local objects. Signed-off-by: Richard Oliver --- builtin/mktree.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/mktree.c b/builtin/mktree.c index 902edba6d2..cfadb52670 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -116,8 +116,15 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing) path, ptr, type_name(mode_type)); } - /* Check the type of object identified by sha1 */ - obj_type = oid_object_info(the_repository, &oid, NULL); + /* Check the type of object identified by oid without fetching objects */ + struct object_info oi = OBJECT_INFO_INIT; + oi.typep = &obj_type; + if (oid_object_info_extended(the_repository, &oid, &oi, + OBJECT_INFO_LOOKUP_REPLACE | + OBJECT_INFO_QUICK | + OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) + obj_type = -1; + if (obj_type < 0) { if (allow_missing) { ; /* no problem - missing objects are presumed to be of the right type */