ESCOMP / CDEPS

Community Data Models for Earth Prediction Systems
https://escomp.github.io/CDEPS/versions/master/html/index.html
18 stars 39 forks source link

Bug in buildlib code to check for compiler warnings #240

Open adrifoster opened 11 months ago

adrifoster commented 11 months ago

In running on my own personal computer, I encountered a bug with the python buildlib script.

nextline is set up as an empty string here, but strings cannot be appended to as they are immutable, so this fails on this line .

I propose changing the line to nextline = nextline + line.

However, I still got additional errors because of this check:

        if len(nextline) > 0:
            expect(False, nextline)`

The specific warning I got was

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: /Users/afoster/projects/scratch/SMS_D_Ld1_P8x1.f10_f10_mg37.I2000Clm50BgcCropQianRs.fleabone_gnu.clm-default.20230821_112432_besy6i/bld/gnu/openmpi/debug/nothreads/nuopc/lib/libFoX_common.a(FoX_common.F90.o) has no symbols

There does seem to be symbols in this file checking on my own and with @billsacks. Commenting out this entire code block, my tests all ran so I'm not sure what this warning is about. However, I believe it is not getting caught by the if "F90" in line and not "fox" in line: above because "fox" is actually "FoX" in the warning output.

So I propose (will submit a PR if get the go ahead) to change this entire code block to:

    if compiler == "gnu" and case.get_value("DEBUG"):
        # Do not allow any warnings except from fox external
        nextline = ""
        for line in e.split("\n"):
            if "f90" in line.lower() and not "fox" in line.lower():
                nextline = nextline + line
        if len(nextline) > 0:
            expect(False, nextline)

@billsacks and @jedwards4b and others does this seem okay? I have done manual testing of this update and it seems to work.

billsacks commented 11 months ago

This suggestion looks good to me, so opening a PR sounds great. Thank you for working through it!