From: Junio C Hamano Date: Wed, 11 Aug 2010 22:19:04 GMT Subject: Re: [PATCH/RFC] Add global and system-wide gitattributes Message-ID: <7vzkwsvllz.fsf@alter.siamese.dyndns.org> In-Reply-To: <1281488646-7108-1-git-send-email-gsvick@gmail.com> Petr Onderka writes: > @@ -480,7 +501,15 @@ static void bootstrap_attr_stack(void) > debug_push(elem); > } > > - elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1); > + elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1, NULL); > + home = get_home_directory(); > + if (git_attr_global() && home) { > + char *user_attr = xstrdup(mkpath("%s/%s", home, GITATTRIBUTES_FILE)); > + elem = read_attr_from_file(user_attr, 1, elem); > + free(user_attr); > + } > + if (git_attr_system()) > + elem = read_attr_from_file(git_etc_gitattributes(), 1, elem); Have you read the comment at the top of prepare-attr-stack? This patch feels triply wrong: - The attribute stack is arranged to have higher precedence file near the top ($GIT_DIR/info/attributes used to be the highest). The above addition means that ~/.gitattributes from user's home trumps what is in a particular repository. That is backwards. You may work on more than one projects and have more than one repositories. What you share among them personally will go to ~/.gitattributes, while a setting specific to a particular repository goes to $GIT_DIR/info/attributes and the latter needs to be able to override the former. - Same thing for git_attr_system() being at the end, which means you set up your own $GIT_DIR/info/attributes (or ~/.gitattributes) carefully but that can be broken by a selfish sysadmin who puts stuff that is only useful to him in /etc/gitattributes, which is not what you want. - Whenever we enter a new directory (either recursing into, or coming back up), prepare_attr_stack() is called to pop the attributes records from now-exited directories and push the attributes records from directories we are about to descend into. The current code knows that the topmost element on the stack is special ($GIT_DIR/info/attributes) and first pops it, adjust the stack for elements that came from the directory hierarcy, and then pushes that back. I don't see any code in the patch to do the equivalent for these two new attribute sources.