Graceful cancellation for network dependent commands

[a-hilaly]

Issue N/A

ack-generate uses go-git to clone and fetch repository tags, these
actions are time consuming (more than a minute for cloning) and do
not tolerate interruption (ctrl+C). Forcing a command to exit when it's
running a git.Clone or a git.FetchTags cause the process to write
incomplete data on disk, therefore re-running ack-generate will
always return an error. The only solution for this problem is to delete
the local sdk cache and let ack-generate reclone (without interruption)

This patch adds a graceful cancellation system for ack-generate
subcommads. Which will allow users to safely ctrl+C and re-run their
commands without any errors.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[RedbackThomson]

I see that it sets up a cancel context and prescribes timeouts to each git operation, but how does this prevent it from creating a faulty local version of the repositories?

[a-hilaly]

I see that it sets up a cancel context and prescribes timeouts to each git operation, but how does this prevent it from creating a faulty local version of the repositories?

Yes on one hand this PR adds default timeouts for each git operation, on the other hand we also propagate a cancellation mechanism to all the functions using the main context (context.Background())

Short answer: It is up the go-git to properly handle context cancellation (which they are "trying" to do). We just have to properly ask go-git functions to cancel what they're doing before killing the process.

The cancellation mechanism doesn't guaranty that go-git will not write incomplete data and/or missing git blobs - but only ensure that we are correctly using go-git functions and any function that takes a context.Context as first parameter.

long answer:

Calling a cancel (function return by context.WIthCancel) sends a signal using the channel that is accessible via ctx.Done(). Every function that needs to implement graceful cancellation needs to listen for a ctx.Done() signal using a select statement.

How does go-git handle graceful cancellation for network calls and disk i/o operations ?
For disk IO operations, go-git uses a specific io.Writer implementations that listens to ctx.Done messages - note line 69 that is waiting for a ctx.Done() signal - which is part of the context.Context interface functions. (and still this implementation is not perfect since there no way to cancel a single write op using go stdlib)

For network calls - the cancellation mechanism is implemented in the standard libraries net, net/http, net/http/httputil ... some examples of handle cancel signals: https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/net/dial.go#L527-L558https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/net/dial.go#L384-L395

[RedbackThomson]

Glad go-git has support for cancellation contexts. Looks good!

/lgtm

[ack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [A-Hilaly,RedbackThomson,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jaypipes]

wouldn't cancel have been called already on line 53?

[a-hilaly]

@jaypipes If signalCh receives a message in L52 yes. However it's safe to call a context.CancelFunc more than once: https://github.com/golang/go/blob/master/src/context/context.go#L404