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

Non-conftest nodes involved in configure checks now get node info cleared after check. #4161

Merged
merged 8 commits into from
Jan 29, 2023

Conversation

dmoody256
Copy link
Contributor

Fixes #2757

from https://pairlist4.pair.net/pipermail/scons-users/2022-May/008989.html

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

@@ -84,10 +84,10 @@
%(sig_re)s \[.*\]
conftest_%(sig_re)s_0_%(sig_re)s%(_obj)s:
%(_sconf_temp_conftest_0_c)s: %(sig_re)s \d+ \d+
%(CC)s: %(sig_re)s \d+ \d+
%(CC)s: %(sig_re)s None None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the compiler which is considered a non-conftest node, so if only configure checks are run, then the node info is not stored.

int test_header = 2;
""")

test.not_up_to_date(read_str="Checking for C header file header1.h... (cached) yes\n")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested before and after and verified this test fails with master, and passed with the PR

@dmoody256
Copy link
Contributor Author

Not sure about the sider issue, should the code style remain consistent?

@bdbaddog
Copy link
Contributor

What happens when you run scons twice in a row where a real file is used by configure, but nothing depends on it?
Does the configure step get rerun?

@bdbaddog
Copy link
Contributor

Not sure about the sider issue, should the code style remain consistent?

punt on this one. There'd be too much to fix.. We'll do a black run on the whole codebase at some point and then enforce.

@dmoody256
Copy link
Contributor Author

What happens when you run scons twice in a row where a real file is used by configure, but nothing depends on it?
Does the configure step get rerun?

No it uses the cached value. I updated the test to show this.

@mwichmann mwichmann added the Configure any issues related to Configure contexts label Jun 9, 2022
@mwichmann mwichmann changed the title Fix for #2757, non conftest nodes involved in configure checks now get node info cleared after check. Non-conftest nodes involved in configure checks now get node info cleared after check. Aug 4, 2022
@bdbaddog
Copy link
Contributor

@dmoody256 - so from looking at the code, I think the only possible functional change is that if you were to only run the configure step and no actual build steps the sconsign would be missing the content signature information for all non-configure generated files uses while processing the configure checks?

Does that sound correct?

@dmoody256
Copy link
Contributor Author

@dmoody256 - so from looking at the code, I think the only possible functional change is that if you were to only run the configure step and no actual build steps the sconsign would be missing the content signature information for all non-configure generated files uses while processing the configure checks?

Does that sound correct?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configure any issues related to Configure contexts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SConf marks files up-to-date while using them in checks
3 participants