Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle --debug containing memoizer #4677

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

mwichmann
Copy link
Collaborator

The memoizer statistics have to be handled specially as they use conditional decorators, those have to be enabled before any of the decorated methods are read by Python. This worked if there was exactly --debug=memoizer on the commandline or in SCONSFLAGS, but not if it was in a multi-value option like --debug=presub,memoizer. Handle the up-front-check a little more completely to fix this.

No doc impacts, documented behavior not changed.

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 RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

The memoizer statistics have to be handled specially as they use
conditional decorators, those have to be enabled before any of the
decorated methods are read by Python.  This worked if there was exactly
"--debug=memoizer" on the commandline or in SCONSFLAGS, but not if it
was in a multi-value option like "--debug=presub,memoizer".
Handle the up-front-check a little more completely to fix this.

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann mwichmann added the args_and_options options processing, arguments, get/setoption and their relationshiop label Feb 3, 2025
@@ -51,36 +48,35 @@ def cat(target, source, env):
# names in the implementation, so if we change them, we'll have to
# change this test...
expect = [
"Memoizer (memory cache) hits and misses",
"Base.stat()",
# "Memoizer (memory cache) hits and misses",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one commented out because it prints even if the memoizer failed to initialize, since the actual heading/print-when-done is handled by the normal option processing, not the special up-front processing. Since the match requires "at least one of" it would still match and obscure the failure in the combined-debug-option case.

@bdbaddog bdbaddog merged commit 797ce27 into SCons:master Feb 3, 2025
7 of 8 checks passed
@mwichmann mwichmann added this to the 4.9 milestone Feb 3, 2025
@mwichmann mwichmann deleted the issue/debug-memoizer branch February 3, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
args_and_options options processing, arguments, get/setoption and their relationshiop
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants