arcade-services icon indicating copy to clipboard operation
arcade-services copied to clipboard

VMR update doesn't fetch additional remotes well

Open mmitche opened this issue 2 years ago • 5 comments

I would expect something like this to work:

darc vmr update installer:823c51323a83639075f6666a06a0f3a48804233a --vmr C:\r\dotnet\ --additional-remotes installer:https://github.com/directhex/installer

or

darc vmr update installer:mariner-vmr-build --vmr C:\r\dotnet\ --additional-remotes installer:https://github.com/directhex/installer

But it appears the vmr doesn't fetch the additional remotes well when running update. It runs fetch against the remote, but this won't necessarily pull the commit specified, or the branch.

mmitche avatar Nov 30 '23 22:11 mmitche

I think the issue is that we don't ever add remotes for any additional remote, or fetch properly.

@premun Let me work on this one so I can understand the tooling better.

mmitche avatar Nov 30 '23 22:11 mmitche

@mmitche please go ahead. I had the following issues with git fetching:

  • fetch --all is the only one that fetches everything but you cannot provide auth headers for each remote
  • fetch remote does less, even if I try to provide all kinds of masks for the type of object
  • I ended up with a combination of remote update and fetch remote as it was promising. However, commits that are not labeled in any way (e.g. no branch pointing after a shallow fetch), do not get fetched. As a workaround in the installer PR, I label the commit artificially and then it gets fetched: https://github.com/dotnet/installer/blob/2292177769e14117b5517583ebed78533d2642af/eng/pipelines/templates/steps/vmr-pull-updates.yml#L25-L31

Your case, I am not sure why is not working but there was a lot of changes around remotes recently. However, we do have an E2E tests for the additional remotes, but maybe it got broken?

If you run with --debug, you will see all calls to git in the logs to verify it's fetching. Also, you probably know, but make sure to update darc. Some of this is pretty new. It is however possible that Maestro is few commits behind. Maybe use the version from dotnet/installer

premun avatar Dec 01 '23 09:12 premun

@mmitche I think I realized what might have gotten broken - passing the branch name instead of SHA. We might not be testing that and it's possibly trying to resolve the branch name as commit SHA. This used to work but possibly got broken. We never use branch names in the infra but for local developer flows it's needed.

Will you have a look?

premun avatar Dec 01 '23 11:12 premun

@mmitche is this still something you want to pursue?

premun avatar Dec 11 '23 13:12 premun

Yes but I'm focused on the VB PoC right now so if you wanted to pick it up.

mmitche avatar Dec 11 '23 15:12 mmitche