Skip to content

Testing: Update the packaging tar bz2 and xz tests on windows - #4741

Merged
bdbaddog merged 5 commits into
SCons:masterfrom
jcbrill:jbrill-wintest-tar
Jul 6, 2025
Merged

Testing: Update the packaging tar bz2 and xz tests on windows#4741
bdbaddog merged 5 commits into
SCons:masterfrom
jcbrill:jbrill-wintest-tar

Conversation

@jcbrill

@jcbrill jcbrill commented Jul 6, 2025

Copy link
Copy Markdown
Contributor

Rework the windows system tar executable handling in the packaging tar tests for bz2 and xz. Remove test test/packaging/tar/xz_packaging.py from the windows runner skip file.

Changes:

  • test/packaging/tar/bz2_packaging.py:
    • Remove searching for and prepending bzip2.exe to the environment path for windows.
    • tar.exe in Windows 10 and GH windows-2022 appears not to support bzip2 compression even when bzip2.exe is on the system path.
    • tar.exe in Windows 11 and GH windows-2025 appears to support bzip2 compression without any additions to the system path.
    • If the candidate tar.exe is the windows system tar.exe, check the version string for bz2lib support.
  • test/packaging/tar/xz_packaging.py:
    • Remove searching for and prepending xz.exe to the environment path for windows.
    • tar.exe in Windows 10 and GH windows-2022 appears not to support xz compression even when xz.exe is on the system path.
    • tar.exe in Windows 11 and GH windows-2025 appears to support xz compression without any additions to the system path.
    • If the candidate tar.exe is the windows system tar.exe, check the version string for liblzma support.
  • Remove test/packaging/tar/xz_packaging.py from the windows ci test skip file.

As of 06-Jul-2025:

  • The packaging tar tests for bz2 and xz are skipped in GH windows-2022.
  • The packaging tar tests for bz2 and xz are run in GH windows-2025.

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

jcbrill added 2 commits July 5, 2025 21:58
…z/liblzma handling.

Changes:
* Remove searching for and prepending xz.exe to the environment path for windows.
* tar.exe in Windows 10 and GH windows-2022 don't appear to support xz even if on the system path.
* tar.exe in Windows 11 and GH windows-2025 appears to support xz without additions to the system path.
* If the candidate tar.exe is the windows tar.exe, check the version string for liblzma support.
* Remove test/packaging/tar/xz_packaging.py from thw windows test skip file.
…bz2/bz2lib handling.

Changes:
* Remove searching for and prepending bzip2.exe to the environment path for windows.
* tar.exe in Windows 10 and GH windows-2022 don't appear to support bzip2 even if on the system path.
* tar.exe in Windows 11 and GH windows-2025 appears to support bzip2 without additions to the system path.
* If the candidate tar.exe is the windows tar.exe, check the version string for bz2lib support.
* Strip the tar versions strings before processing.
@jcbrill jcbrill changed the title [WIP] Test: Update the packaging tar.xz test on windows [WIP] Test: Update the packaging tar xz test on windows Jul 6, 2025
@jcbrill jcbrill changed the title [WIP] Test: Update the packaging tar xz test on windows [WIP] Test: Update the packaging tar bz2 and xz tests on windows Jul 6, 2025
… xz tests updates and GitHub workflow changes on Windows.
@jcbrill jcbrill changed the title [WIP] Test: Update the packaging tar bz2 and xz tests on windows Testing: Update the packaging tar bz2 and xz tests on windows Jul 6, 2025
bz2 = test.where_is('bzip2')

if sys.platform == 'win32':
# windows 10 causes fresh problems by supplying a tar, not bzip2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you ignoring the git tar? Which could be used in the case where the windows tar doesn't support bz2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't.

When either os.environ["SystemRoot"] or os.environ["windir"] is defined:

  • The test is performed if the tar.exe is not %SystemRoot%/system32/tar.exe.
  • The test may be performed if the tar.exe is %SystemRoot%/system32/tar.exe subject to the version string check.

One of these two environment variables should be defined.

If neither variables are defined, then the "--version" string is inspected for bsdtar, - libarchive and other fields. If there is not a match for the known fields, then the test is performed.

It should be unlikely that a bsdtar executable is used when the two Windows system environment variables are undefined.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which could be used in the case where the windows tar doesn't support bz2?

The test, as written prior to changes, only checks the first tar executable found on the path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Locally annotated change (not pushed yet)

        windir = os.environ.get("SystemRoot")
        if not windir:
            windir = os.environ.get("windir")
        if not windir:  # <== ADDED
            windir = os.path.join(os.environ.get("SystemDrive", "C:") + os.path.sep, "Windows")  # <== ADDED
        expected = os.path.normcase(os.path.abspath(os.path.join(windir, "System32", "tar.exe")))
        if tar != expected:
            return False

Additional clause added to always test the target executable path against what should be the windows system32 path.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't windir always defined on windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should be.

Depends on your level of paranoia:

(3.12.9) S:\SCons\Test-WinSkip>type test-env.bat
@setlocal
@python -c "import os; print([os.environ.get(var) for var in ['SystemDrive', 'SystemRoot', 'windir']])"
@set SystemDrive=
@set SystemRoot=
@set windir=
@python -c "import os; print([os.environ.get(var) for var in ['SystemDrive', 'SystemRoot', 'windir']])"
@endlocal

(3.12.9) S:\SCons\Test-WinSkip>test-env
['C:', 'C:\\Windows', 'C:\\Windows']
[None, None, None]

Comment thread test/packaging/tar/xz_packaging.py Outdated

xz_path = os.path.dirname(xz)
# Windows 10 and later supplies windows/system32/tar.exe (bsdtar).
# Not all versions support xz compression. Check the version string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probable makes sense to move this logic into TestSCons?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After copy/pasting/modifying from the xz source code it seemed like it would be a good idea to have only one parameterized implementation.

Wasn't sure where or what name to use.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to move common windows system tar code in bz2 and xz scripts to a private common module in the packaging/tar folder.

It should be easier now to move somewhere else if desired. At present, only used in two places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well that didn't work as I had hoped. The source file test/packaging/tar/_windows_tar.py was "run" as a test (which passes) which is undesirable.

Seems unfortunate to have to add to TestSCons for two tests.

@bdbaddog bdbaddog Jul 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bdbaddog bdbaddog added Op Sys: Windows testsuite Things that only affect the SCons testing. Do not use just because a PR has tests. labels Jul 6, 2025
@bdbaddog

bdbaddog commented Jul 6, 2025

Copy link
Copy Markdown
Contributor

Good enough. Merging.

@bdbaddog
bdbaddog merged commit 84842be into SCons:master Jul 6, 2025
9 of 11 checks passed
@jcbrill

jcbrill commented Jul 6, 2025

Copy link
Copy Markdown
Contributor Author

@bdbaddog Did you see the last commit about a minute before the merge?

@bdbaddog

bdbaddog commented Jul 6, 2025

Copy link
Copy Markdown
Contributor

@bdbaddog Did you see the last commit about a minute before the merge?

The TestSConsTar.py.. Yes.

At this point, enough CPU cycles spent for this issue. Seems good enough and merged.

@jcbrill
jcbrill deleted the jbrill-wintest-tar branch July 25, 2025 21:01
@mwichmann mwichmann added this to 4.10 Sep 30, 2025
@mwichmann mwichmann moved this to Done in 4.10 Sep 30, 2025
@mwichmann mwichmann added this to the 4.10.0 milestone Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Op Sys: Windows testsuite Things that only affect the SCons testing. Do not use just because a PR has tests.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants