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

ABI breakage in libtools #1668

Closed
markus2330 opened this Issue Oct 29, 2017 · 15 comments

Comments

Projects
None yet
3 participants
@markus2330
Contributor

markus2330 commented Oct 29, 2017

Describe what you wanted to do

Installing libelektra-test_0.8.19-1_amd64.deb and then run all tests with Elektra 0.8.20.

Describe what you expected

Some failing tests are expected:

# heavily changed internal ABIs
test_opmphm_vheap
test_opmphm_vstack
# breaks on every internal KDB change
test_splitget
test_splitset
# changes in fcrypt?
testmod_fcrypt
# changes in INI parser
testmod_ini   
# contract from dump changed:
testtool_samemountpoint
testtool_backend

Fcrypt:

FCRYPT       TESTS
==================

/home/markus/Projekte/Elektra/libelektra/src/plugins/fcrypt/testmod_fcrypt.c:162: error in test_file_operations: kdb set failed
/home/markus/Projekte/Elektra/libelektra/src/plugins/fcrypt/testmod_fcrypt.c:163: error in test_file_operations: file content did not change during encryption
/home/markus/Projekte/Elektra/libelektra/src/plugins/fcrypt/testmod_fcrypt.c:166: error in test_file_operations: kdb get (pregetstorage) failed
/home/markus/Projekte/Elektra/libelektra/src/plugins/fcrypt/testmod_fcrypt.c:170: error in test_file_operations: kdb get (postgetstorage) failed
/home/markus/Projekte/Elektra/libelektra/src/plugins/fcrypt/testmod_fcrypt.c:171: error in test_file_operations: postgetstorage did not encrypt the file again

fcrypt RESULTS: 27 test(s) done. 5 error(s).

INI:

INI         TESTS
==================

/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:44: error in test_plainIniRead: section value was not empty
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:44: error in test_plainIniRead: section value was not empty
Compare <emptykey
>, with <#@META ini/empty = 
>
in file /usr/share/libelektra-test/test-data/ini/emptyval, line 6.
/home/markus/Projekte/Elektra/libelektra/tests/cframework/tests.c:174: error in compare_line_files: comparing lines failed
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:106: error in test_plainIniEmptyWrite: files do not match as expected
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:298: error in test_sectionRead: empty section key is not a binary key
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:299: error in test_sectionRead: section key contains non null data
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:303: error in test_sectionRead: section1 key is not a binary key
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:304: error in test_sectionRead: section1 contains non null data
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:316: error in test_sectionRead: section2 key is not a binary key
/home/markus/Projekte/Elektra/libelektra/src/plugins/ini/testmod_ini.c:317: error in test_sectionRead: section2 contains non null data

test_ini RESULTS: 194 test(s) done. 10 error(s).

Describe what actually happened

Some crashes indicate an ABI break of public ABIs, in particular libelektra-tools:

testtool_backend
testtool_backendbuilder
testtool_backendparser
testtool_keyhelper
testtool_mergingkdb
testtool_pluginspec
testtool_specreader
testtool_umount

with the errors:

--- running testtool_backendbuilder ---


Running main() from gtest_main.cc
[==========] Running 30 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 26 tests from BackendBuilder
[ RUN      ] BackendBuilder.withDatabase
/usr/lib/elektra/tool_exec/testtool_backendbuilder: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_backendbuilder: undefined symbol: _ZN3kdb5tools10PluginSpecC1ESsNS_6KeySetE
error: testtool_backendbuilder
--- running testtool_backendparser ---


Running main() from gtest_main.cc
[==========] Running 10 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 10 tests from MountBackendBuilder
[ RUN      ] MountBackendBuilder.parsePluginArguments
/usr/lib/elektra/tool_exec/testtool_backendparser: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_backendparser: undefined symbol: _ZN3kdb5tools20parsePluginArgumentsERKSsS2_
error: testtool_backendparser

--- running testtool_keyhelper ---


Running main() from gtest_main.cc
[==========] Running 10 tests from 4 test cases.
[----------] Global test environment set-up.
[----------] 4 tests from RebasePath
[ RUN      ] RebasePath.RebasesCorrectlyWithValidArguments
/usr/lib/elektra/tool_exec/testtool_keyhelper: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_keyhelper: undefined symbol: _ZN3kdb5tools6helper10rebasePathERKNS_3KeyES4_S4_
error: testtool_keyhelper
--- running testtool_mergingkdb ---


Running main() from gtest_main.cc
[==========] Running 3 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 3 tests from MergingKDBTest
[ RUN      ] MergingKDBTest.HandlesUnconflictingKeySets
/usr/lib/elektra/tool_exec/testtool_mergingkdb: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_mergingkdb: undefined symbol: _ZN3kdb5tools10PluginSpecC1ESsNS_6KeySetE
error: testtool_mergingkdb
--- running testtool_specreader ---


Running main() from gtest_main.cc
[==========] Running 12 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 12 tests from SpecReader
[ RUN      ] SpecReader.withDatabase
/usr/lib/elektra/tool_exec/testtool_specreader: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_specreader: undefined symbol: _ZN3kdb5tools10PluginSpecC1ESsNS_6KeySetE
error: testtool_specreader
--- running testtool_umount ---


Running main() from gtest_main.cc
[==========] Running 20 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 20 tests from Umount
[ RUN      ] Umount.SimpleRoot
/usr/lib/elektra/tool_exec/testtool_umount: symbol lookup error: /usr/lib/elektra/tool_exec/testtool_umount: undefined symbol: _ZN3kdb5tools8Backends6umountERKSsRNS_6KeySetE
error: testtool_umount

System Information

  • Elektra Version: master (a351691) and 0.8.19 tests

Further Log Files and Output

@markus2330 markus2330 added this to the 0.8.20 milestone Oct 29, 2017

@sanssecours sanssecours self-assigned this Oct 30, 2017

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Oct 31, 2017

Contributor

Could you please tell me where I can find libelektra-test_0.8.19-1_amd64.deb? The debian repo already contains the lastest release (0.8.20).

Contributor

sanssecours commented Oct 31, 2017

Could you please tell me where I can find libelektra-test_0.8.19-1_amd64.deb? The debian repo already contains the lastest release (0.8.20).

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

I am afraid it is not available in the Internet anymore, I have sent you an email so that you do not have to rebuild it.

But I am not 100% if the build I have is correct (from the 0.8.19 tag and not from something shortly before/after). So a rebuild might be necessary anyway.

Contributor

markus2330 commented Oct 31, 2017

I am afraid it is not available in the Internet anymore, I have sent you an email so that you do not have to rebuild it.

But I am not 100% if the build I have is correct (from the 0.8.19 tag and not from something shortly before/after). So a rebuild might be necessary anyway.

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Oct 31, 2017

Contributor

Since installing the Debian image did not work (missing dependencies), I am going to test under macOS. Below you can find the current results:

Contributor

sanssecours commented Oct 31, 2017

Since installing the Debian image did not work (missing dependencies), I am going to test under macOS. Below you can find the current results:

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

Can you please explain the results? 2c6c701 does not seem like it contains C++ ABI breaks. It might break something in the kdb ls command-line interface.

Contributor

markus2330 commented Oct 31, 2017

Can you please explain the results? 2c6c701 does not seem like it contains C++ ABI breaks. It might break something in the kdb ls command-line interface.

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Oct 31, 2017

Contributor

Can you please explain the results?

I just installed Elektra 0.8.19. Now I am in the process of checking the first commit that broke one of the testtool tests. I will append the list in my comment above accordingly. After I find the responsible commit I will add another comment about the commit/PR that broke the tests. Basically I execute a non-intelligent very bad version of something that looks a bit like binary search 😊.

Contributor

sanssecours commented Oct 31, 2017

Can you please explain the results?

I just installed Elektra 0.8.19. Now I am in the process of checking the first commit that broke one of the testtool tests. I will append the list in my comment above accordingly. After I find the responsible commit I will add another comment about the commit/PR that broke the tests. Basically I execute a non-intelligent very bad version of something that looks a bit like binary search 😊.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

You can safely assume that only changes in src/libs/tools/include are relevant (for C++ libtools breakage).

Contributor

markus2330 commented Oct 31, 2017

You can safely assume that only changes in src/libs/tools/include are relevant (for C++ libtools breakage).

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

@e1528532 Maybe you can also take a look? Would be good if you can think about how to integrate the type checks into libtools, so that the type checks are always done.

Contributor

markus2330 commented Oct 31, 2017

@e1528532 Maybe you can also take a look? Would be good if you can think about how to integrate the type checks into libtools, so that the type checks are always done.

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Oct 31, 2017

Contributor

Turns out I am unable to reproduce this error. I installed Elektra 0.8.19 but the current tests still work on my machine.

Contributor

sanssecours commented Oct 31, 2017

Turns out I am unable to reproduce this error. I installed Elektra 0.8.19 but the current tests still work on my machine.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

Its the other way round: you need to install the tests from 0.8.19, and Elektra 0.8.20.

Contributor

markus2330 commented Oct 31, 2017

Its the other way round: you need to install the tests from 0.8.19, and Elektra 0.8.20.

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Oct 31, 2017

Contributor

How can I only install the tests? Is there a target to do that? So If I understand you right, the procedure should be:

  1. Install Elektra 0.8.20
  2. Overwrite the tests with the ones from 0.8.19
  3. Run kdb test

?

Contributor

sanssecours commented Oct 31, 2017

How can I only install the tests? Is there a target to do that? So If I understand you right, the procedure should be:

  1. Install Elektra 0.8.20
  2. Overwrite the tests with the ones from 0.8.19
  3. Run kdb test

?

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

Yes, you simply install the libelektra-test_0.8.19-1_amd64.deb (might need dpkg -i --force-all), then you have the tests of libelektra 0.8.19.

Contributor

markus2330 commented Oct 31, 2017

Yes, you simply install the libelektra-test_0.8.19-1_amd64.deb (might need dpkg -i --force-all), then you have the tests of libelektra 0.8.19.

@e1528532

This comment has been minimized.

Show comment
Hide comment
@e1528532

e1528532 Oct 31, 2017

Contributor

@markus2330 I'm not sure if i can be a big help regarding the abi changes, but your suggestion to think about the type system integration was very useful. I've opened a separate issue for discussions about that, as it doesn't really belong to this issue and got quite large.

Contributor

e1528532 commented Oct 31, 2017

@markus2330 I'm not sure if i can be a big help regarding the abi changes, but your suggestion to think about the type system integration was very useful. I've opened a separate issue for discussions about that, as it doesn't really belong to this issue and got quite large.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

Okay, the problem is that even constness changes the ABI ;(

Seems I have to revert the rest of the changes in API. We will reintroduce them later.

7a5e069

Contributor

markus2330 commented Oct 31, 2017

Okay, the problem is that even constness changes the ABI ;(

Seems I have to revert the rest of the changes in API. We will reintroduce them later.

7a5e069

markus2330 added a commit that referenced this issue Oct 31, 2017

Avoid ABI break
Revert "cpp: add const for parentKey"
This reverts commit 7a5e069.
see #1668
@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Oct 31, 2017

Contributor

Looks much better now: Following test cases failed:

  • test_opmphm_vheap
  • test_opmphm_vstack
  • test_splitget
  • test_splitset
  • testmod_fcrypt
  • testmod_ini
  • testtool_backend
  • check_kdb_internal_check
Contributor

markus2330 commented Oct 31, 2017

Looks much better now: Following test cases failed:

  • test_opmphm_vheap
  • test_opmphm_vstack
  • test_splitget
  • test_splitset
  • testmod_fcrypt
  • testmod_ini
  • testtool_backend
  • check_kdb_internal_check

@markus2330 markus2330 removed this from the 0.8.20 milestone Oct 31, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Jun 3, 2018

Contributor

There are many API/ABI breaks waiting as TODOs in libtools. We should do them all at once when we also add a new feature (e.g. import/export) in libtools.

Contributor

markus2330 commented Jun 3, 2018

There are many API/ABI breaks waiting as TODOs in libtools. We should do them all at once when we also add a new feature (e.g. import/export) in libtools.

@markus2330 markus2330 closed this Jun 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment