From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 823D81FA14 for ; Sat, 27 May 2017 01:51:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934516AbdE0Bvs (ORCPT ); Fri, 26 May 2017 21:51:48 -0400 Received: from avasout08.plus.net ([212.159.14.20]:51029 "EHLO avasout08.plus.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754533AbdE0BK7 (ORCPT ); Fri, 26 May 2017 21:10:59 -0400 Received: from [10.0.2.15] ([143.159.212.80]) by avasout08 with smtp id RDAu1v0021keHif01DAvGU; Sat, 27 May 2017 02:10:55 +0100 X-CM-Score: 0.00 X-CNFS-Analysis: v=2.2 cv=FLJr/6gs c=1 sm=1 tr=0 a=n+zECcf3rkBNBoU0FNF4VQ==:117 a=n+zECcf3rkBNBoU0FNF4VQ==:17 a=IkcTkHD0fZMA:10 a=EBOSESyhAAAA:8 a=wmTNtsIs5fuJ5Pnzfd0A:9 a=QEXdDO2ut3YA:10 a=yJM6EZoI5SlJf8ks9Ge_:22 X-AUTH: ramsayjones@:2500 Subject: Re: [GSoC][PATCH v5 1/3] submodule: fix buggy $path and $sm_path variable's value To: Stefan Beller Cc: Prathamesh Chavan , "git@vger.kernel.org" , Brandon Williams , Christian Couder References: <20170521125814.26255-2-pc44800@gmail.com> <20170526151713.10974-1-pc44800@gmail.com> <01e8c552-fd3a-ee05-1ff1-3b3ea0f7feeb@ramsayjones.plus.com> From: Ramsay Jones Message-ID: <24ebb17b-4324-c6ef-7e3a-5576cda3b595@ramsayjones.plus.com> Date: Sat, 27 May 2017 02:10:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 26/05/17 18:07, Stefan Beller wrote: > On Fri, May 26, 2017 at 9:31 AM, Ramsay Jones > wrote: >> Hmm, I'm not sure which documentation you are referring to, > > Quite likely our fine manual pages. ;) > > foreach [--recursive] > Evaluates an arbitrary shell command in each checked out submodule. > The command has access to the variables $name, $path, $sha1 and > $toplevel: $name is the name of the relevant submodule section in > .gitmodules, $path is the name of the submodule directory relative > to the superproject, $sha1 is the commit as recorded in the > superproject, and $toplevel is the absolute path to the top-level > of the superproject. Any submodules defined in the superproject but > not checked out are ignored by this command. Unless given --quiet, > foreach prints the name of each submodule before evaluating the > command. If --recursive is given, submodules are traversed > recursively (i.e. the given shell command is evaluated in nested > submodules as well). A non-zero return from the command in any > submodule causes the processing to terminate. This can be > overridden by adding || : to the end of the command. I suspected as much, but I was wondering specifically if $sm_path had been documented anywhere. I didn't think so, but ... > As $path is documented and $sm_path is not, we should care about > $path first to be correct and either fix the documentation or the implementation > such that we have a consistent world view. :) Sure, but what is that world view? :-D I suspect that commit 091a6eb0fe did not intend (should not have) used $sm_path in that test. If we were to 'fix' that test, would it still work? Back in 2012, the submodule list was generated by filtering the output of 'git ls-files --error-unmatch --stage --'; but I don't recall if (at that time) git-ls-files required being at the top of the working tree, or if it would execute fine in a sub-directory. So, it's possible that the documentation of $path was wrong all along. ;-) At that time, by definition, $path == $sm_path. However, you know this stuff much better than me (I don't use git-submodule), so ... >> but if >> $path != $sm_path then something is wrong. (unless their definition >> has changed, of course). > > I would lean in doing so (changing their definition): > > $path (as documented) is the name of the submodule directory > relative to the direct superproject (so in nested submodules you > go up only one level). > > $sm_path on the other hand is not documented at all and yields > non-sense results in corner cases. Hmm, at what point did '$sm_path yields non-sense results' start being the case? (perhaps the corner cases need to be fixed first). > With this patch it becomes less non-sensey and could be documented as: > > $sm_path is the relative path from the current working directory > to the submodule (ignoring relations to the superproject or nesting > of submodules). OK. > This documentation also fits into the narrative of > the test in t7407. Hmm, does it? ATB, Ramsay Jones