On 7/18/2018 4:17 PM, Jeff King wrote:
We can already disable replace refs using a command line
option or environment variable, but those are awkward to
apply universally. Let's add a config option to do the same
thing.
That raises the question of why one might want to do so
universally. The answer is that replace refs violate the
immutability of objects. For instance, if you wanted to
cache the diff between commit XYZ and its parent, then in
theory that never changes; the hash XYZ represents the total
state. But replace refs violate that; pushing up a new ref
may create a completely new diff.
The obvious "if it hurts, don't do it" answer is not to
create replace refs if you're doing this kind of caching.
But for a site hosting arbitrary repositories, they may want
to allow users to share replace refs with each other, but
not actually respect them on the site (because the caching
is more important than the replace feature).
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
We've been using this patch for about 4 years at GitHub. I'm not sure
why I never sent it upstream until now, since it's pretty trivial.
I think this should interact OK with Stefan's recent c3c36d7de2
(replace-object: check_replace_refs is safe in multi repo environment,
2018-04-11), because we still consider check_replace_refs before doing
anything in lookup_replace_object(). So even if you accidentally
_loaded_ the replace refs due to a timing issue (e.g., looking at them
before reading config) we still wouldn't respect them as long as you've
loaded config by the time you're actually looking at objects.
I followed the existing style of t6050 here, but it looks like it could
use some modernization (indent with spaces, and piping git command
output losing the exit codes).
Documentation/config.txt | 5 +++++
config.c | 5 +++++
t/t6050-replace.sh | 6 ++++++
3 files changed, 16 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1cc18a828c..92b277d27b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -908,6 +908,11 @@ core.commitGraph::
Enable git commit graph feature. Allows reading from the
commit-graph file.
+core.useReplaceRefs::
+ If set to `false`, behave as if the `--no-replace-objects`
+ option was given on the command line. See linkgit:git[1] and
+ linkgit:git-replace[1] for more information.
+
core.sparseCheckout::
Enable "sparse checkout" feature. See section "Sparse checkout" in
linkgit:git-read-tree[1] for more information.
diff --git a/config.c b/config.c
index f4a208a166..ce103ebc20 100644
--- a/config.c
+++ b/config.c
@@ -1346,6 +1346,11 @@ static int git_default_core_config(const char *var, const char *value)
var, value);
}
+ if (!strcmp(var, "core.usereplacerefs")) {
+ check_replace_refs = git_config_bool(var, value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index aa3e249639..86374a9c52 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -113,6 +113,12 @@ test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
'
+test_expect_success 'test core.usereplacerefs config option' '
+ test_config core.usereplacerefs false &&
+ git cat-file commit $HASH2 | grep "author A U Thor" &&
+ git show $HASH2 | grep "A U Thor"
+'
+
cat >tag.sig <<EOF
object $HASH2
type commit
This patch looks good to me. The only thing I saw was when I ran 'git
grep check_replace_refs' and saw the following in environment.c:
int check_replace_refs = 1; /* NEEDSWORK: rename to
read_replace_refs */
This does help me feel confident that the case where the config value is
missing will default to 'yes, please check replace refs', but also the
NEEDSWORK could be something to either (1) do, or (2) remove the
comment. Neither needs to happen as part of this patch.
Reviewed-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
