From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id B9D8D1F5AE for ; Fri, 11 Jun 2021 04:32:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230281AbhFKEdP (ORCPT ); Fri, 11 Jun 2021 00:33:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34130 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230332AbhFKEdJ (ORCPT ); Fri, 11 Jun 2021 00:33:09 -0400 Received: from mav.lukeshu.com (mav.lukeshu.com [IPv6:2001:19f0:5c00:8069:5400:ff:fe26:6a86]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61D30C061574 for ; Thu, 10 Jun 2021 21:31:12 -0700 (PDT) Received: from lukeshu-dw-thinkpad (unknown [IPv6:2601:281:8200:42e:4e34:88ff:fe48:5521]) by mav.lukeshu.com (Postfix) with ESMTPSA id 85F0A80590; Fri, 11 Jun 2021 00:31:10 -0400 (EDT) Date: Thu, 10 Jun 2021 22:31:09 -0600 Message-ID: <877dj16n02.wl-lukeshu@lukeshu.com> From: Luke Shumaker To: Junio C Hamano Cc: Luke Shumaker , "Johannes Schindelin via GitGitGadget" , git@vger.kernel.org, Luke Shumaker , Johannes Schindelin Subject: Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows In-Reply-To: References: <87bl8d6xoq.wl-lukeshu@lukeshu.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Thu, 10 Jun 2021 19:37:12 -0600, Junio C Hamano wrote: > Luke Shumaker writes: > > >> +if test -z "$GIT_EXEC_PATH" || { > >> + test "${PATH#"${GIT_EXEC_PATH}:"}" = "$PATH" && { > >> + # On Windows, PATH might be Unix-style, GIT_EXEC_PATH not > >> + ! type -p cygpath >/dev/null 2>&1 || > >> + test "${PATH#$(cygpath -au "$GIT_EXEC_PATH"):}" = "$PATH" > > > > Nit: That should have a couple more `"` in it: > > > > test "${PATH#"$(cygpath -au "$GIT_EXEC_PATH"):"}" = "$PATH" > > > > But no need to re-roll for just that. > > Does the nit purely cosmetic, or does it affect correctness? I'd > assume the former, as it would not make sense to say "no need to > reroll" if leaving it as-is would mean a breakage, but trying to > make sure. > > Thanks. Quoting in that shell construct can in theory affect correctness, but my intuition was that it can't affect this specific case. However, upon thinking about it a bit more, I realize that I was mistaken. If the git exec path contains a shell glob that is not self-matching, then it will break in that `git subtree` will refuse to run even though the install is fine. For instance, GIT_EXEC_PATH => \Home\Tricky[x]User\git-core PATH => /Home/Tricky[x]User/git-core:/bin I'd think that this type of thing would be unlikely to happen in the wild, but yeah, it needs to be fixed. So a reroll is needed. It is also broken in the other direction (it might run even though the install is broken), but only in situations that I don't think I actually care about. It would happen if the glob is self-matching, your GIT_EXEC_PATH and PATH disagree, and the glob matches PATH. The point of the check is to look for ways that installs are actually accidentally broken in the wild, I'm pretty sure the only way all 3 of those things can happen together is if you're actively trying to break it. And if you're actively trying to break a shell script, you can do so a lot simpler by just setting PATH to something silly. -- Happy hacking, ~ Luke Shumaker