Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-18 Thread Fairuzan Roslan

> On Feb 18, 2015, at 10:05 PM, Matthieu Moy  
> wrote:
> 
> Fairuzan Roslan  writes:
> 
>> Client: OS X 10.9 - 10.10.2
>> git client: git version 1.9.3 (Apple Git-50) and git version 2.2.1
>> 
>> Server : Linux 3.2.40 (Synology DSM 5.1)
>> AFP : Netatalk afpd 3.1.1
> 
> Any chance you can test this with a Mac OS server?
> 
> Perhaps because the server is not a Mac OS, it doesn't have the uchg
> flag, and maps it to the w bit in the POSIX permission system (this is
> pure speculation from me).
> 
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

I don’t have any Mac OS X server in my disposal at this moment but if you want 
to test out with netatalk AFPD I believe it’s available in debian & ubuntu apt 
repo

Regards,
Fairuzan


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-18 Thread Fairuzan Roslan

> On Feb 18, 2015, at 4:15 PM, Matthieu Moy  
> wrote:
> 
> And now I'm starting to wonder whether other people do have the same
> issue. Sure, googling "Git AFP" shows a lot of people having problems
> with Git and AFP, but are they really the same problem?
> 
> I googled 'git afp "unable to unlink"', and all results except one point
> to this thread:
> 
> https://www.google.com/search?q=git+afp+%22warning%3A+unable+to+unlink%22
> 
> The only one which doesn't actually does not mention afp.
> 
> Fairuzan: are you sure you're not the only one having the issue? Can you
> give more info on your system (OS version client and server side, …)?

No I don’t think I’m the only one with the permission issue here but I’m sure 
many people prefer using smb over afp


Client: OS X 10.9 - 10.10.2
git client: git version 1.9.3 (Apple Git-50) and git version 2.2.1

Server : Linux 3.2.40 (Synology DSM 5.1)
AFP : Netatalk afpd 3.1.1

From what I have tested whenever a file’s permission set to read-only on the 
AFP shares a user immutable flag were set as well hence why any action such as 
unlink or rename failed.

$ ls -laO
total 0
drwxr-xr-x  1 user  staff  - 264 Feb 18 21:31 .
drwx--  1 user  staff  - 264 Feb 18 21:31 ..
-rw-r--r--  1 user  staff  -   0 Feb 18 21:31 testfile
$ chmod 0444 testfile; ls -laO
total 0
drwxr-xr-x  1 user  staff  -264 Feb 18 21:31 .
drwx--  1 user  staff  -264 Feb 18 21:32 ..
-r--r--r--  1 user  staff  uchg   0 Feb 18 21:31 testfile

I believe this to stop a read-only file to be modified by the user in  Netatalk 
AFPD.

$ chmod 0644 testfile; ls -laO
total 0
drwxr-xr-x  1 user  staff  - 264 Feb 18 21:32 .
drwx--  1 user  staff  - 264 Feb 18 21:43 ..
-rw-r--r--  1 user  staff  -   0 Feb 18 21:31 testfile

Changing the permission to read-write will remove the user immutable flag.

Regards,
Fairuzan



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-17 Thread Fairuzan Roslan

> On Feb 17, 2015, at 4:51 PM, Matthieu Moy  
> wrote:
> 
> Fairuzan Roslan  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_XX and tmp_pack_XX 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  staff0 Feb 18 00:26 testfile

$ rm -rf testdir
rm: testdir/testfile: Operation not permitted
rm: testdir: Directory not empty

$ 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  staff0 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_XX and tmp_pack_XX 
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.

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




signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-16 Thread Fairuzan Roslan

> On Feb 17, 2015, at 1:34 PM, Torsten Bögershausen  wrote:
> 
> On 02/17/2015 04:22 AM, Fairuzan Roslan wrote:
>>> On Feb 17, 2015, at 3:08 AM, Matthieu Moy  
>>> wrote:
>>> 
>>> [ Please, don't top post on this list ]
>>> 
>>> Fairuzan Roslan  writes:
>>> 
>>>> I don’t see the issue for the owner of his/her own file to have write
>>>> access.
>>> Object and pack files are not meant to be modified. Hence, they are
>>> read-only so that an (accidental) attempt to modify them fails.
>>> 
>>>> Setting tmp idx & pack files to read-only even for the file owner is
>>>> not a safety feature.
>>> Yes it is. If you do not think so, then please give some arguments.
>>> 
>>>> You should at least give the user the option to set the permission in
>>>> the config file and not hardcoded the permission in the binary.
>>> This is the kind of thing I meant by "investigate alternate solutions".
>>> I have no AFP share to test, so it would help if you answered the
>>> question I asked in my previous message:
>>> 
>>>>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy  
>>>>> wrote:
>>>>> 
>>>>> Fairuzan Roslan  writes:
>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are
>>>>>> causing a lot of issues (unable to unlink/write/rename) to those
>>>>>> people who use AFP shares.
>>>>> Is it a problem when using Git (like "git gc" failing to remove old
>>>>> packs), or when trying to remove files outside Git?
>>> (BTW, why did you try to write/rename pack files?)
>>> 
>>> --
>>> Matthieu Moy
>>> http://www-verimag.imag.fr/~moy/
>> I think its easier if I just show you…
>> 
>> OS : OS X 10.10.0 - 10.10.2
>> Client :  git version 1.9.3 (Apple Git-50) and git version 2.2.1
>> AFP share : //user@hostname._afpovertcp._tcp.local/installer on 
>> /Volumes/installer (afpfs, nodev, nosuid, mounted by user)
>> 
>> 1. git clone example
>> 
>> $ 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
>> error: unable to write sha1 filename 
>> /Volumes/installer/oh-my-zsh/.git/objects/pack/pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.pack:
>>  Operation not permitted
>> fatal: cannot store pack file
>> fatal: index-pack failed
>> 
>> $ ls -l oh-my-zsh/.git/objects/pack
>> total 5008
>> -rw---  1 user  staff   32 Feb 17 09:59 
>> pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.keep
>> -r--r--r--  1 user  staff   332312 Feb 17 09:59 tmp_idx_oUN1sb
>> -r--r--r--  1 user  staff  2223007 Feb 17 09:59 tmp_pack_zjPxuc
>> 
>> $ 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
>> 
>> Detail Errors:
>> 1. delete_ref_loose (refs.c) -> unlink_or_msg (wrapper.c) -> "unable to 
>> unlink %s: %s"
>> 2. move_temp_to_file (sha1_file.c ) -> “unable to write sha1 filename %s: %s”
>> 
>> 2. git pull example
>> 
>> Textual git:master $ git pull
>> remote: Counting objects: 435, done.
>> remote: Compressing objects: 100% (398/398), done.
>> remote: Total 435 (delta 219), reused 18 (delta 12)
>> Receiving objects: 100% (435/435), 1.22 MiB | 756.00 KiB/s, done.
>> Resolving deltas: 100% (219/219), done.
>> warning: unable to unlink .git/objects/pack/tmp_pack_vDaIZa: Operation not 
>> permitted
>> error: unable to write sha1 filename 
>> .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack: 
>> Operation not permitted
> I'm somewhat unsure how this is connected to 0444 ?
> 
> It seems as if you don't have write permissions for some reasons.
> (on the higher directory), what does
> ls -ld  .git/objects/pack/
> ls -ld  .git/objects/
> give ?
> 
> can you run
> rm .git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack
> 
> on the command line ?

No. I have write permission on all of the folders.
drwxr-xr-x  1 user  staff   264 Feb 17 11:05 .
drwxr-xr-x  1 user  staff   264 Jan 30 12:52 ..

It has nothing to do with my folder permissions. Like I said earlier this only 
happened to people who use AFP shares.

When odb_mkstemp being called and sets the tmp idx & pack files to 0444 and 
later functions like unlink_or_msg or finish_tmp_packfile tries to unlink or 
rename those files, it will fail

It would be much faster and easier if you can try it on a AFP shares or I can 
talk you through it over irc @freenode #git (riaf^)

Regards,
Fairuzan



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-16 Thread Fairuzan Roslan

> On Feb 17, 2015, at 3:08 AM, Matthieu Moy  
> wrote:
> 
> [ Please, don't top post on this list ]
> 
> Fairuzan Roslan  writes:
> 
>> I don’t see the issue for the owner of his/her own file to have write
>> access.
> 
> Object and pack files are not meant to be modified. Hence, they are
> read-only so that an (accidental) attempt to modify them fails.
> 
>> Setting tmp idx & pack files to read-only even for the file owner is
>> not a safety feature.
> 
> Yes it is. If you do not think so, then please give some arguments.
> 
>> You should at least give the user the option to set the permission in
>> the config file and not hardcoded the permission in the binary.
> 
> This is the kind of thing I meant by "investigate alternate solutions".
> I have no AFP share to test, so it would help if you answered the
> question I asked in my previous message:
> 
>>> On Feb 17, 2015, at 2:23 AM, Matthieu Moy  
>>> wrote:
>>> 
>>> Fairuzan Roslan  writes:
>>> 
>>>> Hi,
>>>> 
>>>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are
>>>> causing a lot of issues (unable to unlink/write/rename) to those
>>>> people who use AFP shares.
>>> 
>>> Is it a problem when using Git (like "git gc" failing to remove old
>>> packs), or when trying to remove files outside Git?
> 
> (BTW, why did you try to write/rename pack files?)
> 
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

I think its easier if I just show you…

OS : OS X 10.10.0 - 10.10.2
Client :  git version 1.9.3 (Apple Git-50) and git version 2.2.1
AFP share : //user@hostname._afpovertcp._tcp.local/installer on 
/Volumes/installer (afpfs, nodev, nosuid, mounted by user)

1. git clone example

$ 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
error: unable to write sha1 filename 
/Volumes/installer/oh-my-zsh/.git/objects/pack/pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.pack:
 Operation not permitted
fatal: cannot store pack file
fatal: index-pack failed

$ ls -l oh-my-zsh/.git/objects/pack
total 5008
-rw---  1 user  staff   32 Feb 17 09:59 
pack-cceafdc9ef02bc58844138ba543ec6cc38252bb1.keep
-r--r--r--  1 user  staff   332312 Feb 17 09:59 tmp_idx_oUN1sb
-r--r--r--  1 user  staff  2223007 Feb 17 09:59 tmp_pack_zjPxuc

$ 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

Detail Errors:
1. delete_ref_loose (refs.c) -> unlink_or_msg (wrapper.c) -> "unable to unlink 
%s: %s"
2. move_temp_to_file (sha1_file.c ) -> “unable to write sha1 filename %s: %s”

2. git pull example

Textual git:master $ git pull
remote: Counting objects: 435, done.
remote: Compressing objects: 100% (398/398), done.
remote: Total 435 (delta 219), reused 18 (delta 12)
Receiving objects: 100% (435/435), 1.22 MiB | 756.00 KiB/s, done.
Resolving deltas: 100% (219/219), done.
warning: unable to unlink .git/objects/pack/tmp_pack_vDaIZa: Operation not 
permitted
error: unable to write sha1 filename 
.git/objects/pack/pack-977a2dc0f4be3996dc1186e565a30d55d14b5e87.pack: Operation 
not permitted
fatal: cannot store pack file
fatal: index-pack failed

Textual git:master $ ls -l .git/objects/pack/tmp_*
-r--r--r--  1 user  staff13252 Feb 17 10:51 .git/objects/pack/tmp_idx_uhnicb
-r--r--r--  1 user  staff  1275487 Feb 17 10:51 
.git/objects/pack/tmp_pack_vDaIZa

= Same explanation as git clone example

3. git gc example

Textual git:master $ git gc
Counting objects: 49691, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (11347/11347), done.
fatal: unable to rename temporary pack file: Operation not permitted
error: failed to run repack

Textual git:master $ ls -l .git/objects/pack/tmp_*
-r--r--r--  1 user  staff   1392420 Feb 17 10:58 
.git/objects/pack/tmp_idx_77nr1b
-r--r--r--  1 user  staff  96260304 Feb 17 10:58 
.git/objects/pack/tmp_pack_RlAZc9

Detail Error:
1. finish_tmp_packfile (pack-write.c) -> die_errno(“unable to rename temporary 
pack file”);


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.

Regards,
Fairuzan







signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-16 Thread Fairuzan Roslan
I don’t see the issue for the owner of his/her own file to have write access.
Setting tmp idx & pack files to read-only even for the file owner is not a 
safety feature.

The real issue here is that in AFP file system can’t even unlink or rename or 
delete the tmp idx and pack file with no write access after it done, while 
other file system like ext4,hfs+, etc can.

You should at least give the user the option to set the permission in the 
config file and not hardcoded the permission in the binary.

Regards,
Fairuzan


> On Feb 17, 2015, at 2:23 AM, Matthieu Moy  
> wrote:
> 
> Fairuzan Roslan  writes:
> 
>> Hi,
>> 
>> Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are
>> causing a lot of issues (unable to unlink/write/rename) to those
>> people who use AFP shares.
> 
> Is it a problem when using Git (like "git gc" failing to remove old
> packs), or when trying to remove files outside Git?
> 
>> The issue was first introduced in
>> https://github.com/git/git/blob/f80c7ae8fe9c0f3ce93c96a2dccaba34e456e33a/wrapper.c
>> line 284.
> 
> I don't think so. The code before this commit did essentially a chmod
> 444 on the file, so object files were already read-only before.
> 
> The pack files have been read-only since d83c9af5c6a437ddaa9dd27 (Junio,
> Apr 22 2007).
> 
>> To fix these issues the permission need to be adjusted to “int mode =
>> 0644;” in odb_mkstemp (environment.c)
> 
> The issue is that having object and pack files read-only on the
> filesystem is a safety feature to prevent accidental modifications (even
> though it's actually not that effective, since brute-force "sed -i" or
> "perl -i" still accept to modify read-only files).
> 
> So, I'd be a bit reluctant to remove this safety feature for all users
> if it's only for the benefit of a minority of users. Not that I think
> the problem shouldn't be fixed, but I'd rather investigate alternate
> solutions before using this mode = 0644.
> 
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/



signature.asc
Description: Message signed with OpenPGP using GPGMail


odb_mkstemp's 0444 permission broke write/delete access on AFP

2015-02-16 Thread Fairuzan Roslan
Hi,

Somehow the “int mode = 0444;” in odb_mkstemp (environment.c) are causing a lot 
of issues (unable to unlink/write/rename) to those people who use AFP shares.

In order to be able to write/unlink/delete/rename a file on AFP filesystem the 
owner of the file must have at least a u+w access to it.

The issue was first introduced in 
https://github.com/git/git/blob/f80c7ae8fe9c0f3ce93c96a2dccaba34e456e33a/wrapper.c
 line 284.

To fix these issues the permission need to be adjusted to “int mode = 0644;” in 
odb_mkstemp (environment.c)

Please let me know if you need further detail.

Regards,
Fairuzan--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html