When using `noCache: true` with `dotnet/build-and-test.yml`, get queue-time error `Checkout of repository 'null' is not supported.`

[dagood]

Steps to reproduce the issue

1. Don't pass versionsRepoRef to eng/common/templates/stages/dotnet/build-and-test.yml and pass noCache: true
2. Try to queue a build-and-test build.

Expected behavior

The template should detect no cache, so the build successfully doesn't try to clone any versions repo.

Actual behavior

A queue-time error:

Checkout of repository 'null' is not supported. Only 'self', 'none', or a repository alias are supported.
Checkout of repository 'null' is not supported. Only 'self', 'none', or a repository alias are supported.

This happens in eng/common/templates/steps/init-matrix-build-publish.yml. Naturally, AzDO doesn't say that, but deleting the step does let the build queue successfully.

Additional information (e.g. issue happens only occasionally)

I thought I'd try using eng/common/templates/stages/dotnet/build-and-test.yml for Go because it looks general enough now. I haven't tried beyond this issue yet--figured I'd write this up first.

I think the issue is AzDO yaml and types, interacting badly in potentially a few places.

First, when I pass in noCache: true, based on earlier experiments, I believe AzDO is coercing the type to a string to fit into dotnet/build-and-test.yml's "dynamic" parameters (rather than type-safe parameters). So: in the context of this template, noCache should have the string True.

This condition then misfires because of the type:

    # Only clone versions repo if we need to reference it during the build in order to cache images.
    ${{ if eq(parameters.noCache, false) }}:
      versionsRepoRef: ${{ parameters.versionsRepoRef }}

Any string that isn't '' is coerced to True when converted to a boolean, so this seems to end up being:

versionsRepoRef: null

I can't quite convince myself of a good reason that ends up causing Checkout of repository 'null' is not supported.. It passes through a few more templates before it's actually used. But a few changes each make it work (allow the build to queue), in isolation, which led me here:

• Change to versionsRepoRef: "" in the default parameter list.
• Delete ${{ if eq(parameters.noCache, false) }}: versionsRepoRef: ${{ parameters.versionsRepoRef }} entirely.
• Delete the checkout step entirely (to reflect the state it should be in).
• Replace the checkout step with a pwsh script that shows convertToJson(parameters).
  • { "publishConfig": "null", "versionsRepoRef": "null", "versionsRepoPath": "versions" }
  • https://dev.azure.com/dnceng-public/public/_build/results?buildId=1140563&view=results
  • https://dev.azure.com/dnceng-public/public/_apis/build/builds/1140563/logs?logid=1

---

All new templates I write use typed parameters, and it seems to help a lot with avoiding stuff like this. Type coercion can still be an issue, but at least the parameter declarations make it explicit.

The inability to pass "extra" parameters in this "parameter mode" was challenging at first (and the verbosity of specifying a lot of passthrough parameters is extreme), until I realized that I could make an object-typed parameter to capture more dynamic params. With that sorted out, the fact that AzDO fails when some parameter was passed but not expected actually becomes nice compile-time assistance. Touched on it in #1771 (comment) but seems worth mentioning in this context too.

[lbussell]

Thanks for the report. This repo's template parameter hygiene isn't great. Pretty much every dotnet build pipeline assumes that VersionsRepoRef exists so I'm not surprised by this bug. We could definitely stand to go through all of the parameters and:

• Convert any implicitly typed parameters to type-safe parameters
• Ensure defaults are only set in one place (for parameters that are passed through multiple template levels)
• Put quotes around strings wherever possible (to prevent other unexpected conversions to numbers, dates, etc.)

It'd also be cool if we had a linter for this type of stuff. Not sure if that exists.

[dagood]

😅

It turns out at some point I swapped from one pipeline to another without properly adjusting the public pipeline's parameters. I was passing all the parameters inside another parent object, so noCache: true wasn't actually being passed at all. The 'null' error is odd, but I think it only shows up because of garbage in. It seems to queue fine once I fixed that.

(I don't know how to explain the dynamic parameter string coercion thing I observed earlier. Maybe it's something more complicated, some deferred eval.)

I guess this is just all an example of how deep a rabbit hole can go with AzDO yml when it's hard to check up on basic assumptions.

[dagood]

• Put quotes around strings wherever possible (to prevent other unexpected conversions to numbers, dates, etc.)

From what I've seen, quoting is more of addressing a "raw" yml shortfall, but AzDO has its own type system built on top of that, and I think it essentially overrides/disregards yml typing.

Perhaps something I should look into is trying to parse https://dev.azure.com/dnceng-public/public/_apis/build/builds/1140593/logs?logid=2 to make it more useful. It's a flood, but if it could be mapped on top of https://dev.azure.com/dnceng-public/public/_apis/build/builds/1140593/logs?logid=1 in some way, maybe direct insight into the eval process would be more feasible...

[lbussell]

Perhaps something I should look into is trying to parse https://dev.azure.com/dnceng-public/public/_apis/build/builds/1140593/logs?logid=2 to make it more useful.

TIL about this output. That does seem useful.

I filed #1799 for cleaning up templates.

[dagood]

Any string that isn't '' is coerced to True when converted to a boolean

Ah. The reason this doesn't apply is that in ${{ if eq(parameters.noCache, false) }}:, the right arg is the one that's coerced, not the left. (And eq is ordinal ignore-case for strings.) And to be fair to AzDO, this is documented.

(I'm sure I've relied on this earlier to make eqs, just remembered it backwards at the worst time. 😄)