Re: [lmi] Unexpected git (mis)behavior

[Greg Chicares]

On 11/13/20 12:56 AM, Vadim Zeitlin wrote:
> On Fri, 13 Nov 2020 00:15:59 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
[...]
> GC> /opt/lmi/src/lmi[0]$git pull
[...]
> GC> Updating 5724e39..b03a344
> GC> error: unable to unlink old '.github/workflows/ci.yml' (Permission denied)
> 
>  The first puzzle is that I don't understand why did this happen.

I don't know, and because we've forcibly fixed it, we can't investigate
the failure any further.

> GC> The file that couldn't be unlinked had permissions "-rw-r--r--",
> 
>  But this is irrelevant, isn't it? It's the permissions of the parent
> directory that matters.

Oh. Yes. I've made that mistake before. Unfortunately, I didn't
record the original permissions of the parent directories.

> GC> which Kim used 'sudo' to reset to "-rw-rw-r--", also repairing
> GC> undesired permissions on other files, thus:
> GC>   sudo chgrp -R lmi /opt/lmi
> GC>   sudo find /opt/lmi -type d -exec chmod g+s {} +
> GC>   sudo chmod -R g=u /opt/lmi
> 
>  If Kim didn't have write access to /opt/lmi/src/lmi/.github/workflows
> directory, this would explain the error above, but I'm still not sure how
> could it have happened. Do you understand this?

No. I can only speculate that when the .github/workflows directory
came into existence because of a prior git-pull, it didn't get the
desired permissions despite core.sharedrepository = true.

>  This could be argued to be a bug in Git, but I don't think it's easy, or
> maybe even possible, to fix, as there is no "atomic update several files"
> operation in the file system API, so it just can't ensure that it has
> updated all files or none of them. And I guess it tries to touch the file
> system as little as possible if an unexpected error occurs to try to avoid
> making things even worse if something really catastrophic has happened. All
> this is pure speculation on my part, of course, but I think it makes sense.

It does. Of course, many other git commands (merge, rebase...)
keep track of what they did before a failure, and offer --abort
options to restore the status quo ante, as though file updates
were atomic. That's not required for validity, but it greatly
improves the quality of implementation. Here, I'd hoped for a
little more QoI.

> GC> What surprised me is that git modified the local tree, then aborted but
> GC> left the local modifications in place, and git-status didn't tell us about
> GC> the problem that running git-pull had created,
> 
>  Sorry, I don't see the output of git-status in your message. I'm pretty
> sure that it would have given an accurate assessment of the situation if
> you had run it before the second git-pull. Did you really do it and didn't
> see any changes?

I believe, but cannot prove, that I ran git-status at that point,
and that it gave no useful advice--i.e., that it said only that
a bunch of files were locally modified.

> GC> or give a clearer suggestion about how we should recover. "Please,
> GC> commit your changes" is misleading because those weren't "our"
> GC> changes--they were git's changes.
> 
>  But git-pull has no way to know about it. You could have arrived to
> exactly the same situation by making these changes manually.

The first git-pull made those changes, so in principle it could have
tracked them and given better advice, either as a result of running
that command, or in git-status (or both).

>  I think "--abort" could have worked after the first pull, but almost
> certainly not after the second one.

OTOH, if '--abort' would have worked at that point, then I would expect
git-status to have suggested it. I believe that it didn't, but, again,
I can't say for certain, and I can no longer investigate that.

> GC> I'm guessing that this is a git QoI issue: git would normally either clean
> GC> up better after a failed pull, or give more helpful suggestions, but maybe
> GC> it stumbles because of the (hidden) .github subdirectory.
> 
>  I'm quite certain that being hidden doesn't play any role here. Or, wait,
> perhaps it does: Git certainly doesn't care, but would it be possible that
> somebody ran chgrp or chmod command using "*", instead of using "find
> -exec"?

This being a corporate server, someone in corporate IT may have done
such a thing, but neither Kim nor I manually did anything like that.
If we did it at all, it can only have been by running a script that's
in the public lmi repository, but I think our scripts are more careful.

> GC> I would have thought that that subdirectory would have had group=user
> GC> permissions because
> GC>   $git config --get-all core.sharedrepository
> GC>   1
> GC> but clearly it didn't.
> 
>  Yes, and this remains a mystery to me...

I have long thought that core.sharedrepository does less than one
might hope, as discussed here in March 2020:
  
https://lists.nongnu.org/archive/cgi-bin/namazu.cgi?query=sharedrepository&submit=Search&idxname=lmi

> GC> Should I make some appropriate hook (post-checkout, perhaps) do this:
> GC>   sudo chgrp -R lmi /opt/lmi
> GC>   sudo find /opt/lmi -type d -exec chmod g+s {} +
> GC>   sudo chmod -R g=u /opt/lmi
> GC> in order to enforce those rules after every pull, so that this problem
> GC> can't occur again?
> 
>  Normally it should never occur in the first place, so there should be no
> need to correct it like this. I'd be curious if the command
> 
>       find /opt/lmi/src/lmi '!' -group lmi -o '(' -type d -a '!' -perm g+s ')'
> 
> returns any files right now?

Yes, that returns thousands of files. One of the results
(retyped manually here) is
  drwxrwsr-x. 2 [MY REDACTED ID] lmi ... aclocal
which has group lmi, is a directory, and has an 's' permission,
so the command isn't doing what I thought it would; I'll have to
take another look at it later when I feel less sleepy.