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

cabal get should not reset the default branch to the given tag #10278

Open
jasagredo opened this issue Aug 26, 2024 · 5 comments
Open

cabal get should not reset the default branch to the given tag #10278

jasagredo opened this issue Aug 26, 2024 · 5 comments

Comments

@jasagredo
Copy link
Collaborator

Currently, having a package that declares the following source-repository stanza in the available repositories (hackage):

source-repository this
  type: git 
  location: <url>
  tag: <tag>
  branch: <branch>

When one runs cabal get <pkg> -sthis -d<somedir> will perform the following sequence of commands:

(cd somedir && git clone <url> --branch <branch>)

if [ -n <tag> ]; then
  (cd somedir/pkg && git reset --hard <tag> --)
fi

(cd somedir/pkg && git submodule sync --recursive)
(cd somedir/pkg && git submodule update --init --force --recursive)

I would argue that the git reset is just wrong. It will reset whichever branch was cloned to the tag, resulting in a state that disagrees with the git server. My proposal is as follows:

  1. branch and tag should be exclusive. To be precise, they could be merged into either of them or some new name. Either way, only one of those has to/should be provided. Probably ref or rev would be a good name.
  2. Invoke git clone <url> --branch <branch-or-tag> as git can handle tags in that argument.
  3. As a side improvement, the directory provided in -d should be used as the output directory, and not as a parent folder for the clone. This means that using cabal get <pkg> -sthis -d<somedir> will clone the repo in ./somedir and not in ./somedir/reponame. This can be achieved by invoking git clone --branch <branch-or-tag> <url> <somedir>

Note that the same reasoning could apply to source-repository-package as those also allow for branch and tag.

@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 26, 2024

mhh do you happen to know what was the rationale behind git reset?

@jasagredo
Copy link
Collaborator Author

I don't know the rationale but through Github's blame I found this PR #5536

@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 26, 2024

Excellent sleuthing. From that discussion:

What do you think about generally using reset --hard (instead of checkout), since that's what we're asked to do, anyway?

The meaning of “since that's what we're asked to do, anyway” eludes me.

@andreasabel
Copy link
Member

@jasagredo wrote:

branch and tag should be exclusive.

I agree.

Let's review the source repository mechanism with its purposes:

  1. source-repository this is supposed to point to the source of a particular version of a published package. Thus, this should be a non-movable reference in a repository. Speaking for git, this would be a tag, and branch (movable reference) should be illegal for source-repository this.
  2. source-repository head is supposed to point to the development head of a package. This is a branch. So tag should be illegal for source-repository head.

cabal check should ensure one uses the right option tag / branch for the respective flavor this / head.

Not really relevant for this issue, but touching the source repository story: I find it irritating that that source-repository-package in the cabal.project file, which is for local development, does not support branch but insists on a non-movable reference like a commit hash. What I typically want to track with source-repository-package is a PR (not published to hackage), and those PRs may evolve. Why would cabal not allow me to track a PR? Why does it force me to manually update the commit hash each time the PR changes, to stay up to date? (Some might prefer this workflow to not be surprised by PR changes, but not all to, so why force them?)

@jasagredo
Copy link
Collaborator Author

I do agree that source-repository this should point to a (fixed) tag/commit, and source-repository head should point to a (moving) branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@andreasabel @ulysses4ever @jasagredo @ffaf1 and others