-
Notifications
You must be signed in to change notification settings - Fork 481
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
Dbld coverage #2031
Dbld coverage #2031
Conversation
Build SUCCESS |
dbld/images/required-apt/artful.txt
Outdated
@@ -6,6 +6,7 @@ libmongoc-dev | |||
librabbitmq-dev | |||
libsystemd-dev | |||
openjdk-8-jdk | |||
libriemann-client-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We install this package from OBS on the following platforms:
$grep -rn libriemann-client-dev
required-obs/trusty.txt:6:libriemann-client-dev
required-obs/xenial.txt:1:libriemann-client-dev
required-obs/jessie.txt:4:libriemann-client-dev
Can we install it from the official repositories on the other platforms? Or should we change this one to?
(Note: Based on the outcome, there is a chance we can move it to the "all.txt")
I would rather decrease dependency on obs packages over time, do if the one
in artful is good enough I would use that.
…On May 13, 2018 23:45, "László Szemere" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In dbld/images/required-apt/artful.txt
<#2031 (comment)>:
> @@ -6,6 +6,7 @@ libmongoc-dev
librabbitmq-dev
libsystemd-dev
openjdk-8-jdk
+libriemann-client-dev
We install this package from OBS on the following platforms:
$grep -rn libriemann-client-dev
required-obs/trusty.txt:6:libriemann-client-dev
required-obs/xenial.txt:1:libriemann-client-dev
required-obs/jessie.txt:4:libriemann-client-dev
Can we install it from the official repositories on the other platforms?
Or should we change this one to?
(Note: Based on the outcome, there is a chance we can move it to the
"all.txt")
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2031 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AArldvZrywTCJtgz_3hfRc94vzfhpqnzks5tyKlmgaJpZM4T853B>
.
|
@bazsi: could you check it and modify the PR accordingly? (I think we could remove obs:libriemann, but we have to test it) |
Right now, I am using libriemann from artful instead of obs. Or you mean checking the rest of the platforms? |
@bazsi: I know it is not strongly related to this PR but also think that we should check this as we want to less depend on obs. |
Build SUCCESS |
Build FAILURE |
@@ -21,6 +21,8 @@ TARBALL=$(BUILD_DIR)/syslog-ng-$(VERSION).tar.gz | |||
MODE=snapshot | |||
CONFIGURE_OPTS=--enable-debug --enable-manpages --with-python=2 --prefix=/install --with-docbook=/usr/share/xml/docbook/stylesheet/docbook-xsl/manpages/docbook.xsl $(CONFIGURE_ADD) | |||
|
|||
-include dbld/rules.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I missed it, but I don't see this rules.conf file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That file can be added locally so you can customize your dbld related options (for me, I override the default docker image to be used and the default bootstrap arguments).
very handy.
lcov is needed for generating coverage reports. Signed-off-by: Balazs Scheidler <[email protected]>
Signed-off-by: Balazs Scheidler <[email protected]>
…ile option The lcov version in our current docker images does not work with libtool properly, as it is looking for the generated files in the source directory. There's a new option called geninfo_auto_base that fixes this issue. Signed-off-by: Balazs Scheidler <[email protected]>
I have now rebased and dropped the riemann bits. This should be clear cut now. I don't see how kira couldn't like it :) Anyhow, I think this is good enough to go in, after a review. |
Build SUCCESS |
@@ -11,3 +11,4 @@ valgrind | |||
linux-tools-generic | |||
libjemalloc-dev | |||
locales | |||
lcov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we want to create coverage reports? I am asking it regarding to the list of dependencies.
- Only by hand, on the local machine. -> devshell-apt/all.txt is a good place.
- Part of Kira/Travis, like the module tests. -> it should be in required-apt / required-yum like other test dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The agreement last time was that the various images should only install packages strictly required for building and dev tools should not be on them (e.g. not even gdb or an editor).
That's why this went into devshell. I agree that it possibly made sense to get these in travis/kira, but let's migrate at least one of them to use these images and get back to this question at that point.
Until now it is manual only.
PS: I could even live with trying to merge the build/dev usecases and only build one image, but in that case, that image would probably be pretty large.
This patch set makes it possible to generate coverage reports in the devshell image. It contains a set of fixes that:
With that, if --enable-gcov is added as a configure option, "make coverage" would work from within the devshell image, generating an HTML report into dbld/build/coverage.html
We roughly have 67% coverage with the unit tests alone. Could be better, but some parts look really nice, others less so.