Re: [GSoC][PATCH v5 1/3] submodule: fix buggy $path and $sm_path variable's value

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: May 31, 2017, 00:13 UTC
Message-ID: <6e1db1f4-abbe-199e-a206-a56d83fac4ff@ramsayjones.plus.com>
In-Reply-To: <CAGZ79kYMA6Me_ZnBZQitW7ZSJ0kfvb-LPnH=1gTwhRN-KOe5pA@mail.gmail.com>

On 31/05/17 00:29, Stefan Beller wrote:

Show 11 quoted lines

>> As I said above, I can't remember how git-ls-files worked back then,
>> but it seems that I thought of it as the path to the submodule from
>> the root of the working tree. Again, by definition, $sm_path == $path
>> (as documented). Of course, that may have changed since then.
> 
> Documented in 64394e3 (git-submodule.sh: Don't use $path variable in
> eval_gettext string, by yourself)
> 
> What I intended to say above was "documented to the end user",
> and I do not count our commit messages as such. The end user facing
> documentation only talks about path, not mentioning sm_path.

Correct, and that is exactly what I was saying. ie. $path as 'documented to the end user'. (again by _definition_ $sm_path _is_ $path).

Show 12 quoted lines

>>> After rereading that test, I would think so?
>>
>> Really? So, if it differs from $path, then something changed between
>> commit 64394e3ae9 and commit 091a6eb0fe. I haven't really read that
>> commit/test, so take what I say with a pinch of salt ...
> 
> Well yes. I am specifically reading 091a6eb0fe, the changes to t7407.
> 
> In that test sm_path contains the relative path from $PWD to the
> submodule. (It does NOT: "$[sm_]path is the name of the submodule
> directory relative to the superproject" as documented but rather
> ... relative to the $PWD)

In that case, the current user documentation does not agree with the current implementation, yes?

So, was the user documentation always wrong? (did git-ls-files work from a sub-directory, limiting its output to the cwd, or did it chdir() to the top of the worktree first?).

Show 11 quoted lines

>> As I said in commit 64394e3ae9, $path was part of the API, so I could
>> not just rename it, without a deprecation period, etc ... So, I was
>> 'crossing my fingers' that nobody would export $path in their user
>> scripts (not very likely, after all).
> 
> Ok. So another approach to get away in the C conversion:
> * export the sm_path as all other environment variables
> * for "$path" we do not export it into the environment, but
>   prefix the command with it, i.e. we'd ask our shell to run
>   "path=%s; %s", sm_path, argv[0]
>   to preserve the historic behavior.

Yes, that would probably work.

ATB, Ramsay Jones