andreww / fox

A Fortran XML library
https://andreww.github.io/fox/
Other
60 stars 51 forks source link

Update to compatibility with modern autoconf #70

Closed bernstei closed 3 years ago

bernstei commented 3 years ago

Update to autoconf 2.69, relying entirely on autoconf distributed lang.m4 and fortran.m4, and therefore losing compatibility with old fortran compilers that cannot do their own preprocessing.

closes #4 and #68

bernstei commented 3 years ago

@andreww What do you think of this? I basically reverted to the default m4 autoconf files, updated some symbol names they define differently from the old custom versions, and stripped out some specific things like the preprocessed fortran filetype.

bernstei commented 3 years ago

I'm looking at the CI failures now.

bernstei commented 3 years ago

Looks like I'm getting a fair number of failures with make check even for the master branch, but it doesn't seem to be saving any information about them - utils/test/failed.out doesn't exist. The patch doesn't change the number of failures (as expected, since everything is still compiling and it's not touching any of the actual fortran code).

andreww commented 3 years ago

That's odd. I've just triggered the tests on the main branch to see if the errors show up there two (but the last build there passed at the weekend). If that doesn't show what's going on I'll pull a copy down and see what testing locally shows.

bernstei commented 3 years ago

On Feb 16, 2021, at 10:40 AM, Andrew Walker notifications@github.com<mailto:notifications@github.com> wrote:

That's odd. I've just triggered the tests on the main branch to see if the errors show up there two (but the last build there passed at the weekend). If that doesn't show what's going on I'll pull a copy down and see what testing locally shows.

I looked at one that fails (test_cmlAddNamespace_0), and I either don't understand what it's doing or I don't understand why it ever passed.

The test source is

program test

  use FoX_wcml
  implicit none

  character(len=*), parameter :: filename = 'test.xml'
  type(xmlf_t) :: xf

  call cmlAddNamespace(xf, 'xhtml', 'http://www.w3.org/1999/xhtml')

end program test

and something below cmlAddNamespace() complains that the xf isn't attached to an open file (lun = -1), but that's not surprising, because there really is nothing that would open a file (and filename is unused, which is suspicious).

andreww commented 3 years ago

Ah - I wonder if all the failures you are seeing locally are tests for the various ways FoX handles errors, and if you are using gfortran? The test harness checks this (that the programme should terminate with an error) but gfortran now adds a backtrace by default which messes things up. If this is the case configuring like this:

FC='gfortran -fno-backtrace' ./configure

should make those failures go away. However, this isn't the problem the CI is seeing. The main branch passed all the tests so I'll pull this branch and have a look (but I don't think I'll get to that today).

bernstei commented 3 years ago

On Feb 16, 2021, at 10:54 AM, Andrew Walker notifications@github.com<mailto:notifications@github.com> wrote:

Ah - I wonder if all the failures you are seeing locally are tests for the various ways FoX handles errors, and if you are using gfortran? The test harness checks this (that the programme should terminate with an error) but gfortran now adds a backtrace by default which messes things up. If this is the case configuring like this:

FC='gfortran -fno-backtrace' ./configure

should make those failures go away. However, this isn't the problem the CI is seeing. The main branch passed all the tests so I'll pull this branch and have a look (but I don't think I'll get to that today).

I am using gfortran. Let me see if that fixes "make check".

bernstei commented 3 years ago

On Feb 16, 2021, at 10:55 AM, Bernstein, Noam CIV USN NRL (6393) Washington DC (USA) noam.bernstein@nrl.navy.mil wrote:

I am using gfortran. Let me see if that fixes "make check".

Unchanged, from what I can tell.

andreww commented 3 years ago

I'm seeing the same errors locally here and I think I know what's going on with at least one of them. The test failure I've looked at is test_dom_getTextContext_4 which is supposed to exit with an error (that ]]> in test_dom_getTextContent_4.xml_in makes the XML badly formed). We are supposed to get the following on standard output:

PARSE_ERR                  
81 runParser

what we actually get is:

PARSE_ERR                  
81 runParser
           0

and the test harness isn't happy with the extra zero, so it marks this as an failure.

The code path for this is a call to throw_exception in dom/m_dom_parse.f90. This is defined in dom/m_dom_error which prints the expected output before calling pxfabort to generate a backtrace / core dump on exit. (This should be useful for debugging.) pxfabort is defined in fsys/fox_m_abort_flush.F90. My guess is that the change means that the precompiler directive FC_HAVE_ABORT is no longer being defined and the zero comes from printing the null pointer in the fallback at the end.

So that's probably the problem. I'm unsure about the best way to fix it (I don't know autoconf that well). As far as I can see the checks to set up the preprocessor directive still exists in aclocal.m4. Humm. I guess the first thing to do is sprinkle some print statements in fsys/fox_m_abort_flush.F90 and check that this idea is correct.

bernstei commented 3 years ago

I'm trying to track down how the autoconf's test results (e.g. whether to set FC_HAVE_ABORT) are supposed to make it into the code. There seems to be some confusion about setting DEFS vs. confdefs.h, and right now I can't figure out how it worked before, but I think it should be straightforward to fix.

bernstei commented 3 years ago

I just checked the master branch. I'm still getting a fail on test_dom_getTextContext_4, probably because the output still has something after the line number (although it's not 0).

rhenium : cat dom/test/failed.out                                                                                      
./test_dom_getTextContent_4
------------
--- test.out    2021-02-23 10:15:40.000000000 -0500
+++ ./test_dom_getTextContent_4.out 2021-02-10 11:07:25.000000000 -0500
@@ -1,11 +1,2 @@
 PARSE_ERR                  
 81 runParser
-
-Program aborted. Backtrace:
-#0  0x10a7a9d3e
-#1  0x10a7aa558
-#2  0x10a8e83c3
-#3  0x10a74c868
-#4  0x10a6dcce0
-#5  0x10a6d9bc8
-#6  0x10a75c16e
------------

Any ideas @andreww ? I can probably get the PR code to behave like the master version, but for me that's not working either.

andreww commented 3 years ago

That looks like a backtrace (which is good). Try with FC='gfortran -fno-backtrace' ./configure (rather than just ./configure). That should suppress the backtrace and the test ought to pass. Alternatively push to this branch - the CI should pass now I think.

bernstei commented 3 years ago

OK. I'm still chasing down DEFS vs. confdefs.h, but once have that straightened out, I'll test locally with "-fno-backtrace" and push to see how the CI responds.

bernstei commented 3 years ago

I think I got all the things that used to go into DEFS to go into confdefs.h, which autoconf then automatically turns back into DEFS, and I also added DEFS explicitly back into arch.make.in, to replace the old m4 code that did it via FPPFLAGS. I think it should be at least somewhat better, but still waiting for the CI tests to finish,

andreww commented 3 years ago

Excellent - passes the tests in the CI and also locally for me. Many thanks @bernstei! I'll get this merged.

bernstei commented 3 years ago

Thanks.