Re: odb_mkstemp's 0444 permission broke write/delete access on AFP
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- Feb 17, 2015, 17:54 UTC
- Message-ID
- <54E3804D.6020301@web.de>
- In-Reply-To
- <CA0F915F-74B1-4292-AFB8-D1A4C76C0137@gmail.com>
On 17/02/15 17:58, Fairuzan Roslan wrote:
Show 75 quoted lines
>
>> On Feb 17, 2015, at 4:51 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>>
>> Fairuzan Roslan <fairuzan.roslan@gmail.com> writes:
>>
>>> $ git clone https://github.com/robbyrussell/oh-my-zsh.git
>>> Cloning into 'oh-my-zsh'...
>>> remote: Counting objects: 11830, done.
>>> remote: Total 11830 (delta 0), reused 0 (delta 0)
>>> Receiving objects: 100% (11830/11830), 2.12 MiB | 481.00 KiB/s, done.
>>> Resolving deltas: 100% (6510/6510), done.
>>> warning: unable to unlink /Volumes/installer/oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
>>
>> This should be fixable from Git itself, by replacing the calls to
>> "unlink" with something like
>>
>> int unlink_or_chmod(...) {
>> if (unlink(...)) {
>> chmod(...); // give user write permission
>> return unlink(...);
>> }
>> }
>>
>> This does not add extra cost in the normal case, and would fix this
>> particular issue for afp shares. So, I think that would fix the biggest
>> problem for afp-share users without disturbing others. It seems
>> reasonable to me to do that unconditionnally.
>>
>>> $ rm -rf oh-my-zsh/.git/objects/pack/tmp_*
>>> rm: oh-my-zsh/.git/objects/pack/tmp_idx_oUN1sb: Operation not permitted
>>> rm: oh-my-zsh/.git/objects/pack/tmp_pack_zjPxuc: Operation not permitted
>>
>> What happens if you do "rm -fr oh-my-zsh/.git/objects/pack/" (i.e.
>> remove the directory, not the files)?
>>
>> If you can still remove the directory, then I'd say the solution above
>> could be sufficient: the user isn't supposed to interfer with the
>> content of .git/objects other than by using Git, and if he or she does,
>> then asking a chmod prior to an rm seems reasonable.
>>
>> If you can't, then it's another problematic use-case (basically, you
>> can't just "rm -fr" a whole clone), and then it deserves at least an
>> opt-in configuration to get writable pack files.
>>
>> (Unfortunately, I suspect we're in the later case)
>>
>>> If you insist on setting the tmp idx & pack file permission to 0444 at
>>> least give it a u+w permission whenever you try to unlink and rename
>>> it so it won’t fail.
>>
>> Yes. In case you hadn't guessed, this is precisely what I had in mind
>> when I asked "Is it a problem when using Git [...] or when trying to
>> remove files outside Git?".
>>
>> --
>> Matthieu Moy
>> http://www-verimag.imag.fr/~moy/
>
> Yes. It’s a problem when using Git where it fails to unlink and rename the tmp idx and pack files.
> The reason I tries to rm -rf the tmp_idx_XXXXXX and tmp_pack_XXXXXX is to proof a point why Git fails
>
> Perhaps my explanation wasn’t clear enough. Maybe it’s hard for you to understand without having to test it yourself on a AFP filesystem.
>
> Let me explain why AFP filesystem is more strict and different from your typical filesystem like ext4,hfs+,etc.
>
> $ mkdir testdir; chmod 0755 testdir; touch testdir/testfile; chmod 0444 testdir/testfile; ls -la testdir
> total 0
> drwxr-xr-x 1 user staff 264 Feb 18 00:26 .
> drwx------ 1 user staff 264 Feb 18 00:26 ..
> -r--r--r-- 1 user staff 0 Feb 18 00:26 testfile
>
> $ rm -rf testdir
> rm: testdir/testfile: Operation not permitted
> rm: testdir: Directory not empty
> This works on my system (Mac OS 10.9 as server and client)
Show 12 quoted lines
> $ chmod +w testdir/testfile; ls -la testdir > total 0 > drwxr-xr-x 1 riaf staff 264 Feb 18 00:26 . > drwx------ 1 riaf staff 264 Feb 18 00:26 .. > -rw-r—r-- 1 riaf staff 0 Feb 18 00:26 testfile > > $ rm -rf testdir <—— No error message > > This show that you cannot delete a directory or a file without a write permission in AFP filesystem. > > The problem with Git failing is not because its inability to delete a directory but its inability to unlink and rename tmp_idx_XXXXXX and tmp_pack_XXXXXX because those files were set to 0444 by odb_mkstemp. > Try google for “Git AFP” and you will see a lot people are facing with the same problem.
Yes, (at least to my knowledge) you seem to be one of the first to report it here, thanks for that.
Show 6 quoted lines
> > Regarding your suggestion, yes I think it would work but you have to take care (chmod) every calls that rename or unlink or delete files with 0444 permission. > > Regards, > Fairuzan >
The "right" solution is to make a wrapper function, and to re-define unlink() and rename() with help of the preprocessor.
git-compat-util.h has an example for fopen(), so that can be used for a patch.
And no, as I can not reproduce it here, I can only help with reviews or so.