RE: On compatibility

[Mark Rivers <rivers@cars.uchicago.edu>]

I certainly agree that maintaining interfaces to the maximum extent possible is a good idea, and I try to do that.

But I really don't see the argument for preserving binary compatibility.  As far as I can see there are 2 reasons for preserving binary compatibility of software packages:

1) The source code is not available.  For example, Windows.

2) The effort to compile everything from source code is overwhelming.  For example, Linux.

In my opinion EPICS does not fit in either of these categories.  

- The source code is available.

- The time to rebuild everything from scratch is trivial.  For example, on my Linux host the time to do a clean rebuild (using "make -j") is:

  - EPICS base: 23 seconds

  - All of synApps (including asyn, seq, areaDetector) and all of our IOC applications: 78 seconds

Thus it takes less than 2 minutes to rebuild base and all applications.  This includes rebuilding for 4 architectures: linux-x86, vxWorks-68040, vxWorks-ppc603_long, and vxWorks-ppc604_long.  This is on a Linux host with 16 cores, which does not cost very much now.

On the other hand, preserving binary compatibilty comes at a significant cost in terms of code legibility and maintainability.  

Consider the following change in the vxi11Configure command between asyn 4-15 and 4-16.

 int vxi11Configure(char *dn, char *hostName, int flags,
-    double defTimeout,
+    char *defTimeoutString,

-static const iocshArg vxi11ConfigureArg3 = { "default timeout",iocshArgDouble};
+static const iocshArg vxi11ConfigureArg3 = { "default timeout",iocshArgString};
 
This change was made because we realized that one cannot pass floating point arguments in the vxWorks shell, so we changed the defTimeout argument in the vxi11Configure command from a double to a string.  For users who were using vxWorks this was not binary compatible, and they needed to change their startup scripts to put quotes around that argument.  For users who were using the iocsh shell it was binary compatible, because that shell does not require quotes on strings.

What should we have done here?  Made a major new release because we broke binary compatibility on some platforms?

As another example, consider a C++ driver that contains something like the following, which is very similar to real areaDetector drivers.

class myClass : public baseClass {
public:
    void publicFunction1();
    int publicParam1;
    #define FIRST_PUBLIC_PARAM publicParam1
     int publicParam2;
    #define LAST_PUBLIC_PARAM publicParam2
    #define NUM_PUBLIC_PARAMS (&LAST_PUBLIC_PARAM - &FIRST_PUBLIC_PARAM + 1)

protected:
    int protectedParam1;
    #define FIRST_PROTECTED_PARAM protectedParam1
    int protectedParam2;
    #define LAST_PROTECTED_PARAM protectedParam2
    #define NUM_PROTECTED_PARAMS (&LAST_PROTECTED_PARAM - &FIRST_PROTECTED_PARAM + 1)

private:
   int privateParam1;
}


Now I want to make a new release in which I add publicFunction2(), publicParam3, and protectedParam3;

The natural way to do this is of course:

class myClass : public baseClass {
public:
    void publicFunction1();
    void publicFunction2();
    int publicParam1;
    #define FIRST_PUBLIC_PARAM publicParam1
    int publicParam2;
    int publicParam3;
    #define LAST_PUBLIC_PARAM publicParam3
    #define NUM_PUBLIC_PARAMS (&LAST_PUBLIC_PARAM - &FIRST_PUBLIC_PARAM + 1)

protected:
    int protectedParam1;
    #define FIRST_PROTECTED_PARAM protectedParam1
    int protectedParam2;
    int protectedParam3;
    #define LAST_PROTECTED_PARAM protectedParam3
    #define NUM_PROTECTED_PARAMS (&LAST_PROTECTED_PARAM - &FIRST_PROTECTED_PARAM + 1)

private:
   int privateParam1;
}

But this breaks binary compatibility.  I need to put the publicParam3 and privateParam3 at the end.  I argue that it makes the code much less legible, and much less maintainable, since public and protected data is now scattered all over the place.  Furthermore,  I cannot use the simple parameter counting algorithm above, which is actually very useful in real life.

What about publicFunction2()?  Does putting that in front of the publicParam1 break binary compatibility?  Does the C++ standard actually define how the function tables are arranged in memory with respect to the class member data, so that adding a new function at the head is guaranteed to be binary compatible and not move class member data? 

Finally, consider an example where I have defined a public function to be:

int myPublicFunction(char *myString);

Later on I realize that I really should have defined this as:

int myPublicFunction(const char *myString);

and I make the change.  Is that binary compatible?  In my experience it probably is with gcc.  But it is not with the Visual Studio compilers, which put "const char *" data in a read-only section of memory, and you will get a crash if you attempt to modify it.  Does that mean I should not fix this bug in order to maintain binary compatibility?  Or that I need to make a major new release number to fix it?

If recompiling is trivial, I also don't buy the requirement for putting new enums at the end, new structure elements at the end, preserving unused and obsolete parameters in function calls, etc.  

The last time I counted, the publicly available EPICS modules for which I am directly responsible consist of over 350,000 lines of code ( and that's just the .c, .cpp, and .h files).  Legibility and maintainability win out over binary compatibility in my book.

Cheers,
Mark

________________________________________
From: tech-talk-bounces@aps.anl.gov [tech-talk-bounces@aps.anl.gov] on behalf of michael.abbott@diamond.ac.uk [michael.abbott@diamond.ac.uk]
Sent: Tuesday, May 29, 2012 4:05 AM
To: dirk.zimoch@psi.ch; jlmuir@imca-cat.org
Cc: wenorum@lbl.gov; tech-talk@aps.anl.gov
Subject: RE: On compatibility

I'm going to quote the entirety of Dirk's message.

From: tech-talk-bounces@aps.anl.gov [mailto:tech-talk-
> I like backward compatibility. This allows to take care only of the
> changes that I really need.
>
> When upgrading to a new version of a certain (3rd party) driver, I
> usually spend some time on tracking the changes. I do that because of
> bad experience. Drivers change behavior, change APIs and so on. Recent
> example: base 3.14.12 broke the ca gateway because of an API change.
> Other example: the OMS VME58 driver once dropped one (unused) argument
> from it's configuration function. Everybody had to change their startup
> scripts. These changes usually induce a lot of work in addition to
> simply downloading the new code and compiling it.
>
> When there are many cross-dependencies like in areaDetector, things are
> getting worse. Suddenly I have to track changes in 5 or more other
> modules (and maybe they require further updates and so on). This costs
> me a lot of my time. This is why I would appreciate to have more
> flexibility in mixing versions and to be able to upgrade only that what
> I really need and want to upgrade.
>
> By the way, I also appreciate binary compatibility. Up to EPICS base
> 3.14.8 (maybe even up to 3.14.11), large parts of base were binary
> compatible. I could for example upgrade all extensions just by
> upgrading base. 3.14.12 broke this mechanism.
>
> Some guidelines would help to to keep binary compatibility:
>
> Do not remove API functions or change their arguments. Do not change
> the meaning (e.g. units) of an argument. Better write additional API
> functions if necessary.
>
> Do not remove or reorder fields of interface structures. Do not insert
> fields in the middle. Add them at the end. Keep supporting old fields.
>
> Do not remove shared libraries.
>
> Also put version macros into your header files if they are used by
> other code. This allows other code to deal with different interfaces.
>
> It *is* possible to improve software without breaking all existing
> interfaces.
>
> Or do it like the Linux kernel developers do: Someone who changes an
> interface is responsible for fixing (and testing ?) all drivers that
> use that interface. This of course only works when all drivers are in one
> common repository.
>
> At the moment I am very hesitant when it comes to upgrades of third
> party drivers because of all the unknown side effects.

What can I say, except: yes, yes, again yes!

Please folks, please pay attention to backwards compatibility.  Mostly it requires just a little attention to the details that Dirk mentions.

One more detail regarding enumerations: when adding values to an enumeration, add them to the *end*, not in the middle, and please *don't* just delete values from the middle of an enumeration list -- if you really want to delete a value then assign explicit (and backwards compatible) values to everything before you delete the offending value.

To be honest I'm speechless to express the frustration I've found in trying to deal with gratuitous changes to interfaces.  *Sometimes* you need to break things.  Sometimes...  Most of the time it's just either inattention or being completely oblivious to the fact that backwards compatibility is a good thing.

So, in summary: please do what Dirk says here!

--
This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail.
Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd.
Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message.
Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom