NBGV sets cloudBuild.buildNumber to null, but this violates the JSON schema

[alexrp]

This happens when running nbgv prepare-release, for example.

See: vezel-dev/zig-sdk@0602c08

[dariusonsched]

@AArnott I concur with this. First snippet is the original version.json from the main branch before nbgv prepare-release. Second and third are results of version.json in the new release branch and back on the main branch.

In addition to the cloudBuild section disappearing, notice the release section is missing my branchName and versionIncrement properties.

This is a show-stopper for us.

{
    "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json",
    "version": "2021.1-beta.{height}",
    "assemblyVersion": {
        "precision": "build"
    },
    "gitCommitIdShortFixedLength": 8,
    "pathFilters": [],
    "publicReleaseRefSpec": [
        "^refs/heads/releases/v\\d+\\.\\d+",
        "^refs/tags/v\\d+\\.\\d+"
    ],
    "cloudBuild": {
        "setVersionVariables": true,
        "buildNumber": {
            "enabled": false,
            "includeCommitId": {
                "when": "nonPublicReleaseOnly",
                "where": "buildMetadata"
            }
        }
    },
    "release" : {
        "branchName" : "releases/v{version}",
        "versionIncrement" : "minor",
        "firstUnstableTag" : "beta"
    }
}

Release version:

{
  "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json",
  "version": "2021.1",
  "assemblyVersion": {
    "precision": "build"
  },
  "gitCommitIdShortFixedLength": 8,
  "publicReleaseRefSpec": [
    "^refs/heads/releases/v\\d+\\.\\d+",
    "^refs/tags/v\\d+\\.\\d+"
  ],
  "release": {
    "firstUnstableTag": "beta"
  },
  "pathFilters": []
}

Main branch after prepare-release:

{
  "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json",
  "version": "2021.2-beta",
  "assemblyVersion": {
    "precision": "build"
  },
  "gitCommitIdShortFixedLength": 8,
  "publicReleaseRefSpec": [
    "^refs/heads/releases/v\\d+\\.\\d+",
    "^refs/tags/v\\d+\\.\\d+"
  ],
  "release": {
    "firstUnstableTag": "beta"
  },
  "pathFilters": []
}

[AArnott]

@dariusonsched In your example, cloudBuild appears to be removed, which is certainly unexpected, although not the same symptom reported in the bug title. That's alright though: I'll consider both symptoms when I investigate.

[dariusonsched]

@AArnott Apologies - The result of a late night working session.

[AArnott]

@dariusonsched said:

In addition to the cloudBuild section disappearing

The cloudBuild section disappears because it is equivalent to the default settings. When we deserialize we don't currently record what was explicitly present, so when it comes to serializing back, we just skip anything that could be omitted because it's the default.

notice the release section is missing my branchName and versionIncrement properties.

versionIncrement is dropped for the same reason: it's the default.

Your branchName is not the default and should have been retained. But it is retained when I try it with nbgv v3.4.240. What version are you using?

This is the final product (in the main branch) after I run the command:

{
  "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json",
  "version": "2021.2-beta.{height}",
  "assemblyVersion": {
    "precision": "build"
  },
  "gitCommitIdShortFixedLength": 8,
  "publicReleaseRefSpec": [
    "^refs/heads/releases/v\\d+\\.\\d+",
    "^refs/tags/v\\d+\\.\\d+"
  ],
  "release": {
    "branchName": "releases/v{version}",
    "firstUnstableTag": "beta"
  },
  "pathFilters": []
}

This looks correct, although it's trimmed of defaults.
Is there any reason you require the defaults to remain?
Does updating your nbgv version resolve the last issue of release.branchName disappearing?

@alexrp I can repro what you're reporting and am investigating.

[dariusonsched]

@AArnott Thank you for your response.

Looks like 3.4.20:

Tool 'nbgv' (version '3.4.240') was successfully installed.

I was reviewing your codebase and noticed how only the top-most properties are being evaluated for consideration of including cloudBuild:

Nerdbank.GitVersioning/src/NerdBank.GitVersioning/VersionOptionsContractResolver.cs

Line 94 in a1d636e

property.ShouldSerialize = instance => !((VersionOptions)instance).CloudBuildOrDefault.IsDefault;

Nerdbank.GitVersioning/src/NerdBank.GitVersioning/VersionOptions.cs

Line 867 in a1d636e

internal static readonly CloudBuildOptions DefaultInstance = new CloudBuildOptions()

Even if cloudBuild is left at defaults, but cloudBuildNumber is set, it must be included. This would seem like an absolute need to resolve and hope you agree.

How do we get around this for now? Should we set setAllVariables = true to force a delta from the default?

Regarding the release not carrying over the branchName, I believe I have traced that back to the main line branch was still set to the default value. The new, incoming value releases/v{version} was part of this PR and since this PR wasn't merged yet, it wasn't there for consideration.

[AArnott]

only the top-most properties are being evaluated for consideration of including cloudBuild:

The equality check is "deep". Each property only looks at equality of the property value to its default, but the equality comparer is recursive. The entire top-level property will be skipped if the default property value includes all the same properties "deeply". In your case, that's what's happening here. All the properties you set on cloudBuild are exactly what NB.GV would do without you setting anything.

Even if cloudBuild is left at defaults, but cloudBuildNumber is set, it must be included.

Why? The default value for cloudBuild includes that cloudBuildNumber is set to the same values you specify. Omitting the whole thing changes no behavior in NB.GV.

How do we get around this for now?

I don't yet see the problem. The nbgv tool is just trimming the fat from your version.json file. No behavioral changes come of them.

I believe I have traced that back to the main line branch was still set to the default value.

Excellent.

[dariusonsched]

@AArnott Oh, forgive me. I totally didn't realize cloudBuildNumber > enabled was the default state. I appreciate your help. It appears the original issue author's concern is still relevant; so I will free you up to addressing it. Warm regards, -Darius

[AArnott]

I totally didn't realize cloudBuildNumber > enabled was the default state.

The default for cloudBuildNumber.enabled is actually false.

I'm glad you're not blocked after all.