* [PATCH v2 1/4] t6050: use test_line_count instead of wc -l
2019-03-31 13:46 [PATCH v2 0/4] fix `git replace --graft` tag related issues Christian Couder
@ 2019-03-31 13:46 ` Christian Couder
2019-03-31 13:46 ` [PATCH v2 2/4] t6050: redirect expected error output to a file Christian Couder
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2019-03-31 13:46 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Andreas Schwab, Taylor Blau, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Christian Couder
This modernizes a test and makes it more portable.
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t6050-replace.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d638119750..41b177936e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -393,9 +393,11 @@ test_expect_success 'replace ref cleanup' '
'
test_expect_success '--graft with and without already replaced object' '
- test $(git log --oneline | wc -l) = 7 &&
+ git log --oneline >log &&
+ test_line_count = 7 log &&
git replace --graft $HASH5 &&
- test $(git log --oneline | wc -l) = 3 &&
+ git log --oneline >log &&
+ test_line_count = 3 log &&
commit_has_parents $HASH5 &&
test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 &&
git replace --force -g $HASH5 $HASH4 $HASH3 &&
--
2.21.0.69.g2d86a08d09
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] t6050: redirect expected error output to a file
2019-03-31 13:46 [PATCH v2 0/4] fix `git replace --graft` tag related issues Christian Couder
2019-03-31 13:46 ` [PATCH v2 1/4] t6050: use test_line_count instead of wc -l Christian Couder
@ 2019-03-31 13:46 ` Christian Couder
2019-03-31 13:46 ` [PATCH v2 3/4] replace: fix --graft when passing a tag as parent Christian Couder
2019-03-31 13:46 ` [PATCH v2 4/4] replace: fix --graft when passing a tag first Christian Couder
3 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2019-03-31 13:46 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Andreas Schwab, Taylor Blau, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Christian Couder
Otherwise the error from `git rev-parse` is uselessly
polluting the debug output.
Redirecting to a file, instead of /dev/null, makes it
possible to check that we got the error we expected, so
let's check that too.
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t6050-replace.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 41b177936e..948d278482 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -40,7 +40,8 @@ commit_peeling_shows_parents ()
test "$_found" = "$_parent" || return 1
_parent_number=$(( $_parent_number + 1 ))
done &&
- test_must_fail git rev-parse --verify $_commit^$_parent_number
+ test_must_fail git rev-parse --verify $_commit^$_parent_number 2>err &&
+ test_i18ngrep "Needed a single revision" err
}
commit_has_parents ()
--
2.21.0.69.g2d86a08d09
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] replace: fix --graft when passing a tag as parent
2019-03-31 13:46 [PATCH v2 0/4] fix `git replace --graft` tag related issues Christian Couder
2019-03-31 13:46 ` [PATCH v2 1/4] t6050: use test_line_count instead of wc -l Christian Couder
2019-03-31 13:46 ` [PATCH v2 2/4] t6050: redirect expected error output to a file Christian Couder
@ 2019-03-31 13:46 ` Christian Couder
2019-03-31 13:46 ` [PATCH v2 4/4] replace: fix --graft when passing a tag first Christian Couder
3 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2019-03-31 13:46 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Andreas Schwab, Taylor Blau, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Christian Couder
When passing a tag as a parent argument to `git replace --graft`,
it can be useful to accept it and use the underlying commit as a
parent.
This already works for lightweight tags, but unfortunately
for annotated tags we have been using the hash of the tag object
instead of the hash of the underlying commit as a parent in the
replacement object we create.
This created invalid objects, but the replace succeeded even if
it showed an error like:
error: object A is a tag, not a commit
This patch fixes that by using the hash of the underlying commit
when an annotated tag is passed.
While at it, let's also update an error message to make it
clearer.
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replace.c | 9 ++++++---
t/t6050-replace.sh | 11 +++++++++++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index f5701629a8..b0a9227f9a 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -370,16 +370,19 @@ static int replace_parents(struct strbuf *buf, int argc, const char **argv)
/* prepare new parents */
for (i = 0; i < argc; i++) {
struct object_id oid;
+ struct commit *commit;
+
if (get_oid(argv[i], &oid) < 0) {
strbuf_release(&new_parents);
return error(_("not a valid object name: '%s'"),
argv[i]);
}
- if (!lookup_commit_reference(the_repository, &oid)) {
+ commit = lookup_commit_reference(the_repository, &oid);
+ if (!commit) {
strbuf_release(&new_parents);
- return error(_("could not parse %s"), argv[i]);
+ return error(_("could not parse %s as a commit"), argv[i]);
}
- strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
+ strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&commit->object.oid));
}
/* replace existing parents with new ones */
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 948d278482..2385a60f68 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -406,6 +406,17 @@ test_expect_success '--graft with and without already replaced object' '
git replace -d $HASH5
'
+test_expect_success '--graft using a tag as the new parent' '
+ git tag new_parent $HASH5 &&
+ git replace --graft $HASH7 new_parent &&
+ commit_has_parents $HASH7 $HASH5 &&
+ git replace -d $HASH7 &&
+ git tag -a -m "annotated new parent tag" annotated_new_parent $HASH5 &&
+ git replace --graft $HASH7 annotated_new_parent &&
+ commit_has_parents $HASH7 $HASH5 &&
+ git replace -d $HASH7
+'
+
test_expect_success GPG 'set up a signed commit' '
echo "line 17" >>hello &&
echo "line 18" >>hello &&
--
2.21.0.69.g2d86a08d09
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] replace: fix --graft when passing a tag first
2019-03-31 13:46 [PATCH v2 0/4] fix `git replace --graft` tag related issues Christian Couder
` (2 preceding siblings ...)
2019-03-31 13:46 ` [PATCH v2 3/4] replace: fix --graft when passing a tag as parent Christian Couder
@ 2019-03-31 13:46 ` Christian Couder
2019-04-01 8:48 ` Junio C Hamano
3 siblings, 1 reply; 6+ messages in thread
From: Christian Couder @ 2019-03-31 13:46 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Andreas Schwab, Taylor Blau, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Christian Couder
When passing a tag as the first argument to `git replace --graft`,
it can be useful to accept it and use the underlying commit as a
the commit that will be replaced.
This already works for lightweight tags, but unfortunately
for annotated tags we have been using the hash of the tag object
instead of the hash of the underlying commit.
Especially we would pass the hash of the tag object to
replace_object_oid() where we would likely fail with an error
like:
"error: Objects must be of the same type.
'annotated_replaced_object' points to a replaced object of type 'tag'
while 'replacement' points to a replacement object of type 'commit'."
This patch fixes that by using the hash of the underlying commit
when an annotated tag is passed.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replace.c | 11 +++++++----
t/t6050-replace.sh | 11 +++++++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index b0a9227f9a..644b21ca8d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -481,15 +481,18 @@ static int create_graft(int argc, const char **argv, int force, int gentle)
strbuf_release(&buf);
- if (oideq(&old_oid, &new_oid)) {
+ if (oideq(&commit->object.oid, &new_oid)) {
if (gentle) {
- warning(_("graft for '%s' unnecessary"), oid_to_hex(&old_oid));
+ warning(_("graft for '%s' unnecessary"),
+ oid_to_hex(&commit->object.oid));
return 0;
}
- return error(_("new commit is the same as the old one: '%s'"), oid_to_hex(&old_oid));
+ return error(_("new commit is the same as the old one: '%s'"),
+ oid_to_hex(&commit->object.oid));
}
- return replace_object_oid(old_ref, &old_oid, "replacement", &new_oid, force);
+ return replace_object_oid(old_ref, &commit->object.oid,
+ "replacement", &new_oid, force);
}
static int convert_graft_file(int force)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 2385a60f68..e7e64e085d 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -417,6 +417,17 @@ test_expect_success '--graft using a tag as the new parent' '
git replace -d $HASH7
'
+test_expect_success '--graft using a tag as the replaced object' '
+ git tag replaced_object $HASH7 &&
+ git replace --graft replaced_object $HASH5 &&
+ commit_has_parents $HASH7 $HASH5 &&
+ git replace -d $HASH7 &&
+ git tag -a -m "annotated replaced object tag" annotated_replaced_object $HASH7 &&
+ git replace --graft annotated_replaced_object $HASH5 &&
+ commit_has_parents $HASH7 $HASH5 &&
+ git replace -d $HASH7
+'
+
test_expect_success GPG 'set up a signed commit' '
echo "line 17" >>hello &&
echo "line 18" >>hello &&
--
2.21.0.69.g2d86a08d09
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 4/4] replace: fix --graft when passing a tag first
2019-03-31 13:46 ` [PATCH v2 4/4] replace: fix --graft when passing a tag first Christian Couder
@ 2019-04-01 8:48 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2019-04-01 8:48 UTC (permalink / raw)
To: Christian Couder
Cc: git, Andreas Schwab, Taylor Blau, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
> When passing a tag as the first argument to `git replace --graft`,
> it can be useful to accept it and use the underlying commit as a
> the commit that will be replaced.
Yeah, another plausible fix would be to make it an error, but the
message for the error would most likely tell the user to peel the
tag, so peeling it ourselves would make sense.
^ permalink raw reply [flat|nested] 6+ messages in thread