Add tool package format version metadata to packed tool in settings file

[wli3]

To support better error message when future version come.

@KathleenDollard should it be SettingsVersion? it is really not about settings file. It is the format or structure(folder structure + setting file content). Maybe ToolPackageFormatVersion ?

example:

<?xml version="1.0" encoding="utf-8"?>
<DotNetCliTool>
  <Commands>
    <Command Name="mytool" EntryPoint="mytool.dll" Runner="dotnet" />
  </Commands>

  <SettingsVersion>1.0.0</SettingsVersion>
</DotNetCliTool>

And also, warning when it is not compatible (start with 1.)

cc @dotnet/dotnet-cli

• change on CLI to warning when major version is different
• change on SDK to put version on packtool

[peterhuene]

Perhaps a Version or SettingsVersion attribute on DotNetCliTool rather than a sub-element?

[wli3]

That's also good. A little vague. But better than settingsVersion since settingsVersion won't cover all

[KathleenDollard]

I like that.

…

________________________________ From: William Lee <notifications@github.com> Sent: Thursday, March 15, 2018 2:51 PM To: dotnet/cli Cc: Kathleen Dollard; Mention Subject: Re: [dotnet/cli] Add tool package format version metadata to packed tool in settings file (#8805) That's also good. A little vague. But better than settingsVersion since settingsVersion won't cover all — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcli%2Fissues%2F8805%23issuecomment-373534400&data=04%7C01%7Ckathleen.dollard%40microsoft.com%7C262d7f875c294323263308d58abee8ca%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636567474911143485%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=xeCPxln0fk5XnaMzbmomNAEunupaoS%2BVCtQLeN646RQ%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAFktXh-cmI0RYkWXC04BVx_I7YOkWX3_ks5teuJggaJpZM4Ss6Jp&data=04%7C01%7Ckathleen.dollard%40microsoft.com%7C262d7f875c294323263308d58abee8ca%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636567474911143485%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwifQ%3D%3D%7C-1&sdata=FIKTn0ginkQAQz7Z%2BXoVHz2bryOzk48l35tB1Is%2FVBQ%3D&reserved=0>.

[wli3]

Do you think we should warning the user as long as minor version is bigger?

If we use semver, when tool package format 1.2.0 come out in the future. It will require more feature, say support self contain, but back compat. That means, if the consumer has a SDK support 1.0.0 format, that SDK cannot support the self contain tools.

[wli3]

@nguerrera @KathleenDollard

Let's move the discussion on PR here. Since it is more of a general issue #2074 (comment)

Nick:

I don't have a huge opinion on this, but if the sole purpose is to mark a breaking boundary for older consumers, we could just use a single number: "1" -> "2" -> "3".

I just wonder when we're expected to change major, minor, patch if there are 3 parts.

Whatever scheme we use, we should be documenting when to change it.

[wli3]

I think either way are fine, since it won't change that dramatic. However, i think semver is well known enough, and it fits here:

If the package format version is 1.2, it may not work on SDK that only support 1.0 - 1.1 since it asks for more feature. It is fine for SDK Support 1.2 - 1.9999. But it should warning again for SDK supporting 2.0+

[wli3]

Maybe the problem is the hammer is too big?

[nguerrera]

If the package format version is 1.2, it may not work on SDK that only support 1.0 - 1.1 since it asks for more feature. It is fine for SDK Support 1.2 - 1.9999. But it should warning again for SDK supporting 2.0+

I'm having trouble interpreting this. Really the only meaningful thing is to poison the package so that old consumers break cleanly instead of unexpectedly. I think we only need one dimension for that.

int formatVersion = 1; // bump whenever the format changes such that it will break old consumers

Right now, when do we change it? Do we need to evaluate if every single change to the format is major, minor, or patch?

If we can come up with clear guidance and document it, I'm fine with a semantic version, but I think the risk is that we won't actually update it to follow such guidelines because they are necessarily more complicated with 3 dimensions.

[KathleenDollard]

There is no implementation, so there is no need for a patch.

The first major represents a change where we think an older version of the SDK will not be able to install or run the version of the tool created with a later version of the SDK.

Those two sound OK?

The question is whether we want to mark a change in the settings structure that is clearly a change, but we do not expect to be a problem for older versions of the SDK.

On the one hand, I think we do because it's a dandy way to keep track of versions. On the other hand, this is only available at install time (correct?) and if we were interested in a new non-breaking feature we would look for the feature not the version number (correct?), so the only reason this exists is to give the warning to avoid attempting forwards compatibility in the tool structure. If that is the only reason we are doing this, yep, one digit does it.

Can we add a second digit in the future if we are wrong about this very narrow usage?

If all that lines up. Yep, one digit does it.

Do we have docs on the settings.xml file that I need to update.

[wli3]

On the other hand, this is only available at install time (correct?)

It is in the disk. But it is only checked during install time.

if we were interested in a new non-breaking feature we would look for the feature not the version number (correct?)

This part I think I may misunderstood from the mail. So there is a feature that is absent. Today the code in master will error out due to the missing feature, say only support any. But do we also want to warn user to update SDK during install time?

[wli3]

if not, the version number only bump with breaking change. And 1 digit is enough

[KathleenDollard]

Can you write the code so it ignores all but the first digit, but doesn't crash if we need two digit numbers later.

If we don't prepare for the possibility of two digits, then we would be forced to add another number if we are wrong and it matters what the version is, even when the change is additive and non-breaking.

Does that make sense? Otherwise I will be here a bit longer if you want to come by.