My Git Dev Blog - Week 9

My Git Dev Blog - Week 9

[Atharva Raykar]

Here's my latest blog post:
https://atharvaraykar.me/gitnotes/week9

A part that may be interesting to list readers:

- In one section of the blog, I describe the problem of the git
  submodule configuration not updating properly, when I launch
  a subprocess that first initialises the submodule. I will
  appreciate it if someone has a possible explanation for the
  issue I faced at:
  https://atharvaraykar.me/gitnotes/week9#launching-the-init-part-as-a-subprocess

The rest of the post talks about my project progress so far, and how I
avoided the above problem entirely for now by taking a different approach.

Have a great day!

---
Atharva Raykar
ಅಥರ್ವ ರಾಯ್ಕರ್
अथर्व रायकर

Re: My Git Dev Blog - Week 9

[Christian Couder]

On Sun, Jul 18, 2021 at 1:59 PM Atharva Raykar <raykar.ath@gmail.com> wrote:
>
> Here's my latest blog post:
> https://atharvaraykar.me/gitnotes/week9

Great!

> A part that may be interesting to list readers:
>
> - In one section of the blog, I describe the problem of the git
>   submodule configuration not updating properly, when I launch
>   a subprocess that first initialises the submodule. I will
>   appreciate it if someone has a possible explanation for the
>   issue I faced at:
>   https://atharvaraykar.me/gitnotes/week9#launching-the-init-part-as-a-subprocess

My wild guess about why is_submodule_active() might not be working as
expected is that submodule_from_path() uses the submodule cache which
might not be properly updated when a submodule is initialized by a
subprocess.

> The rest of the post talks about my project progress so far, and how I
> avoided the above problem entirely for now by taking a different approach.

Nice that you found a possible better way!

Re: My Git Dev Blog - Week 9

[Atharva Raykar]

On 18-Jul-2021, at 18:45, Christian Couder <christian.couder@gmail.com> wrote:
> 
> On Sun, Jul 18, 2021 at 1:59 PM Atharva Raykar <raykar.ath@gmail.com> wrote:
>> 
>> Here's my latest blog post:
>> https://atharvaraykar.me/gitnotes/week9
> 
> Great!
> 
>> A part that may be interesting to list readers:
>> 
>> - In one section of the blog, I describe the problem of the git
>>  submodule configuration not updating properly, when I launch
>>  a subprocess that first initialises the submodule. I will
>>  appreciate it if someone has a possible explanation for the
>>  issue I faced at:
>>  https://atharvaraykar.me/gitnotes/week9#launching-the-init-part-as-a-subprocess
> 
> My wild guess about why is_submodule_active() might not be working as
> expected is that submodule_from_path() uses the submodule cache which
> might not be properly updated when a submodule is initialized by a
> subprocess.

Right, I have made a note of this. It does feel like the most plausible
explanation, which I might look into after my update series is done.

>> The rest of the post talks about my project progress so far, and how I
>> avoided the above problem entirely for now by taking a different approach.
> 
> Nice that you found a possible better way!

Re: My Git Dev Blog - Week 9

[Kaartic Sivaraam]

Hi Atharva and all,

On 18/07/21 5:29 pm, Atharva Raykar wrote:
> Here's my latest blog post:
> https://atharvaraykar.me/gitnotes/week9
> 

Nice blog!

> A part that may be interesting to list readers:
> 
> - In one section of the blog, I describe the problem of the git
>    submodule configuration not updating properly, when I launch
>    a subprocess that first initialises the submodule. I will
>    appreciate it if someone has a possible explanation for the
>    issue I faced at:
>    https://atharvaraykar.me/gitnotes/week9#launching-the-init-part-as-a-subprocess
> 

Christian mentioned a possible reason. I'll try to take a look
later and see if I could find anything.

> Passing the superprefix explicitly

My gut instinct tells me we could get away without having to go this way but
I haven't yet been able to figure out how. How 'prefix', 'super-prefix' and
'recursive-prefix' is still puzzling me. In case anyone has knowledge about
this please chime in and enlighten us. It would be very helpful.

> I am also waiting on my list patch to land on master, so that I can
> then send the next patch for the submodule add conversion.

I think you don't have to wait until it lands on 'master'. Now that it
is set to merge to 'next', I believe it should be fine for you to send
the rest of the series to the list. Just make sure you mention the
dependency of the series on the one you've already sent to the list.

> Have a great day!
>

Thanks! Hope you have a great weak ahead and do well in your exams.

-- 
Sivaraam

Re: My Git Dev Blog - Week 9

[Kaartic Sivaraam]

Hi Atharva and all,

On 19/07/21 12:49 am, Kaartic Sivaraam wrote:
> 
> On 18/07/21 5:29 pm, Atharva Raykar wrote:
> 
>> Passing the superprefix explicitly
> 
> My gut instinct tells me we could get away without having to go this way but
> I haven't yet been able to figure out how. How 'prefix', 'super-prefix' and
> 'recursive-prefix' is still puzzling me. In case anyone has knowledge about
> this please chime in and enlighten us. It would be very helpful.
>

I just noticed I was short and unhelpful here. To expand on what information would be
helpful on this respect, here's a snippet of a private e-mail I sent which I've
tweaked a little to make it relevant for the list audience:

-- 8< --
>> Problem:
>> -------
>> I cannot get 'submodule update --init --recursive' to work properly.
>> 
>> Cause:
>> -----
>> The paths get malformed in my first implementation[1] because I am not able
>> to transfer the git 'superprefix' to the 'init_submodule_cb' callback with the
>> current interface. The superprefix is required by the
>> 'get_submodule_displaypath()' function to create the correct display path.
>>
>> Thus, t406.5 fails.
>> 
>> For more details on this and the exact error,
>> cf.: https://atharvaraykar.me/gitnotes/week8#path-pains
>>
>> [ snip ]
>>
>> The link at [1] is the one where it fails only one test in my machine.
>>
>> [ snip ]
>>
>> This version does *not* start a subprocess for running init before the
>> update. The relevant lines for this are here:
>> https://github.com/tfidfwastaken/git/blob/7ad995f465072b60a8c33e51c4f91d08ee3d2484/builtin/submodule--helper.c#L3325-L3344
>>
>> [1]: https://github.com/tfidfwastaken/git/commits/submodule-helper-update-1b
>>

I took a look at this. I'm not able to find the exact issue but my gut instinct tells me
it has something to do with how prefix, super-prefix and recursive-prefix
are handled before and after the conversion. To be more specific, I'm having doubts
about whether the following code snippet[4] in shell ...

     prefix=$(git submodule--helper relative-path "$prefix$sm_path/" "$wt_prefix")
     wt_prefix=
     sanitize_submodule_env
     cd "$sm_path" &&
     eval cmd_update

... has been properly converted to C. Particularly, the fact that recursive calls to
'cmd_update' is made in the existing shell implementation in case '--recusive' is
passed needs to be observed well. This would subsequently result in multiple calls to
'cmd_init' with different 'prefix' and 'wt_prefix' values. So, that needs to be
observed well too.

I have not been able to get a complete picture of this myself. I'll try to take better
look later and see if I could get anything.

Note that this could also be me just not being able to get a proper picture of it.
Just wanted to share it in case others are able to get any ideas or able to think of a
possible cause.

[4]: https://github.com/git/git/blob/abb21c7263616f01c5e950861a29279ab21cb02f/git-submodule.sh#L654-L658
-- >8 --

I hope that gives a better idea about what kind of information would be helpful.
Kindly let me know if it doesn't.

Atharva,

In the meanwhile, I think you could continue polishing your attempted solution
and preparing to send it to the list. I'll try to dig more on my gut instinct
and let you know if I get anything. If you have some free time left after
polishing your series, you could try digging into it too :)

-- 
Sivaraam

Re: My Git Dev Blog - Week 9

[Atharva Raykar]

On 19-Jul-2021, at 08:28, Kaartic Sivaraam <kaartic.sivaraam@gmail.com> wrote:
> 
> Hi Atharva and all,
> 
> On 19/07/21 12:49 am, Kaartic Sivaraam wrote:
>> On 18/07/21 5:29 pm, Atharva Raykar wrote:
>>> Passing the superprefix explicitly
>> My gut instinct tells me we could get away without having to go this way but
>> I haven't yet been able to figure out how. How 'prefix', 'super-prefix' and
>> 'recursive-prefix' is still puzzling me. In case anyone has knowledge about
>> this please chime in and enlighten us. It would be very helpful.
>> 
> 
> I just noticed I was short and unhelpful here. To expand on what information would be
> helpful on this respect, here's a snippet of a private e-mail I sent which I've
> tweaked a little to make it relevant for the list audience:
> 
> -- 8< --
>>> ...
>>> 
> 
> I took a look at this. I'm not able to find the exact issue but my gut instinct tells me
> it has something to do with how prefix, super-prefix and recursive-prefix
> are handled before and after the conversion. To be more specific, I'm having doubts
> about whether the following code snippet[4] in shell ...
> 
>    prefix=$(git submodule--helper relative-path "$prefix$sm_path/" "$wt_prefix")
>    wt_prefix=
>    sanitize_submodule_env
>    cd "$sm_path" &&
>    eval cmd_update
> 
> ... has been properly converted to C. Particularly, the fact that recursive calls to
> 'cmd_update' is made in the existing shell implementation in case '--recusive' is
> passed needs to be observed well. This would subsequently result in multiple calls to
> 'cmd_init' with different 'prefix' and 'wt_prefix' values. So, that needs to be
> observed well too.

The whole deal with prefixes confused me initially a lot as well, so I'll try to
explain what's going on there first, and then show why I had trouble with it.

The terminology of 'recursive-prefix' was something I borrowed from the update-clone
helper[5]. Just to give you a summary of which shell variable maps to which C
variable, along with my understanding of what they represent:

| shell      | C                | description                               |
|------------+------------------+-------------------------------------------|
| $prefix    | recursive_prefix | this represents the superproject prefix,  |
|            |                  | ie, the path of the superproject relative |
|            |                  | to the current subproject.                |
|            |                  |                                           |
| $wt_prefix | prefix           | this is the path into the working tree,   |
|            |                  | ie, the path of the directory from the    |
|            |                  | root of the git directory's worktree      |

The display paths that are generated for the error messages require one of these to
be non-NULL, but not both[6].

I feel confident the values of the various prefixes are handled properly during the
recursion because I setup a new worktree on based master as my "control", and a
combination of running with GIT_TRACE2 and printing the values showed me that both
of them handle the prefixes identically.

The issue with the subprocess approach[7] seems to be not with the prefixes, but
rather with the configuration not being properly read when it is set in the same
command invocation.

[5] https://github.com/git/git/blob/75ae10bc75336db031ee58d13c5037b929235912/git-submodule.sh#L536-L550
[6] https://github.com/git/git/blob/75ae10bc75336db031ee58d13c5037b929235912/builtin/submodule--helper.c#L258-L260
[7] https://github.com/tfidfwastaken/git/commits/submodule-helper-update-1a

> I have not been able to get a complete picture of this myself. I'll try to take better
> look later and see if I could get anything.
> 
> Note that this could also be me just not being able to get a proper picture of it.
> Just wanted to share it in case others are able to get any ideas or able to think of a
> possible cause.
> 
> [4]: https://github.com/git/git/blob/abb21c7263616f01c5e950861a29279ab21cb02f/git-submodule.sh#L654-L658
> -- >8 --
> 
> I hope that gives a better idea about what kind of information would be helpful.
> Kindly let me know if it doesn't.
> 
> Atharva,
> 
> In the meanwhile, I think you could continue polishing your attempted solution
> and preparing to send it to the list. I'll try to dig more on my gut instinct
> and let you know if I get anything. If you have some free time left after
> polishing your series, you could try digging into it too :)

Sure. Thanks for the time and effort you are putting into this!

> -- 
> Sivaraam

Re: My Git Dev Blog - Week 9

[Bagas Sanjaya]

On 18/07/21 18.59, Atharva Raykar wrote:
> Here's my latest blog post:
> https://atharvaraykar.me/gitnotes/week9

You wrote:
> Interact more with the community by asking questions in your patch cover letters and replies to reviews.

Did you mean something like [RFC PATCH]?

-- 
An old man doll... just what I always wanted! - Clara

Re: My Git Dev Blog - Week 9

[Atharva Raykar]

On 19-Jul-2021, at 06:21, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> 
> On 18/07/21 18.59, Atharva Raykar wrote:
>> Here's my latest blog post:
>> https://atharvaraykar.me/gitnotes/week9
> 
> You wrote:
>> Interact more with the community by asking questions in your patch cover letters and replies to reviews.
> 
> Did you mean something like [RFC PATCH]?

That would be one part of it, but I meant it in a more general sense, ie,
it is better to be detailed and specific about design decisions in a patch
that one may not be sure of, and mention it up front.

That, and asking questions to reviewers to clarify what they mean, and their
motivations behind it, if one is not sure of those.

> -- 
> An old man doll... just what I always wanted! - Clara