Dave Olszewski <cxreg@xxxxxxxxx> writes:
> Generally, tags are considered a write-once ref (or object), and updates
> to them are the exception to the rule.
This may be just the naming issue and you could say "moving them",
"updates to them" or "changing them" interchangeably in the above;
among them, "updates to them" sounds the most natural.
Can you change the "moving" in the patch to make them consistent with the
above description?
> However, there is presently nothing preventing a tag from being
> fast-forwarded, which can happen intentionally or accidentally.
> ... the user should be aware that they are changing something that is
> expected to be immutable and stable.
I actually think prevention of non-fast-forward updates for tags actually
is a misfeature that didn't even come from any concious design; the check
for fast-forwarding refs was to make sure we do not lose histories from
branches. IOW, I would say this would have been a good feature if things
were like this from day one.
> diff --git a/remote.c b/remote.c
> index 9143ec7..fbca1e6 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -50,6 +50,8 @@ static int explicit_default_remote_name;
> static struct rewrites rewrites;
> static struct rewrites rewrites_push;
>
> +static int deny_moving_tags;
> +
> #define BUF_SIZE (2048)
> static char buffer[BUF_SIZE];
>
> @@ -385,6 +387,10 @@ static int handle_config(const char *key, const char *value, void *cb)
> add_instead_of(rewrite, xstrdup(value));
> }
> }
> + if (!strcmp(key, "push.denymovingtags")) {
> + deny_moving_tags = git_config_bool(key, value);
> + return 0;
> + }
Hmm, shouldn't this be per-remote (rather, shouldn't a per-remote variant
be allowed to override this)?
> @@ -1266,6 +1272,31 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
> continue;
> }
>
> + /* If a tag already exists on the remote and points to
> + * a different object, we don't want to push it again
> + * without requiring the user to indicate that they know
> + * what they are doing.
> + */
/*
* We try to format
* multi-line comment
* like this.
*/
> + if (!prefixcmp(ref->name, "refs/tags/") &&
> + !ref->deletion &&
> + !is_null_sha1(ref->old_sha1)) {
> + if (deny_moving_tags) {
> + /* Set `nonfastforward` for the sake of displaying
> + * this update as forced
> + */
> + ref->nonfastforward = 1;
I think you are propagating this bit to print_ok_ref_status() in
transport.c; it indicates that after your change, "nonfastforward" does
not mean non-fast-forward anymore, doesn't it?
Perhaps the bit needs to be renamed to "update_forced" or something?
> + if (!ref->force && !force_update) {
> + ref->status = REF_STATUS_REJECT_MOVING_TAG;
> + }
> + } else {
> + if (!ref->force && !force_update)
> + warning("You are changing the value of an upstream tag. This may\n"
> + "be deprecated in a future version of Git. Please use --force\n"
> + "if this was intentional, and consider setting push.denyMovingTags.");
> + }
> + continue;
> + }
> +
> /* This part determines what can overwrite what.
> * The rules are:
> *
You are changing the rule that determine what can overwrite what, aren't
you? It is Ok (although it is in general frowned upon if you do so when
you do not have to) to add your new rule before an existing rule, but your
rule should be added as a new rule to the enumeration in the comment, and
the code that implements the new rule after the comment, no?
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index c718253..7906ba5 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -106,6 +106,20 @@ test_expect_success 'denyNonFastforwards trumps --force' '
> test "$victim_orig" = "$victim_head"
> '
>
> +test_expect_success 'denyMovingTags trumps --force' '
> + (
> + cd victim &&
> + ( git tag moving_tag master^ || : ) &&
In which circumstance is it allowed for this "git tag" command to
fail and the entire test to succeed?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html