Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added SOVERSION for symlinked library generation from just a version number #3753

Closed
wants to merge 13 commits into from

Conversation

dmoody256
Copy link
Contributor

@dmoody256 dmoody256 commented Jul 24, 2020

This PR covers just the SOVERSION updates from #3661

The SOVERSION is a new environment variable which works very similar to the SONAME environment variable, except instead of providing the full library name as you do with SONAME, you just provide an number which will generally be the major version number in the symlinked library.

The SHLIBVERSION environment variable is also very similar to this, except that the SHLIBVERSION is generally the specific version number of the library, and used on the actual output library, instead of the symlinked library.

Please review https://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html for more information on the standard conventions for library versioning in linux.

Also note that macOS uses a similar convention for versioning libraries, so this PR also adds the variable to apple linker.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

soname = ShLibSonameGenerator(env, libnode)
else:
soname = '.'.join([libname, major, _suffix])
soname = '.'.join([libname, major, _suffix])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove using supplied SONAME?

Copy link
Contributor Author

@dmoody256 dmoody256 Jul 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not the correct order of the calling functions. ShLibSonameGenerator calls into LINKCALLBACK, which calls this function if SONAME is not supplied. Since ShLibSonameGenerator calls to the more specific naming functions, it should handle SONAME and SOVERSION in ShLibSonameGenerator, and then pass the naming info down.

<example_commands>
env.SharedLibrary('test', 'test.c', SHLIBVERSION='0.1.2', SOVERSION='2')
</example_commands>
The variable is used by linker tools which use the &t-link-link; tool such as &t-link-gnulink; or &t-link-applelink;.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand upon the SONAME generated when SOVERSION is specified. (what would it be in the example above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 13527e8

@@ -243,17 +235,14 @@ def _versioned_lib_symlinks(env, libnode, version, prefix, suffix, name_func, so

def _versioned_shlib_symlinks(env, libnode, version, prefix, suffix):
name_func = env['LINKCALLBACKS']['VersionedShLibName']
soname_func = env['LINKCALLBACKS']['VersionedShLibSoname']
soname_func = env['ShLibSonameGenerator']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShLibSonameGenerator ends up calling that linkcallback if SONAME is not supplied. Without this change, the ShLibSonameGenerator is skipped out.

@dmoody256 dmoody256 force-pushed the soversion_for_soname_libs branch from 4cc125e to 13527e8 Compare July 26, 2020 02:26
@dmoody256 dmoody256 force-pushed the soversion_for_soname_libs branch from 13527e8 to 027be11 Compare July 26, 2020 02:33
@dmoody256 dmoody256 force-pushed the soversion_for_soname_libs branch from 027be11 to 0fa4feb Compare July 27, 2020 14:30
@dmoody256
Copy link
Contributor Author

Ok tests are failing now that I removed that code I thought was left from the SONAME_GENERATOR. Maybe I took too much away, once I get a minute I'll go check it.

@dmoody256 dmoody256 force-pushed the soversion_for_soname_libs branch from 41ae370 to 69bbae8 Compare July 28, 2020 21:22
@bdbaddog
Copy link
Contributor

bdbaddog commented Aug 16, 2020

So if we allow setting SOVERSION (in place of setting SONAME) it should have the following impact:

  1. the soname set in the shared library should be libname.SOVERSION
  2. The chain of symlinks should be as below, where libname.so.SHLIBVERSION is the actual file.
   libname.so -> libname.so.SOVERSION
   libname.so.SOVERSION -> libname.so.SHLIBVERSION
  1. For apple, the flag link flags:
    gcc -o libfoo.SHLIBVERSION.dylib -dynamiclib -Wl,-current_version,SHLIBVERSION -Wl,-compatibility_version,SOVERSION.o foo.os
  2. For gnu the link flags should be
    gcc -o libfoo.so.SHLIBVERSION -shared -Wl,-soname=libfoo.so.SOVERSION foo.os

In the case we have SONAME and not SOVERSION, should we extract the SOVERSION from SONAME for use in the -compatibility_version?

Note we also have the following env vars

  • APPLELINK_NO_CURRENT_VERSION - don't specify a -current_version
  • APPLELINK_CURRENT_VERSION - use this for the -current_version and don't extract from SHLIBVERSION
  • APPLELINK_NO_COMPATIBILITY_VERSION - don't specify a -compatibility_version
  • APPLELINK_COMPATIBILITY_VERSION - use this for -compatibility_version and don't extract from SHLIBVERSION

The apple bits aren't really (yet) addressed in this PR.
(Note there's also some validation of the version numbers for apple. 0-65536[.0-255[.0-255]] are the only valid formats for those numbers and there's pre-existing code to check that)

Of note, some PR's for meson and links to cmakes implementation details

I'm guessing it's better for our user base if we match cmake/meson's implementations (rather than libtool, especially for apple).

@bdbaddog
Copy link
Contributor

Closing in favor of PR #3848 which is a rewrite of existing logic and implements this logic as well.

@bdbaddog bdbaddog closed this Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants