Re: BUG: git clean -d cannot remove files from read-only directories

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The fragmenting and duplication of this discussion via email is unfortunate, but I'm not sure what can be done about that.

> Yeah, arguing to change existing behavior (as you started with by labelling the thread with "BUG") will probably get you a lot of pushback, but let me address your new angle of suggesting a new option...

Sorry if the subject prefix annoyed people, but I wasn't trying to. There's no template that I could find for bug reports... sorry, feature suggestions. :-)

I appreciate your responding to my arguments here and you make some good objections. I'm not sure at this point that they're insuperable, but let me try to respond.

> Let's say we did implement a new flag that said we should override standard permissions. In such a case, shouldn't ACLs also be overridden?

Maybe, and probably ideally. I suppose it depends in part on the difficulty of implementation. I don't know how other tools, like rsync (which has no problem deleting files from read-only directories), handle ACLs. But I would say that if rsync can do it then Git can do it.

> Can we handle that in any semi-portable fashion? Assuming we do attempt to override ACLs, do we destroy portability of git?

I don't know. I know there are POSIX ACLs, but I'd be surprised if even the POSIX layer on Windows implements them, for example. I've never used a Unix-like system with ACLs. I'd be tempted to say again "do what rsync does", but I don't actually know what that is and don't have an easy way to test it out. I think Unix-like systems with ACLs are very rare, much less than 1% of installations, but of course Windows is ACL-based.

Let me turn your questions around. Git knows about, tracks, and stores standard Unix file permissions. It manipulates them and cares about them. What about ACLs? Does Git apply the same versioning to ACLs as it does to files? If the answer is "no", as I expect, doesn't that mean Git's portability is already "destroyed", or nonexistent? And that therefore it's wrong for Git to even try to maintain Unix file permissions since they don't apply to all operating systems? I could put it another way: since Git knows and cares about and already manipulates Unix file permissions but (presumably) not ACLs, it is not inconsistent with the current design to continue doing exactly that - supporting Unix permissions but not ACLs - in new features.

> If we don't try to overrule ACLs, doesn't that defeat your whole argument that "git clean" (with various levels of forcing) is about trying to do as much as possible to put the repository in a clean state?

I think the discussion of "would it be a good feature?" is fairly independent from the question of "is it easy to implement?". The question of what "git clean" is about is separate from the question of what limitations we might have due to implementation difficulties. You've probably seen the "Known limitations" sections of many man pages. If you can solve 90% of a problem with 10% of the effort, it's usually reasonable to do so. I.e. "don't let perfection be the enemy of good" and all that. And I suspect explicit ACL support is missing from Git generally, so having another feature without explicit ACL support is not a showstopper.

> what if we attempt to override the permissions by marking directories as writable, but find out the user isn't the owner of the directory and thus can't change its permissions?

Well, then you fail. If the user isn't the owner of the thing being deleted, that's a different ballgame.

> Do we try harder by attempting to invoke chmod under sudo to see if we can override the permissions?

Certainly not. :-)  The user can sudo git if they have permission to do such a thing.

> At what point do we give up? What's the clear line...and why does that line not just get drawn at not changing permissions at all?

I would say that first it should be determined whether the feature is useful. If not, then the line should get drawn at not changing permissions. But if it would be useful, then the line should be initially drawn at wherever gives the most "bang for the buck", the 80/20 case, etc. If it turns out that it's an unreasonable effort even to get a minimally useful implementation, then the stance of the Git team should be "It'd be nice to have, but if you want it you'll have to write it yourself and send us a patch." Which is, I think, how more esoteric features like ACLs should be handled, if they are to be handled at all.

To be specific, I propose that if the unlink or rmdir system call returns EPERM, and you are going to delete the parent directory anyway, then you stat the parent directory, see if it's missing write permissions, and if so, try to chmod u+w and retry the deletion of the child. And if any of those steps fail, you print an error message exactly like today. Alternately, and preferably, you do it even if you won't delete the parent directory, but in that case you have to put the permissions back when you're done. I think both of those are simple to implement, but I don't know about Git's implementation.

> Here you've changed the goalposts slightly.  I didn't compare "git clean" to "rm", I compared "git clean -fd" to "rm -rf" on untracked directories.

True, but I only realized that after sending the email.

> In other words, "git clean -fd" avoids bringing the repository back to a pristine state in a certain circumstance; it's actually more cautious than rm.

Which is the main trouble with it. It's very difficult to know exactly what it's going to want to delete, hence the difficulty of fixing the problem myself (where myself is a script invoking "git clean") with any kind of workaround outside Git, and hence my request for it to be handled inside Git.

But I come back to rsync, which I think is perhaps spiritually closer to "git clean" than rm is. It has no problem syncing files even if it has to twiddle some permission bits to do it. And that's convenient.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux