fix: keep the schema version inside the package

[rix0rrr]

Currently, the schema version espoused by this package is the package version itself (i.e., the version of its own package.json). Move this version to a resource file that maintained inside the package and is bumped explicitly.

There are a couple goals here of decoupling the package version and the schema version:

• For a practical reason: in a projen (mono)repo, all packages have version 0.0.0. At development time, the Cloud Assembly Schema has to know its own version, for various versioning checks. It can’t get it from the package version (which is 0.0.0, so it needs to get it elsewhere, which will be schema.version.json).
• Semi-practical reason: updates flow smoother. A minor version bump of the Cloud Assembly schema more accurately reflects changes: new features that are added, without breaking changes. As such, Dependabot will automatically update to newer versions. As opposed to major version bumps, for which we will need to have a custom npm-check-updates script that upgrades major versions.
• For a user focused future feature: better error messaging. If we give the Cloud Assembly Schema package version the same version as the CLI and we write that version to the manifest, it will be very easy to produce an error message like You must install at least CDK CLI >= X.Y.Z to use this CDK application.

[iliapolo]

@rix0rrr

It can’t get it from the package version (which is 0.0.0, so it needs to get it elsewhere, which will be schema.version.json)

But this is true now as well even without the monorepo change. How does it work now? And why do we need to know the version during development?

[iliapolo]

@rix0rrr

A minor version bump of the Cloud Assembly schema more accurately reflects changes: new features that are added, without breaking changes.

Not sure I follow. What do you mean by "new features"? Every change to schema itself is considered a breaking change from a CDK deployment perspective. If you mean features that don't change the schema (like adding an API on top of it) - then we can still release a minor version for that change (in fact it will happen automatically).

As such, Dependabot will automatically update to newer versions.

Dependabot can be configured to bump major versions. Its just that we'll need an exclude/include setup in such a case.

[iliapolo]

@rix0rrr

it will be very easy to produce an error message like You must install at least CDK CLI >= X.Y.Z to use this CDK application.

This is what sold me. I was never comfortable with the generic "update to the latest version" message.

[iliapolo]

Overall I want this to happen, but I don't know that it should happen before the repo split. Here's why:

Because this package is not in the same repo as the CLI, nothing guarantees that a CLI version that consumes the latest schema has already been released. This means that consumers who pickup a new schema version may be left hanging, with no CLI version compatible to it. The current versioning scheme drastically decreases the chances of this happening, because it requires a major version bump. If we do this now, consumers are gonna start getting automatic updates and might be baffled as to how to solve it (even if for a little while...).

[iliapolo]

Also, @TheRealAmazonKendra can you have a look at this too?

[mrgrain]

@rix0rrr

it will be very easy to produce an error message like You must install at least CDK CLI >= X.Y.Z to use this CDK application.

This is what sold me. I was never comfortable with the generic "update to the latest version" message.

@iliapolo @rix0rrr This is selling me as well. But... in the past we already had both packages in the same monorepo. Why was that not possible then? Is this actually a benefit that is coming out of this change? Or is it just something we need to do one way or another?

[mrgrain]

A minor version bump of the Cloud Assembly schema more accurately reflects changes: new features that are added, without breaking changes.

I'd argue this is the other way around. A newer CLI can always use older schemas, but newer schemas require an up-to-date CLI. The breaking change is that you need to update your CLI. Is this not correct?

[mrgrain]

For a practical reason: in a projen (mono)repo, all packages have version 0.0.0. At development time, the Cloud Assembly Schema has to know its own version, for various versioning checks. It can’t get it from the package version (which is 0.0.0, so it needs to get it elsewhere, which will be schema.version.json).

This is clearly solved right now. Storing the version in a file seems completely unrelated to the version number.

[mrgrain]

As such, Dependabot will automatically update to newer versions. As opposed to major version bumps, for which we will need to have a custom npm-check-updates script that upgrades major versions.

Neither Dependabot nor ncu do anything else but what they are told to do. I think what you meant to say is: The current system will require additional work.

[mrgrain]

I'm not really against this change. My main concern is the messed up versioning we will get (from 2.x. to [31-37.x], back to 2.x and deprecating some versions). Seems risky and unnecessary to me.

Finally I don't think the arguments you brought up for this are convincing. The main point seems to be "if schema and CLI have the same version, we have less work to do". That's a reasonable point. Let's just make it then.

[rix0rrr]

It can’t get it from the package version (which is 0.0.0, so it needs to get it elsewhere, which will be schema.version.json)

But this is true now as well even without the monorepo change. How does it work now? And why do we need to know the version during development?

There are no tests in this package that depend on the version of the schema, but there are tests in the CLI that depend on the version of the Cloud Assembly schema. As in, the behavior of the test depends on a line in the CLI that says:

if (schema.version < 10) { 
  doThis();
} else {
  doThat();
}

This test now fails because schema.version == 0 so the CLI will take the doThis() branch instead of the doThat branch.

Not sure I follow. What do you mean by "new features"? Every change to schema itself is considered a breaking change from a CDK deployment perspective.

I don't think I agree with this. An addition to the schema represents a new capability of the CLI. It is not a breaking change for the CLI to add a new capability.

I guess it could be argued that it is disruptive for the aws-cdk-lib to consume a new version of the schema, because that requires a new version of the CLI. Still, changing either the major or the minor version doesn't really change anything about the disruptiveness of that change; and I would argue that that is an implementation choice (*), and not inherent to the description of the protocol.

(*) One might imagine a scenario in which we emit the version of the schema file as MAX(actually_used_capability_version) to reduce the disruptiveness of changes.

Dependabot can be configured to bump major versions. Its just that we'll need an exclude/include setup in such a case.

Perhaps you're right. I'd argue there is a value to working with defaults... but this is a weaker argument.

Overall I want this to happen, but I don't know that it should happen before the repo split.

Well here's a catch-22: we can't do the repo split before this has happened, because the CLI tests won't pass before this change has been made (they fail due to the reason I said above).

I suppose we could do another workaround: disable the tests that fail. But we need to do something to unblock the CLI migration right now, and I figured moving this package into its desired state would be the simplest and cheapest overall solution.

[mrgrain]

There are no tests in this package that depend on the version of the schema, but there are tests in the CLI that depend on the version of the Cloud Assembly schema. As in, the behavior of the test depends on a line in the CLI that says:

if (schema.version < 10) { 
  doThis();
} else {
  doThat();
}

This test now fails because schema.version == 0 so the CLI will take the doThis() branch instead of the doThat branch.

Honestly I think the test is wrong and it needs to run with the schema being set as both < 10 and >=10.
Cause otherwise we don't ever test the doThis(); branch.

[rix0rrr]

I can't really proceed with the migration without this change though.

The projen scripts runs git --sort=-creatordate to find the latest tag of the repository, to decide what argument to pass to minMajorVersion.

Since my target repository doesn't have tags yet, and I don't want to put an artificial v38.0.0 tag on the entire repository just for this one package, I cannot make the build work without applying post-migration changes to this repository that I do not want to do.