cabal icon indicating copy to clipboard operation
cabal copied to clipboard

(partially) fix issue #6888, source-repository-package branch failure

Open noinia opened this issue 2 years ago • 7 comments

As the title of the issue states, this (partially) fixes issue #6888 ; i.e. if a user specifies a git branch in the 'source-repository-package' stanza in cabal.project cabal actually properly checks out that branch.

The core of the issue is that (1) the (existing) implementation always uses 'vcsSyncRepos' to check out a (git) repository rather than 'vcsCloneRepo', and (2) vcsSyncRepos did not specify the '--branch' field to clone. (Afterwards it also runs git reset --hard ', but that fails since it did not clone the right initial branch.

This PR provides a somewhat minimalistic approach to solving this issue: i.e. it now also provides the branch to clone in the implementation of vcsSyncRepos in vcsGit, which fixes the issue.

I do think this reveals some opportunity for more refactoring though. In particular it seems that

  • vcsCloneRepo is not actually used anywhere in the project !?
  • I only added the --branch stuff to the implementation of vcsGit; I would think that the other vcs systems (hg, bazaar etc) probably also need some sort of --branch argument in order to work correctly. Just glancing at the code there I'm guessing that does not work either.

Not entirely sure what to do about these two remaining parts. I can't easily figure out/test what the right --branch invocations would be for the other VCSes. If desired, ripping out vcsCloneRepo, is probably done best in a separate PR. I'm happy to hear input about this. (Afterwards I can write the changelog entry).

** Include the following checklist in your PR:

noinia avatar Jul 27 '23 10:07 noinia

I do think this reveals some opportunity for more refactoring though.

YES. Please make yourself at home :) How would you write that code?

vcsCloneRepo is called by cloneSourceRepo.

andreabedini avatar Jul 27 '23 15:07 andreabedini

I do think this reveals some opportunity for more refactoring though.

YES. Please make yourself at home :) How would you write that code?

Good question. Admittedly, I don't have a concrete proposal; but it seems strange to me that: vcsCloneRepo does not seem to be called at all when cloning a new git repository, and that vcsSyncRepo duplicates some of the cloning logic.

vcsCloneRepo is called by cloneSourceRepo.

Hmm, well spotted. Still, it seems that even on a fresh clone vcsSyncRepo is used rater than vcsCloneRepo. So I don't think vcsCloneRepo is actually invoked when cloning a fresh repository.

noinia avatar Jul 27 '23 16:07 noinia

Hmm, for whatever reason CI seems to timeout with ghc 9.0.2 on OS X. Not sure why though.

noinia avatar Jul 27 '23 16:07 noinia

How is the state of it.

soulomoon avatar Apr 10 '24 15:04 soulomoon

Patch seems to work fine here; I've been using it locally ever since opening this PR.

I don't have the time and interest to do any (further) refactoring steps that may or may not be possible here. In any case: there has not been much input from the maintainers on whether they want this patch/solution and/or what else is required (other than me writing a changelog entry I guess)

noinia avatar Apr 11 '24 21:04 noinia

I now also wrote the changelog entry, so this should really be done now.

The failure on macos-13 ghc-9.10.1 is because there was some (temporary?) internet issue it seems. Not sure if I can easily restart just that runner; I didn't really see a way to do that. (But I don't really have any reason to believe it would not sucessfully build there otherwise; I'm locally using mac os and 9.10.1 at the moment even actually).

noinia avatar Sep 18 '24 21:09 noinia