On 07/03 12:29, Junio C Hamano wrote:
> Shourya Shukla <periperidip@xxxxxxxxx> writes:
>
> > On 22/02 11:29, Junio C Hamano wrote:
> >> Shourya Shukla <periperidip@xxxxxxxxx> writes:
> >>
> >> > + if (git_config_rename_section_in_file(index_only ? GITMODULES_INDEX :
> >> > + GITMODULES_FILE, sect.buf, NULL) < 0) {
> >>
> >> Also, is it really sufficient to pass GITMODULES_INDEX as the first
> >> argument to this function to tweak what is in the index?
> >>
> >> git_config_copy_or_rename_section_in_file() which is the
> >> implementation of that helper seems to always want to work with a
> >> file that is on disk, by making unconditional calls to
> >> hold_lock_file_for_update(), fopen(), fstat(), chmod(), etc.
> >>
> >> So I suspect that there are much more work needed.
> >
> > I am not able to comprehend _why_ we need so much more work. To me it
> > seems to work fine.
>
> > The flow now is something like:
> >
> > 1. If !index_only i.e., '--cached' is not passed then remove the entry
> > of the SM from the working tree copy of '.gitmodules' i.e.,
> > GITMODULES_FILE. If there are any unstaged mods in '.gitmodules', we do
> > not proceed with 'git rm'.
>
> That side is fine, especially if we are extending the "when doing
> 'git rm PATH' (without '--cached'), PATH must match between the
> index and the working tree" to "when doing 'git rm SUBMODULE', not
> just SUBMODULE but also '.gitmodules' must match between the index
> and the working tree", then adjusting the entry for SUBMODULE in
> '.gitmodules' in the working tree and adding the result to the index
> would give the same result as editing '.gitmodules' both in the
> index and in the working tree independently.
>
> But the problem is that there is no way "--cached" case would work
> with your code.
>
> > What exactly do we need to change then?
>
> Have you traced what happens when you make this call
>
> >> > + if (git_config_rename_section_in_file(index_only ? GITMODULES_INDEX :
> >> > + GITMODULES_FILE, sect.buf, NULL) < 0) {
>
> with index_only set? i.e. GIT_MODULES_INDEX passed as the
> config_filename argument?
>
> The first parameter to the git_config_rename_section_in_file() names
> a filename in the working tree to be edited. Writing ':.gitmodules'
> does not make the function magically work in-core without touching
> the working tree. It will make it update a file (likely not
> tracked) whose name is ":.gitmodules" in the working tree, no?
>
> Presumably you want to edit in-index .gitmodules without touching
> the working tree file, but the call is not doing that---and it would
> take much more work to teach it do so.
>
> And a cheaper way out would be how I outlined in the message you are
> responding to, i.e. write out the in-index .gitmodules to a
> temporary file, let git_config_rename_section_in_file() tweak that
> temporary file, and add it back into the index.
Ahhh. Understood and will work on it. BTW then when does
GITMODULES_INDEX even fulfill its purpose? Its name can confuse anyone
into thinking what it made me think: it is the index copy of the
gitmodules.
Is it something which is to be changed in the near future?
