Re: [PATCH] push: warn users about updating existing tags on push

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Sep 1, 2010, 03:51 UTC
Message-ID: <AANLkTinn0Evi6tYMSt+FevJnFt1taQVzqhJKuiGKudOy@mail.gmail.com>
In-Reply-To: <1282983736-3233-1-git-send-email-cxreg@pobox.com>

Hi,

On Sat, Aug 28, 2010 at 4:22 PM, Dave Olszewski <cxreg@pobox.com> wrote:

Show 10 quoted lines

> Generally, tags are considered a write-once ref (or object), and updates
> to them are the exception to the rule.  This is evident from the
> behavior of "git fetch", which will not update a tag it already has
> unless --tags is specified, from the --force option to "git tag", and
> the fact that Git does not keep reflogs for tags.
>
> However, there is presently nothing preventing a tag from being
> fast-forwarded, which can happen intentionally or accidentally.  In both
> cases, the user should be aware that they are changing something that is
> expected to be immutable and stable.

Sounds like a pretty good idea.

I think we could also expose this as a command-line option - say, --force-tags.

Also, how will this handle a remote config like this?

  [remote "foo"]
    push = +refs/tags/*

Show 17 quoted lines

> [snip]
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 05ec3fe..02dfc96 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> [snip]
> @@ -1545,6 +1548,10 @@ push.default::
>  * `tracking` push the current branch to its upstream branch.
>  * `current` push the current branch to a branch of the same name.
>
> +push.denyMovingTags::
> +       Whether or not a user will be allowed to push a tag that already
> +       exists on the remote for a different object.  False by default.
> +
>  rebase.stat::
>        Whether to show a diffstat of what changed upstream since the last
>        rebase. False by default.

Hmm, it's a little weird to speak of "allowing" the user to do this and that. Perhaps

	Whether or not a push will be allowed to proceed if a tag...

Show 12 quoted lines

> @@ -1593,6 +1600,11 @@ receive.denyNonFastForwards::
>        even if that push is forced. This configuration variable is
>        set when initializing a shared repository.
>
> +receive.denyMovingTags::
> +       If set to true, git-receive-pack will deny an update to a tag which
> +       already points to a different object.  Use this to prevent such an
> +       update via a push, even if that push is forced.
> +
>  receive.updateserverinfo::
>        If set to true, git-receive-pack will run git-update-server-info
>        after receiving data from git-push and updating refs.

Perhaps

	If set to true, git-receive-pack will refuse to update to a tag to point the
	tag to a	different object.  Use this...

Show 15 quoted lines

> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 658ff2f..1d53e04 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -112,7 +112,10 @@ nor in any Push line of the corresponding remotes file---see below).
>        Usually, the command refuses to update a remote ref that is
>        not an ancestor of the local ref used to overwrite it.
>        This flag disables the check.  This can cause the
> -       remote repository to lose commits; use it with care.
> +       remote repository to lose commits; use it with care.  This
> +       flag will also allow a previously pushed tag to be updated
> +       to point to a new commit, which is refused if
> +       push.denyMovingTags is set to true.
>  --repo=<repository>::
>        This option is only relevant if no <repository> argument is

Perhaps

	remote repository to lose commits; use it with care.

	Note that for tags that have already been pushed and have been updated
	locally, \--force will not update them if push.denyMovingTags is set to true.

-- 
Cheers,
Ray Chuan