On Thu, 28 Feb 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
>
> > We need to rewrite the index file when we check out files, even if we
> > haven't modified the blob info by reading from another tree, so that
> > we get the stat cache to include the fact that we just modified the
> > file so it doesn't need to be refreshed.
>
> Thanks, Alexandre, for spotting.
>
> > -static int checkout_paths(const char **pathspec)
> > +static int checkout_paths(const char **pathspec, int newfd, struct lock_file *lock_file)
> > {
> > ...
> > @@ -554,11 +549,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> > }
> > }
> >
> > + int newfd;
> > + struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
> > + newfd = hold_locked_index(lock_file, 1);
> > + read_cache();
> > +
> > if (source_tree)
> > read_tree_some(source_tree, pathspec);
> > - else
> > - read_cache();
> > - return checkout_paths(pathspec);
> > + return checkout_paths(pathspec, newfd, lock_file);
>
> Aside from decl-after-statement, I suspect that at this point
> these all should go to checkout_paths() function itself, that
> takes pathspec and source_tree (which could be NULL), but that
> is only "logical code organization" issue. Thanks for fixing.
Yeah, that's a better organization, and makes the code nice. Want it as a
replacement patch or as an additional patch on top of this one?
> I however would have liked if this were caught while the topic
> was still cooking in 'next'.
>
> There was a process failure somewhere, which makes me worry more
> than just this single bug that escaped to 'master'. Alex
> Riesen's segv fix in another thread makes it double X-<.
This one is a stat cache problem, and those tend not to get noticed for a
while in normal use, simply because everything works fine, but makes
subsequent operations a bit slower.
Alex's is a case where an operation that should fail fails ungracefully,
which is again unlikely to be uncovered by normal use, because people just
don't do that.
I think that cooking in next isn't likely to matter much to either of
these classes of problems, and we need to catch them in regression
testing. They're also not the sort of thing that we're likely to test for
specifically, so it would be nice to have something in test-lib to do it
in general. It would be nice to have automatic checks after every test
that nothing segfaulted, and a helper (which could be sprinkled liberally
through the test suite) to make sure that nothing has stale stat cache
info (without being actually changed).
-Daniel
*This .sig left intentionally blank*
--
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
