Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix changing branch of pins with 'git+ssh://' and 'xyz+https://' urls #1722

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

Seb-MCaw
Copy link
Contributor

@Seb-MCaw Seb-MCaw commented Jul 24, 2024

Currently, specifying the branch of pinned remote repositories fails for git+ssh:// urls. The requested branch is fetched correctly when alr build is first run (or after manually clearing the cache), but subsequent changes to the branch = component of the pin in alire.toml always results in the default branch being checked out. The same bug is also observed with [^+]*\+https: urls.

git+ssh:// urls actually appear to be deprecated, but we should probably support them anyway.

The issue manifests in the Update procedure. This detects if the url has changed and, if so, it clears the cached copy and does a fresh git clone (presumably on the basis that a different url probably points to a different version of the repository, which may have diverged sufficiently that an update will fail).

For some reason, this fresh git clone always checks out the default branch (calls Checkout, rather than Checkout (Branch => Branch)). I'm not sure whether this is intended behaviour, but it is not ordinarily a problem as Deploy always seems to be called several times. As such:

  • The first call identifies that the url has changed, and consequently does a fresh clone of the default branch
  • The second (and subsequent) call(s) considers the url to be unchanged (since it is the same as the first call), and therefore triggers the normal update procedure (which does checkout the correct branch)

This fails for git+ssh:// and [^+]*\+https: urls because the Repo function (which constructs the url when performing the clone) removes the prefix. A change to the url is detected by comparing the appropriate line in git config --list (here) with the string defined in alire.toml (here). git config returns the form passed to the clone command (without the prefix), while the string in alire.toml retains the full git+ssh:// form. The net effect is that a pin with a git+ssh:// url will always be treated as having changed (from ssh:// to git+ssh://), and the default branch will always be used (for every call to Deploy).

This pull request resolves the issue by substituting git+ssh://ssh:// and xyz+https://https:// when reading alire.toml.

@Seb-MCaw Seb-MCaw marked this pull request as draft July 24, 2024 13:39
@Seb-MCaw Seb-MCaw force-pushed the bugfix/Pins-with-git-ssh-urls branch from 45754ca to 23c2542 Compare July 24, 2024 14:00
@Seb-MCaw Seb-MCaw marked this pull request as ready for review July 24, 2024 14:22
@mosteo
Copy link
Member

mosteo commented Jul 29, 2024

For some reason, this fresh git clone always checks out the default branch (calls Checkout, rather than Checkout (Branch => Branch)).

AFAIR, this is because in the past some git versions found in the wild wouldn't have yet the option to check out a particular branch. I think it is safe to use this now when needed.

@mosteo mosteo merged commit 1604866 into alire-project:master Jul 29, 2024
25 checks passed
@mosteo
Copy link
Member

mosteo commented Jul 29, 2024

Thanks for the comprehensive test

@Seb-MCaw Seb-MCaw deleted the bugfix/Pins-with-git-ssh-urls branch July 29, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants