From: Junio C Hamano <hidden> Date: 2016-06-15 22:55:37
Grégory Pakosz [off-list ref] writes:
Are you suggesting $sha1 should be obtained differently before
entering case "$rewritten" ?
That would mean changing sha1=$(git rev-parse "$ref"^0) at line 376 to
something like $(git cat-file -t "$ref") = 'tag' && sha1=$(git
rev-parse "$ref") || sha1=$(git rev-parse "$ref^0") ?
I was wondering if it should be
sha1=$(git rev-parse --verify "$ref")
or something that does not dereference a tag at all.
The way I read what that loop seems to want to do is:
Read each refname that was given originally from the file
$tempdir/heads, find out the object it used to refer to and
have it in $sha1, find out what new object the object was
rewritten to and have it in $rewritten, and:
(1) if the rewrite left the object unchanged, do nothing but
warn users just in case this was a mistake;
(2) if the rewrite told us to remove it, then delete the
ref; or
(3) if the rewrite gave us a new object, replace the ref to
point to that new one.
And in the latter two cases, save the original one in
$orig_namespace so that the user can choose to recover if
this filter-branch was done by mistake.
So I do not think unwraping the ref at that point makes any sense,
unless it is not prepared to handle annotated tags at all by
unwrapping tags too early.
What am I missing?
I was wondering if it should be
sha1=$(git rev-parse --verify "$ref")
or something that does not dereference a tag at all.
The way I read what that loop seems to want to do is:
Read each refname that was given originally from the file
$tempdir/heads, find out the object it used to refer to and
have it in $sha1, find out what new object the object was
rewritten to and have it in $rewritten, and:
(1) if the rewrite left the object unchanged, do nothing but
warn users just in case this was a mistake;
(2) if the rewrite told us to remove it, then delete the
ref; or
(3) if the rewrite gave us a new object, replace the ref to
point to that new one.
And in the latter two cases, save the original one in
$orig_namespace so that the user can choose to recover if
this filter-branch was done by mistake.
So I do not think unwraping the ref at that point makes any sense,
unless it is not prepared to handle annotated tags at all by
unwrapping tags too early.
What am I missing?
So we have an annotated tag that points to a commit that is rewritten
to nothing as the result of the filtering. What should happen?
My initial questions and patching suggestions are based on git
1.7.10.4 behavior.
However, playing with git HEAD exhibits a slightly different behavior:
it breaks when invoking git mktag line 459 (introduced by
1bf6551e42c79a594689a356a9b14759d55f3cf5):
error: char7: could not get SHA1 hash
fatal: invalid tag signature file
Could not create new tag object for tag-a
It's basically the same problem. In my opinion, lines 447-466 should
take into account $new_sha1 is empty.
Please forgive me again for not having configured my mailer yet :(
When I'm ready to provide a patch that implements a solution we all
agree with I'll use git send-email.
In the mean time, I would like to pursue the discussion in this mail
thread so please find attached a patch that deletes a tag instead of
invoking the tag-name-filter when it detects $new_sha1 is empty.
I tested the patch doesn't break t7003. What do you think?
Gregory
From: Junio C Hamano <hidden> Date: 2016-06-15 22:55:38
Grégory Pakosz [off-list ref] writes:
So we have an annotated tag that points to a commit that is rewritten
to nothing as the result of the filtering. What should happen?
If the user asked to filter that tag itself, it may make sense to
remove it, rather than keeping it pointing at the original commit,
because the commit it used to point at no longer exists in the
alternate history being created by filter-branch.
It's basically the same problem. In my opinion, lines 447-466 should
take into account $new_sha1 is empty.
Yeah, I think that is a sensible observation.
Having said that, I welcome comments from others, of course. My
involvement in this script has been very limited.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:55:38
Am 03.01.2013 00:19, schrieb Junio C Hamano:
Grégory Pakosz [off-list ref] writes:
quoted
So we have an annotated tag that points to a commit that is rewritten
to nothing as the result of the filtering. What should happen?
If the user asked to filter that tag itself, it may make sense to
remove it, rather than keeping it pointing at the original commit,
because the commit it used to point at no longer exists in the
alternate history being created by filter-branch.
IOW, if the command was something like
git filter-branch ...filter options... -- v1.0 master ...
and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
deleted if the commit it points to goes away. But if the commit did not
go away, but was rewritten, then it is equally reasonable to expect that
the tag is also rewritten. But I don't think that we currently do the
latter.
Therefore, IMO, a change that implements the former behavior should also
implement the latter behavior.
-- Hannes
IOW, if the command was something like
git filter-branch ...filter options... -- v1.0 master ...
and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
deleted if the commit it points to goes away. But if the commit did not
go away, but was rewritten, then it is equally reasonable to expect that
the tag is also rewritten. But I don't think that we currently do the
latter.
When the commit doesn't go away, the tag is currently being rewritten properly.
Therefore, IMO, a change that implements the former behavior should also
implement the latter behavior.
The patch in my latest email does both. (yet lacks unit tests for now)
Gregory
From: Johannes Sixt <hidden> Date: 2016-06-15 22:55:38
Am 03.01.2013 10:50, schrieb Grégory Pakosz:
quoted
IOW, if the command was something like
git filter-branch ...filter options... -- v1.0 master ...
and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
deleted if the commit it points to goes away. But if the commit did not
go away, but was rewritten, then it is equally reasonable to expect that
the tag is also rewritten. But I don't think that we currently do the
latter.
When the commit doesn't go away, the tag is currently being rewritten properly.
Indeed, but only if a --tag-name-filter was specified.
quoted
Therefore, IMO, a change that implements the former behavior should also
implement the latter behavior.
The patch in my latest email does both. (yet lacks unit tests for now)
If it deletes a tag only when --tag-name-filter was specified, than that
should be fine.
-- Hannes
On Thu, Jan 3, 2013 at 2:33 AM, Johannes Sixt [off-list ref] wrote:
Am 03.01.2013 10:50, schrieb Grégory Pakosz:
quoted
quoted
IOW, if the command was something like
git filter-branch ...filter options... -- v1.0 master ...
and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be
deleted if the commit it points to goes away. But if the commit did not
go away, but was rewritten, then it is equally reasonable to expect that
the tag is also rewritten. But I don't think that we currently do the
latter.
When the commit doesn't go away, the tag is currently being rewritten properly.
Indeed, but only if a --tag-name-filter was specified.
quoted
quoted
Therefore, IMO, a change that implements the former behavior should also
implement the latter behavior.
The patch in my latest email does both. (yet lacks unit tests for now)
If it deletes a tag only when --tag-name-filter was specified, than that
should be fine.
Hmm, if a tag name filter _other_ than 'cat' is supplied, I think a
user will expect that the original tags will _not_ be touched, and
especially not deleted.
Rather than blindly deleting the original tag ref, maybe we should
still call the user's tag name filter, and then attempt to delete the
"new" name provided by the filter, if it exists. If the filter was
'cat', then the new and old names will be the same.
-Brandon