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

Change the default to error on missing SConscript #4391

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

mwichmann
Copy link
Collaborator

This completes a change begun in 3.0.2, when the behavior changed from "skip silently" to "skip but issue a warning"; that behavior was marked deprecated in 3.1. Use must_exist=False to get the old "skip silently" behavior - this option has been available since the change in 3.0.2, this PR flips the default for that kwarg from None meaning "False but warn", to True, meaning by default a UserError is raised for missing SConscript, the flag is now treated as boolean rather than tri-state since there is no further need for the warn behavior.

Fixes #3958

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

This completes a change begun in 3.0.2, when the behavior changed
from "skip silently" to "skip but issue a warning"; that behavior
was marked deprecated in 3.1.  Use must_exist=False to get the old
"skip silently" behavior.

Fixes SCons#3958

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann mwichmann added the maintenance Tasks to maintain internal SCons code/tools label Aug 6, 2023
@mwichmann
Copy link
Collaborator Author

Goody... revised test fails on Windows. Possibly path-separator thing.

@mwichmann
Copy link
Collaborator Author

Yes, indeed. That should be a straightforward fix:

STDERR =========================================================================
2c2
< scons: *** Fatal: missing SConscript 'missing/SConscript'
---
> scons: *** Fatal: missing SConscript 'missing\SConscript'
5c5
< scons: *** Fatal: missing SConscript 'missing/SConscript'
---
> scons: *** Fatal: missing SConscript 'missing\SConscript'

Make the path to the missing script in expected errors
be OS-independent to avoid Windows failing on wrong path separator

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann
Copy link
Collaborator Author

Grumble... still broken. Now it says

STDERR =========================================================================
2c2
< scons: *** Fatal: missing SConscript 'missing\\SConscript'
---
> scons: *** Fatal: missing SConscript 'missing\SConscript'
5c5
< scons: *** Fatal: missing SConscript 'missing\\SConscript'
---
> scons: *** Fatal: missing SConscript 'missing\SConscript'

Also dropped the word Fatal from the error, it's not consistent with
any other scons-generated exception.

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann
Copy link
Collaborator Author

This time I've actually tested it on Windows. Also dropped the word "Fatal" from the exception, SCons doesn't say that anywhere else (tests adjusted to match).

@bdbaddog bdbaddog merged commit d3f899a into SCons:master Aug 24, 2023
@mwichmann mwichmann added this to the NextRelease milestone Aug 24, 2023
@mwichmann mwichmann deleted the feature/missing-sconscript-error branch August 24, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Tasks to maintain internal SCons code/tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change SConscript() default behavoir to error if SConscript is not present.
2 participants