Libelektra: dini: Plugin Breaks Test Suite

[sanssecours]

Description

Unfortunately the latest merge commit to master breaks

• [X] the Jenkins build job elektra-ini-mergerequest and
• [x] the Travis Linux/ASAN build job
• [x] the Jenkins Linux/ASAN

.

Proposal

It would be great if we require that everyone opens a pull request before she or he merges non-trivial changes. This way we can check if new code breaks anything. Merging non-working code to master might break the workflow of other developers, who regularly rebase their work on the lastest version of the code base.

[markus2330]

Thank you for reporting the issue!

It would be great if we require that everyone opens a pull request before she or he merges non-trivial changes. This way we can check if new code breaks anything. Merging non-working code to master might break the workflow of other developers, who regularly rebase their work on the lastest version of the code base.

Sorry, my mistake. Unfortunately, we do not have a workflow to build a Debian package with a new plugin without merging to master first. Thus I merged it without creating a PR. I opened a separate issue for the workflow problem in #1714. Nevertheless, you are right: not breaking the master is more important than being able to build Debian packages sooner. I thought that a new experimental plugin in master is acceptable (even if it does not work) but it seems like my assumption was wrong.

[breaks] the Jenkins build job elektra-ini-mergerequest and

This build job should be changed to use dini instead. But more work is needed until this works.

[breaks] the Travis Linux/ASAN build job

I saw this already but I have not found the problem yet. (And I get different ASAN errors locally, not caused by this PR, so I cannot reproduce.) Might be fixed with 518440b27d43c9c0abb97c0fe7fe601b758a4c29.

[markus2330]

Seems like the change introduced a new error (warning):

https://travis-ci.org/ElektraInitiative/libelektra/jobs/310956065

Hopefully fixed in 59d73b015ce67d451f6fa94d3a3509263a72f527

[markus2330]

Proposal accepted for future (thus proposal removed) but ASAN error unfortunately still not fixed (thus help wanted added).

Reproducing is difficult because there are still plenty of ASAN errors in Debian Stretch.

[markus2330]

Seems like also the testshell_markdown_dini sometimes fails. I added the todos to #1693.

[markus2330]

@sanssecours What about dropping support for Ubuntu Trusty (14.04)? (Or at least the ASAN build for it.) It is even older than Debian Jessie (April 26th, 2015).

I wonder in which ways travis modified Trusty. (Or is it even using Ubuntu Trusty?) When I bootstrapped trusty I got cmake v2.8, not supported by Elektra anymore. Thus I could not reproduce the ASAN problem.

[sanssecours]

@sanssecours What about dropping support for Ubuntu Trusty (14.04)? (Or at least the ASAN build for it.) It is even older than Debian Jessie (April 26th, 2015).

I do not like the idea. I just fixed two memory leaks reported by the Travis ASAN build. However, I agree that Ubuntu Trusty is (horribly) outdated. As far as I can tell Travis should add support for the less outdated Ubuntu Xenial on December 11th 🙌.

I wonder in which ways travis modified Trusty.

The Travis documentation lists some of the modifications. The documentation is quite vague though:

The build-essential metapackage is installed, as well as modern versions of:

• GCC
• Clang
• make
• autotools
• cmake
• scons

. At least I do not really know what version of CMake is considered “modern”.

[markus2330]

Thank you for the quick reply!

lists some of the modifications

If they exchange core components they should not call it Ubuntu Trusty.

As far as I can tell Travis should add support for the less outdated Ubuntu Xenial on December 11th 🙌.

So lets wait for 3 days and see which ASAN errors we get with Ubuntu Xenial. (Hopefully they provide a real Ubuntu Xenial and not a modified one.)

[markus2330]

Seems like elektra-ini-mergerequest is now fixed. For ASAN multiple problems exist:

https://build.libelektra.org/jenkins/job/elektra-clang-asan/lastFailedBuild/console

Were they all introduced with the dini plugin?

[sanssecours]

Were they all introduced with the dini plugin?

The tests:

• testshell_selftest,
• testshell_markdown_dini and
• testkdb_allplugins

fail both on Travis and on Jenkins. Since the Travis ASAN build worked before commit 2bac4050, I would assume the failures were introduced by dini. Anyway, all of the messages of the form

runtime error: downcast of address … with insufficient space for an object of type 
'_Rb_tree_node<std::pair<const std::basic_string<char>, std::basic_string<char> > >'

show problems in libstdc++ and not in Elektra.

[markus2330]

Thank you for fixing it!

[markus2330]

I thought we found a way to suppress wrongly found ASAN errors. Can't we add the libstdc++ problems to tests/sanitizer.blacklist?

[sanssecours]

Can't we add the libstdc++ problems to tests/sanitizer.blacklist?

Hm, I never thought about that 😄. I guess you are right. I added an ToDo to issue #1725.