[PATCH] For compiling on Mac OS X 10.4

[Blair Zajac]

Yes, who would have guessed people are still using 10.4. Here's the patch:

https://trac.macports.org/attachment/ticket/40155/patch-sconstruct.diff

Here's the bug report:

https://trac.macports.org/ticket/40155#comment:21

Let me know if this is a proper fix.

Blair

[Lieven Govaerts]

Hi,

Patch inlined for easier review:

> --- SConstruct~ 2013-08-15 11:13:20.000000000 +0200
> +++ SConstruct 2013-08-27 20:35:28.000000000 +0200
> @@ -220,8 +220,7 @@
> env.Append(LINKFLAGS='-Wl,-install_name,%s/%s.dylib' % (thisdir, LIBNAME,))
> # 'man ld' says positive non-zero for the first number, so we add one.
> # Mac's interpretation of compatibility is the same as our MINOR version.
> - env.Append(LINKFLAGS='-Wl,-compatibility_version,%d' % (MINOR+1,))
> - env.Append(LINKFLAGS='-Wl,-current_version,%d.%d' % (MINOR+1, PATCH,))
> + env['SHLIBVERSION']='${%d}.0.0' % (MINOR+1,)
>
> if sys.platform != 'win32':
> ### gcc only. figure out appropriate test / better way to check these

Two disadvantages of this patch:
1. we loose the patch level in '-current_version'.
2. the version number is different on Mac OS X compared to other platforms.

I don't think the first is much of a problem, except for making
trouble shooting shared lib problems a bit more difficult. I do think
we need to align version numbers on all platforms, even if that means
that serf 2.0.0 will be labelled as 2.1.0 in the .so. We can do this
in 1.4.0.

The fact remains that this is a workaround for one of the issues in
the shared library support in scons 2.3.0. I've identified so far:
1. the shared library version has to consist of 3 components. As serf
adds the major version in the library name, 2 components would have
been sufficient.
2. creating of the .so/.dylib symlinks fails when these were already
installed, breaking of the installation
3. on Mac OS X, no way to differentiate between compatibility_version
and current_version
4. on Mac OS X, no support for setting install_name
5. mandatory upgrade to scons 2.3.0, which is not available yet in
most Linux distributions (but is very easy to install)

We've worked around issues 1, 3 (but the workound will be reverted by
this patch) and 4. Issue 2 remains open and there is no entry point in
scons to solve this easily, so will require a hack.

I'll apply this patch with some added comments.

> Blair
>

Thanks!

Lieven

[Blair Zajac]

Switching gears and all the examples below are with an unpatched serf 1.3.1.

One other problem is that svn is getting linked to the dylib name with
the complete version number:

$ otool -L /opt/local/bin/svn | grep libserf
/opt/local/lib/libserf-1.3.0.0.dylib (compatibility version 4.0.0,
current version 4.1.0)

When serf 1.4.0 is released, it will create libserf-1.4.0.0.dylib and
break Subversion. Also, the shared library thinks its name is
libserf-1.3.0.0.dylib when it should think it is libserf-1.dylib.

$ otool -L /opt/local/lib/libserf-1.dylib
/opt/local/lib/libserf-1.dylib:
/opt/local/lib/libserf-1.3.0.0.dylib (compatibility version 4.0.0,
current version 4.1.0)

<unhappy>
Why did you guys switch to Scons? automake was working fine and didn't
require as much work and time and effort.
</unhappy>

Blair

[Blair Zajac]

On 08/31/2013 11:52 AM, Blair Zajac wrote:
> On 08/31/2013 01:35 AM, Blair Zajac wrote:
>> Yes, who would have guessed people are still using 10.4. Here's the
>> patch:
>>
>> https://trac.macports.org/attachment/ticket/40155/patch-sconstruct.diff
>>
>> Here's the bug report:
>>
>> https://trac.macports.org/ticket/40155#comment:21
>>
>> Let me know if this is a proper fix.
>>
>> Blair
>
> Switching gears and all the examples below are with an unpatched serf
> 1.3.1.
>
> One other problem is that svn is getting linked to the dylib name with
> the complete version number:
>
> $ otool -L /opt/local/bin/svn | grep libserf
> /opt/local/lib/libserf-1.3.0.0.dylib (compatibility version 4.0.0,
> current version 4.1.0)
>
> When serf 1.4.0 is released, it will create libserf-1.4.0.0.dylib and
> break Subversion. Also, the shared library thinks its name is
> libserf-1.3.0.0.dylib when it should think it is libserf-1.dylib.
>
> $ otool -L /opt/local/lib/libserf-1.dylib
> /opt/local/lib/libserf-1.dylib:
> /opt/local/lib/libserf-1.3.0.0.dylib (compatibility version 4.0.0,
> current version 4.1.0)

Here it is setting the install_name:

scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
Install file: "libserf-1.a" as
"/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_ports_www_serf1/serf1/work/destroot/opt/local/lib/libserf-1.a"
Install file: "libserf-1.3.0.0.dylib" as
"/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_ports_www_serf1/serf1/work/destroot/opt/local/lib/libserf-1.3.0.0.dylib"
install_name_tool -id /opt/local/lib/libserf-1.3.0.0.dylib
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_ports_www_serf1/serf1/work/destroot/opt/local/lib/libserf-1.3.0.0.dylib

Blair

[Blair Zajac]

Which is being done here:

if sys.platform == 'darwin':
# If --install-sandbox=<path> is specified, install_shared_path will
point
# to a path in the sandbox. The shared library install name (id)
should be the
# final targat path.
install_shared_path = install_shared[0].abspath
target_install_shared_path = os.path.join(libdir, lib_shared[0].name)
env.AddPostAction(install_shared, ('install_name_tool -id %s %s'
% (target_install_shared_path,
install_shared_path)))


So target_install_shared_path is wrong. What variable or code should be
used here?

Blair

[Blair Zajac]

Here's my final patch, committed to MacPorts.

Even though the shared library is named libserf-1.dylib, I think that
SHLIBVERSION should include the major version number for all platforms.
This avoids needing to add 1 to MINOR on OS X because the first number
in the (X, Y, Z) tuple must be non-zero and positive. But it also means
that the embedded shared library version number honors the APR
versioning guidlines [1]. If one drops the MAJOR version from the
version tuple, then technically each 1.X to 1.Y upgrade breaks the ABI
according to the version number.

Blair

[1] http://apr.apache.org/versioning.html


--- ../serf-1.3.1.orig/SConstruct 2013-08-15 02:13:20.000000000 -0700
+++ ./SConstruct 2013-08-31 14:10:43.000000000 -0700
@@ -204,9 +204,9 @@
libdir = '$LIBDIR'
incdir = '$PREFIX/include/serf-$MAJOR'

-env['SHLIBVERSION']='${MINOR}.0.0'
+env['SHLIBVERSION'] = '%d.%d.%d' % (MAJOR, MINOR, PATCH)

-LIBNAME = 'libserf-${MAJOR}'
+LIBNAME = 'libserf-%d' % (MAJOR,)
if sys.platform != 'win32':
LIBNAMESTATIC = LIBNAME
else:
@@ -218,10 +218,6 @@
if sys.platform == 'darwin':
# linkflags.append('-Wl,-install_name,@executable_path/%s.dylib' %
(LIBNAME,))

env.Append(LINKFLAGS='-Wl,-install_name,%s/%s.dylib' % (thisdir,
LIBNAME,))

- # 'man ld' says positive non-zero for the first number, so we add one.
- # Mac's interpretation of compatibility is the same as our MINOR version.

- env.Append(LINKFLAGS='-Wl,-compatibility_version,%d' % (MINOR+1,))
- env.Append(LINKFLAGS='-Wl,-current_version,%d.%d' % (MINOR+1, PATCH,))

if sys.platform != 'win32':
### gcc only. figure out appropriate test / better way to check these

@@ -401,7 +397,7 @@

# to a path in the sandbox. The shared library install name (id)
should be the
# final targat path.
install_shared_path = install_shared[0].abspath

- target_install_shared_path = os.path.join(libdir, lib_shared[0].name)
+ target_install_shared_path = os.path.join(libdir, '%s.dylib' % LIBNAME)

[Blair Zajac]

On Saturday, August 31, 2013 3:31:09 AM UTC-7, lgo wrote:

Hi,


On Sat, Aug 31, 2013 at 10:35 AM, Blair Zajac <bl...@orcaware.com> wrote:
> Yes, who would have guessed people are still using 10.4.  Here's the patch:
>
> https://trac.macports.org/attachment/ticket/40155/patch-sconstruct.diff
>
> Here's the bug report:
>
> https://trac.macports.org/ticket/40155#comment:21
>
> Let me know if this is a proper fix.

Patch inlined for easier review:

I didn't see your reply till right now, for some reason, Google didn't email me the reply.

I ended up going with the patch at the end of this thread, also committed here:

https://svn.macports.org/repository/macports/trunk/dports/www/serf1/files/patch-SConstruct.diff

 

> --- SConstruct~ 2013-08-15 11:13:20.000000000 +0200
> +++ SConstruct  2013-08-27 20:35:28.000000000 +0200
> @@ -220,8 +220,7 @@
>    env.Append(LINKFLAGS='-Wl,-install_name,%s/%s.dylib' % (thisdir, LIBNAME,))
>   # 'man ld' says positive non-zero for the first number, so we add one.
>   # Mac's interpretation of compatibility is the same as our MINOR version.
> -  env.Append(LINKFLAGS='-Wl,-compatibility_version,%d' % (MINOR+1,))
> -  env.Append(LINKFLAGS='-Wl,-current_version,%d.%d' % (MINOR+1, PATCH,))
> +  env['SHLIBVERSION']='${%d}.0.0' % (MINOR+1,)
>
> if sys.platform != 'win32':
>   ### gcc only. figure out appropriate test / better way to check these

Two disadvantages of this patch:
1. we loose the patch level in '-current_version'.
2. the version number is different on Mac OS X compared to other platforms.

I don't think the first is much of a problem, except for making
trouble shooting shared lib problems a bit more difficult. I do think
we need to align version numbers on all platforms, even if that means
that serf 2.0.0 will be labelled as 2.1.0 in the .so. We can do this
in 1.4.0.

The fact remains that this is a workaround for one of the issues in
the shared library support in scons 2.3.0. I've identified so far:
1. the shared library version has to consist of 3 components. As serf
adds the major version in the library name, 2 components would have
been sufficient.

I think we should duplicate the major version number, since the so version number as (MINOR, PATCH) breaks the APR versioning guidelines.

I don't mind duplicating the MAJOR number, as in libserf-$(MAJOR).$(MAJOR).$(MINOR).$(PATCH).

2. creating of the .so/.dylib symlinks fails when these were already
installed, breaking of the installation
3. on Mac OS X, no way to differentiate between compatibility_version
and current_version
4. on Mac OS X, no support for setting install_name

The SConstruct does this though?

 

5. mandatory upgrade to scons 2.3.0, which is not available yet in
most Linux distributions (but is very easy to install)

We've worked around issues 1, 3 (but the workound will be reverted by
this patch) and 4. Issue 2 remains open and there is no entry point in
scons to solve this easily, so will require a hack.

This is an issue when not installing into a sandbox?

[Lieven Govaerts]

Hi,

On Sun, Sep 1, 2013 at 1:48 AM, Blair Zajac <bl...@orcaware.com> wrote:
>
> On Saturday, August 31, 2013 3:31:09 AM UTC-7, lgo wrote:
>>
>> Hi,
>>
>>
>> On Sat, Aug 31, 2013 at 10:35 AM, Blair Zajac <bl...@orcaware.com> wrote:
>> > Yes, who would have guessed people are still using 10.4. Here's the
>> > patch:
>> >
>> > https://trac.macports.org/attachment/ticket/40155/patch-sconstruct.diff
>> >
>> > Here's the bug report:
>> >
>> > https://trac.macports.org/ticket/40155#comment:21
>> >
>> > Let me know if this is a proper fix.
>>
>> Patch inlined for easier review:
>
>
> I didn't see your reply till right now, for some reason, Google didn't email
> me the reply.
>
> I ended up going with the patch at the end of this thread, also committed
> here:
>
> https://svn.macports.org/repository/macports/trunk/dports/www/serf1/files/patch-SConstruct.diff
>

Alright. I've committed the "major version only in the install name"
part in r2161.
I've been looking for a solution that allows us to customise
current_version and compatibility_version

[..]

>> The fact remains that this is a workaround for one of the issues in
>> the shared library support in scons 2.3.0. I've identified so far:
>> 1. the shared library version has to consist of 3 components. As serf
>> adds the major version in the library name, 2 components would have
>> been sufficient.
>
>
> I think we should duplicate the major version number, since the so version
> number as (MINOR, PATCH) breaks the APR versioning guidelines.
>
> I don't mind duplicating the MAJOR number, as in
> libserf-$(MAJOR).$(MAJOR).$(MINOR).$(PATCH).

Ok for the MAJOR number, as we want that in the compatibility_version
option. But is there a reason why you include the PATCH level?
Using your patch with the 1.3.x branch has as consequence that the
compatibility_version is now set to 'serf 1.3.2'. So an application
built with serf 1.3.2 will not work with serf 1.3.0. I haven't tested
that, it's my assumption based on the dyld man page.

Looks like the general problem here is that scons doesn't allow us to
define on which version number level we guarantee compatibility, aka
doesn't allow us to set the 'compatibility_version' option separately
from the library_name/current_version option. If that'd be possible
I'd agree with your use of full version number.

>> 2. creating of the .so/.dylib symlinks fails when these were already
>> installed, breaking of the installation
>> 3. on Mac OS X, no way to differentiate between compatibility_version
>> and current_version
>> 4. on Mac OS X, no support for setting install_name
>
>
> The SConstruct does this though?

Yes, but I consider that a workaround for a missing feature in scons.

>>
>> 5. mandatory upgrade to scons 2.3.0, which is not available yet in
>> most Linux distributions (but is very easy to install)
>>
>> We've worked around issues 1, 3 (but the workound will be reverted by
>> this patch) and 4. Issue 2 remains open and there is no entry point in
>> scons to solve this easily, so will require a hack.
>
>
> This is an issue when not installing into a sandbox?
>

Scons can't overwrite a symlink if that already exists, so if you're
starting from a clean sandbox you won't see the issue.
The issue has been solved in scons dev, but not yet released:
https://bitbucket.org/scons/scons/commits/e12adc6d670af107f1cbc08bdf4c55f0b592f37e

Lieven

[Lieven Govaerts]

Forgot to add: so for now I think it's best we use MAJOR and MINOR
number in libname, current_version and compatibility_version options.
So:
env['SHLIBVERSION'] = '%d.%d.%d' % (MAJOR, MINOR, 0)

Assuming this doesn't have any negative consequences on Linux or Windows.
Lieven

[Blair Zajac]

On 9/6/13 11:21 AM, Lieven Govaerts wrote:
> Hi,
>
>
> On Sun, Sep 1, 2013 at 1:48 AM, Blair Zajac<bl...@orcaware.com> wrote:
>> I didn't see your reply till right now, for some reason, Google didn't email
>> me the reply.
>>
>> I ended up going with the patch at the end of this thread, also committed
>> here:
>>
>> https://svn.macports.org/repository/macports/trunk/dports/www/serf1/files/patch-SConstruct.diff
>>
>
> Alright. I've committed the "major version only in the install name"
> part in r2161.
> I've been looking for a solution that allows us to customise
> current_version and compatibility_version

Great, thanks.

>>> The fact remains that this is a workaround for one of the issues in
>>> the shared library support in scons 2.3.0. I've identified so far:
>>> 1. the shared library version has to consist of 3 components. As serf
>>> adds the major version in the library name, 2 components would have
>>> been sufficient.
>>
>> I think we should duplicate the major version number, since the so version
>> number as (MINOR, PATCH) breaks the APR versioning guidelines.
>>
>> I don't mind duplicating the MAJOR number, as in
>> libserf-$(MAJOR).$(MAJOR).$(MINOR).$(PATCH).
>
> Ok for the MAJOR number, as we want that in the compatibility_version
> option. But is there a reason why you include the PATCH level?
> Using your patch with the 1.3.x branch has as consequence that the
> compatibility_version is now set to 'serf 1.3.2'. So an application
> built with serf 1.3.2 will not work with serf 1.3.0. I haven't tested
> that, it's my assumption based on the dyld man page.

Good point, yes, we should not use the PATCH level. When serf goes to
1.4.0 I'll drop the PATCH from the MacPorts' version number, too late to
do that now.

Blair

[Blair Zajac]

I don't think so. It'll change it for people that have deployed serf
already, but it's better to get this change out sooner rather than
later. Distributions that have deployed it already will probably patch
this anyway.

Or maybe we take the highest number we've sent out as major, would that
be 3 == MINOR, and use MAJOR = MAJOR + 2?

Blair

[Bert Huijben]

In the current Scons this number is ignored on Windows, so you don't have to check for compatibility there yet.

Bert

---

From: Blair Zajac
Sent: ‎7-‎9-‎2013 21:07
To: serf...@googlegroups.com
Subject: Re: [serf-dev] [PATCH] For compiling on Mac OS X 10.4

--
You received this message because you are subscribed to the Google Groups "Serf Development List" group.
To unsubscribe from this group and stop receiving emails from it, send an email to serf-dev+u...@googlegroups.com.
To post to this group, send email to serf...@googlegroups.com.
Visit this group at http://groups.google.com/group/serf-dev.
For more options, visit https://groups.google.com/groups/opt_out.

[Lieven Govaerts]

On Sat, Sep 7, 2013 at 9:07 PM, Blair Zajac <bl...@orcaware.com> wrote:
> On 9/6/13 11:58 AM, Lieven Govaerts wrote:
>>
>> On Fri, Sep 6, 2013 at 8:21 PM, Lieven Govaerts
>> <lieven....@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On Sun, Sep 1, 2013 at 1:48 AM, Blair Zajac<bl...@orcaware.com> wrote:
>>>>

..

>>>>
>>>> I think we should duplicate the major version number, since the so
>>>> version
>>>> number as (MINOR, PATCH) breaks the APR versioning guidelines.
>>>>
>>>> I don't mind duplicating the MAJOR number, as in
>>>> libserf-$(MAJOR).$(MAJOR).$(MINOR).$(PATCH).
>>>
>>>
>>> Ok for the MAJOR number, as we want that in the compatibility_version
>>> option. But is there a reason why you include the PATCH level?
>>> Using your patch with the 1.3.x branch has as consequence that the
>>> compatibility_version is now set to 'serf 1.3.2'. So an application
>>> built with serf 1.3.2 will not work with serf 1.3.0. I haven't tested
>>> that, it's my assumption based on the dyld man page.
>>>
>>> Looks like the general problem here is that scons doesn't allow us to
>>> define on which version number level we guarantee compatibility, aka
>>> doesn't allow us to set the 'compatibility_version' option separately
>>> from the library_name/current_version option. If that'd be possible
>>> I'd agree with your use of full version number.
>>
>>
>> Forgot to add: so for now I think it's best we use MAJOR and MINOR
>> number in libname, current_version and compatibility_version options.
>> So:
>> env['SHLIBVERSION'] = '%d.%d.%d' % (MAJOR, MINOR, 0)

r2163 implements SHLIBVERSION like this, and removes the lines that
add the current_version and compatibility_version LINKFLAGS.

>> Assuming this doesn't have any negative consequences on Linux or Windows.
>
>
> I don't think so. It'll change it for people that have deployed serf
> already, but it's better to get this change out sooner rather than later.
> Distributions that have deployed it already will probably patch this anyway.
>
> Or maybe we take the highest number we've sent out as major, would that be 3
> == MINOR, and use MAJOR = MAJOR + 2?

These numbers are also used in the library name. There is no real
requirement that the version in the library name matches the actual
version, but I like that better.
Linux and Mac users will have to clean previous versions first though
when installing the new serf version with these fixes. I'll add that
in the release notes.

Lieven

>
> Blair
>