Skip to content

Commit

Permalink
Merge pull request #378: Scalar: Fix some flaky tests with retry logi…
Browse files Browse the repository at this point in the history
…c and better GVFS Protocol awareness

I ran the Scalar functional tests at least 10 times before getting a full green build. One of the most common failures was a failure during `scalar clone` with a strange error. Looking at the TRACE2 logs, I found these entries:

```
d0 | main                     | child_start  |     |  0.159207 |           |              | [ch2] class:? argv:[git fetch --quiet origin]
d0 | main                     | child_exit   |     | 15.823433 | 15.664226 |              | [ch2] pid:16571 code:128
d0 | main                     | error        |     |           |           |              | Partial clone failed; Trying full clone
d0 | main                     | error        |     |           |           |              | could not configure for full clone
d0 | main                     | exit         |     | 15.823775 |           |              | code:1
d0 | main                     | atexit       |     | 15.823790 |           |              | code:1
```

This means that the first `git fetch` failed for some reason, and we started running `set_config()` to un-set the partial-clone config options. However, we are not in a partial clone at this state! We are working against Azure Repos.

There are two things in this PR that should help:

1. Add retry logic to `run_git()` which will retry the `git fetch` when it fails (also other subcommands).
2. Fix this use of `set_config()` by skipping it if we are using the GVFS Protocol.
  • Loading branch information
derrickstolee authored Jun 18, 2021
2 parents 57eeed4 + cbad21e commit b4032c0
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions contrib/scalar/scalar.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,25 @@ static void setup_enlistment_directory(int argc, const char **argv,
setup_git_directory();
}

static int git_retries = 3;

static int run_git(const char *arg, ...)
{
struct strvec argv = STRVEC_INIT;
va_list args;
const char *p;
int res;
int res, attempts;

va_start(args, arg);
strvec_push(&argv, arg);
while ((p = va_arg(args, const char *)))
strvec_push(&argv, p);
va_end(args);

res = run_command_v_opt(argv.v, RUN_GIT_CMD);
for (attempts = 0, res = 1;
res && attempts < git_retries;
attempts++)
res = run_command_v_opt(argv.v, RUN_GIT_CMD);

strvec_clear(&argv);
return res;
Expand Down Expand Up @@ -857,6 +862,7 @@ static int cmd_clone(int argc, const char **argv)
char *cache_key = NULL, *shared_cache_path = NULL;
struct strbuf buf = STRBUF_INIT;
int res;
int gvfs_protocol;

argc = parse_options(argc, argv, NULL, clone_options, clone_usage, 0);

Expand Down Expand Up @@ -984,8 +990,10 @@ static int cmd_clone(int argc, const char **argv)
goto cleanup;
}

if (cache_server_url ||
supports_gvfs_protocol(url, &default_cache_server_url)) {
gvfs_protocol = cache_server_url ||
supports_gvfs_protocol(url, &default_cache_server_url);

if (gvfs_protocol) {
if (!cache_server_url)
cache_server_url = default_cache_server_url;
if (set_config("core.useGVFSHelper=true") ||
Expand Down Expand Up @@ -1017,7 +1025,12 @@ static int cmd_clone(int argc, const char **argv)
return error(_("could not configure '%s'"), dir);

if ((res = run_git("fetch", "--quiet", "origin", NULL))) {
warning(_("Partial clone failed; Trying full clone"));
if (gvfs_protocol) {
res = error(_("failed to prefetch commits and trees"));
goto cleanup;
}

warning(_("partial clone failed; attempting full clone"));

if (set_config("remote.origin.promisor") ||
set_config("remote.origin.partialCloneFilter")) {
Expand Down

0 comments on commit b4032c0

Please sign in to comment.