From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-a8-smtp.messagingengine.com (fhigh-a8-smtp.messagingengine.com [103.168.172.159]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1A72F24EABD for ; Tue, 4 Mar 2025 18:30:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.159 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741113006; cv=none; b=eAKMK9YOtAxoLm9wVBoS0EuVUnqLeBfm/McwzNaTD6BJ4o4Ebrh+9xLXtizKieFEEWwnAWbMju6+rsrjqtat3dAYaBkPR7IN4k1ZPxJ1andlaDcJy3Iq+Sx/sauym2ixWr5cvg8nzR0XEM4Ge0Ru6vglOJOTmMCi97pBYhXr3rE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741113006; c=relaxed/simple; bh=HnyiRq4W727nKqgczeNEb2iYmR5JIbk0L/mOxN/v2eY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=p5/1GgNCc82DObzQnK/puWxVvdHVzZfH14/sB+6qOdMFr+BP+Q+dcmESwnVaDw1uqkBc+KfSZIpraIJku00MpEUb5EwdJBIxJ1VCm/F3fLT6W3VL1jEbhAZMc7ZIwpZ/CyLECgzewjUQrgpkMuLNofJMrg+V2v9W8QCYzEC2QY0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (2048-bit key) header.d=pobox.com header.i=@pobox.com header.b=C+EiJrWr; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=4SolJ31c; arc=none smtp.client-ip=103.168.172.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pobox.com header.i=@pobox.com header.b="C+EiJrWr"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="4SolJ31c" Received: from phl-compute-12.internal (phl-compute-12.phl.internal [10.202.2.52]) by mailfhigh.phl.internal (Postfix) with ESMTP id 192831140093; Tue, 4 Mar 2025 13:30:03 -0500 (EST) Received: from phl-frontend-01 ([10.202.2.160]) by phl-compute-12.internal (MEProxy); Tue, 04 Mar 2025 13:30:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pobox.com; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm3; t=1741113003; x=1741199403; bh=IQQt8et78IbSOinIxnnAxQMaHAUqRFrzMQAUfzmygXs=; b= C+EiJrWr2hCzAEkmCot6MH6d/v1wxQx5lbZbnYLINtgKKdRE6Tebp2RWNRhaFHj+ SnqauhatejYM2M6MHTCNCH2LZb46djerb/NY4uQYsCBj/xrtkwxu72ScOZ61vHdK x4tNh0eLA3a3cg+mA6IGOrVI+D11JLXJCQav7HPneJDtMAqOn+sHy5zSxmLzpmnm 45KFMsU+lK1ASekL8JazPLh49DtRsB/AabrNfNkRciXsvUpt5m3IRs7Z3heSvG0g DkCbqhVBi3Bvtq5RqedB18nLseKeq8Rcnl2mnwWbii01DRXalv9KEkTnlkTa6r6w jaPXvYFZUkJ3ZCpl0uhhQQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741113003; x= 1741199403; bh=IQQt8et78IbSOinIxnnAxQMaHAUqRFrzMQAUfzmygXs=; b=4 SolJ31cUGzM6oQxumPwHfOSHSA7nvASEFGRop6U74MUdUutEy7FeXNNNLm8OWyWR tQkFrxRd6qOVwQPlUjEg/2vYfv1ZSzA8YKtrXzqBV0SulD3O2AcOapEGwAmRvXxu /LWmiTRFupklZKDfM01OabugXm71hhjFHYSDHzb5AMp7EetHzqVC+bI4P46So+KV 5/wQOwdiiyiYzPwt9VYAReKDtpok7CYq6qvioLgiiy6lkNFoHcM5zWyrQr5DNnfC HnGus865+eQ++ZeXhMzAOmc7BSU17HHoD/KcDFP1swPyFxdHWBj5+C6nJWuJdYYs 9FWuOgcAln2HRWgaIwjow== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddutddvjeeiucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephffvvefujghffffkfgggtgfgsehtkeertddt reejnecuhfhrohhmpefluhhnihhoucevucfjrghmrghnohcuoehgihhtshhtvghrsehpoh gsohigrdgtohhmqeenucggtffrrghtthgvrhhnpedtffdvteegvddtkeetfeevueevlefg keefheeigfehveehvdekheelveevfedtheenucevlhhushhtvghrufhiiigvpedtnecurf grrhgrmhepmhgrihhlfhhrohhmpehgihhtshhtvghrsehpohgsohigrdgtohhmpdhnsggp rhgtphhtthhopeehpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehsuhhnshhhih hnvgesshhunhhshhhinhgvtghordgtohhmpdhrtghpthhtohepuggrnhhimhgrhhgvnhgu rhgrtdeltdegsehgmhgrihhlrdgtohhmpdhrtghpthhtohepphhssehpkhhsrdhimhdprh gtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepghhi thhsthgvrhesphhosghogidrtghomh X-ME-Proxy: Feedback-ID: if26b431b:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 4 Mar 2025 13:30:01 -0500 (EST) From: Junio C Hamano To: Eric Sunshine Cc: Mahendra Dani , Patrick Steinhardt , git@vger.kernel.org Subject: Re: [PATCH 1/1] t1403: prefer test_path_exists helper function In-Reply-To: (Eric Sunshine's message of "Tue, 4 Mar 2025 13:07:22 -0500") References: <20250301105838.1481-1-danimahendra0904@gmail.com> <20250301105838.1481-2-danimahendra0904@gmail.com> Date: Tue, 04 Mar 2025 10:30:00 -0800 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Eric Sunshine writes: > On Tue, Mar 4, 2025 at 12:49 PM Junio C Hamano wrote: >> Eric Sunshine writes: >> > On Tue, Mar 4, 2025 at 7:05 AM Junio C Hamano wrote: >> >> Mahendra Dani writes: >> >> >> > remove_object() { >> >> >> > file=$(sha1_file "$*") && >> >> >> > - test -e "$file" && >> >> >> > + test_path_exists "$file" && >> >> >> > rm -f "$file" >> >> >> > } && > ... > Yes, I understood the implication of your suggestion, but as mentioned > above, it's not clear (at least to me) why `test -e "$file"` is there > at all since this test is not about checking functionality of `git > commit`. Yup, I do not see much point in "test -e" there in the original, and it does not change even if it were "test -f". I would understand if the author wanted to have a "slightly more intelligent 'rm -f' that knows where a loose object is located, and removes the named object no matter what", but if the objective were to ensure the object is missing, I wouldn't have written it to return non-zero when the object were missing in the first place. And if the purpose of the function is to catch unexpected cases, such as "the loose object file should be there but isn't" and "we located the file but we failed to remove it", then it shouldn't have the 'test -e' guard and 'rm' shouldn't have been used with '-f'. So, I agree with you that the original is already iffy. Thanks.