goulgold / gflags

Automatically exported from code.google.com/p/gflags
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Bad soname version in current git master #85

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Commit cd7aece14e2aac0669292c95ab42de6e267a5034 sets the library SOVERSION to 
the major version of the package (currently 2).

However, the current git master (or even 2.1.1) is not backward compatible at 
an ABI level with previous versions (2.0) of the library. At least one symbol 
was dropped at some point.

Please increment the SOVERSION, or go back to using MAJOR.MINOR (or maybe even 
MAJOR.MINOR.MICRO).

Original issue reported on code.google.com by floppymaster@gmail.com on 16 Jul 2014 at 3:42

GoogleCodeExporter commented 9 years ago
Hmm... I have just seen #83... maybe I need to look into that.

Original comment by floppymaster@gmail.com on 16 Jul 2014 at 4:08

GoogleCodeExporter commented 9 years ago
Ok, so having reviewed that issue, I would definitely say that changing the 
namespace for all C++ functions warrants a change in the major version of the 
package.

Original comment by floppymaster@gmail.com on 17 Jul 2014 at 12:26

GoogleCodeExporter commented 9 years ago
I would agree. However, considering that version 2.0 of gflags was to reflect 
the change of community and thus namespace in which the package lives without 
important code changes, I would rather stick to major version 2 and live with 
the incompatibility to version 2.0 which is being fixed up/completed by these 
patches.

Original comment by andreas....@gmail.com on 17 Jul 2014 at 12:46

GoogleCodeExporter commented 9 years ago
What about setting GFLAGS_NAMESPACE to "google;gflags" (requires issue #83 
branch) ? In this case the library symbols are all still in the google 
namespace and gflags is instead an alias namespace name that can be already 
used in user code. That way the 2.x versions can serve as interim step for 
moving from google to the gflags namespace. A 3.0 version may then drop 
backwards compatibility regarding the namespace all together in the future.

Original comment by andreas....@gmail.com on 17 Jul 2014 at 1:11

GoogleCodeExporter commented 9 years ago
That sounds like a reasonable solution. I'll do some testing to see how that 
works when upgrading from 2.0.

Original comment by floppymaster@gmail.com on 17 Jul 2014 at 1:20

GoogleCodeExporter commented 9 years ago
Ok, GFLAGS_NAMESPACE="google;gflags" works well for binary compatibility. 
However, that causes the headers to be installed under /usr/include/google 
instead of /usr/include/gflags.

Reversing the order (gflags;google) puts the headers in the right place, but 
breaks binary compatibility. For example:

vcdiff: symbol lookup error: vcdiff: undefined symbol: 
_ZN6google14FlagRegistererC1EPKcS2_S2_S2_PvS3_

Original comment by floppymaster@gmail.com on 17 Jul 2014 at 1:34

GoogleCodeExporter commented 9 years ago
Good. Then we also need to set GFLAGS_INCLUDE_DIR to "gflags" which should 
force the headers to be installed in /usr/include/gflags then.

Original comment by andreas....@gmail.com on 17 Jul 2014 at 1:35

GoogleCodeExporter commented 9 years ago
Ah, yes. That works nicely.

Original comment by floppymaster@gmail.com on 17 Jul 2014 at 1:45

GoogleCodeExporter commented 9 years ago
I just noticed that I had a previously cached version of GFLAGS_INCLUDE_DIR in 
my test build tree. It seems I removed the cache entry for this variable. It 
can be set on the command-line using "c[c]make -D 
GFLAGS_INCLUDE_DIR:STRING=gflags .". By default it is set to the first entry in 
GFLAGS_NAMESPACE, i.e., the namespace of the library symbols. Will then add 
GFLAGS_INCLUDE_DIR to the cache again with default value "gflags" and set 
GFLAGS_NAMESPACE by default to "google;gflags".

I just pushed these changes to the feature/#83-alternative-namespace branch.

Original comment by andreas....@gmail.com on 17 Jul 2014 at 1:46

GoogleCodeExporter commented 9 years ago

Original comment by andreas....@gmail.com on 9 Nov 2014 at 8:59