# [PATCH] Add read_cache to builtin-check-attr 8 messages from 2007-08-14 to 2007-08-15. Participants: Brian Downing, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/9522 ## Brian Downing, 2007-08-14 13:18 Subject: [PATCH] Add read_cache to builtin-check-attr Message-ID: <11870975181798-git-send-email-bdowning@lavos.net> URL: https://gitlist.dev/e/11870975181798-git-send-email-bdowning%40lavos.net ``` We can now read .gitattributes files out of the index, but the index must be loaded for this to work. Signed-off-by: Brian Downing --- builtin-check-attr.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/builtin-check-attr.c b/builtin-check-attr.c index 9d77f76..d949733 100644 --- a/builtin-check-attr.c +++ b/builtin-check-attr.c @@ -1,4 +1,5 @@ #include "builtin.h" +#include "cache.h" #include "attr.h" #include "quote.h" @@ -10,6 +11,10 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) struct git_attr_check *check; int cnt, i, doubledash; + if (read_cache() < 0) { + die("invalid cache"); + } + doubledash = -1; for (i = 1; doubledash < 0 && i < argc; i++) { if (!strcmp(argv[i], "--")) -- 1.5.3.GIT ``` ## Brian Downing, 2007-08-14 13:22 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: <20070814132209.GJ21692@lavos.net> URL: https://gitlist.dev/e/20070814132209.GJ21692%40lavos.net In-Reply-To: <11870975181798-git-send-email-bdowning@lavos.net> ``` On Tue, Aug 14, 2007 at 08:18:38AM -0500, Brian Downing wrote: > We can now read .gitattributes files out of the index, but the index > must be loaded for this to work. This was supposed to be In-Reply-To Junio's patch, "attr.c: read .gitattributes from index as well." It's not much use without it. -bcd ``` ## Johannes Schindelin, 2007-08-14 14:08 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0708141506260.25989%40racer.site In-Reply-To: <20070814132209.GJ21692@lavos.net> ``` Hi, On Tue, 14 Aug 2007, Brian Downing wrote: > On Tue, Aug 14, 2007 at 08:18:38AM -0500, Brian Downing wrote: > > We can now read .gitattributes files out of the index, but the index > > must be loaded for this to work. > > This was supposed to be In-Reply-To Junio's patch, "attr.c: read > .gitattributes from index as well." It's not much use without it. Shouldn't read_cache() be _only_ called if - it has not been read yet, and - .gitattributes was not found in the work tree? IOW check-attr is the wrong place for your patch IMHO. Ciao, Dscho ``` ## Brian Downing, 2007-08-14 14:24 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: <20070814142428.GK21692@lavos.net> URL: https://gitlist.dev/e/20070814142428.GK21692%40lavos.net In-Reply-To: ``` On Tue, Aug 14, 2007 at 03:08:52PM +0100, Johannes Schindelin wrote: > Shouldn't read_cache() be _only_ called if > > - it has not been read yet, and > - .gitattributes was not found in the work tree? > > IOW check-attr is the wrong place for your patch IMHO. I admit I just cargo-culted what builtin-checkout-index did upon starting. Off the cuff, though, I don't see how the cache could ever already be loaded upon the start of cmd_check_attr, and the way the attr.c code is written, the cache be loaded when we check attributes or it will default to the old behavior (only checking the working directory.) What would you suggest here? -bcd ``` ## Johannes Schindelin, 2007-08-14 14:46 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0708141540420.25989%40racer.site In-Reply-To: <20070814142428.GK21692@lavos.net> ``` Hi, On Tue, 14 Aug 2007, Brian Downing wrote: > On Tue, Aug 14, 2007 at 03:08:52PM +0100, Johannes Schindelin wrote: > > Shouldn't read_cache() be _only_ called if > > > > - it has not been read yet, and > > - .gitattributes was not found in the work tree? > > > > IOW check-attr is the wrong place for your patch IMHO. > > I admit I just cargo-culted what builtin-checkout-index did upon starting. > Off the cuff, though, I don't see how the cache could ever already be > loaded upon the start of cmd_check_attr, Right. I was talking more about read_cache() being called later anyway, so you do not have to read the cache if a .gitattributes is there and you do not need the index to begin with. > and the way the attr.c code is > written, the cache be loaded when we check attributes or it will default > to the old behavior (only checking the working directory.) Why not just make sure that the index is read in read_index_data()? Something like /* read index if that was not already done yet */ if (!istate->mmap) read_index(&istate); (Yes, I know that read_index() calls read_index_from(), which in turn checks that, but read_attr() is called possibly pretty often, right? So we might just as well spare a few cycles here.) Ciao, Dscho ``` ## Junio C Hamano, 2007-08-14 18:38 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: <7vhcn2c673.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vhcn2c673.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <11870975181798-git-send-email-bdowning@lavos.net> ``` Brian Downing writes: > We can now read .gitattributes files out of the index, but the index > must be loaded for this to work. That interface is at too low a level, I am afraid. Many commands do want to control when they read the index and it affects the result, especially when the work tree traversal implemented in dir.c is involved. I am not rejecting/objecting, but just raising concerns. I do not have time to review this today, but just wanted to see if you fully assessed the implications (and if so that would save work on my end). ``` ## Brian Downing, 2007-08-14 18:45 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: <20070814184541.GL21692@lavos.net> URL: https://gitlist.dev/e/20070814184541.GL21692%40lavos.net In-Reply-To: <7vhcn2c673.fsf@assigned-by-dhcp.cox.net> ``` On Tue, Aug 14, 2007 at 11:38:24AM -0700, Junio C Hamano wrote: > That interface is at too low a level, I am afraid. Many > commands do want to control when they read the index and it > affects the result, especially when the work tree traversal > implemented in dir.c is involved. > > I am not rejecting/objecting, but just raising concerns. I do > not have time to review this today, but just wanted to see if > you fully assessed the implications (and if so that would save > work on my end). I really don't understand the implications. That was just something that got it working on my end, and I figured I should send it along in case it was just that simple. -bcd ``` ## Junio C Hamano, 2007-08-15 05:45 Subject: Re: [PATCH] Add read_cache to builtin-check-attr Message-ID: <7veji5jqp6.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7veji5jqp6.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070814184541.GL21692@lavos.net> ``` Ah, silly me. The patch was to builtin-check-attr. I somehow thought it was to attr.c::git_checkattr(). Thanks, will apply. ```