More robustness in checking out tags

[billsacks]

Since I'm paranoid, I started worrying about the possibility that the wrong commit will be checked out when checkout_externals tries to checkout a tag. While this doesn't seem like a big risk, it seems possible - especially after someone has been doing some work locally and then reruns checkout_externals after the .cfg file has been updated.

There are two scenarios that I'm worried about:

(1) There happens to be a branch named the same as the given tag

If the reference is ambiguous in this way, git will checkout the branch rather than the tag. This one seems easy to fix: rather than just git checkout TAGNAME, we should do git checkout tags/TAGNAME (tested with git 2.14.1).

(2) This tag is defined differently in different places - i.e., the same tag name points to a different sha in different places.

While this one seems unlikely (I hope), it could cause massive confusion if it did happen.

(2a) Multiple remotes define this tag differently

I believe that the current behavior - git fetch --all --tags will give indeterminate behavior in this case.

To address this, I think we should just do a fetch from the single remote listed in the .cfg file rather than doing a fetch from --all.

(2b) There is already a local tag with this name but pointing to the wrong sha

All of my testing with git 2.14.1 suggests that local tags are overwritten with the tag from the remote when doing a fetch, but some answers on StackOverflow suggest that this may be version-dependent (see the second answer, by torek, here: https://stackoverflow.com/questions/35979642/how-to-checkout-remote-git-tag (especially the section on Tags), and see the comments by torek in the answer by eckes here https://stackoverflow.com/questions/9662249/how-to-overwrite-local-tags-with-git-fetch/21475727 ).

I can see a couple of ways to address this.

We could use the method suggested by torek here https://stackoverflow.com/questions/9662249/how-to-overwrite-local-tags-with-git-fetch/21475727

git fetch <remote> '+refs/tags/*:refs/tags/*'

Though maybe just specifying the single tag of interest rather than *. (I could imagine first fetching all tags from the given remote, because it's useful to update all tags, and then doing something like git fetch <remote> '+refs/tags/TAGNAME:refs/tags/TAGNAME'.)

(NOTE: I haven't tested this method myself.)

Alternatively, it seems we could first delete any local tag with the given name (git tag -d TAGNAME, ignoring errors in case that tag doesn't exist yet locally), then do a fetch. I slightly prefer this approach simply because it's easier to understand.

[billsacks]

Looking through the recent changes on master, and looking back through my comments, I'm mostly happy with how things are handled now.

However, I'm still a bit concerned about (2b) - and more so because according to git's documentation (git help tag), the behavior I saw (where a tag is overwritten upon a fetch) is not what's supposed to happen:

       However, Git does not (and it should not) change tags behind users back. So if somebody already got the old tag, doing a git pull on your tree shouldn't just make them
       overwrite the old one.

       If somebody got a release tag from you, you cannot just change the tag for them by updating your own one. This is a big security issue, in that people MUST be able to trust
       their tag-names. If you really want to do the insane thing, you need to just fess up to it, and tell people that you messed up. You can do that by making a very public
       announcement saying:

           Ok, I messed up, and I pushed out an earlier version tagged as X. I
           then fixed something, and retagged the *fixed* tree as X again.

           If you got the wrong tag, and want the new one, please delete
           the old one and fetch the new one by doing:

                   git tag -d X
                   git fetch origin tag X

           to get my updated tag.

We should decide what we want the behavior to be in the case where someone already has a tag with the given name, but pointing to a different sha than the tag on the remote listed in CESM.cfg. Personally, I think the most robust behavior would be to force use of the tag on the remote in this case, but I could be convinced otherwise.

If we do want to force use of the tag on the remote, we could accomplish this by doing a git tag -d of the given tag name (ignoring errors in case the tag doesn't exist).

Or, if we're concerned about leaving the repo in a usable state if the git fetch fails (e.g., due to a network issue), we could do something like this:

• If the given tag name already exists:

  • Set tag_existed_before_fetch = True (otherwise, we'll have tag_existed_before_fetch = False)

  • Make a temporary tag that points to the original; give it a unique name by appending a string of random characters to the end of the original tag name. For example, if the given tag name is foo, we might end up with a tag named foo_rsa3oe5udq, via git tag foo_rsa3oe5udq foo

  • Delete the original tag

• Fetch from the remote

• if tag_existed_before_fetch:

  • If the given tag name does NOT exist: remake the original tag - e.g., git tag foo foo_rsa3oe5udq

  • Delete the temporary tag

I don't feel this absolutely needs to be solved before we "go live". But I'd like to keep this issue open until we decide what behavior we want in this situation.

[bandre-ucar]

I personally don't feel it's appropriate to do anything more about the tag issue. It is fundamentally the responsibility of git and the user to manage tags in their repo. We should trust the default behavior of git. Trying to manipulate tags as described above is not only a security issue (as described above) but also has the potential to destroy user data. Using the example above, the user would be rightly upset if they created a tag for a commit and the only reference to that commit was deleted automatically by this tool!

[bandre-ucar]

This is beyond the scope of v1.0. I'm marking for future discussion/development.

[billsacks]

Based on @bandre-ucar 's points, I'm fine closing this issue, leaving (2b) unresolved.