Skip to content

Conversation

@dmoody256
Copy link
Contributor

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

@dmoody256 dmoody256 changed the title Added option to allow scons to determine if it should skip ninja regeneration. [NINJA] Added option to allow scons to determine if it should skip ninja regeneration. Jun 7, 2022
@bdbaddog bdbaddog added the Ninja label Jun 11, 2022
@bdbaddog bdbaddog added this to the 4.4 milestone Jun 11, 2022
import SCons
import random
SetOption('experimental','ninja')
SetOption('skip_ninja_regen', True)
Copy link
Contributor

Choose a reason for hiding this comment

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

If I comment this line out, the test still passes? Is that expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah actually this flag is not relevant to the current test. This PR makes ninja files deterministic, but the flag is for controlling skipping the regeneration and restarting of the daemon. The flag is to protect users who may be using Glob or SCons generated files in command lines. Its assumed if the user enables the flag, they know what they are doing.

I will update this test to also make sure the mtime of the ninja file did not change, which would indicate it was never overwritten.

Copy link
Contributor

Choose a reason for hiding this comment

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

o.k. so run this test with master branch code and it should fail then?

Copy link
Contributor Author

@dmoody256 dmoody256 Jun 15, 2022

Choose a reason for hiding this comment

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

yeah I tested that in the latest

@dmoody256
Copy link
Contributor Author

@bdbaddog fixed the notes, and updated the test

@bdbaddog bdbaddog merged commit 3b8e2b8 into SCons:master Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants