threads / patch / 43089

patchRe: [PATCH] Enable reflogs by default in any repository with a working directory.

Subject: Re: [PATCH] Enable reflogs by default in any repository with a working directory.

## tl;dr

9 messages between Dec 14, 2006 and Dec 15, 2006. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Junio C Hamano· Dec 14, 2006, 20:35 UTC · lore

Re: What's in git.git (stable)

Shawn Pearce <spearce@spearce.org> writes:
> Does anyone on the mailing list really have an objection to having
> reflogs on by default?

When you talk about potential breakage for existing users, you should not be asking people on THIS list. You instead should talk with or at least think about people on linux-kernel, x.org and wine people, and possibly others. git is maturing, and we cannot expect that most of the users are paying attention to what is happening on this list anymore.

I 100% agree that it makes sense to have reflog enabled for a repository with an associated worktree. I would say that we do not even need it to be conditional on the configuration variable for such a repository.

My answer to your question is:
	kernel.org:/pub/scm/

I would REALLY be worried to have reflog enabled at a public distribution point where the only ways the owners interact with it daily are 'git push' and 'git pull'. As you mentioned, there is one extra potential receive-pack failure, and in general it is one more thing that can go wrong, and hard to notice breakage because it is on the other side of the connection.

Worse yet, there is no easy way to garbage collect. Even in an end-user repository with a worktree, the only way to garbage collect older reflog entries is to edit the reflog files to remove the top part.

Maybe a check to say if $GIT_DIR is ".git" or ends with "/.git" then enable it and otherwise honor the configuration variable, without changing the default in the code (with your patch) nor in the default configuration ("enable for new repositories" as I suggested) might be a workable compromise.

Shawn O. Pearce· Dec 14, 2006, 22:41 UTC · re: Junio C Hamano · lore

[PATCH] Enable reflogs by default in any repository with a working directory.

New and experienced Git users alike are finding out too late that they forgot to enable reflogs in the current repository, and cannot use the information stored within it to recover from an incorrectly entered command such as `git reset --hard HEAD^^^` when they really meant HEAD^^ (aka HEAD~2).

So enable reflogs by default in all future versions of Git, unless the user specifically disables it with:

  [core]
    logAllRefUpdates = false
in their .git/config or ~/.gitconfig.

We only enable reflogs in repositories that have a working directory associated with them, as shared/bare repositories do not have an easy means to prune away old log entries, or may fail logging entirely if the user's gecos information is not valid during a push. This heuristic was suggested on the mailing list by Junio.

Documentation was also updated to indicate the new default behavior. We probably should start to teach usuing the reflog to recover from mistakes in some of the tutorial material, as new users are likely to make a few along the way and will feel better knowing they can recover from them quickly and easily, without fsck-objects' lost+found features.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/config.txt |    7 +++++--
 builtin-init-db.c        |    4 ++++
 cache.h                  |    1 +
 environment.c            |    9 +++++++++
 4 files changed, 19 insertions(+), 2 deletions(-)
Show changes to 4 files +19 −2

Documentation/config.txt, builtin-init-db.c, cache.h, environment.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a3587f8..8abb082 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -79,8 +79,11 @@ core.logAllRefUpdates::
 	file is automatically created for branch heads.
 
 	This information can be used to determine what commit
-	was the tip of a branch "2 days ago".  This value is
-	false by default (no automated creation of log files).
+	was the tip of a branch "2 days ago".
+
+	This value is true by default in a repository that has
+	a working directory associated with it, and false by
+	default in a bare repository.
 
 core.repositoryFormatVersion::
 	Internal variable identifying the repository format and layout
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 235a0ee..214fc8e 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -239,6 +239,10 @@ static void create_default_files(const char *git_dir, const char *template_path)
 		git_config_set("core.filemode",
 			       filemode ? "true" : "false");
 	}
+
+	/* Enable logAllRefUpdates if a working tree is attached */
+	git_config_set("core.logallrefupdates",
+		!is_bare_git_dir(git_dir) ? "true" : "false");
 }
 
 static const char init_db_usage[] =
diff --git a/cache.h b/cache.h
index f2ec5c8..2d3df98 100644
--- a/cache.h
+++ b/cache.h
@@ -123,6 +123,7 @@ extern int cache_errno;
 #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
 #define GRAFT_ENVIRONMENT "GIT_GRAFT_FILE"
 
+extern int is_bare_git_dir(const char *dir);
 extern const char *get_git_dir(void);
 extern char *get_object_directory(void);
 extern char *get_refs_directory(void);
diff --git a/environment.c b/environment.c
index 84d870c..b7256eb 100644
--- a/environment.c
+++ b/environment.c
@@ -48,6 +48,15 @@ static void setup_git_env(void)
 	git_graft_file = getenv(GRAFT_ENVIRONMENT);
 	if (!git_graft_file)
 		git_graft_file = xstrdup(git_path("info/grafts"));
+	log_all_ref_updates = !is_bare_git_dir(git_dir);
+}
+
+int is_bare_git_dir (const char *dir)
+{
+	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
+		return 0;
+	const char *s = strrchr(dir, '/');
+	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
 }
 
 const char *get_git_dir(void)
J. Bruce Fields· Dec 14, 2006, 23:10 UTC · re: Shawn O. Pearce · lore
On Thu, Dec 14, 2006 at 05:41:17PM -0500, Shawn O. Pearce wrote:
Show 5 quoted lines
> New and experienced Git users alike are finding out too late that
> they forgot to enable reflogs in the current repository, and cannot
> use the information stored within it to recover from an incorrectly
> entered command such as `git reset --hard HEAD^^^` when they really
> meant HEAD^^ (aka HEAD~2).

Stupid question--I assume a mention in the reflog doesn't count as a real reference to an object, so they won't save you in the case when you pruned recently?

Shawn Pearce· Dec 14, 2006, 23:18 UTC · re: J. Bruce Fields · lore
"J. Bruce Fields" <bfields@fieldses.org> wrote:
Show 10 quoted lines
> On Thu, Dec 14, 2006 at 05:41:17PM -0500, Shawn O. Pearce wrote:
> > New and experienced Git users alike are finding out too late that
> > they forgot to enable reflogs in the current repository, and cannot
> > use the information stored within it to recover from an incorrectly
> > entered command such as `git reset --hard HEAD^^^` when they really
> > meant HEAD^^ (aka HEAD~2).
> 
> Stupid question--I assume a mention in the reflog doesn't count as a
> real reference to an object, so they won't save you in the case when you
> pruned recently?

Not a stupid question. Your assumption is correct, its not a real reference, so prune will remove things that the log mentions but that refs don't currently mention.

J. Bruce Fields· Dec 14, 2006, 23:42 UTC · re: Shawn Pearce · lore
On Thu, Dec 14, 2006 at 06:18:32PM -0500, Shawn Pearce wrote:
Show 8 quoted lines
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > Stupid question--I assume a mention in the reflog doesn't count as a
> > real reference to an object, so they won't save you in the case when you
> > pruned recently?
> 
> Not a stupid question.  Your assumption is correct, its not a real
> reference, so prune will remove things that the log mentions but
> that refs don't currently mention.
OK, thanks.  So we just need to make sure that's documented someplace.
Johannes Schindelin· Dec 15, 2006, 00:13 UTC · re: Shawn O. Pearce · lore
Hi,
On Thu, 14 Dec 2006, Shawn O. Pearce wrote:
Show 7 quoted lines
> +int is_bare_git_dir (const char *dir)
> +{
> +	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
> +		return 0;
> +	const char *s = strrchr(dir, '/');
> +	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
>  }

This function does not really determine if the repo is bare. I have no better name for it, though.

Ciao, Dscho

Shawn Pearce· Dec 15, 2006, 00:20 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 14 quoted lines
> Hi,
> 
> On Thu, 14 Dec 2006, Shawn O. Pearce wrote:
> 
> > +int is_bare_git_dir (const char *dir)
> > +{
> > +	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
> > +		return 0;
> > +	const char *s = strrchr(dir, '/');
> > +	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
> >  }
> 
> This function does not really determine if the repo is bare. I have no 
> better name for it, though.
guess_if_bare_git_dir ?

I struggled to name that thing because it can't really tell, its just guessing... but it is going to be right most of the time. Of course I'm sure there's some Git user somewhere who will confuse it.

Junio C Hamano· Dec 15, 2006, 21:55 UTC · re: Shawn Pearce · lore
Shawn Pearce <spearce@spearce.org> writes:
Show 21 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> Hi,
>> 
>> On Thu, 14 Dec 2006, Shawn O. Pearce wrote:
>> 
>> > +int is_bare_git_dir (const char *dir)
>> > +{
>> > +	if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))
>> > +		return 0;
>> > +	const char *s = strrchr(dir, '/');
>> > +	return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);
>> >  }
>> 
>> This function does not really determine if the repo is bare. I have no 
>> better name for it, though.
>
> guess_if_bare_git_dir ?
>
> I struggled to name that thing because it can't really tell, its just
> guessing... but it is going to be right most of the time.  Of course
> I'm sure there's some Git user somewhere who will confuse it.

I think the name is fine, but probably a comment in front would help unconfuse people.

	/* Does it look like a repository without a working tree? */

Unfortunately there currently are public bare repositories that have index under them because they were primed by rsync from developers' working repositories. I do not think it is unreasonable to persuade owners of them to drop index -- then we could use absence of $GIT_DIR/index as a strong clue that the repository is bare.

Shawn Pearce· Dec 14, 2006, 22:44 UTC · re: Junio C Hamano · lore

Re: What's in git.git (stable)

Junio C Hamano <junkio@cox.net> wrote:
Show 5 quoted lines
> Maybe a check to say if $GIT_DIR is ".git" or ends with "/.git"
> then enable it and otherwise honor the configuration variable,
> without changing the default in the code (with your patch) nor
> in the default configuration ("enable for new repositories" as I
> suggested) might be a workable compromise.

See my latest patch. Though that patch also sets the value in the config file, much as core.filemode is also set in the config file, based on the guess determined by init-db at the time it was executed.

← back to recent threads