Skip to content

Conversation

@mwichmann
Copy link
Collaborator

The mocked tools mylex.py and myyacc.py now understand the file-generation options, and generate a dummy file with predictable contents, for checking. This allows more testing of the path through the SCons support for these two without needing live commands.

New tests added which invoke the file-generation options, and make sure the extra files are created, and that SCons detects and tracks the added targets. Work is done in a subdirectory, which exposes some existing known inconsistent behavior (the regular generated file goes in the subdir per the LEXCOM and YACCOM generated line, while the ones generated from command line options go in the topdir) - but we're going to allow that behavior to continue for backwards compat.

Same fix applied to yacc tool that PR #4168 did for lex - do subst_list() instead of subst() to preserve spaces in paths. That fix left the lex tool unable to pass the new test, as it could not see the individual arguments in the FLAGS variable, which was solved by indexing into the subst'd list so we can iterate over the args again.

Test and tool cleanup; add DefaultEnvironment calls, etc.

Note this mentions, but does not address the problem described in issue 4154.

Signed-off-by: Mats Wichmann [email protected]

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

The mocked tools mylex.py and myyacc.py now understand the file-generation
options, and generate a dummy file with predictable contents, for
checking.  This allows more testing of the path through the SCons support
for these two without needing live commands.

New tests added which invoke the file-generation options, and make
sure the extra files are created, and that SCons detects and tracks
the added targets.  Work is done in a subdirectory, which exposes some
existing known inconsistent behavior (the regular generated file goes
in the subdir per the LEXCOM and YACCOM generated line, while the ones
generated from commandline options go in the topdir) - but we're going
to allow that behavior to continue for backwards compat.

Same fix applied to yacc tool that PR SCons#4168 did for lex - do subst_list()
instead of subst() to preserve spaces in paths.  That fix left the lex
tool unable to pass the new test, as it could not see the individual
arguments in the FLAGS variable, which was solved by indexing into the
subst'd list so we can iterate over the args again.

Test and tool cleanup; add DefaultEnvironment calls, etc.

Note this mentions, but does not address the problem described in issue 4154.

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann mwichmann added the Tools Issues related to tools subsystem label Jun 30, 2022
Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann
Copy link
Collaborator Author

Question: issue #4154 is mentioned in code comments, but not in the updated manpage sections. This is because there's no replacement yet - didn't feel like saying "this won't be completely correct if you are in a subdirectory (including variant directory), be we can't tell you what to use instead". Should it be mentioned in some way?

but was not applied to other dialects, and e2e tests explicitly checked
that FORTRANFLAGS did not propagate outside the FORTRAN dialect,
so the conclusion is that behavior is intentional (issue #2257)
- Improvements to lex and yacc tools: better documentation of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add which flags are interpretted correctly now? also add blurb to RELEASE.txt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only behavioral change is in Tool/lex.py: the previous lex PR left us with a list-of-lists after the change from subst to subst_list on LEXFLAGS, which meant we couldn't iterate over the results. That's not material for RELEASE - the briefly broken bit was never visible in an SCons release. I don't know if you want that mentioned in CHANGES: fix interpretation of LEXFLAGS in one very specific circumstance (previously untested) which was broken for a few days. Let me know...

@bdbaddog bdbaddog merged commit 4e70c9f into SCons:master Jul 19, 2022
@mwichmann mwichmann added this to the 4.4 milestone Jul 19, 2022
@mwichmann mwichmann deleted the maint/lex_yacc branch July 19, 2022 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tools Issues related to tools subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants