Fetch target revision which is not merged to HEAD

[yujunz]

Is your feature request related to a problem? Please describe.

I was trying to create application from an under review patch in Gerrit. However, the target revision is considered invalid in argo-cd.

Unable to update application: application spec is invalid: InvalidSpecError: Unable to determine app source type: rpc error: code = Unknown desc = Unable to resolve 'refs/changes/84/6684/3' to a commit SHA

The reason is probably this commit is NOT merged to HEAD and the ref is NOT included in default fetched heads i.e. refs/heads/*.

But as a developer, I would like to have a test deployment before merging the patch.

Describe the solution you'd like

Fetch the revision before checkout, e.g.

git fetch gerrit refs/changes/84/6684/3
git checkout FETCH_HEAD

Describe alternatives you've considered
A full support of Gerrit would be even better. This is how git-review helper query, fetch and checkout a target revision.

Query gerrit ssh://yujunz@gerrit-host:29418/repo --current-patch-set change:6684
2019-05-10 13:28:18.735892 Running: ssh -xp29418 yujunz@gerrit-host gerrit query --format=JSON --current-patch-set change:6684
{..."currentPatchSet":{"number":3,"revision":"109539535bcaea2c1a35bb404285e04b1ed0b407","parents":["c65b8bf11f3c4295c4a987072de5d8e0d6c3b7ae"],"ref":"refs/changes/84/6684/3",...}
...
2019-05-10 13:28:20.973543 Running: git fetch gerrit refs/changes/84/6684/3
2019-05-10 13:28:25.350308 Running: git checkout -b review/yujun_zhang/erica/argocd FETCH_HEAD

It would be nice to support spec like

  source:
    repoURL: ssh://argocd@gerrit-host:29418/repo.git
    targetGerritChange:
      number: 6684
      patchSet: 3

Additional context
N/A

[alexec]

I think I've also encountered this myself. This feels like a bug to me. Gerrit support - Argo CD is typically agnostic to such things.

@yujunz are you able to provide reproduction steps please?

[yujunz]

I think I've also encountered this myself. This feels like a bug to me. Gerrit support - Argo CD is typically agnostic to such things.

As I described, "The reason is probably this commit is NOT merged to HEAD and the ref is NOT included in default fetched heads i.e. refs/heads/*.". Gerrit has its own way to manage refs.

You may reproduce it on any Gerrit server. Just try to deploy from a patch that is NOT merged yet.

[yujunz]

It seems such refs are skipped on-purpose in https://github.com/argoproj/argo-cd/blob/master/util/git/client.go#L204

		if refName != "HEAD" && !strings.HasPrefix(refName, "refs/heads/") && !strings.HasPrefix(refName, "refs/tags/") {
			// ignore things like 'refs/pull/' 'refs/reviewable'
			continue

[jessesuen]

@yujunz the goal of this logic was to be able to support:

git clone https://giturl.com/org/proj
git checkout XXXXX

Where XXXXXX is a symbolic reference (e.g. HEAD), branch name, tag, commit sha (basically anything supported after git checkout). In the case of these refs/pull refs/reviewable and refs/changes, these do not have a canonical name that would be distinguishable from the other four supported references: symbolic references, branches, tags, commit sha.

I think for your use case, you would need to supply the commit sha instead of the reference for this to work.

[yujunz]

@jessesuen I tried commit SHA, since it is NOT merged to HEAD nor any fetched refs, check out will fail to find it.

[jessesuen]

But there has to be some repo (e.g. the downstream repo) hosted somewhere. In other words, the commit SHA has to be valid someplace clone-able, right? The trick to address your use case is supplying both the clone-able repo URL and commit-SHA for the preview.

Another way to ask this is, for the command: git fetch gerrit refs/changes/84/6684/3, what is the git URL for the remote named gerrit ?

[yujunz]

Just regular ssh url

remote.gerrit.url=ssh://yujunz@gerrit-host:29418/repo
remote.gerrit.fetch=+refs/heads/*:refs/remotes/gerrit/*

[yujunz]

supplying both the clone-able repo URL and commit-SHA

How? could you give an example. If I put them separately, it doesn't work. Since refs/changes/* are not fetched in a regular clone.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[yujunz]

Part of the problem is addressed in #2201 .

To make it more user friendly, it would be nice if we can infer refs/changes/84/6684/3 from change number 6684 and patch set 3

[magthe]

I'd love to see this picked up again, I'd have loved finishing #2201, but after a quick look at the changes I have a feeling my extremely limited knowledge of Go and complete ignorance of Argo CD internals will make it too tall an order to pull of on my own 😞

[danbirle]

@yujunz the goal of this logic was to be able to support:

git clone https://giturl.com/org/proj
git checkout XXXXX
Where XXXXXX is a symbolic reference (e.g. HEAD), branch name, tag, commit sha (basically anything supported after git checkout). In the case of these refs/pull refs/reviewable and refs/changes, these do not have a canonical name that would be distinguishable from the other four supported references: symbolic references, branches, tags, commit sha.

I think for your use case, you would need to supply the commit sha instead of the reference for this to work.

commit sha doesn't work though..... and i'm supplying the correct format as i'm getting some other error if i supply only the first 7 / 8 / 9 characters from the sha ........ neither supplying a branch name that's not merged into master...

[Velyks]

Should this make it possible to set the target revision to pull request refs that are from forked repos? I'm currently getting "reference is not a tree" when using something along the lines of refs/pull/1234/head as a targetRevision, where this reference points to a commit in a forked repo.

[yujunz]

There is a regression introduced in d516f47.

PR #5605 proposed to fix it but still under review.