git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Oct 9, 2011, 09:17 UTC
Message-ID
<20111009091617.GA29150@elie.hsd1.il.comcast.net>
In-Reply-To
<1313652227-48545-4-git-send-email-davvid@gmail.com>

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@gmail.com>
---
David Aguilar wrote:
> +++ b/Makefile
[...]
Show 7 quoted lines
> @@ -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
Previous: David AguilarNext: Jonathan Nieder
Message 8 of 18 in “Refactor git-mergetool--lib.sh”
  1. 0/4 Refactor git-mergetool--lib.shDavid Aguilar, Aug 18, 2011
  2. 1/4 difftool--helper: Make style consistent with gitDavid Aguilar, Aug 18, 2011
  3. Sebastian SchuberthAug 18, 2011
  4. David AguilarAug 18, 2011
  5. Junio C HamanoAug 18, 2011
  6. 2/4 mergetool--lib: Make style consistent with gitDavid Aguilar, Aug 18, 2011
  7. 3/4 mergetool--lib: Refactor tools into separate filesDavid Aguilar, Aug 18, 2011
  8. Makefile: fix permissions of mergetools/ checked out with permissive umaskJonathan Nieder, Oct 9, 2011
  9. Jonathan NiederOct 9, 2011
  10. David AguilarOct 9, 2011
  11. Jonathan NiederOct 9, 2011
  12. Junio C HamanoOct 11, 2011
  13. 4/4 mergetools/meld: Use '--output' when availableDavid Aguilar, Aug 18, 2011
  14. Jonathan NiederAug 18, 2011
  15. David AguilarAug 18, 2011
  16. 4/4 mergetools/meld: Use '--output' when availableDavid Aguilar, Aug 19, 2011
  17. Junio C HamanoAug 19, 2011
  18. Sebastian SchuberthAug 18, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.