[PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask

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

 



Ever since mergetool--lib was split into multiple files in
v1.7.7-rc0~3^2~1 (2011-08-18), the Makefile takes care to reset umask
and use tar --no-owner when installing merge tool definitions to
$(gitexecdir)/mergetools/.  Unfortunately it does not take into
account the possibility that the permission bits of the files being
copied might already be wrong.

Rather than fixing the "tar" incantation and making it even more
complicated, let's just use the "install" utility.  This only means
losing the ability to install executables and subdirectories of
mergetools/, which wasn't used.

Noticed by installing from a copy of git checked out with umask 002.
Compare v1.6.0.3~81^2 (Fix permission bits on sources checked out with
an overtight umask, 2008-08-21).

Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
David Aguilar wrote:

> +++ b/Makefile
[...]
> @@ -2266,6 +2274,9 @@ install: all
>  	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
> +	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> +	(cd mergetools && $(TAR) cf - .) | \
> +	(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)

Last month I tried this out and found that, strangely, my files under
/usr/lib/git-core/mergetools/ had the g+w bit set.  Leading me to
wonder: does the "umask" here have any effect at all?

Since debian/rules install is run as root, the default is for tar to
act as thought --preserve-permissions were passed, so the umask when
running "tar" is not relevant.  Luckily I think "tar" is overkill
here, anyway.

Thoughts?  Sorry to have taken so long to send this out.

 Makefile |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1e91b19c..e27755e7 100644
--- a/Makefile
+++ b/Makefile
@@ -2275,8 +2275,7 @@ install: all
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
-	(cd mergetools && $(TAR) cf - .) | \
-	(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)
+	$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C gitweb install
-- 
1.7.7.rc1

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


[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]