scons icon indicating copy to clipboard operation
scons copied to clipboard

gcc command-line option '-include foo' doesn't create a dependency

Open bdbaddog opened this issue 7 years ago • 8 comments

This issue was originally created at: 2005-08-11 14:34:17. This issue was reported by: gregnoel.

gregnoel said at 2005-08-11 14:34:17

If a GCC compile command line has one or more '-include foo' options, no dependency is created for the file 'foo'. Although the use of this option is rare and tends to be to unchanging files, it's disconcerting to change such a base file and not have the dependent files recompiled.

stevenknight said at 2006-01-05 18:16:19

Changing version to "unspecified" to conform to new scheme.

kmaples said at 2006-05-20 17:03:31

making the default milestone consistent across the project

kmaples said at 2006-05-20 17:14:14

making the version consistent across the project

bdbaddog said at 2008-03-15 22:43:18

Is scons supposed to be scanning the command line, or is it just supposed to use CPPPATH to determine include paths?

gregnoel said at 2008-03-15 23:03:14

SCons frequently scans command lines to make decisions about what to do. For example, the SWIG builder will generate different output depending upon whether the -python or -c++ flag is present. This is no different.

Since it can lead to incorrect builds if the dependency isn't created, which SCons will go to great lengths to avoid.

stevenknight said at 2008-10-02 10:42:18

See issue #1107, which we could possibly dup this to.

I'd prefer to see this configured by an explicit variable like $CPPFORCEINCLUDES that then gets added to the command line with the (configurable) "-include" option. This is more like the way we handle many other things, and allows the user to be explicit about what .h file is used for the dependency.

Greg pointed out in IRC that doing so would have impacts on MergeFlags() and the like, which makes sense.

I'll update #1107 with a comment. Greg, your call on whether we can go ahead and dup this.

gregnoel said at 2008-10-02 16:34:32

Issue #1107 is the better approach, and it preceeds this one by five months, so it gets the nod.

I don't like the name CPPFORCEINCLUDES, but I'll make that comment over there.

*** This issue has been marked as a duplicate of 1107 ***

gregnoel said at 2009-02-06 06:25:34

Bug party triage. Reopen this issue to add a CPPINCLUDE variable that contains the list of headers to be used with -include on the command line; also add CPPINCLUDE{PRE,SUF}FIX variables to expand to the correct wrapping. Names (and Files) in this list should be dependencies of the compile step. See issue #1107 for a patch that does most of this.

stevenknight said at 2009-02-22 07:41:44

Assigning to SK and target milestone per bug party triage.

stevenknight said at 2009-11-10 18:00:20

stevenknight => issues@scons

gregnoel said at 2009-12-09 12:06:40

*** Issue 2475 has been marked as a duplicate of this issue. ***

Votes for this issue: 10.

gregnoel said this issue is duplicated by #2475 at 2009-12-09 12:06:41.

bdbaddog avatar Feb 09 '18 16:02 bdbaddog

Oooh, this is an old issue. I've run into something similar... -isystem directives. The problem is once scons makes the committment to be "smart" it has to keep handling all of the special cases, most of which are compiler-specific, a pain when scons itself tries to be compiler agnostic as much as it can. I had a similar fight for about a decade with lsbcc, the LSB project's compiler wrapper to help make it easy to build conforming binaries - there's always a special case we didn't think of that a user figured out they needed.

gcc has a whole bunch of ways to affect the way headers are found: -include, -iquote, -isystem, -idirafter, -isysroot, etc. Since scons is going to look for headers it needs a way to understand these. Further, order matters: we can't add these paths to CPPPATH, because those get "transformed" by the addition of -I, but if you add them to a different construction variable you may end up getting include paths out of order. Don't have an answer for this.

FYI. my particular use-case for -isystem is a project that includes several other open source libraries, not under control of this project. The project wants to enforce a policy of warnings-treated-as-errors, but only for project code, which gcc allows by considering certain include paths as referring to system files - system headers get a more lenient treatment for the very same reason. So we want to use -isystem for those external projects, which we do by adding them to CPPFLAGS instead, which has several downsides - scons doesn't know about the headers (which has not proven a problem in practice because they change really slowly), and we have to special-case the linux+gcc case for this, and because CPPFLAGS isn't going to be processed further, we have to munge the path, something like this example (ugh):

if 'gcc' in env.get('CC'):
    env.AppendUnique(CPPFLAGS='-isystem ' + 
                     Dir('#extlibs/rapidjson/rapidjson/include/rapidjson').path)
else:
    env.AppendUnique(CPPPATH='#extlibs/rapidjson/rapidjson/include/rapidjson')

mwichmann avatar Jan 24 '19 19:01 mwichmann

This issue surfaced again on an scons chat recently. I'm trying to see if the relevant part of the patch in #1107 can be extracted. It doesn't include a test.

mwichmann avatar Aug 27 '19 23:08 mwichmann

Sorry about the verbosity of this message.

I have a solution for this issue that I use in my projects. I haven't made a pull request for this, however, I am happy to do so if someone thinks it is worthwhile.

Here is a sample SConstruct that adds support for -include and -iquote. NOTE: This is python3 only. Also the _defines function in the environment has changed recently so you may need to add ,TARGET, SOURCE to _defines arguments if you are using newer SCons.

import SCons


class _CScanner(SCons.Scanner.ClassicCPP):
    '''Adds support for -include to CScanner'''

    def __init__(self, *args, **kwargs):
        self.include_variable = kwargs.pop('include_variable', None)
        super().__init__(*args, **kwargs)
        def _scan(node, env, path=(), self=self):
            node = node.rfile()
            if not node.exists():
                return []
            return self.scan(node, path, env=env)
        self.function = _scan

    def get_includes(self, env):
        return env.Flatten(env.get(self.include_variable, []))
        
    def scan(self, node, *args, **kwargs):
        env = kwargs.pop('env', None)
        nodes = super().scan(node, *args, **kwargs)
        if env and node.get_suffix() in self.get_skeys(env):
            nodes += self.get_includes(env)
        return nodes


# Also support for multiple CPPPATH's so we can easily add support for
# '-iquote' and friends
CScanner = _CScanner(
    'CIncludeScanner',
    '$CPPSUFFIXES',
    'CPPPATHS',
    '^[ \t]*#[ \t]*(?:include|import)[ \t]*(<|")([^>"]+)(>|")',
    include_variable='CPPINCLUDE',
)


# replace the existing scanner for C suffixes
for suffix in SCons.Tool.CSuffixes:
    SCons.Tool.SourceFileScanner.add_scanner(suffix, CScanner)


# Create an environment that supports CPPINCLUDE '-include' and CPPQUOTEPATH
# '-iquote'. NOTE: '-isystem' etc can be added in a similar manner to the
# method used to add '-iquote', However '-include' needs special consideration
# (hence the modified scanner)
env = Environment(

    # '-include' support
    CPPINCLUDE=[],
    CPPINCPREFIX='-include ',
    CPPINCSUFFIX='',
    _CPPDEFFLAGS=' '.join([
        '${_defines(CPPDEFPREFIX, CPPDEFINES, CPPDEFSUFFIX, __env__)}',
        '${_defines(CPPINCPREFIX, CPPINCLUDE, CPPINCSUFFIX, __env__)}',
    ]),

    # '-iquote' support
    INCQUOTEPREFIX='-iquote ',
    INCQUOTESUFFIX='',
    CPPPATHS=['$CPPPATH', '$CPPQUOTEPATH'],
    _CPPINCFLAGS=' '.join(['$(',
        '${_concat(INCQUOTEPREFIX, CPPQUOTEPATH, INCQUOTESUFFIX, __env__, RDirs, TARGET, SOURCE)}',
        '${_concat(INCPREFIX, CPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)}',
    '$)']),
)


# Test '-include' and '-iquote'
env.Textfile('foo/time.h', r'''#include <time.h>
static inline time_t now(void) {
    return time(NULL);
}''')
autoconf_h = env.Textfile('autoconf.h', r'#define CONF_WHAT "stuff"')
test_c = env.Textfile('test.c', r'''
#include "time.h"
#include <stdio.h>
int main(void) {
    printf("what = " CONF_WHAT "\n");
    printf("now = %ld\n", now());
    return 0;
}''')
env.Append(
    CPPINCLUDE=[autoconf_h],
    CPPQUOTEPATH=['foo'],
)
test = env.Program('test', [test_c])
env.Alias('run', test, './$SOURCE')

Build result:

$ scons run
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
Creating 'test.c'
Creating 'foo/time.h'
Creating 'autoconf.h'
gcc -o test.o -c -include autoconf.h -iquote foo test.c
gcc -o test test.o
./test
what = stuff
now = 1632808347
scons: done building targets.

Build tree:

$ scons --tree=prune
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
scons: `.' is up to date.
+-.
  +-SConstruct
  +-autoconf.h
  | +-#define CONF_WHAT "stuff"
  +-foo
  | +-foo/time.h
  |   +-#include <time.h>
static inline time_t now(void) {
    return time(NULL);
}
  +-test
  | +-test.o
  | | +-test.c
  | | | +-
#include "time.h"
#include <stdio.h>
int main(void) {
    printf("what = " CONF_WHAT "\n");
    printf("now = %ld\n", now());
    return 0;
}
  | | +-[foo/time.h]
  | | +-[autoconf.h]
  | | +-/bin/gcc
  | +-/bin/gcc
  +-[test.c]
  +-[test.o]
scons: done building targets.

greenbender avatar Sep 28 '21 05:09 greenbender

Adding a note with linkages - precompiled headers are a current topic of interest (see #1435 and #4026), and the GCC documentation on using precompiled headers (https://gcc.gnu.org/onlinedocs/gcc/Precompiled-Headers.html) describes using -include as a way to add the precompiled header without modifying the source file. Seems like it wouldn't be great if such an inclusion wasn't factored into the dep calculations...

mwichmann avatar Sep 28 '21 14:09 mwichmann

We built a tool at MongoDB to handle -include, you can have a look at it here: https://github.com/mongodb/mongo/blob/master/site_scons/site_tools/forceincludes.py

acmorrow avatar Sep 28 '21 15:09 acmorrow

@mwichmann, we, at PlatformIO, also experience this issue, see https://github.com/platformio/platformio-core/issues/4578

Does it make sense to leverage MongoDB's tool https://github.com/mongodb/mongo/blob/master/site_scons/site_tools/forceincludes.py ?

ivankravets avatar Mar 20 '23 21:03 ivankravets

We should perhaps look at that.

I'd like to add to the initial comment:

If a GCC compile command line has one or more -include foo options, no dependency is created for the file foo. Although the use of this option is rare and tends to be to unchanging files...

Actually, these days -include is a recommended approach for including a precompiled header with GCC, as noted here in the project docs:

This also works with -include. So yet another way to use precompiled headers, good for projects not designed with precompiled header files in mind, is to simply take most of the header files used by a project, include them from another header file, precompile that header file, and -include the precompiled header.

So this is perhaps not as much of a corner case as originally described.

mwichmann avatar Mar 20 '23 21:03 mwichmann

Does it make sense to leverage MongoDB's tool https://github.com/mongodb/mongo/blob/master/site_scons/site_tools/forceincludes.py ?

I'd be very happy to see that work get upstreamed. I think the only missing piece is a way to achieve the ListScanner concept mentioned in the tool source. Right now SCons doesn't provide a good way to chain together multiple Scanners like it does for Emitters, and the forceincludes tool currently assumes that it will be the only target_scanner in play for the relevant nodes, which isn't a great assumption. I don't think such a widget is hard to make, we just didn't have a need in our particular setup, so we didn't spend time on it.

acmorrow avatar Mar 21 '23 00:03 acmorrow