VCS: Fix weird tag creation

[9999years]

Follow-up to #10254; @alt-romes is there a reason to create the tag matching FETCH_HEAD here instead of using FETCH_HEAD directly?

Creating a tag with an arbitrary user-supplied name can cause problems, like the warning below. Fortunately, we can just use FETCH_HEAD as the ref name directly instead of creating a tag to match it!

Running: git fetch origin d1dc91fd977bb4b28f0e01966fa08640a1283318
From /private/var/folders/z5/fclwwdms3r1gq4k4p3pkvvc00000gn/T/vcstest-90401/src
 * branch            d1dc91fd977bb4b28f0e01966fa08640a1283318 -> FETCH_HEAD

Running: git tag -f d1dc91fd977bb4b28f0e01966fa08640a1283318 FETCH_HEAD

Running: git reset --hard d1dc91fd977bb4b28f0e01966fa08640a1283318 --
warning: refname 'd1dc91fd977bb4b28f0e01966fa08640a1283318' is ambiguous.
Git normally never creates a ref that ends with 40 hex characters
because it will be ignored when you just specify 40-hex. These refs
may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please
examine these refs and maybe delete them. Turn this message off by
running "git config advice.objectNameWarning false"

• Patches conform to the coding conventions.
• Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

[alt-romes]

Oh, this is problematic -- Cabal "tags" can be commit refs of course...

I'm not looking at the code at the moment, but this decision may have been caused by how the "synchronization" step works: you'll only fetch once, but you may want to synchronize using the tag later on. I'm not entirely sure, but if you mean to look in the meantime, that's what I'd do: does the tag have to exist to synchronize later on?

It's also a bit unfortunate the testsuite missed this. It'd be great if you could add a regression test.

[9999years]

Hmm, we only use the FETCH_HEAD immediately after git fetching, so I think it's fine if we don't make a specific tag for it.

[9999years]

It's also a bit unfortunate the testsuite missed this. It'd be great if you could add a regression test.

I'm not sure it's possible, because it's only a warning...

[geekosaur]

Anyway, what's here is fine, I'm just trying to figure the logic behind the original version. FETCH_HEAD can change, tags can't (unless you remove, recreate, and force push).

[geekosaur]

Okay, I can't make sense of it. In particular, there seems to be no reason why you can't do

git fetch refspec # using the refspec from source-repository-package
git switch -d refspec
git submodule update --init # optional

and not even bother with a tag. It's minimal, it's faster than what we were doing before, and I can't find any reason why anything else would be necessary given that it's only intended to be built, with no other git commands and no editing. In particular, this is more or less what submodules do, and while submodules have their problems this isn't one of them.

[ulysses4ever]

I hope it's okay to comment on the PR even though it's a draft. If not, please let me know.

My understanding of git aligns completely with what @geekosaur is saying. I think we could switch to the simpler approach.

[alt-romes]

I may have overcomplicated the design when I was down in the weeds.

If that is what submodules do, it seems sensible it would work for us too.

I wasn't aware of git switch -d, so maybe that's why

[alt-romes]

What's the status of this patch @9999years?

[9999years]

@alt-romes The patch is ready, just needs a second review before it can be merged.