Re: [Scons-dev] [PATCH] scons soname on OpenBSD
Hi Stefan, On 09.09.2013 19:45, Stefan Sperling wrote: On Sat, Sep 07, 2013 at 04:24:41PM -0400, Gary Oberbrunner wrote: [...] The ideal way to contribute to SCons is to fork the mercurial repo at https://bitbucket.org/scons/scons, make your change, then submit a pull request. Patches sent to the mailing list can get lost. If possible I'd like to ask someone on this list to commit the patch for me. I don't have an account on bitbucket. I used the time it would take me to read through atlassian's terms of services to write a unit test instead :) thanks a lot for the patch and the test case. I'll file a bug in the Tigris tracker and then submit a pull request for your fix. Best regards, Dirk ___ Scons-dev mailing list Scons-dev@scons.org http://two.pairlist.net/mailman/listinfo/scons-dev
Re: [Scons-dev] [PATCH] scons soname on OpenBSD
On Sat, Sep 07, 2013 at 04:24:41PM -0400, Gary Oberbrunner wrote:
> > > If you can submit this as a mercurial patch, ideally with a testcase
> > (SCons
> > > uses TDD), we should be able to work it in.
> >
> > Pardon my ignorance: I sent the output of 'hg diff'.
> > Is a "mercurial patch" something else?
> >
>
> The ideal way to contribute to SCons is to fork the mercurial repo at
> https://bitbucket.org/scons/scons, make your change, then submit a pull
> request. Patches sent to the mailing list can get lost.
If possible I'd like to ask someone on this list to commit
the patch for me. I don't have an account on bitbucket.
I used the time it would take me to read through atlassian's
terms of services to write a unit test instead :)
> What kind of test case do you expect? Something that checks linker
> > invokation command lines and fails if a soname is used on OpenBSD?
>
>
> Yes -- TDD says that the best test case is one that fails before your
> change (on OpenBSD in this case) and succeeds afterward.
Thanks, does the below look ok? It fails without my patch and
succeeds with it.
Note that sys.platform in Python currently returns 'openbsd5', hence
the startswith() to keep things working when OpenBSD's version number
reaches 6.0 (which given OpenBSD's release cycle and numbering will
happen in about 3 years).
diff -r 05db74dca43c src/engine/SCons/Tool/__init__.py
--- a/src/engine/SCons/Tool/__init__.py Sun Mar 03 19:34:10 2013 -0500
+++ b/src/engine/SCons/Tool/__init__.py Mon Sep 09 19:38:58 2013 +0200
@@ -257,6 +257,10 @@
print "VersionShLibLinkNames: linkname = ",linkname
linknames.append(linkname)
elif platform == 'posix':
+if sys.platform.startswith('openbsd'):
+# OpenBSD uses x.y shared library versioning numbering convention
+# and doesn't use symlinks to backwards-compatible libraries
+return []
# For libfoo.so.x.y.z, linknames libfoo.so libfoo.so.x.y libfoo.so.x
suffix_re = re.escape(shlib_suffix + '.' + version)
# First linkname has no version number
@@ -302,13 +306,17 @@
if version:
# set the shared library link flags
if platform == 'posix':
-suffix_re = re.escape(shlib_suffix + '.' + version)
-(major, age, revision) = version.split(".")
-# soname will have only the major version number in it
-soname = re.sub(suffix_re, shlib_suffix, libname) + '.' + major
-shlink_flags += [ '-Wl,-Bsymbolic', '-Wl,-soname=%s' % soname ]
-if Verbose:
-print " soname ",soname,", shlink_flags ",shlink_flags
+shlink_flags += [ '-Wl,-Bsymbolic' ]
+if sys.platform.startswith('openbsd'):
+pass # OpenBSD doesn't usually use SONAME for libraries
+else:
+suffix_re = re.escape(shlib_suffix + '.' + version)
+(major, age, revision) = version.split(".")
+# soname will have only the major version number in it
+soname = re.sub(suffix_re, shlib_suffix, libname) + '.' + major
+shlink_flags += [ '-Wl,-soname=%s' % soname ]
+if Verbose:
+print " soname ",soname,", shlink_flags ",shlink_flags
elif platform == 'cygwin':
shlink_flags += [ '-Wl,-Bsymbolic',
'-Wl,--out-implib,${TARGET.base}.a' ]
diff -r 05db74dca43c test/Libs/SharedLibrary.py
--- a/test/Libs/SharedLibrary.pySun Mar 03 19:34:10 2013 -0500
+++ b/test/Libs/SharedLibrary.pyMon Sep 09 19:38:58 2013 +0200
@@ -60,6 +60,13 @@
Default(env.Library(target = 'foo', source = obj))
""")
+test.write('SConstructBaz', """
+env=Environment()
+env['SHLIBVERSION'] = '1.0.0'
+obj = env.SharedObject('baz', 'foo.c')
+Default(env.SharedLibrary(target = 'baz', source = obj))
+""")
+
test.write('foo.c', r"""
#include
@@ -280,6 +287,12 @@
test.run(program = test.workpath('progbar'),
stdout = "f4.c\nprogbar.c\n")
+if sys.platform.startswith('openbsd'):
+# Make sure we don't link libraries with -Wl,-soname on OpenBSD.
+test.run(arguments = '-f SConstructBaz')
+for line in test.stdout().split('\n'):
+test.fail_test(line.find('-Wl,-soname=libbaz.so') != -1)
+
test.pass_test()
# Local Variables:
___
Scons-dev mailing list
Scons-dev@scons.org
http://two.pairlist.net/mailman/listinfo/scons-dev
Re: [Scons-dev] [PATCH] scons soname on OpenBSD
Hi, Stefan. On Sat, Sep 7, 2013 at 5:57 AM, Stefan Sperling wrote: > On Fri, Sep 06, 2013 at 10:04:07PM -0400, Gary Oberbrunner wrote: > > I don't have any problem with this conceptually. The > > sys.platform.startswith() would be better as a function perhaps > > (is_openbsd() or maybe just is_bsd()). Is this also true for freebsd for > > instance? > > I don't know if this applies to FreeBSD, but I wouldn't count on it. > The various BSDs can be very different in such matters. > OK, sounds good. > I do know that FreeBSD's ports system wasn't as strict about shared > library versioning years ago when I used it, but that might have > changed since. If I were you, I would assume FreeBSD is fine with > whatever scons is doing, unless someone complains or sends a patch. > > > And a more flexible way of handling the multi-part version > > numbers would be welcome, perhaps as a separate patch. > > I agree, but I don't think I have the time to do that. I'm not > sure what the implications are for scons code base, and I don't > really know the code base. So if you're asking me to do that, > I'm afraid I'll have to decline. > > I'm just trying to provide a drive-by fix for a scons issue that I > have to deal with in order to provide an up-to-date port of Subversion > for OpenBSD (a problem I only have because serf, a dependency of > Subversion, has switched to using scons exclusively -- else, I wouldn't > be here). > Understood. You're not responsible for cleaning up our code. Of course, any time you want to help... it's always welcome. :-) :-) But no, that's more an idea for future enhancement. > > If you can submit this as a mercurial patch, ideally with a testcase > (SCons > > uses TDD), we should be able to work it in. > > Pardon my ignorance: I sent the output of 'hg diff'. > Is a "mercurial patch" something else? > The ideal way to contribute to SCons is to fork the mercurial repo at https://bitbucket.org/scons/scons, make your change, then submit a pull request. Patches sent to the mailing list can get lost. What kind of test case do you expect? Something that checks linker > invokation command lines and fails if a soname is used on OpenBSD? Yes -- TDD says that the best test case is one that fails before your change (on OpenBSD in this case) and succeeds afterward. For info on writing tests, start with http://www.scons.org/wiki/DeveloperGuide/TestingMethodology and http://www.scons.org/wiki/DevelopingTests. There's also a good intro in QMTest/test-framework.rst if you have a local copy of the source. If you can make a test case, I actually have access to a FreeBSD system so I could check whether it has your issue or not. Hope that's helpful; -- Gary ___ Scons-dev mailing list Scons-dev@scons.org http://two.pairlist.net/mailman/listinfo/scons-dev
Re: [Scons-dev] [PATCH] scons soname on OpenBSD
On Fri, Sep 06, 2013 at 10:04:07PM -0400, Gary Oberbrunner wrote: > I don't have any problem with this conceptually. The > sys.platform.startswith() would be better as a function perhaps > (is_openbsd() or maybe just is_bsd()). Is this also true for freebsd for > instance? I don't know if this applies to FreeBSD, but I wouldn't count on it. The various BSDs can be very different in such matters. I do know that FreeBSD's ports system wasn't as strict about shared library versioning years ago when I used it, but that might have changed since. If I were you, I would assume FreeBSD is fine with whatever scons is doing, unless someone complains or sends a patch. > And a more flexible way of handling the multi-part version > numbers would be welcome, perhaps as a separate patch. I agree, but I don't think I have the time to do that. I'm not sure what the implications are for scons code base, and I don't really know the code base. So if you're asking me to do that, I'm afraid I'll have to decline. I'm just trying to provide a drive-by fix for a scons issue that I have to deal with in order to provide an up-to-date port of Subversion for OpenBSD (a problem I only have because serf, a dependency of Subversion, has switched to using scons exclusively -- else, I wouldn't be here). > If you can submit this as a mercurial patch, ideally with a testcase (SCons > uses TDD), we should be able to work it in. Pardon my ignorance: I sent the output of 'hg diff'. Is a "mercurial patch" something else? What kind of test case do you expect? Something that checks linker invokation command lines and fails if a soname is used on OpenBSD? Can you point me at docs for writing test cases? ___ Scons-dev mailing list Scons-dev@scons.org http://two.pairlist.net/mailman/listinfo/scons-dev
Re: [Scons-dev] [PATCH] scons soname on OpenBSD
I don't have any problem with this conceptually. The
sys.platform.startswith() would be better as a function perhaps
(is_openbsd() or maybe just is_bsd()). Is this also true for freebsd for
instance? And a more flexible way of handling the multi-part version
numbers would be welcome, perhaps as a separate patch.
If you can submit this as a mercurial patch, ideally with a testcase (SCons
uses TDD), we should be able to work it in.
(As I look at it, it's clear that Tool/__init__.py is not the right place
for this logic anyway. But that's a separate issue. Better to capture the
correct behavior now and refactor it later when redoing Tools.)
On Thu, Sep 5, 2013 at 11:40 AM, Stefan Sperling wrote:
> Hi,
>
> as discussed on IRC today, and on the OpenBSD ports list at
> http://marc.info/?l=openbsd-ports&m=137831857808220&w=2,
> soname support in scons 2.3.0 is interfering a little bit with
> OpenBSD's way of handling shared libraries.
>
> The patch below (against the rel_2.3.0 branch) does the minimum
> necessary to prevent that. Is it acceptable?
>
> I was thinking that instead of making behaviour conditional on the host
> platform, a generic switch could be used to toggle generation of a soname,
> and the same for creation of symlinks. Though perhaps that would be
> overkill or conflict with scons goals, I don't know.
> Anyway, the below works for me.
>
> One additional issue is that OpenBSD uses an x.y shared library version
> numbering scheme, which scons does not support. This can be worked around
> by renaming shared libraries after the build.
> I'm not sure how much needs to be done to fix this. There is a comment in
> the VersionShLibLinkNames() function that some adjustments would need to
> be made. Are there plans to support such versioning schemes in a future
> release of scons?
>
> Thanks.
>
> Suggested log message:
>
> [[[
> OpenBSD-specific fixes for shared library handling.
>
> Don't put a SONAME into shared lbraries, and don't create symlinks
> between .so files. OpenBSD uses a different approach to shared library
> handling, and in particular setting a SONAME that doesn't match the .so
> file name can confuse automated checks performed by its ports framework.
> See http://marc.info/?l=openbsd-ports&m=137831857808220&w=2
> ]]]
>
> diff -r 05db74dca43c src/engine/SCons/Tool/__init__.py
> --- a/src/engine/SCons/Tool/__init__.py Sun Mar 03 19:34:10 2013 -0500
> +++ b/src/engine/SCons/Tool/__init__.py Thu Sep 05 16:26:07 2013 +0200
> @@ -257,6 +257,10 @@
> print "VersionShLibLinkNames: linkname = ",linkname
> linknames.append(linkname)
> elif platform == 'posix':
> +if sys.platform.startswith('openbsd'):
> +# OpenBSD uses x.y shared library versioning numbering
> convention
> +# and doesn't use symlinks to backwards-compatible libraries
> +return []
> # For libfoo.so.x.y.z, linknames libfoo.so libfoo.so.x.y
> libfoo.so.x
> suffix_re = re.escape(shlib_suffix + '.' + version)
> # First linkname has no version number
> @@ -302,13 +306,17 @@
> if version:
> # set the shared library link flags
> if platform == 'posix':
> -suffix_re = re.escape(shlib_suffix + '.' + version)
> -(major, age, revision) = version.split(".")
> -# soname will have only the major version number in it
> -soname = re.sub(suffix_re, shlib_suffix, libname) + '.' +
> major
> -shlink_flags += [ '-Wl,-Bsymbolic', '-Wl,-soname=%s' % soname
> ]
> -if Verbose:
> -print " soname ",soname,", shlink_flags ",shlink_flags
> +shlink_flags += [ '-Wl,-Bsymbolic' ]
> +if sys.platform.startswith('openbsd'):
> +pass # OpenBSD doesn't usually use SONAME for libraries
> +else:
> +suffix_re = re.escape(shlib_suffix + '.' + version)
> +(major, age, revision) = version.split(".")
> +# soname will have only the major version number in it
> +soname = re.sub(suffix_re, shlib_suffix, libname) + '.' +
> major
> +shlink_flags += [ '-Wl,-soname=%s' % soname ]
> +if Verbose:
> +print " soname ",soname,", shlink_flags ",shlink_flags
> elif platform == 'cygwin':
> shlink_flags += [ '-Wl,-Bsymbolic',
>'-Wl,--out-implib,${TARGET.base}.a' ]
> ___
> Scons-dev mailing list
> Scons-dev@scons.org
> http://two.pairlist.net/mailman/listinfo/scons-dev
>
--
Gary
___
Scons-dev mailing list
Scons-dev@scons.org
http://two.pairlist.net/mailman/listinfo/scons-dev
