SDK Versioning sceme, plugin sdk version check

[BlueHazzard]

Hi,
i would like to add a new function to the sdk, this means increasing the sdk version.
I have found some problems i  would like to discuss.

I think we should (and i think we are using) use semantic versioning scheme: https://semver.org/

According to my understanding we have "shiftet" the scheme by one and are not using the "patch" number.:
PLUGIN_SDK_VERSION_MINOR  -> patch version  (internal changes, probably behaviour changes)
PLUGIN_SDK_VERSION_MAJOR  -> minor version (adding functions)
PLUGIN_SDK_VERSION_RELEASE -> major version (removing functions, non backwards compatible interface changes)

So adding new features should be covered by incrementing the PLUGIN_SDK_VERSION_MAJOR. This should allow all plugins to still be usable without recompiling.

Our current implementation does not allow me to change any version number and still use old compiled plugins.
The check is performed in pluginmanager.cpp:700

Code

if (major != PLUGIN_SDK_VERSION_MAJOR ||
        minor != PLUGIN_SDK_VERSION_MINOR ||
        release != PLUGIN_SDK_VERSION_RELEASE)
    {
        // wrong version: in this case, inform the user...
        wxString fmt;
        fmt.Printf(_("SDK version mismatch for %s (%d.%d.%d). Expecting %d.%d.%d"),
                    name.c_str(),
                    major,
                    minor,
                    release,
                    PLUGIN_SDK_VERSION_MAJOR,
                    PLUGIN_SDK_VERSION_MINOR,
                    PLUGIN_SDK_VERSION_RELEASE);
        Manager::Get()->GetLogManager()->LogError(fmt);
        return;
    }

I think this is wrong and the PLUGIN_SDK_VERSION_MINOR and PLUGIN_SDK_VERSION_MAJOR check should be a ">" check. So if we add functions old plugins should still be working without problems.
Removing functions or changing the interface of functions should result in increase of the major version. Here we can perform an = check because no backwards compatibility

any comments?

[killerbot]

seems reasonable to me. If we don't foresee any regressions, might be the way forward.

[AndrewCot]

My 2c of feedback is:

1) Looks like a good change.
2) The WIKI documentation and the code will need to have info in them to ensure (okay try) people follow the semantic versioning as it's been like this for a very long time.
3) Be aware that most of the plugin's manifest.xml file are very old and specify the release as 1. With all of the changes over the years this may not be correct. These could be bumped in the same change or left for later.

[BlueHazzard]

Looking at this notes: https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
The recommend to increase the PLUGIN_SDK_VERSION_RELEASE  number only after public releases or else the numbers could get quite large. Public releasases are quite rare for us, but we have a lot nightly builds and this leads to the same problem as now. If we do not increase the PLUGIN_SDK_VERSION_RELEASE  version on every change the nightly releases can still not be used correctly with old plugins. I think this is some point. Do we mind if we get PLUGIN_SDK_VERSION_RELEASE  in the 100s range?

[AndrewCot]

This afternoon I jot clobbered on the Mac with the version check change failing (plugin 2.16.0 and CB SDK 2.19.0 btw).. I have no idea why the plugin loaded on Windows without triggering the failure.
If you look at the some of the version number of MS DLL's that are to do with the MSVC the last octect of the version number jumps 100's between releases in allot of cases.

BTW I changed the check to the following as it makes more sense and resolved the issue I had:

Code

    if (    major > PLUGIN_SDK_VERSION_MAJOR ||
            (
                major == PLUGIN_SDK_VERSION_MAJOR &&
                minor > PLUGIN_SDK_VERSION_MINOR
            ) ||
            (
                major == PLUGIN_SDK_VERSION_MAJOR &&
                minor == PLUGIN_SDK_VERSION_MINOR && 
                release > PLUGIN_SDK_VERSION_RELEASE
            )
       )

[BlueHazzard]

I think a better check is

Code

if((  release != PLUGIN_SDK_VERSION_RELEASE ||
     ( major > PLUGIN_SDK_VERSION_MAJOR ) ||
     ( minor > PLUGIN_SDK_VERSION_MINOR && major >= PLUGIN_SDK_VERSION_MAJOR)))

PLUGIN_SDK_VERSION_RELEASE  is a total breaker of plugins. Change in this version means binary incompatibility: signature of function change, or function removal this means no backwards compatibility.


Big call to other devs: What are you thinking about this?

Increasing PLUGIN_SDK_VERSION_RELEASE  will mean break binary compatibility. So a plugin previously compiled will not work. When do you want increase this? On an effective change in code or only after a release?
We use a lot nightly builds so it would be reasonable to change it after every effective code change. But in the end it could happen that we have high release numbers.
This is only a thing of taste, but we have to decide...

[AndrewCot]

Feedback on plugin manifest.xml files:
If you update all of the existing manifest.xml files that are still stuck with the major set to 1 then it will work, but it then brings up the question of what do you update them to? This could be done way before any of the changes being discussed in this thread. In my local source I have bumped the version to 2.16.0 in the old manifest.xml file just because it was the SDK version in the nightly when I did the change, but I could have picked 2.0.0 for the manifest files with 1.x.y version in them.

This also brings up a need to bump all of the manifest.xml files when the release is updated so the plugins still work. This would need to be done for windows, linux and Mac. Be aware that currently on Windows it is not built via a bash shell as the changes have not been looked at in SF, so there will not be a consistent way of doing it via the different build processes.

Other Feedback
I am a SF end user and do not like SVN, but is there a way on a SVN update to hook into the update and therefore run a script to update the PLUGIN_SDK_VERSION_MINOR to the SVN revision. This would need to be done by one of the SF C::B admins. This would need someone with SVN and SF knowledge to investigate.Manually updating the PLUGIN_SDK_VERSION_MINOR to the SVN revision will not work.