Clone repo normally

[HebaruSan]

Problem

As @techman83 noted (see #33 (comment)), the new bot's git repos aren't being created properly.

A regular git clone's .git/config file:

[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
[remote "origin"]
	url = /tmp/tmpnvd6f81d/upstream
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master

The .git/config file from the new bot's repo:

[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
[remote "origin"]
	url = /tmp/tmpnvd6f81d/upstream
	fetch = +refs/heads/*:refs/remotes/origin/*

Cause

The new bot attempts to set up its git repos manually in multiple steps: mkdir, init, create_remote, and pull. This leaves out the step of setting the upstream for the master branch.

Changes

Now we use clone_from to set up the working dir normally. This will take care of the whole process for us in one step.

In a dir set up with the old code:

$ git status
On branch master
nothing to commit, working tree clean

In a dir set up with the new code:

$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

Note the new bit about "up to date with 'origin/master'". This means the upstream is set for the branch.

May relate to #33, but I'm not sure about that. I had expected the problem to relate more to the Perl web hook code and/or how it's configured, since the problem in that issue happens immediately without waiting for the new bot to mess things up.

[techman83]

@HebaruSan this is the thing I fought with for hours when it came to staging. Using clone_from doesn't map the refs properly. :-(

I'm probably gonna drop that create_remote and call out to the git cli directly (which you can do with repo.git.<function>().

[HebaruSan]

Using clone_from doesn't map the refs properly. :-(

Howso? I didn't notice any problems when I was trying it out.

[techman83]

Howso? I didn't notice any problems when I was trying it out.

The ref mapping. It's very subtly different. I spent hours debugging the staging process, only to figure out the clone was where the problem was. d485953

[HebaruSan]

The ref mapping. It's very subtly different.

Can you give me a hint of where to look to see that? I'm suspicious that we appear to have had a bug that was fixed by introducing another bug, suggesting that the original problem may have been something other than it appeared.

[HebaruSan]

I'm probably gonna drop that create_remote and call out to the git cli directly (which you can do with repo.git.().

That's exactly how Repo.clone_from is implemented, FWIW:

https://github.com/gitpython-developers/GitPython/blob/3d7eaf1253245c6b88fd969efa383b775927cdd0/git/repo/base.py#L960-L961

[techman83]

It was this mapping here, I don't have the example specifically though this is an example of what it looks like:

[remote "origin"]
	url = /tmp/tmpnvd6f81d/upstream
	fetch = +refs/heads/*:refs/remotes/origin/*

I think this is how it was configuring it from memory.

[remote "origin"]
	url = /tmp/tmpnvd6f81d/upstream
	fetch = +refs/heads/master:refs/remotes/origin/master

Which relates to the refspec. I definitely introduced a new bug fixing it, however reverting it will cause it to blow up when it hits a staging branch on a fresh clone.

Worst case scenario we can just inject the master block config into the config and move on 🤷‍♂️

[branch "master"]
	remote = origin
	merge = refs/heads/master

I feel I'm missing something stupidly obvious somewhere, but the documentation is a little tough to follow.

[techman83]

That's exactly how Repo.clone_from is implemented, FWIW

Interesting. I wonder what option I was failing to pass that makes it behave like the cli version.

[techman83]

I think we should write a test case for the clone. One that matches the desired output of the config, cause we have come unstuck several times by this. Manually setting that master block does fix it and I reckon I can massage the bot a little to prove that. The repo only gets brought into memory when the context manager is active (it leaks memory, so gets closed after every block sqs batch)

[HebaruSan]

OK, so this script:

import os, sys, git, pathlib

url = "https://github.com/KSP-CKAN/CKAN-meta.git"
dir = pathlib.Path("cloned")

repo = git.Repo.clone_from(url, dir)

Generates this .git/config file:

[core]
        repositoryformatversion = 0
        filemode = false
        bare = false
        logallrefupdates = true
        symlinks = false
        ignorecase = true
[remote "origin"]
        url = https://github.com/KSP-CKAN/CKAN-meta.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
        remote = origin
        merge = refs/heads/master

And running git clone https://github.com/KSP-CKAN/CKAN-meta.git from the command line gave me this:

[core]
        repositoryformatversion = 0
        filemode = false
        bare = false
        logallrefupdates = true
        symlinks = false
        ignorecase = true
[remote "origin"]
        url = https://github.com/KSP-CKAN/CKAN-meta.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
        remote = origin
        merge = refs/heads/master

I'm not seeing a problem with Repo.clone_from.

[techman83]

Thanks for your persistence @HebaruSan - I'll check this branch when I have a coffee and see if I can repeat the error I had. It might even be down to the git version in the container, maybe its newer and the defaults are done differently.

If it works I'll go ahead and merge it.

[techman83]

I do note that we're doing a full clone here, but I can't replicate the fault either way (shallow or full).

Thanks for persisting, I literally have no idea why that wasn't working last week when I was doing the testing. But honestly it's been the week for it and I'm looking forward to going home and resting!

[HebaruSan]

Are the repos used by the various containers persistent? If so, we should delete them so they can be re-cloned to get the benefits of this fix.

[techman83]

Nope, they are re-cloned on container redeploy. So it should be all done now in theory :-)