[WIP] Git support: first steps

[wilzbach]

trafficstars

This is a WIP approach to support to new features. Based on a label (or maybe comment, but label is easier for now as authorization is implicitly given), support the following "actions":

• "bot-rebase" (label) -> "git clone" -> rebase to latest master -> "git push" (if no failure happened)
• "bot-squash" (label) -> "git clone" -> squash all commits -> "git push"

Imho both are very useful features

• rebase can be used to re-trigger Jenkins
• squash can be used by reviewers to automatically squash everything before applying "auto-merge"

This PR implements only "rebase", but adding "squash" later won't be difficult

What's there?

• Implemented SmartGit server protocol (only the needed parts, of course)
• Added unittest abstraction over SmartGit server

This means that we can fully handle git push commands as "faked" server, inject the requests and control the responses like we already do for other mocked APIs.

> git push -v --force http://0.0.0.0:9006/aa/bar HEAD:Issue_8573

The command will be handled by the mocked backend :)

// first request by the git client
// server returns a list of all refs it knows
gitInfoRefs = (string user, string repo) {
    "requesting refs".writeln;
    return [
        GitInfoRef("6ed2e15cf4a35e274adb8385b3ee8125326509f9", "refs/heads/foo"),
        GitInfoRef("6e79d22fdfda446601d969ce77e406b9a5506de9", "refs/heads/Issue_8573"),
        GitInfoRef("6e79d22fdfda446601d969ce77e406b9a5506de8", "refs/heads/master"),
    ];
};

// git client "reports" new refs to the server
gitReportRefs = (ClientReq clientReq) {
    clientReq.writeln;
    // GitClientRequest([], [Command(update, "6e79d22fdfda446601d969ce77e406b9a5506de9", "e4afacadb8a291f51444e948dd7b53592e799dbe", "refs/heads/Issue_8573")])
    return [
        GitReportRef(GitReportRef.status.ok, "refs/heads/Issue_8573")
    ];
};

What's missing?

• [ ] fix reason for Vibe.d blocking (not sure about this, but std.process.wait seems to block the entire Vibe.d thread even if we are in a new fiber (e.g. runTask), on a worker thread (e.g. runWorkerTask) and when spawned with std.concurrency`)
• [ ] add simple, "immutable" test repository with a couple of small PRs
• [ ] extensive testing (on GH with dummy repo)
• [ ] remove hard-coded gitURL
• [ ] add support for authentication (this requires saving the bot's password or private key on heroku)
• [ ] final cleanup

[dlang-bot]

Thanks for your pull request, @wilzbach!

[CyberShadow]

Implemented SmartGit server protocol (only the needed parts, of course)

That seems like overkill. Why not just use a bare repo?

[wilzbach]

That seems like overkill. Why not just use a bare repo?

Why? I explicitly want to test what the bot will send to GitHub, so mocking the API seemed very reasonable. Btw this is the way we test the other bot actions as well and force rebasing is a potentially dangerous action, hence I want to have an extensive test coverage before testing this in the wild.

[CyberShadow]

Why?

Well, it tests implementation detail of third-party software, relies on the git implementation used to behave as it does now, and it would make it overly onerous to write tests for more complicated functionality in the same vein.

It would make sense if dlang-bot had its own git client and SmartGit protocol implementation, but it doesn't (and shouldn't) - it uses the git shell commands, so I think it would be more logical to test their effects, not implementation details.

IMHO. shrug

[wilzbach]

It would make sense if dlang-bot had its own git client and SmartGit protocol implementation, but it doesn't - it uses the git shell commands, so I think it would be more logical to test their effects, not implementation details.

How would you test the effect of git push?

[CyberShadow]

You can push to a bare repo on the filesystem.

[PetarKirov]

What do you guys think about using comments, instead of labels for this? Specifically about squashing, it would be much better if a reviewer could manually specify a commit message.

[JinShil]

What do you guys think about using comments, instead of labels for this?

I kindof like the idea of giving some functionality to contributors through the comments. If you want to build the basic infrastructure around labels, you could still have commands in the comments to label/unlabel a PR.