Fix distcheck #303

chrissie-c · 2018-03-20T13:39:15Z

Miscellaneous fixes and cleanups to get 'make distcheck' to work on most platforms.

There's a thing in here to speed up the FreeBSD IPC stress tests. I know it means that the BSD test is no longer really a stress test, but it can take over an hour to run with 70000 iterations and that;s just getting in everyone's way.

jnpkrn · 2018-03-20T15:30:29Z

There's a lot of context and some alignment with the existing checks
missing.

Under which circumstances does the check take 60+ minutes and does this
anomaly happen only with FreeBSD?

Just tried, "make check" incl. compilation took about 8 minutes,
FreeBSD 11.1, virtualized under KVM hypervisor, single CPU allocated
from 8 of i7-6820HQ.

jnpkrn · 2018-03-20T15:32:38Z

Makefile.am

@@ -37,6 +37,10 @@ MAINTAINERCLEANFILES	= Makefile.in aclocal.m4 configure depcomp \

 ACLOCAL_AMFLAGS		= -I m4

+DISTCHECK_CONFIGURE_FLAGS = --with-socket-dir=/tmp/libqb-$(VERSION)-$(shell echo $$$$)


Only to be removed with subsequent commit?

Yes, I decided it was better done in the CI itself rather than hard-coding it in the Makefile.

But ... introducing temporary, unjustified inter-patchset noise is not
nice should anyone care about history, bisecting etc.

It would have disappeared when I committed the final patch.

I thought the typical approach is to evolve the patchset towards the final
form directly as a part of the review. I see you want to see the response
from the CI, but cannot you prototype using your fork for that?

TBH I just want something that works. I hate working with autotools and it's ghastly friends. Every keypress I make regarding them makes me less and less motivated.

jnpkrn · 2018-03-20T15:33:43Z

tests/functional/GNUmakefile

@@ -10,7 +10,7 @@ distclean maintainer-clean:
 	# includes, which are swiped when processing one subdir and then
 	# missing, as a hard error, for the other)
 	@$(MAKE) -C log_external $@
-	@mkdir .deps
+	@mkdir .deps 2>/dev/null || true


You likely want mkdir -p instead of suppressing permission issues etc.

chrissie-c · 2018-03-20T15:38:32Z

I am no FreeBSD expert. All I know is that all the FreeBSD VMs I have ever tried this on (the 3 I have here on two different machines) and the one in the Jenkins CI all take around an hour to run. If a FreeBSD expert wants to fix this then it's fine by me, but I don't have the time to do it.

jnpkrn · 2018-03-20T15:43:27Z

Wrt. alignment with the existing checks, there's --enable-slow-tests
to configure script. We can either reuse that, or perhaps better yet,
apply a similar logic with optional --disable-sticky-tests (work name).
As demonstrated, it should not be hard-wired to apply on FreeBSD
unconditionally because there are cases with this system where the
testsuite runs reasonably out-of-the-box.

chrissie-c · 2018-03-20T15:45:43Z

Either of those is fine by me - and certainly tidier than what I had in there. My main aim here is just to get the stupid thing working :)

jnpkrn · 2018-03-20T18:47:57Z

lib/Makefile.am

@@ -137,6 +137,8 @@ install-exec-hook: qblog_script.ld
 	  sed -i -- "s/libqb.so.<digit>/$${so_ver}/" \
 	    "$(DESTDIR)$(libdir)/libqb.so-t" "$(DESTDIR)$(pkgconfigexecdir)/libqb.pc"
 	mv -f "$(DESTDIR)$(libdir)/libqb.so-t" "$(DESTDIR)$(libdir)/libqb.so"
+	rm -f "$(DESTDIR)$(pkgconfigexecdir)/libqb.pc--"
+	rm -f "$(DESTDIR)$(libdir)/libqb.so-t--"
 endif


Took me some moments to realize the issue -- there's an ambiguity
across systems as to whether -- should be considered as an argument
to -i option or whether this option should be considered orphan (not
accompanied with any argument as it's optional, anyway), followed with
delimiter indicating the end of options:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02

Do we have a better answer to this other than just removing the junk files? That chunk of shell code is extremely opaque (I remember commenting on that at the time.)

Let me include the fix for that into #288 I am going to overhaul a bit
(incl. a solution for backdated OpenBSD per #299). We need to get
rid of sed -i as it's not POSIX :-/

Ah, don't get me started ... spent way too much time yesterday trying to come up with a portable function to replace sed -i and gave up -- it's not impossible, but far uglier than this simple workaround:

sed ... "$INFILE" > "${INFILE}.$$" mv -- "${INFILE}.$$" "$INFILE"

It obviously only works on one input file at a time. If you need to preserve permissions, you have to do this first:

cp -p "$INFILE" > "${INFILE}.$$"

(A portable way to preserve permissions without copying the entire file is also too ugly for words.)

If you had suspicion what triggered this response of mine, you were right,
I came by that pacemaker commit, thanks for insights.

jnpkrn · 2018-03-20T18:59:43Z

tests/Makefile.am

@@ -82,6 +82,7 @@ check-headers: $(auto_c_files:.c=.o)

 distclean-compile:
 	rm -rf auto_*.c
+	rm -rf .deps


Looks to me the target should rather be named distclean-local,
cannot find any evidence for this one to be supported, hence we
should avoid surprises with the gray zone.

interesting! All these built-in destinations still throw me, and some are hard to find in the doc. disclean-local works for me so I''m happy to change it.

Is there a good reference for these things? the info files seem to be unhelpful.

https://www.gnu.org/software/automake/manual/automake.html#Clean
(as applicable to many other cases incl. pacemaker: single-page compilation,
finding through the page, even though info pages map 1:1 for most GNU
projects, IIRC)

jnpkrn · 2018-03-22T13:52:38Z

tests/check_ipc.c

@@ -64,7 +64,7 @@ static const int MAX_MSG_SIZE = DEFAULT_MAX_MSG_SIZE;
 #define GIANT_MSG_DATA_SIZE MAX_MSG_SIZE - sizeof(struct qb_ipc_response_header) - 8

 /* This can take AGES (like, an hour) on FreeBSD */
-#ifdef __FreeBSD__
+#ifndef HAVE_SLOW_TESTS


I think we should relax testing being in place for several years already
only on demand, through that sketched --disable-sticky-tests equivalent.
The hypothesis that it's always slow on FreeBSD has been invalidated,
so only a very very specific subset of all setups is affected (VMs with
FreeBSD lacking virtio support under KVM?).

chrissie-c · 2018-03-22T14:44:56Z

We don' t know whether it's a small or a large subset of FreeBSD installations that are slow - we have 3 data points. As it stands (because it affects the ones I'm using), I'm more inclined to put the original #ifdef FreedBSD back in (I agree, disabling it on Linux is a a bad move) unless someone has a proper fix or can explain why all all my VMs (and Fabio's) exhibit the problem and how to fix it.

jnpkrn · 2018-03-22T16:44:40Z

Can you at least collect which FreeBSD versions are you talking about?
freebsd-version should do. Point is, if they are rather outdated, I guess
we should not care at all in the build not instructed otherwise.

fabbione · 2018-03-23T05:55:58Z

On 3/22/2018 3:44 PM, Chrissie Caulfield wrote: We don' t know whether it's a small or a large subset of FreeBSD installations that are slow - we have 3 data points. As it stands (because it affects the ones I'm using), I'm more inclined to put the original #ifdef *FreedBSD* back in (I agree, disabling it on Linux is a a bad move) unless someone has a proper fix or can explain why all all my VMs (and Fabio's) exhibit the problem and how to fix it.

I am a bit lost on what the original issue was, but to be honest, a test suite should work regardless if the VM is slow or fast. The test suite passes on sparc64 that is 100x slower than any of those VMs, so the problem might be real somewhere else. Fabio

…

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#303 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAW6rWPoXkAn0HzT19nzBxCZXR0s714cks5tg7jogaJpZM4Sx6OX>.

chrissie-c · 2018-03-23T08:30:10Z

Both the slow FreeBSD VMs are 11.1 and were recently installed. They are also on three different hosts - though all using KVM as the virtualisation platform. I really don't want to get stuck on this. I'm happy to take the ifdef out completely if it annoys people.

chrissie-c · 2018-03-23T10:47:20Z

OK. I just moved my FreeBSD VM to another host and the full 70000 iteration tests completed in 8 minutes, same as yours Poki, so I'm happy it's a hosting problem. from that I suspect that people running it on bare metal are probably not seeing the problem and I'm going to remove that ifdef completely.

jnpkrn · 2018-03-26T11:04:05Z

Sounds like it may actually be the host the causes this (either with dated
kernel/KVM and or being virtualized on its own, i.e., nested virtualization?).

fabbione · 2018-03-26T11:10:31Z

On 3/26/2018 1:04 PM, Jan Pokorný wrote: Sounds like it may actually be the host the causes this (either with dated kernel/KVM and or being virtualized on its own, i.e., nested virtualization?).

7.4 kernel and kvm, no nested virt. Fabio

…

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <#303 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAW6rYl_JjaYM8YYXCq7IUhXMzztMteoks5tiMslgaJpZM4Sx6OX>.

chrissie-c · 2018-03-26T11:39:02Z

7.4 kernels are slow here too. (no nested virt - that's insane). Fedora 27 kernel seems to run reasonably though

Miscellaneous fixes and cleanups to get 'make distcheck' to work on most platforms. Signed-off-by: Christine Caulfield <[email protected]>

jfriesse · 2018-03-27T12:06:08Z

@chrissie-c Patchset looks ok for me, so ACK from my side.

chrissie-c · 2018-03-27T12:11:25Z

Thanks. I appreciate that the bit around the .ld file generation is suboptimal but we can fix that later if needs be. My main aim here is to get the tests back working.

chrissie-c · 2018-04-12T08:34:18Z

Update on the 'slow BSD' problem - it's a Linux kernel bug and is fixed in the RHEL7.5 kernel where the BSD tests take 8 minutes again.

jnpkrn · 2018-04-12T10:02:30Z

Thanks for the upgrade, resisting the immediate urge paid off this time.

fabbione · 2018-04-12T10:43:03Z

We will upgrade the CI infrastructure probably next week as 7.5 just went GA and we need to plan downtime. The distcheck is temporary disabled for BSD for this few days we need for preparation.

jnpkrn reviewed Mar 20, 2018

View reviewed changes

jnpkrn reviewed Mar 22, 2018

View reviewed changes

test: Fix 'make distcheck'

9a86921

Miscellaneous fixes and cleanups to get 'make distcheck' to work on most platforms. Signed-off-by: Christine Caulfield <[email protected]>

chrissie-c force-pushed the fix-distcheck branch from 81c5644 to 9a86921 Compare March 27, 2018 08:48

chrissie-c merged commit 3730644 into master Mar 27, 2018

chrissie-c deleted the fix-distcheck branch April 12, 2018 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix distcheck #303

Fix distcheck #303

chrissie-c commented Mar 20, 2018

jnpkrn commented Mar 20, 2018 •

edited

Loading

jnpkrn Mar 20, 2018

chrissie-c Mar 20, 2018

jnpkrn Mar 20, 2018

chrissie-c Mar 22, 2018

jnpkrn Mar 22, 2018

chrissie-c Mar 22, 2018

jnpkrn Mar 20, 2018

chrissie-c Mar 20, 2018

chrissie-c commented Mar 20, 2018

jnpkrn commented Mar 20, 2018

chrissie-c commented Mar 20, 2018

jnpkrn Mar 20, 2018

chrissie-c Mar 22, 2018

jnpkrn Apr 11, 2018

kgaillot Apr 11, 2018

jnpkrn Apr 11, 2018

jnpkrn Mar 20, 2018 •

edited

Loading

chrissie-c Mar 22, 2018

jnpkrn Mar 22, 2018

jnpkrn Mar 22, 2018

chrissie-c commented Mar 22, 2018

jnpkrn commented Mar 22, 2018

fabbione commented Mar 23, 2018 via email

chrissie-c commented Mar 23, 2018

chrissie-c commented Mar 23, 2018

jnpkrn commented Mar 26, 2018

fabbione commented Mar 26, 2018 via email

chrissie-c commented Mar 26, 2018

jfriesse commented Mar 27, 2018

chrissie-c commented Mar 27, 2018

chrissie-c commented Apr 12, 2018

jnpkrn commented Apr 12, 2018

fabbione commented Apr 12, 2018

		@@ -37,6 +37,10 @@ MAINTAINERCLEANFILES = Makefile.in aclocal.m4 configure depcomp \

		ACLOCAL_AMFLAGS = -I m4

		DISTCHECK_CONFIGURE_FLAGS = --with-socket-dir=/tmp/libqb-$(VERSION)-$(shell echo $$$$)

Fix distcheck #303

Fix distcheck #303

Conversation

chrissie-c commented Mar 20, 2018

jnpkrn commented Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrissie-c commented Mar 20, 2018

jnpkrn commented Mar 20, 2018

chrissie-c commented Mar 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnpkrn Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrissie-c commented Mar 22, 2018

jnpkrn commented Mar 22, 2018

fabbione commented Mar 23, 2018 via email

chrissie-c commented Mar 23, 2018

chrissie-c commented Mar 23, 2018

jnpkrn commented Mar 26, 2018

fabbione commented Mar 26, 2018 via email

chrissie-c commented Mar 26, 2018

jfriesse commented Mar 27, 2018

chrissie-c commented Mar 27, 2018

chrissie-c commented Apr 12, 2018

jnpkrn commented Apr 12, 2018

fabbione commented Apr 12, 2018

jnpkrn commented Mar 20, 2018 •

edited

Loading

jnpkrn Mar 20, 2018 •

edited

Loading