Hi,
On Fri, 6 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > And yeah, the git-gui part should be factored out, I guess. Shawn?
>
> I'll exclude git-gui part and commit with a minor tweaks;
Thank you. I planned to redo the patch when consensus is reached that this
is actually a good patch. But as usual, you're faster.
> > -int cmd_rerere(int argc, const char **argv, const char *prefix)
> > +int is_rerere_enabled(void)
> > {
>
> This will be "static".
But of course!
> > + const char *rr_cache = git_path("rr-cache");
> > + int rr_cache_exists;
> >
> > - if (stat(git_path("rr-cache"), &st) || !S_ISDIR(st.st_mode))
> > + if (!rerere_enabled)
> > return 0;
>
> As git_path() is not zero-cost, assignment to rr_cache will be
> moved here.
Yes, fully agree. It probably does not matter much right now, as
git-rerere is mostly called from scripts, and thus does a fork() && exec()
anyway, but I am smilingly awaiting the first full-fledged builtin
implementation of "git commit", in which case this very function should
move somewhere else, and there it does matter a bit more.
> > + rr_cache_exists = !stat(rr_cache, &st) && S_ISDIR(st.st_mode);
> > + if (rerere_enabled < 0)
> > + return rr_cache_exists;
> > +
> > + if (!rr_cache_exists && (mkdir(rr_cache, 0777) ||
> > + adjust_shared_perm(rr_cache)))
> > + die("Could not create directory %s", rr_cache);
> > + return 1;
> > +}
>
> If rr-cache is a regular file, we will hit "Could not create
> directory" which is exactly what we want anyway. Even if it is
> a dangling symlink, it would fail with "File exists", so that
> should be Ok.
That was the intended error path, yes. It does not safeguard against old
behaviour, where you could ": > .git/rr-cache", and be wondering why it is
not activating rr-cache ;-)
Ciao,
Dscho
-
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
