SCons / scons

SCons - a software construction tool
http://scons.org
MIT License
2.08k stars 316 forks source link

Fortran compile fails with module dependencies #4177

Open james-thunes opened 2 years ago

james-thunes commented 2 years ago

Describe the bug scons does not appear to be correctly find dependencies on *.mod files in Fortran files.

My project includes a Fortran library that includes a number of source files and a module including some shared variables. When compiling on windows with ifort, I get the following error when doing a clean build: error #7002: Error in opening the compiled module file. Check INCLUDE paths. Subsequent builds complete successfully.

Investigation shows that the above compiler error is seen because the .mod file required by the source files is not built before the source file compilation. Reordering the list of source files improves the situation, but it still fails periodically when building in parallel. Subsequent attempts to compile work as the .mod file is created during the first scons attempt and are thus already available.

Per suggestion from discord, I ran scons with --tree=prune to check if the consumers of the module correctly listed the *.mod file as a build dependency. The output below (anonymized) shows the output for one of the source files:

  |   +-<path>\<objName>.staticrt.obj
  |   | +-<relativePath>\<srcFile>.f
  |   | +-C:\Program Files (x86)\IntelSWTools\compilers_and_libraries_2019.5.281\windows\bin\intel64\ifort.EXE

Note that the source file does not list the *.mod file as a dependency.

The project is cross-platform so I also checked the result of --tree=prune on linux (compiled with gfortran). Scons also did not recognize the dependency on the mod file (output was the same as above with the exception of the compiler path). However, the code compiles without issue on linux.

Since the above shows that the source files are not identifying the *.mod file as a build dependency, my assumption is that there is some issue in the way that scons is determining dependencies for Fortran files. I'm not sure why there's not an issue with gfortran on linux.

Required information

bdbaddog commented 1 year ago

Do you always want the .mod files in the same directory as the .o's ?

dnwillia commented 1 year ago

Good question. I would say possibly not, but it's certainly a much better default than what happens at the moment (i.e. having them dumped in the cwd with the SConstruct) and will get the variant_dir builds working properly.

mwichmann commented 1 year ago

@dnwillia it seems like your repository of tests has differing behavior between runs. I tried SConstructVariant once and saw the error you saw. Edited - I misread, this looks like a different error...

gfortran -o TestSConscript/builds1/linux/main_prog.exe TestSConscript/builds1/linux/module_one.o TestSConscript/builds1/linux/module_two.o TestSConscript/builds1/linux/main_prog.o
scons: *** [TestSConscript/builds2/linux/builds2] TypeError : Tried to lookup Dir 'TestSConscript/builds2' as a File.

then tried it again and the build worked. (!?!? I hate those). Is this maybe some case of the sconsign file getting populated?

@bdbaddog

Do you always want the .mod files in the same directory as the .o's ?

Thinking about it, I believe not, unless it's a fairly flat tree. If you USE a module, then its .mod file has to be available at compilation time. If the source file being compiled is in a different directory than the module's source, then having the module file over in that other directory isn't any good either unless FORTRANMODDIR points over there.

       -Jdir
           This option specifies where to put .mod files for compiled modules. 
           It is also added to the list of directories to searched by an "USE" statement.

           The default is the current directory.

So if we do put them together with the module's objects, then that directory should somehow get added to FORTRANPATH so it gets searched later:

       -Idir
           ...
           This path is also used to search for .mod files when previously compiled modules are required
           by a "USE" statement.

Can we leave that step up to the SConscript file author?

dnwillia commented 1 year ago

Regarding the first issue, yes I think I ran into that as well. Not sure why. The example SCons input files are not particularly complicated and are based on what is written into the user documentation for SCons so I'm just following the pattern. :-)

Regarding the .mod location I think it just comes down to what you want the default behavior to be. The default right now is to leave them in the SConstruct directory when using the variant_dir feature. Without the change to FORTRANMODDIR the --warn=all option reports warnings about the .mod files not being found (as I noted already). I think that the .mod files are an output of the build process and so should be in the variant_dir if you use that as in my repo. Maybe my solution is not the right way to make this happen, but this would be my expected UX.

I agree that if you want to modify this behavior then of course you need to resort to using -J and -I and/or modifying the FORTRANMODDIR variable.

bdbaddog commented 1 year ago

@dnwillia it seems like your repository of tests has differing behavior between runs. I tried SConstructVariant once and saw the error you saw. Edited - I misread, this looks like a different error...

gfortran -o TestSConscript/builds1/linux/main_prog.exe TestSConscript/builds1/linux/module_one.o TestSConscript/builds1/linux/module_two.o TestSConscript/builds1/linux/main_prog.o
scons: *** [TestSConscript/builds2/linux/builds2] TypeError : Tried to lookup Dir 'TestSConscript/builds2' as a File.

then tried it again and the build worked. (!?!? I hate those). Is this maybe some case of the sconsign file getting populated?

@bdbaddog

Do you always want the .mod files in the same directory as the .o's ?

Thinking about it, I believe not, unless it's a fairly flat tree. If you USE a module, then its .mod file has to be available at compilation time. If the source file being compiled is in a different directory than the module's source, then having the module file over in that other directory isn't any good either unless FORTRANMODDIR points over there.

       -Jdir
           This option specifies where to put .mod files for compiled modules. 
           It is also added to the list of directories to searched by an "USE" statement.

           The default is the current directory.

So if we do put them together with the module's objects, then that directory should somehow get added to FORTRANPATH so it gets searched later:

       -Idir
           ...
           This path is also used to search for .mod files when previously compiled modules are required
           by a "USE" statement.

Can we leave that step up to the SConscript file author?

Nope. Fairly certain that means the DAG isn't right, and once the build fails, the needed files (which Scons isn't aware of) are in the right place to succceed.. Check scons --tree=all ?

mwichmann commented 1 year ago

Here's a dump, I have to run off and do other stuff....

Variant-output.txt

bdbaddog commented 1 year ago

Variant-output.txt

Ugh. that's odd.

mwichmann commented 1 year ago

Had a bit of a dig, and it's just too convoluted and too many layers to tease out much info. For the case of non-duplicating variantdir, we die in the Fortran scanner, in scan, where it:

        for dep in mods_and_includes:                                          
            n, i = self.find_include(dep, source_dir, path)  

and doesn't find the module it's looking for... it looks like it's looking for them as if they were static files that will be present in the source tree? but there must be more going on here since the mod files are derived, not static sources, and this comes out correct in other usages. Anyway, with verbose enabled in the file finder, first the copying-variant form:

scons: Building targets ...
  find_file: looking for 'module_two.mod' in 'TestSConscript/builds1/linux' ...
  find_file: ... FOUND 'module_two.mod' in 'TestSConscript/builds1/linux'
gfortran -o TestSConscript/builds1/linux/module_two.o -c -JTestSConscript/builds1/linux TestSConscript/builds1/linux/module_two.f90
gfortran -o TestSConscript/builds1/linux/module_one.o -c -JTestSConscript/builds1/linux TestSConscript/builds1/linux/module_one.f90
  find_file: looking for 'module_one.mod' in 'TestSConscript/builds1/linux' ...
  find_file: ... FOUND 'module_one.mod' in 'TestSConscript/builds1/linux

vs the non-copying variant:

scons: Building targets ...
  find_file: looking for 'module_two.mod' in 'TestSConscript' ...

scons: warning: No dependency generated for file: module_two.mod (referenced by: TestSConscript/module_one.f90) -- file not found
File "/home/mats/.pyenv/versions/venv-system/bin/scons", line 8, in <module>
gfortran -o TestSConscript/builds12/linux/module_one.o -c -JTestSConscript/builds12/linux TestSConscript/module_one.f90
TestSConscript/module_one.f90:2:7:

    2 |   use module_two
      |       1
Fatal Error: Cannot open module file 'module_two.mod' for reading at (1): No such file or directory

Notice the different path the file finder is passed.

On a separate note, when the revised gfortran.py from the test branch sets up env["FORTRANMODDIR"] = "${TARGET.dir}", that doesn't get expanded as expected in the emitter when it's called, the emitter does this:

moddir = env.subst('$FORTRANMODDIR', target=target, source=source)

and always comes back here with moddir set to .; the expansion works right when the command line is actually generated as can be seen above.

dnwillia-work commented 1 year ago

and doesn't find the module it's looking for... it looks like it's looking for them as if they were static files that will be present in the source tree?

That was what I managed to work out too but from there it's not clear to me what to do about it so I stopped digging.

mwichmann commented 1 year ago

Okay, a little more light on the non-copying variantdir: sources are absolute paths in this case, but targets shouldn't be so they can be handled by the variant code. But the module that will be generated, as detected by the emitter (not via the scanner, btw, but by using a regex that's supposed to stay in sync with the one in the scanner), has the absolute-path form.

Fortran emitter: module_one.f90 -> module_one.o module_one.mod
Fortran emitter: module_two.f90 -> module_two.o module_two.mod
Fortran emitter: main_prog.f -> main_prog.o
...
Fortran emitter: /home/mats/github/scons/exp/fort-subdir/SConsTests/TestSConscript/module_one.f90 -> module_one.o /home/mats/github/scons/exp/fort-subdir/SConsTests/TestSConscript/module_one.mod
Fortran emitter: /home/mats/github/scons/exp/fort-subdir/SConsTests/TestSConscript/module_two.f90 -> module_two.o /home/mats/github/scons/exp/fort-subdir/SConsTests/TestSConscript/module_two.mod
Fortran emitter: /home/mats/github/scons/exp/fort-subdir/SConsTests/TestSConscript/main_prog.f -> main_prog.o

So I presume that this snip from FortranCommon is the wrong way to generate the target node(s) for modules:

    for module in modules:                                                     
        target.append(env.fs.File(module, moddir))
mwichmann commented 1 year ago

If it helps to be explicit - it's not just that the module paths here are absolute, it's that they're to what would be considered the "source directory", as in:

absolute-path-to-topdir/TestSConscript/module_one.mod

the module file will never appear there... it will either be in the topdir if there's no FORTRANMODDIR, or in the build directory specified by the variantdir if it is set as in the sample.

mwichmann commented 1 year ago

Okay, I'm just repeating stuff that's already been said, but module handling seems pretty broken. I set up another small test with a trivial module source file, repeated four times (uniqued for each version):

  1. source in top-level directory
  2. source in src subdirectory with own SConscript
  3. source in vsrc subdirectory, SConscript called as duplicating variantdir build
  4. source in vsrc-no subdirectory, SConscript called as non-duplicating variantdir build-no

Calling Object() on each, with no setting of FORTRANMODDIR, so the default empty string, which won't expand to any -J option to gfortran, you expect to get four object files and four .mod files, and so we do:

build-no/test4.o
build/test3.o
mod_foo1.mod
mod_foo2.mod
mod_foo3.mod
mod_foo4.mod
src/test2.o
test1.o

But that doesn't match what SCons thinks it did, which means we have a problem (the tree is trimmed to show only these eight):

+-.
  +-build
  | +-build/mod_foo3.mod
  | +-build/test3.o
  +-build-no
  | +-build-no/mod_foo4.mod
  | +-build-no/test4.o
  +-mod_foo1.mod
  +-src
  | +-src/mod_foo2.mod
  | +-src/test2.o
  +-test1.o

It's never good when SCons builds up a picture of what exists, and then that's not what the compiler did. The simplest problem from that is you can't clean, but there's plenty more issues, like rebuilds being horked, etc.

mwichmann commented 1 year ago

Now, if we redo the above with the experimental setting of FORTRANMODDIR, so it's force-passed on every build, we at least get consistency:

f95 -o build/test3.o -c -Jbuild build/test3.f
f95 -o build-no/test4.o -c -Jbuild-no vsrc-no/test4.f
f95 -o test1.o -c -J. test1.f
f95 -o src/test2.o -c -Jsrc src/test2.f

Giving us:

build/mod_foo3.mod
build/test3.o
build-no/mod_foo4.mod
build-no/test4.o
mod_foo1.mod
src/mod_foo2.mod
src/test2.o
test1.o

And the tree at least now matches what was built:

+-.
  +-build
  | +-build/mod_foo3.mod
  | +-build/test3.o
  +-build-no
  | +-build-no/mod_foo4.mod
  | +-build-no/test4.o
  +-mod_foo1.mod
  +-src
  | +-src/mod_foo2.mod
  | +-src/test2.o
  +-test1.o

But that hasn't got into the other case of trying to USE these modules, and whether they're found where they got put so other sources will build.

dnwillia commented 1 year ago

Yup. At least you can reproduce what I’m also seeing.

mwichmann commented 1 year ago

I thought I had a cute fix for the "if there's no FORTRANMODDIR, SCons is out of sync with what the compiler does" problem, but it turns out an extra bit of variantdir cleverness defeats that in one case. Still thinking. This was the idea:

    for module in modules:
        if not moddir:
           target.append(env.fs.File(module, directory='#'))
        else:
           target.append(env.fs.File(module, directory=moddir))
mwichmann commented 1 year ago

So the odd bit is recorded, it seems, if you set a relative path for FORTRANMODDIR, and use a non-duplicating variantdir, scons will double the flag usage, emitting one module dir for the target (variant) directory and one for the original source directory, and that's illegal. Like

env = Environment(FORTRANMODDIR="modules")

which leads to:

gfortran -o build-no/test4.o -c -Jbuild-no/modules -Jvsrc-no/modules vsrc-no/test4.f
Fatal Error: gfortran: Only one '-J' option allowed
compilation terminated.

I don't think I've caused this....

Leaving it out (the default is an empty string), or using a top-relative path ("#/modules") does not cause this effect.

mwichmann commented 1 year ago

If a relative path is used, then RDirs, the function passed in the _concat call, is the one that makes this mess.

So this:

env['_FORTRANMODFLAG'] = '$( ${_concat(FORTRANMODDIRPREFIX, FORTRANMODDIR, FORTRANMODDIRSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)'

has the prospect of producing something illegal, with Fortran compilers not accepting multiple module-directory options. I think it's better not to apply this particular transformation (i.e., leave out RDirs from the generation of this flag.)

mwichmann commented 1 year ago

Hoo boy, this simple little matter might end up being a hair-loss event...

mwichmann commented 1 year ago

On a separate note, when the revised gfortran.py from the test branch sets up env["FORTRANMODDIR"] = "${TARGET.dir}", that doesn't get expanded as expected in the emitter when it's called, the emitter does this:

moddir = env.subst('$FORTRANMODDIR', target=target, source=source)

and always comes back here with moddir set to .; the expansion works right when the command line is actually generated as can be seen above.

Okay, this conclusion was just bogus. At SConscript evaluation time, the context is the directory of the SConscript, so it makes sense for the subst here to come back with .. Ignore me...