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

Symbol Versioning #2809

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@kodebach
Copy link
Contributor

commented Jun 25, 2019

This introduces a setup for proper symbol versioning. I hope I listed all symbols correctly.

  • Stuff that isn't part of any header is not listed at all, even if it was exported until now. This only affects a few elektra* helper functions.
  • Stuff that is listed in a non-installed header like kdbprivate.h and actually used in another compilation unit (i.e. must be available during linking), is listed in libelektraprivate_1.0 so that it gets exported.
  • Everything else, i.e. the symbols that are actually public API is listed in libelektra_0.8. Symbols that were not part of the last release should be listed in libelektra_0.9. Right now that is only the case for the highlevel API for everything else I still need to check what was released and what wasn't.

Like stated in #1271 the basis of the whole system is the FreeBSD source.

Basics

  • Short descriptions should be in the release notes (added as entry in
    doc/news/_preparation_next_release.md which contains _(my name)_)
    Please always add something to the the release notes.
  • Longer descriptions should be in documentation or in design decisions.
  • Describe details of how you changed the code in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, should be in the commit messages.

Checklist

  • I added unit tests
  • I ran all tests locally and everything went fine
  • affected documentation is fixed
  • I added code comments, logging, and assertions (see Coding Guidelines)
  • meta data is updated (e.g. README.md of plugins and METADATA.ini)

Review

Reviewers will usually check the following:

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the build server is happy and also you
    say that everything is ready to be merged.
@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

Sidenote: The reason for this PR is so that elektraOpen can be changed to include kdbEnsure functionality.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2019

Amazing that you work on it!

Btw. you also should ABI tests to see if test cases compiled against the old Elektra still work after this PR.

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

Btw. you also should ABI tests to see if test cases compiled against the old Elektra still work after this PR.

You mean compile the libraries, git checkout the last release and then compile its test cases against the new libraries?

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

Also a separate question:

libtools has a separate version script already. Should that be integrated or is there any special reason that it is separate?

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2019

You mean compile the libraries, git checkout the last release

Yes.

and then compile its test cases against the new libraries?

No, not compiling against the new libraries. Compile both versions separately and then simply replace the files by coping over them (dynamic linking will do the rest).

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

libtools has a separate version script already. Should that be integrated or is there any special reason that it is separate?

this questions is still open

Btw. you also should ABI tests to see if test cases compiled against the old Elektra still work after this PR.

Automating this will be problematic. Ideally we would execute the test cases from the last release against the current source code to ensure backwards compatibility. However, that causes certain problems, e.g. #2821.

Some of these problems are not easily solved. For example:

  • If internal functions not part of the public API are changed or removed tests will fail (e.g. the elektraGenInternal* functions have been removed since the last release). Such test failures are not a problem, but impossible to detect automatically and ignoring a test case entirely might hide other incompatibilities (e.g. ignoring testkdb_highlevel because elektraGenInternal* was removed, hides all problems with the highlevel API).
  • Running the plugin tests is a good way to cover a lot of the API surface, but if a plugin changed some of its functionality since the last release, the test cases will fail. e.g. boolean had some changes since the last release that required changes in keySetString (namely the origvalue stuff).

We could of course just run a small subset of tests that should always work (e.g. the testabi_*) tests, but I think the best solution is to just do manual ABI compatibility testing.


EDIT: also I'm not sure why, but Cirrus CI didn't trigger for the last commit in this PR, I'm also don't know how to trigger it manually Nevermind, it works again...

@kodebach kodebach force-pushed the kodebach:symver branch from dd8b10f to 44b9ac2 Jul 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.