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

Modernize stat usage #4627

Merged
merged 4 commits into from
Dec 16, 2024
Merged

Modernize stat usage #4627

merged 4 commits into from
Dec 16, 2024

Conversation

mwichmann
Copy link
Collaborator

Since Python 2.2, the object returned by an os.stat() call presents attributes matching the 10-tuple of stat values. Use these instead of indexing into the tuple (which is now considered "backwards-compatibility" mode).

As usual for non-removed tests, minor tweaks made if needed - copyright header and DefautlEnvironment() call for performance.

This change has no visibility into docs.

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

@mwichmann mwichmann added the maintenance Tasks to maintain internal SCons code/tools label Nov 8, 2024
@mwichmann
Copy link
Collaborator Author

mwichmann commented Nov 8, 2024

Hmmm, problems went undetected because my dev machine wasn't reporting results properly due to previous virtualenv damage I thought I'd finished repairing. Will sort out shortly.

@mwichmann
Copy link
Collaborator Author

Ah... affects sconsign - stat_result returns floats for times, but using the backwards-compat indexing form returns ints, because that's what it did back in the earliest days. So that mismatch is tripping things up in the sconsign tests.

@mwichmann mwichmann force-pushed the maint/st_mode branch 2 times, most recently from 68e41a9 to 162cdcc Compare November 18, 2024 14:32
@bdbaddog
Copy link
Contributor

bdbaddog commented Dec 6, 2024

@mwichmann I merged from main with the intent to clear up the failing ninja tests and get an all pass on the CI tests.

Since Python 2.2, the object returned by an os.stat() call presents
attributes matching the 10-tuple of stat values. Use these instead of
indexing into the tuple.

As usual for non-removed tests, minor tweaks made if needed -
copyright header and DefautlEnvironment() call for performance.

Signed-off-by: Mats Wichmann <[email protected]>
If a stat_result object's st_*time attributes are used, a float is
returned instead of an int. Adjust sconsign test regexes to optionally
accept a .digits tail of the number.  Confirmed this works for both
indexed attributes (which return an int) and named attributes (float).

Signed-off-by: Mats Wichmann <[email protected]>
A previous change proposed flipping all of the Python stat references
from the older tuple index style (st[stat.ST_XXX]) to the more modern stat
structure references (st.st_xxx). This change rolls back the one place in
the actual SCons code (in Node/FS) that used the stat mtime attribute.
There's a small chance that switching scons versions back and forth
could cause some time comparison issues, since st_mtime is a float,
but for backwards compatibility reasons with ancient Python 2 versions
Python retained the integer behavior of the index form.  While problems
from this seem a low probability, avoid the issue for now.

Signed-off-by: Mats Wichmann <[email protected]>
@bdbaddog bdbaddog merged commit 0c21de9 into SCons:master Dec 16, 2024
8 of 10 checks passed
@mwichmann mwichmann added this to the NextRelease milestone Dec 16, 2024
@mwichmann mwichmann deleted the maint/st_mode branch December 16, 2024 10:22
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
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants