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, and from the --force option to "git tag".
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.
This change forces a user to specify "git push --force" to push a tag
which points to a different object than it does on the upstream
repository, regardless of whether it's a fast-forward or not.
The config option receive.denyMovingTags can be set on the upstream
repository to disallow this, even with --force.
Signed-off-by: Dave Olszewski <cxreg@xxxxxxxxx>
---
Documentation/config.txt | 8 ++++++++
Documentation/git-push.txt | 21 ++++++++++++++++++---
Documentation/git-receive-pack.txt | 3 ++-
advice.c | 2 ++
advice.h | 1 +
builtin/push.c | 10 ++++++++--
builtin/receive-pack.c | 14 ++++++++++++++
builtin/send-pack.c | 9 ++++++++-
cache.h | 1 +
contrib/completion/git-completion.bash | 1 +
remote.c | 13 +++++++++++++
t/t5400-send-pack.sh | 13 +++++++++++++
transport-helper.c | 6 ++++++
transport.c | 15 ++++++++++++---
transport.h | 4 ++--
15 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 05ec3fe..587cccd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -122,6 +122,9 @@ advice.*::
pushNonFastForward::
Advice shown when linkgit:git-push[1] refuses
non-fast-forward refs. Default: true.
+ pushMovingTag::
+ Advice shown when linkgit:git-push[1] refuses
+ to push a changed tag. Default: true.
statusHints::
Directions on how to stage/unstage/add shown in the
output of linkgit:git-status[1] and the template shown
@@ -1593,6 +1596,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.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 658ff2f..f7f8f17 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -112,7 +112,9 @@ 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 by default.
--repo=<repository>::
This option is only relevant if no <repository> argument is
@@ -215,8 +217,9 @@ remote rejected::
of the following safety options in effect:
`receive.denyCurrentBranch` (for pushes to the checked out
branch), `receive.denyNonFastForwards` (for forced
- non-fast-forward updates), `receive.denyDeletes` or
- `receive.denyDeleteCurrent`. See linkgit:git-config[1].
+ non-fast-forward updates), `receive.denyDeletes`,
+ `receive.denyDeleteCurrent`, or `receive.denyMovingTags`. See
+ linkgit:git-config[1].
remote failure::
The remote end did not report the successful update of the ref,
@@ -324,6 +327,18 @@ overwrite it. In other words, "git push --force" is a method reserved for
a case where you do mean to lose history.
+Note about moving tags
+----------------------
+
+Tags are widely considered 'read-only', and are not expected to change.
+See the 'On Re-tagging' section of linkgit:git-tag[1] to learn more about
+why this is so.
+
+If you really need to change an already-propagated tag, you must force it
+with "git push --force". This can be overridden on the upstream repository
+with receive.denyMovingTags.
+
+
Examples
--------
diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt
index 2790eeb..55f2830 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -30,7 +30,8 @@ post-update hooks found in the Documentation/howto directory.
'git-receive-pack' honours the receive.denyNonFastForwards config
option, which tells it if updates to a ref should be denied if they
-are not fast-forwards.
+are not fast-forwards, and the receive.denyMovingTags config option,
+which disallows updating a tag to point to a new object.
OPTIONS
-------
diff --git a/advice.c b/advice.c
index 0be4b5f..51021d8 100644
--- a/advice.c
+++ b/advice.c
@@ -1,6 +1,7 @@
#include "cache.h"
int advice_push_nonfastforward = 1;
+int advice_push_moving_tag = 1;
int advice_status_hints = 1;
int advice_commit_before_merge = 1;
int advice_resolve_conflict = 1;
@@ -12,6 +13,7 @@ static struct {
int *preference;
} advice_config[] = {
{ "pushnonfastforward", &advice_push_nonfastforward },
+ { "pushmovingtag", &advice_push_moving_tag},
{ "statushints", &advice_status_hints },
{ "commitbeforemerge", &advice_commit_before_merge },
{ "resolveconflict", &advice_resolve_conflict },
diff --git a/advice.h b/advice.h
index 3244ebb..8b1a33c 100644
--- a/advice.h
+++ b/advice.h
@@ -4,6 +4,7 @@
#include "git-compat-util.h"
extern int advice_push_nonfastforward;
+extern int advice_push_moving_tag;
extern int advice_status_hints;
extern int advice_commit_before_merge;
extern int advice_resolve_conflict;
diff --git a/builtin/push.c b/builtin/push.c
index e655eb7..8547554 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -106,7 +106,7 @@ static void setup_default_push_refspecs(void)
static int push_with_options(struct transport *transport, int flags)
{
int err;
- int nonfastforward;
+ int nonfastforward, moving_tag;
transport_set_verbosity(transport, verbosity, progress);
@@ -119,7 +119,7 @@ static int push_with_options(struct transport *transport, int flags)
if (verbosity > 0)
fprintf(stderr, "Pushing to %s\n", transport->url);
err = transport_push(transport, refspec_nr, refspec, flags,
- &nonfastforward);
+ &nonfastforward, &moving_tag);
if (err != 0)
error("failed to push some refs to '%s'", transport->url);
@@ -134,6 +134,12 @@ static int push_with_options(struct transport *transport, int flags)
"'Note about fast-forwards' section of 'git push --help' for details.\n");
}
+ if (moving_tag && advice_push_moving_tag) {
+ fprintf(stderr, "A tag which already exists upstream was attempted to be pushed while\n"
+ "pointing to a different object. This is unsafe, and disabled by default.\n"
+ "See the 'Note about moving tags' section of 'git push --help' for details.\n");
+ }
+
return 1;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 760817d..1a96e55 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -22,6 +22,7 @@ enum deny_action {
static int deny_deletes;
static int deny_non_fast_forwards;
+static int deny_moving_tags;
static enum deny_action deny_current_branch = DENY_UNCONFIGURED;
static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects;
@@ -63,6 +64,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (strcmp(var, "receive.denymovingtags") == 0) {
+ deny_moving_tags = git_config_bool(var, value);
+ return 0;
+ }
+
if (strcmp(var, "receive.unpacklimit") == 0) {
receive_unpack_limit = git_config_int(var, value);
return 0;
@@ -416,6 +422,14 @@ static const char *update(struct command *cmd)
return "non-fast-forward";
}
}
+ if (deny_moving_tags && !is_null_sha1(new_sha1) &&
+ !is_null_sha1(old_sha1) &&
+ hashcmp(old_sha1, new_sha1) &&
+ !prefixcmp(name, "refs/tags/")) {
+ rp_error("denying moving tag %s", name);
+ return "moving tag";
+ }
+
if (run_update_hook(cmd)) {
rp_error("hook declined to update %s", name);
return "hook declined";
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 481602d..c41a455 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -198,6 +198,11 @@ static void print_helper_status(struct ref *ref)
msg = "non-fast forward";
break;
+ case REF_STATUS_REJECT_MOVING_TAG:
+ res = "error";
+ msg = "moving tag";
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = "error";
@@ -275,6 +280,7 @@ int send_pack(struct send_pack_args *args,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_MOVING_TAG:
case REF_STATUS_UPTODATE:
continue;
default:
@@ -395,6 +401,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
const char *receivepack = "git-receive-pack";
int flags;
int nonfastforward = 0;
+ int moving_tag = 0;
argv++;
for (i = 1; i < argc; i++, argv++) {
@@ -516,7 +523,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
ret |= finish_connect(conn);
if (!helper_status)
- transport_print_push_status(dest, remote_refs, args.verbose, 0, &nonfastforward);
+ transport_print_push_status(dest, remote_refs, args.verbose, 0, &nonfastforward, &moving_tag);
if (!args.dry_run && remote) {
struct ref *ref;
diff --git a/cache.h b/cache.h
index eb77e1d..cca7499 100644
--- a/cache.h
+++ b/cache.h
@@ -913,6 +913,7 @@ struct ref {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_MOVING_TAG,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6756990..dc29e0b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1964,6 +1964,7 @@ _git_config ()
receive.denyCurrentBranch
receive.denyDeletes
receive.denyNonFastForwards
+ receive.denyMovingTags
receive.fsckObjects
receive.unpackLimit
repack.usedeltabaseoffset
diff --git a/remote.c b/remote.c
index 9143ec7..cc7ce93 100644
--- a/remote.c
+++ b/remote.c
@@ -1266,6 +1266,19 @@ 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.
+ */
+ if (!prefixcmp(ref->name, "refs/tags/") &&
+ !ref->deletion &&
+ !is_null_sha1(ref->old_sha1) &&
+ !ref->force &&
+ !force_update) {
+ ref->status = REF_STATUS_REJECT_MOVING_TAG;
+ }
+
/* This part determines what can overwrite what.
* The rules are:
*
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index c718253..573eb20 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -106,6 +106,19 @@ test_expect_success 'denyNonFastforwards trumps --force' '
test "$victim_orig" = "$victim_head"
'
+test_expect_success 'denyMovingTags trumps --force' '
+ (
+ cd victim &&
+ ( git tag moving_tag master^ || : ) &&
+ git config receive.denyMovingTags true
+ ) &&
+ git tag moving_tag &&
+ victim_orig=$(cd victim && git rev-parse --verify moving_tag) &&
+ test_must_fail git send-pack --force ./victim moving_tag &&
+ victim_tag=$(cd victim && git rev-parse --verify moving_tag) &&
+ test "$victim_orig" = "$victim_tag"
+'
+
test_expect_success 'push --all excludes remote tracking hierarchy' '
mkdir parent &&
(
diff --git a/transport-helper.c b/transport-helper.c
index acfc88e..e9730a0 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -575,6 +575,7 @@ static int push_refs_with_push(struct transport *transport,
/* Check for statuses set by set_ref_status_for_push() */
switch (ref->status) {
case REF_STATUS_REJECT_NONFASTFORWARD:
+ case REF_STATUS_REJECT_MOVING_TAG:
case REF_STATUS_UPTODATE:
continue;
default:
@@ -655,6 +656,11 @@ static int push_refs_with_push(struct transport *transport,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, "moving tag")) {
+ status = REF_STATUS_REJECT_MOVING_TAG;
+ free(msg);
+ msg = NULL;
+ }
}
if (ref)
diff --git a/transport.c b/transport.c
index 4dba6f8..e3b2ee8 100644
--- a/transport.c
+++ b/transport.c
@@ -692,6 +692,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
"non-fast-forward", porcelain);
break;
+ case REF_STATUS_REJECT_MOVING_TAG:
+ print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+ "moving tag", porcelain);
+ break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
ref->deletion ? NULL : ref->peer_ref,
@@ -711,7 +715,8 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
}
void transport_print_push_status(const char *dest, struct ref *refs,
- int verbose, int porcelain, int *nonfastforward)
+ int verbose, int porcelain,
+ int *nonfastforward, int *moving_tag)
{
struct ref *ref;
int n = 0;
@@ -727,6 +732,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,
n += print_one_push_status(ref, dest, n, porcelain);
*nonfastforward = 0;
+ *moving_tag = 0;
for (ref = refs; ref; ref = ref->next) {
if (ref->status != REF_STATUS_NONE &&
ref->status != REF_STATUS_UPTODATE &&
@@ -734,6 +740,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)
*nonfastforward = 1;
+ if (ref->status == REF_STATUS_REJECT_MOVING_TAG)
+ *moving_tag = 1;
}
}
@@ -1004,9 +1012,10 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
int transport_push(struct transport *transport,
int refspec_nr, const char **refspec, int flags,
- int *nonfastforward)
+ int *nonfastforward, int *moving_tag)
{
*nonfastforward = 0;
+ *moving_tag = 0;
transport_verify_remote_names(refspec_nr, refspec);
if (transport->push) {
@@ -1047,7 +1056,7 @@ int transport_push(struct transport *transport,
if (!quiet || err)
transport_print_push_status(transport->url, remote_refs,
verbose | porcelain, porcelain,
- nonfastforward);
+ nonfastforward, moving_tag);
if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index c59d973..8e95e09 100644
--- a/transport.h
+++ b/transport.h
@@ -138,7 +138,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
int transport_push(struct transport *connection,
int refspec_nr, const char **refspec, int flags,
- int * nonfastforward);
+ int *nonfastforward, int *moving_tag);
const struct ref *transport_get_remote_refs(struct transport *transport);
@@ -163,6 +163,6 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v
int transport_refs_pushed(struct ref *ref);
void transport_print_push_status(const char *dest, struct ref *refs,
- int verbose, int porcelain, int *nonfastforward);
+ int verbose, int porcelain, int *nonfastforward, int *moving_tag);
#endif
--
1.7.2.2.176.g3e15d
--
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