git_config confuses branches and tags

[victorusu]

We used to install Spack on CSCS systems using EasyBuild. After the 4.4.2 release, the tag option from git_config doesn't change to the correct develop branch of Spack. EB outputs the warning and it fails to change branches:

...
... cmd "git checkout refs/tags/develop" exited with exit code 1 and output:
...
WARNING: Tag develop was not downloaded in the first try due to https://github.com/spack/spack containing a branch with the same name. You might want to alert the maintainers of spack about that issue.

I have tested and confirmed that versions are 4.4.2 and 4.5.0 give this error, while 4.4.1 works.

The easyconfig file that we use can be found here.

I suggest to try to reproduce the error using

eb Spack-daint-git-develop.eb --force-download -f

[boegel]

Thanks for the bug report @victorusu!

Based on this being introduced in EasyBuild v4.4.2, I'd say the changes made in #3795 are a likely culprit...

[migueldiascosta]

indeed, the problem is with this block https://github.com/Flamefire/easybuild-framework/blob/6a831521af6be42d103acdf157421d6d680fe502/easybuild/tools/filetools.py#L2501-L2525 from #3795, it works without it

that block is assuming we asked for a tag (because there is no commit), but tag is used for boths tags and branches (passed to --branch), and in this case the intention is actually to get a branch

it seems that in order to do this check, we'd need a way to distinguish tags from branches in the easyconfig (?)

cc @Flamefire

[Flamefire]

Well it is not meant to be used with a branch. It is named "tag" not "branch" as that is what we usually want.
If we want to support branches (which I think we should not because that means a moving target and not a specific version) then we need a new feature.

[migueldiascosta]

@Flamefire indeed, but could we at least make the warning message contemplate this possibility?

something like "either a branch was specified instead of a tag or the repository contains a tag and a branch with the same name (if the latter, you might want to alert the maintainers of the repository about that issue)"?

[boegel]

There's no way we can detect it's actually a branch rather than a tag, and then instruct to use branch instead of tag (for which we need to implement support first)?

[Flamefire]

something like "either a branch was specified instead of a tag or the repository contains a tag and a branch with the same name (if the latter, you might want to alert the maintainers of the repository about that issue)"?

Erm... Something like this?

WARNING: Tag develop was not downloaded in the first try due to https://github.com/spack/spack containing a branch with the same name. You might want to alert the maintainers of spack about that issue.

Or do you mean to add that you should not pass a branch to an attribute called "tag"?

[boegel]

@Flamefire Well, can we actually detect that develop is a valid branch name, but not a tag name?

In that case, we can produce an error message like "Tag 'develop' does not exist in <repo>, but a branch with that name does (use 'branch' rather than 'tag' in git_config?)."

[migueldiascosta]

@Flamefire I meant to at least make clear that the problem may come from what's specified in the easyconfig (a branch instead of a tag) - I get it that the current message is technically correct, but imho it makes it seem that the problem is with the repository (or with easybuild).

@boegel Regarding detecting, at that point (after cloning) for instance the first line of git status should allow that, it's # Not currently on any branch. for a tag (in which case one would still need the git describe if one wants to make sure it's the correct tag) and # On branch ... for a branch.

[Flamefire]

Well, can we actually detect that develop is a valid branch name, but not a tag name?

Well we already are in the situation that we checked out a branch, not a tag. So what is left is checking whether there exists a tag, i.e. if https://github.com/Flamefire/easybuild-framework/blob/6a831521af6be42d103acdf157421d6d680fe502/easybuild/tools/filetools.py#L2519 will fail

Before being able to do so we have to unshallow the repo first which may fail: https://github.com/Flamefire/easybuild-framework/blob/6a831521af6be42d103acdf157421d6d680fe502/easybuild/tools/filetools.py#L2517

So my vote would be to improve the warning message to mention that branches should not be specified in the EC. Basically add "Make sure you actually specified a tag and not a branch or you might want to alert the maintainers of %s about that issue"

[migueldiascosta]

So my vote would be to improve the warning message to mention that branches should not be specified in the EC. Basically add "Make sure you actually specified a tag and not a branch or you might want to alert the maintainers of %s about that issue"

I'd say this would already help, so it should be the immediate action?

we can think about support for branches (even though they're a moving target?) after 4.5.1...

[boegel]

I've improved the warning message in #3911, and also added support for using 'branch' in git_config (which was trivial, since no extra logic is needed, same procedure as for a specific commit)

[boegel]

I'm going to move the milestone on this to the release after EasyBuild v4.5.1, since there's an easy workaround fox the problem that was reported: use 'commit': 'develop' rather than 'tag': 'develop'.

Also, the fact that 'tag': 'develop' was sort of unintentional; develop is not a tag at all here, so it was basically just luck that it was working at all.

We're working out a proper way forward in #3911, but that's turning out less trivial than I anticipated at first, and this issue is not worth blocking a release over imho...