make check on Windows [WAS Re: 16690 breaks `make check`]

[Daniel Harding]

Derek Atkins wrote:

 > Quoting Daniel Harding <dharding at gmail.com>:
 >
 > > Found a build problem, at least on Windows.  The Makefile attempts to
 > > create links to several .scm files from the build directory to the
 > > source directory.  Because Windows filesystems do not support links, it
 > > simply does a copy instead.  But, because the Makefile assumes that it
 > > is really is a link, it does not contain dependencies between the .scm
 > > files in the build directory and those in the source directory.  Thus
 > > any changes made to the .scm files in the source directory will not get
 > > propagated to the build directory after the initial copies are made.
 >
 > Correct.  On Windows you should use a separate build-dir, or you
 > should "make distclean" between each build.  I'm pretty sure
 > that the "install.sh" scripts do this automatically for you.  Are
 > you using the win32 build scripts or are you building it yourself
 > by hand?

I've always built using install.sh, which does use a separate 
build-dir.  It doesn't appear to be doing a make distclean before 
starting the build though.

 > > When make checks the dependency on iso-4217-currencies.scm for
 > > iso-4217-currencies.c, it uses the version in the build directory
 > > instead of the version in the source directory.  Because the version in
 > > the build directory hasn't been changed, it doesn't rebuild
 > > iso-4217-currencies.c.
 >
 > Hmm....
 >
 > > I don't know of any reason why iso-4217-currencies.scm needs to be
 > > copied/linked, because it isn't actually used in the program at all,
 > > just for generating the .c file.  I have attached a patch for
 > > Makefile.am to no longer generate the link to/copy
 > > iso-4217-currencies.scm into the build directory, and have verified 
that
 > > everything still builds properly, even without iso-4217-currencies.scm
 > > in the build directory.
 >
 > True, for THAT case you could just run it specifically out of
 > $(srcdir).

Which is what the Makefile does, except that because it is also copying 
iso-4217-currencies.scm into the build-dir, it messes up the dependency 
check.  I don't think that iso-4217-currencies.scm actually needs to be 
copied into the build-dir.  It builds fine without doing so, and 
iso-4217-currencies.scm isn't referenced by any tests.

 > > If I run make check in src/engine/test, nearly everything passes for
 > > me. I had to disable test-book-merge because it isn't building for
 > > me, while test-date and test-transaction-voiding both fail, but that
 > > appears to be simply because of Windows issues rather than this
 > > patch. Everything else completes successfully, including test-lots.
 >
 > You shouldn't have to disable any tests.  If you do, then there's
 > a problem.  "make check" should pass on all platforms, and I know
 > that Christian spent much time working on that.

Well, then either I'm doing something wrong, or something has since 
broken.  I'll post a couple of example failures here, and if nothing 
jumps out as an obvious mistake on my part, I'll start filing bugs, so 
that they can actually be tracked and fixed (which I am glad to help out 
with).

Before running make check, I make sure that the all the environment 
variables are set as they would be if I ran the gnucash shell script 
which starts gnucash-bin.  If I then run "make check" from the root 
directory, 4 of 12 tests in gnc-module fail and then make error exits.  
Here's example output from one of the failing tests (test-gwrapped-c):

(guile.exe:2060): gnc.module-WARNING **: Failed to open module gnucash/foo

(guile.exe:2060): gnc.module-WARNING **: : could not locate gnucash/foo 
interface v.0

ERROR: Unbound variable: foo-hello
FAIL: test-gwrapped-c

This looks to me like either a search directory is not being set up 
correctly or a file is not being copied into the right place.  However, 
because I don't know exactly what file it is looking for or where it 
should be looking for it, I haven't been able to dig into this further 
to know whether this is really a bug or just a configuration issue on my 
part.

Moving on, if I run "make check" from src/engine, I get the following 
compile error:

../../../../repos/src/engine/test/test-book-merge.c: In function `main':
../../../../repos/src/engine/test/test-book-merge.c:508: warning: 
implicit declaration of function `sleep'

That call to sleep is the first line of the main() function, right 
before qof_init().  It was added in revision 16041, but without any 
explanatory comment and does not seem to be serving any useful purpose.  
I will try to get a bug filed tomorrow, and if removing it does not 
appear to have any ill affects, will include a patch to remove it.

If I disable test-book-merge, the rest of the tests build, but test-date 
and test-transaction-voiding fail.  I still need to look at those, and 
will probably end up filing bugs for them as well.

If there are any comments/suggestions before I start hitting Bugzilla, 
let me know.  Thanks,

-Daniel