scons icon indicating copy to clipboard operation
scons copied to clipboard

`LEX_HEADER_FILE` doesn't work correctly when there is more than one lex source in the construction environment

Open piotrsiupa opened this issue 3 years ago • 7 comments

Describe the bug I've set LEX_HEADER_FILE to a function that takes source[0] and modifies it to get the custom target path. When there is multiple CXXFile builders, they call the function only for the first source file. For the remaining ones, they appear to use the result of the first call instead of calling the function again.

Required information

  • Link to SCons Users thread discussing your issue: https://discord.com/channels/571796279483564041/571796280146133047/1004464208726990848
  • Version of SCons: v4.4.0.fc8d0ec215ee6cba8bc158ad40c099be0b598297, Sat, 30 Jul 2022 14:11:34 -0700, by bdbaddog on M1Dog2021
  • Version of Python: 3.8.10
  • Which python distribution if applicable (python.org, cygwin, anaconda, macports, brew,etc): the one from the Linux Mint's apt repository
  • How you installed SCons: pip install scons
  • What Platform are you on? (Linux/Windows and which version): Linux Mint 20.3 Una
  • How to reproduce your issue? Please include a small self contained reproducer. Likely a SConstruct should do for most issues.
env = DefaultEnvironment()

def make_target(env, target, source, for_signature):
    print('source:', source[0])
    new_target = env.File(str(source[0]) + '.xyz')
    print('new target:', new_target)
    return new_target
print('Calling `env.Replace()` for "LEX_HEADER_FILE"...')
env.Replace(LEX_HEADER_FILE=make_target)

print('Calling `env.CXXFile()` for "foo.ll"...')
env.CXXFile('foo.ll')
print('Calling `env.CXXFile()` for "bar.ll"...')
env.CXXFile('bar.ll')

print('The script has finished.')
  • How you invoke scons (The command line you're using "scons --flags some_arguments")
scons -Q

piotrsiupa avatar Aug 03 '22 19:08 piotrsiupa

This is probably correct. As an attempted workaround, can you set the variable in the builder call (I have no idea if it will work). Will need to investigate, but when adding this did not anticipate that people were going to use multiple different files, so we probably ended up with something that evaluates only once, and at sconscript time at that, not at build time.

I'm thinking of something like:

env.CXXFILE('foo.ll', LEX_HEADER_FILE='foo.ll.xyz')
env.CXXFILE('bar.ll', LEX_HEADER_FILE='bar.ll.xyz')

mwichmann avatar Aug 03 '22 20:08 mwichmann

I looked into the code of the Lex builder.

It seems that these two lines in the Lex emitter are at fault: https://github.com/SCons/scons/blob/4aaa505e7f5e3f0f75c2d17d91afa3a7162f69f9/SCons/Tool/lex.py#L87 https://github.com/SCons/scons/blob/4aaa505e7f5e3f0f75c2d17d91afa3a7162f69f9/SCons/Tool/lex.py#L93 They change values of the construction variables LEX_HEADER_FILE and LEX_TABLES_FILE from functions (or whatever is there at the moment) to pre-calculated nodes. Unfortunately, these new values are required because SCons assumes that the paths of files are relative to SConscript's directory, not the main directory of the build. If they were calculated later, that assertion would not be true. The lines that would not work without replacing values are: https://github.com/SCons/scons/blob/4aaa505e7f5e3f0f75c2d17d91afa3a7162f69f9/SCons/Tool/lex.py#L161 https://github.com/SCons/scons/blob/4aaa505e7f5e3f0f75c2d17d91afa3a7162f69f9/SCons/Tool/lex.py#L162

I attempted to fix the issue without any luck. It's gonna require better knowledge of the code base than I have.

piotrsiupa avatar Aug 05 '22 16:08 piotrsiupa

Then maybe use the "old way" - embedding the options in LEXFLAGS? The whole reason for introducing the new variables was to provide a way for these generated extra files to be tracked relative to the directory of the sconscript, rather than relative to the top of the build - tracking them from the top of the build made a mess when you were in a subdirectory (such as any time a variant dir was in effect). Unless I'm misunderstanding what you're after...

mwichmann avatar Aug 05 '22 16:08 mwichmann

To add - it's quite true that the force-rewrite of the variables doesn't work for passing a function - as mentioned earlier, had never considered that case. If that's a use-case we need to support, then some thought will definitely be needed.

mwichmann avatar Aug 05 '22 16:08 mwichmann

Yeah, for now I'm using the old way. I haven't tested it but I'm assuming the method with setting the construction variable in each call of the builder would also work.

Honestly, I don't really care to which directory the paths are relative as long as it works reliably. The reason for introduction those new variables was the fact that SCons was misinterpreting --header-file and --tables-file in LEXFLAGS which caused problems with the build and required weird workarounds. Currently I still use these hacks.

I wanted to switch to the new and cleaner way and then I found this bug. In my project, there are 3 parsers using Lex and one Lex lexer that is not a part of a parser. However, the Lex builder assumes that there is only one lex file in the project.

piotrsiupa avatar Aug 05 '22 16:08 piotrsiupa

To add - it's quite true that the force-rewrite of the variables doesn't work for passing a function - as mentioned earlier, had never considered that case. If that's a use-case we need to support, then some thought will definitely be needed.

If you want to do it only for my sake, I would recommend to just stop accepting functions in those variables, as I already have my awful hacks that work reliably. Although, I don't know if it can be done, especially without making construction variables work inconsistently between builders.

piotrsiupa avatar Aug 05 '22 16:08 piotrsiupa

Well, f... I have some bad news. I just thought to check what would happen if I used a normal subst text concatenation instead of a function. The error is still there. It seems that the only thing LEX_HEADER_FILE works with is a constant path, that is exactly the same for each lex file in the construction environment. So effectively there may be only one lex file in each environment regardless of what is the content of LEX_HEADER_FILE.

Here is the new and simpler example:

env = DefaultEnvironment()

print('Calling `env.Replace()` for "LEX_HEADER_FILE"...')
env.Replace(LEX_HEADER_FILE='${SOURCE}.xyz')

print('Calling `env.CXXFile()` for "foo.ll"...')
env.CXXFile('foo.ll')
print('Calling `env.CXXFile()` for "bar.ll"...')
env.CXXFile('bar.ll')

print('The script has finished.')

For the sake of completeness, I've tested:

env.CXXFile('foo.ll', LEX_HEADER_FILE='foo.ll.xyz')
env.CXXFile('bar.ll', LEX_HEADER_FILE='bar.ll.xyz')

This seems to work fine.

piotrsiupa avatar Aug 05 '22 17:08 piotrsiupa