v0.34.0 - Running `vendir sync` fetches outdated repo when pointed at tag

[Laevos]

What steps did you take: In my vendir.yml file:

apiVersion: vendir.k14s.io/v1alpha1
directories:
- contents:
  - git:
      depth: 1
      ref: v8.291.0 # landed to branch within the last hour
      # [...]

ran the following:

$ vendir sync

What happened:

   * [new tag]           v0.2.0                  -> v0.2.0
   * [new tag]           v8.206.1                -> v8.206.1
  --> git -c advice.detachedHead=false checkout v8.291.0
  error: pathspec 'v8.291.0' did not match any file(s) known to git

vendir: Error: Syncing directory 'vendor':
  Syncing directory 'example' with git contents:
    Fetching git repository:
      Git [-c advice.detachedHead=false checkout v8.291.0]: exit status 1 (stderr: error: pathspec 'v8.291.0"' did not match any file(s) known to git)

v8.206.1 shows up as the most recent tag, although that tag is more than several months old.

What did you expect: vendir sync should pull the latest version of the repo.

Anything else you would like to add: If I replace :

ref: v8.291.0 

with:

ref: master

Then vendir sync will pull the latest commit without issue (the commit from which v8.291.0 was generated), but we prefer to run against tags rather than master.

I was able to work around the issue by downloading the 0.33.2 release of vendir and using that to run vendir sync, so the problem seems to be contained in 0.34.0. Since this is the development version, maybe it shouldn't be what Homebrew installs by default? At Ieast splitting it out into development and release channels might be useful?

Environment:

• vendir version (execute vendir --version): vendir version 0.34.0 (from Homebrew)
• OS (e.g. from /etc/os-release): macos 13.4 (22F66)

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[Laevos]

Note: This seems to only happen in some repos, not others, currently unsure why that is.

[neil-hickey]

Perhaps related to this change https://github.com/carvel-dev/vendir/pull/251

[Laevos]

Good call, this bit strikes me as especially suspect:

if strings.HasPrefix(t.opts.Ref, "origin/") {
// only fetch the exact ref we're seeking
fetchArgs = append(fetchArgs, t.opts.Ref[7:])
}

[Laevos]

Unfortunately, removing the contents of #251 and re-building still results in an error; I'll keep digging

[neil-hickey]

Is the repository you are trying this out on public? Would you be able to share the full vendir.yml you are using or no?

[Laevos]

Looking at it again, the following does fix it:

diff --git a/pkg/vendir/fetch/git/git.go b/pkg/vendir/fetch/git/git.go
index 2601d9b..c5b6e27 100644
--- a/pkg/vendir/fetch/git/git.go
+++ b/pkg/vendir/fetch/git/git.go
@@ -149,13 +149,12 @@ func (t *Git) fetch(dstPath string, tempArea ctlfetch.TempArea) error {
                {"remote", "add", "origin", gitURL},
        }

-       if t.opts.RefSelection != nil {
-               // fetch tags for selection
-               argss = append(argss, []string{"config", "remote.origin.tagOpt", "--tags"})
-       }
+       // fetch tags for selection
+       argss = append(argss, []string{"config", "remote.origin.tagOpt", "--tags"})

It doesn't look like specifying a ref causes t.opts.RefSelection to not be nil, whether I set it to a tag or the master branch, namely because I don't see the --> git config remote.origin.tagOpt --tags output in vendir sync as I do when running on 0.33.2

@neil-hickey the repo is not public, but if there's any further information I can provide I'm happy to assist

[neil-hickey]

sure thing - no worries, I'm trying to just get a simple reproducable vendir.yml put together - hence checking if what you were trying was sharable or not.

[Laevos]

I can say that we aren't using a RefSelection, which I imagine is why RefSelection is nil, looking into it. Basically, the redacted version of the offending block looks like this

  - git:
      depth: 1
      ref: v8.291.0
      url: https://example.com
    includePaths:
    - jsonnet/**/*
    path: com.example
  path: vendor

[Laevos]

The following change does fix the issue, is this desirable? if so I can submit a PR:

diff --git a/pkg/vendir/fetch/git/git.go b/pkg/vendir/fetch/git/git.go
index 2601d9b..eea8efe 100644
--- a/pkg/vendir/fetch/git/git.go
+++ b/pkg/vendir/fetch/git/git.go
@@ -149,13 +149,14 @@ func (t *Git) fetch(dstPath string, tempArea ctlfetch.TempArea) error {
                {"remote", "add", "origin", gitURL},
        }

-       if t.opts.RefSelection != nil {
+       if t.opts.RefSelection != nil || t.opts.Ref != "" {

Though this is the same as just not guarding it with an if clause at all, since one of Ref or RefSelection must be present, trying to understand the usecase, do we only want to fetch tags if either a RefSelection or a specific tag is provided, perhaps?

[neil-hickey]

That's the bug :) Nice find, sure a PR would be awesome! TY @Laevos !

[Laevos]

Sure thing, would you prefer that I just remove that if-clause entirely or keep it there to guard against unexpected input? I'll default to the latter but let me know and I can update if needed

[Laevos]

opened #264 to implement this change

[neil-hickey]

Hmm, I'm thinking about this one a little more, and I think it's actually "working as intended".

The intention was the following for the original feature:

1. When you specify an exact tag, we don't fetch them all. Only the one you asked for.
2. When you choose the "here's some constraints, fetch me a tag that matches" you need to have all tags available. Hence these three lines

So I think in your case

apiVersion: vendir.k14s.io/v1alpha1
directories:
- contents:
  - git:
      depth: 1
      ref: v8.291.0 # landed to branch within the last hour
      # [...]

The ref is missing origin/ in front of it. Now officially in the docs, we say

# branch, tag, commit; origin is the name of the remote (required)
# optional if refSelection is specified (available in v0.11.0+)
ref: origin/master

But it technically worked without it previously. The PR here introduced a requirement for having origin/ in the tag - https://github.com/carvel-dev/vendir/pull/251/files#diff-dd14009dbf6d077d1b3aa8b3df9aa3b1310ec2b5a47895c54214d5ed95681058R159

[neil-hickey]

Could you try adding origin/v8.291.0 to your ref section and see if it works with the current release? @Laevos

[Laevos]

      ref: origin/v8.291.0

$ vendir sync
Fetching: vendor + example (git from https://github.com/example/example@origin/v8.291.0)

  --> git init
  Initialized empty Git repository in /example/git/.git/
  --> git config credential.helper store --file /example/git-auth/.git-credentials
  --> git remote add origin https://github.com/example/example
  --> git fetch origin v8.291.0 --depth 1
  From github.com:example/example
   * tag               v8.291.0   -> FETCH_HEAD
  --> git -c advice.detachedHead=false checkout origin/v8.291.0
  error: pathspec 'origin/v8.291.0' did not match any file(s) known to git

vendir: Error: Syncing directory 'vendor':
  Syncing directory 'example' with git contents:
    Fetching git repository:
      Git [-c advice.detachedHead=false checkout origin/v8.291.0]: exit status 1 (stderr: error: pathspec 'origin/v8.291.0' did not match any file(s) known to git)

Unfortunately even prefixing origin doesn't seem to allow us to pull a specific tag 😞

[Laevos]

I'd love to find a solution that doesn't require us to change all of our vendir.yml files (as we have quite a few of them), but failing that, would the solution here maybe be to expose a RefType where we can specify whether the ref is a branch or a tag or etc., since the git commands needed differ between types?

[neil-hickey]

but failing that, would the solution here maybe be to expose a RefType where we can specify whether the ref is a branch or a tag or etc., since the git commands needed differ between types?

Yeh I was just about to comment that git fetch origin v8.291.0 --depth 1 ought to be git fetch origin tag v8.291.0 --depth 1 to actually work.... le sigh

[neil-hickey]

Let me chat with the folks that merged it in and see what they tested it with. Will for sure take into account the origin/ stuff. Appreciate you working through it with me :) was fun

[Laevos]

sgtm, thanks for helping dig into it! :)

[neil-hickey]

hey @kumaritanushree / @joaopapereira this issue is related to

https://github.com/carvel-dev/vendir/pull/251

Did you all test this use-case and it worked before?

[Laevos]

One followup question: I see in the homebrew update-formula action that the homebrew formula seems to be updated on changes to the develop branch. I also see that there is a release branch, but that it hasn't been updated in some time. Would it be desirable to base homebrew updates off of a known-stable version instead of the release branch? As far as I can tell in the docs, v0.34.0 is still in development, with v0.32.0 still tagged as latest. Maybe utilizing the head directive to get the latest commit to develop would be appropriate?

Happy to contribute a PR if that seems reasonable, but I figured I'd check and see what you all thought, thanks!

[neil-hickey]

I think the docs are just out of sync with the releases unfortunately. Any "released" item on github ought to be stable and docs should exist. But it seems like it's missing.