Teach git apply to respect core.fileMode

[dscho]

When applying a patch that adds an executable file, git apply ignores the core.fileMode setting. On Windows, where Git's idea of file mode bits makes no sense, it therefore spits out warnings like:

warning: a-new-script.sh has type 100644, expected 100755

This ticket, should you choose to accept it, is about teaching git apply to respect the core.fileMode setting and not even attempt to chmod() any added files (or modified, if the diff wants to change the file mode).

[LemmingAvalanche]

Backlink: https://lore.kernel.org/git/CAJHH8bE0g1N6a2jTeHYiUCzsR1-VnAp5dWu-9RMJEP0ixt=NRA@mail.gmail.com/T/#u

[Chand-ra]

Hey @dscho, I was trying to work on this issue but I couldn't find any instance in git/apply.c where file modes are explicitly changed/assigned except for when dealing with a new patch. Am I missing something or am I completely off?

[dscho]

As mentioned in the description, adding a file is precisely the scenario we're talking about, yes.

[Chand-ra]

So, is the fix doing something like:

if (core.fileMode) {   /*this line is added*/
 if (!patch->new_mode) {  
         if (0 < patch->is_new) 
           patch->new_mode = S_IFREG | 0644;  
         else  
           patch->new_mode = patch->old_mode
    }
}

in all those places where where a new file mode is explicitly assigned?

[dscho]

@Chand-ra in case core.fileMode = false, what we want in those code paths is to infer the correct mode from the Git index instead of the stat() information before warning the user about changing the file mode (and then ensuring that the file mode is unchanged in the Git index).

[Chand-ra]

So would doing something like this in the code snippet above suffice?

if (!patch->new_mode) {
      if (core.filemode) {  
         if (0 < patch->is_new) 
           patch->new_mode = S_IFREG | 0644;  
         else  
           patch->new_mode = patch->old_mode;
      }
      else {
         if (0 < patch->is_new) {
           patch->new_mode = ce->ce_mode;
           warning(_("%s has now type %o", name, patch->new_mode));
           \*code to ensure file mode is unchanged in git index*\
         }
         else 
           patch->new_mode = patch->old_mode;
       }
}

and then ensuring that the file mode is unchanged in the Git index

I am unsure of what exactly is it that we need to do here. Do we assert that the patch's new mode matches the file mode in the git index?

[dscho]

Do we assert that the patch's new mode matches the file mode in the git index?

That is exactly what I tried to say by "then ensuring that the file mode is unchanged in the Git index"

would doing something like this in the code snippet above suffice?

You can answer that question yourself by implementing a test case that only succeeds if the desired behavior is achieved.

[Chand-ra]

Hey @dscho I was trying to trigger the warning you mentioned in your original comment but couldn't do it. I initialized an empty Git folder on my (Windows) desktop -> added a script.sh file to it -> used git diff --cached > new_patch.patch to create a patch that adds the said script file -> applied the patch to another Git folder but I didn't recieve any warnings.
I tried again on an Ubuntu Virtual Machine but that didn't help either. I tried a bunch of other combinations, creating the patch on Windows -> applying it in Ubuntu ; creating the patch on Ubuntu -> applying it in Windows but nothing triggered the warning.

What did seem to work though, was to create a folder with the same script.sh file on both Ubuntu and Windows, creating a patch that changes the said file on one of them and applying the change on the other using git apply. I wanted to confirm with you that this is the behavior that we're trying to rectify or if I have missed something in between.

[dscho]

I came up with this script:

#!/bin/sh

{ test ! -d apply-and-filemode || rm -rf apply-and-filemode; } &&
git init apply-and-filemode &&
cd apply-and-filemode &&
git commit -m initial --allow-empty &&

git config core.fileMode false &&
echo true >script.sh &&
git add --chmod=+x script.sh &&
git commit -m "Add script" &&

echo true >>script.sh &&
git commit -m "Modify script" script.sh &&
git format-patch -1 --stdout >patch &&

git switch -c branch HEAD^ &&
git apply patch

Both on Windows and on Linux, running the script will show this line as last one in its output:

warning: script.sh has type 100644, expected 100755

[Chand-ra]

Hey @dscho, the patch corresponding to this issue was recently committed to the 'seen' branch so this issue can now be closed.

[dscho]

I typically leave the tickets open until the patch entered master... 🙂

[Chand-ra]

@dscho the patch for this issue landed in master some time ago, so I think this is safe to close now.

[dscho]

the patch for this issue landed in master some time ago, so I think this is safe to close now.

Yes, indeed, thank you for keeping an eye on things @Chand-ra !