filemode=false somewhat broken

filemode=false somewhat broken

[Juergen Ruehle]

Commit fd28b34afd9bbd58297a25edced3f504c9a5487a tried to ignore the
executable bit if filemode=false, but instead forced all files to be
regular with 644 permission bits nuking symlink support. The attached
diff works better for me, but note that I'm completely posix agnostic.

Unfortunately there still seems to be another problem somewhere: files
merged by octopus (wanted to try one, too, after the recent
discussions on the list:-) lose the x bits in the process. But as
usual I'm not smart enough to find the problem:-(

diff --git a/builtin-update-index.c b/builtin-update-index.c
index 7f9c638..f4b4bc4 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -112,13 +112,13 @@ static int add_file_to_cache(const char
 	ce->ce_mode = create_ce_mode(st.st_mode);
 	if (!trust_executable_bit) {
 		/* If there is an existing entry, pick the mode bits
-		 * from it, otherwise force to 644.
+		 * from it, otherwise use mask 666.
 		 */
 		int pos = cache_name_pos(path, namelen);
 		if (0 <= pos)
 			ce->ce_mode = active_cache[pos]->ce_mode;
 		else
-			ce->ce_mode = create_ce_mode(S_IFREG | 0644);
+			ce->ce_mode = create_ce_mode((S_IFMT | 0666) & st.st_mode);
 	}
 
 	if (index_path(ce->sha1, path, &st, !info_only))
diff --git a/read-cache.c b/read-cache.c
index 97c3867..6cbbecf 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -347,13 +347,13 @@ int add_file_to_index(const char *path,
 	ce->ce_mode = create_ce_mode(st.st_mode);
 	if (!trust_executable_bit) {
 		/* If there is an existing entry, pick the mode bits
-		 * from it, otherwise force to 644.
+		 * from it, otherwise use mask 666.
 		 */
 		int pos = cache_name_pos(path, namelen);
 		if (pos >= 0)
 			ce->ce_mode = active_cache[pos]->ce_mode;
 		else
-			ce->ce_mode = create_ce_mode(S_IFREG | 0644);
+			ce->ce_mode = create_ce_mode((S_IFMT | 0666) & st.st_mode);
 	}
 
 	if (index_path(ce->sha1, path, &st, 1))
-- 
1.4.4.g540c

Re: filemode=false somewhat broken

[Junio C Hamano]

Juergen Ruehle <j.ruehle@bmiag.de> writes:

> Commit fd28b34afd9bbd58297a25edced3f504c9a5487a tried to ignore the
> executable bit if filemode=false, but instead forced all files to be
> regular with 644 permission bits nuking symlink support.

Thanks for noticing.  Yes, that change is _seriously_ broken.  I
should really raise the bar for patch acceptance.

Re: filemode=false somewhat broken

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Juergen Ruehle <j.ruehle@bmiag.de> writes:
> 
> > Commit fd28b34afd9bbd58297a25edced3f504c9a5487a tried to ignore the
> > executable bit if filemode=false, but instead forced all files to be
> > regular with 644 permission bits nuking symlink support.
> 
> Thanks for noticing.  Yes, that change is _seriously_ broken.  I
> should really raise the bar for patch acceptance.

Whoops.

I don't work with symlinks so I never noticed the breakage myself.
Clearly the test cases that I created in fd28b3 should also have
included symlinks, but they did not...  and we see this breakage
occur.

--