-
-
Notifications
You must be signed in to change notification settings - Fork 356
Testing: Update the packaging tar bz2 and xz tests on windows #4741
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
Changes from 3 commits
bc87c18
4337335
8258c87
bb5b162
188875b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,10 @@ | |
| This tests the SRC xz packager, which does the following: | ||
| - create a tar package from the specified files | ||
| """ | ||
| import os | ||
| import os.path | ||
| import subprocess | ||
| import sys | ||
| import TestSCons | ||
|
|
||
| python = TestSCons.python | ||
|
|
@@ -38,14 +41,75 @@ | |
| if not tar: | ||
| test.skip_test('tar not found, skipping test\n') | ||
|
|
||
| # Windows 10 now supplies tar, but doesn't support xz compression | ||
| # assume it's just okay to check for an xz command, because don't | ||
| # want to probe the command itself to see what it supports | ||
| xz = test.where_is('xz') | ||
| if not xz: | ||
| test.skip_test('tar found, but helper xz not found, skipping test\n') | ||
| if sys.platform == 'win32': | ||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probable makes sense to move this logic into TestSCons?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well that didn't work as I had hoped. The source file Seems unfortunate to have to add to TestSCons for two tests.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can exclude |
||
| # for liblzma support. | ||
|
|
||
| def windows_system32_bsdtar_have_xz(tar): | ||
|
|
||
| # Don't skip test if not confident this is the windows/system32/tar.exe (bsdtar). | ||
|
|
||
| tar = os.path.normcase(os.path.abspath(tar)) | ||
|
|
||
| # windows tar.exe: | ||
| # %systemroot%/system32/tar.exe | ||
|
|
||
| windir = os.environ.get("SystemRoot") | ||
| if not windir: | ||
| windir = os.environ.get("windir") | ||
| if windir: | ||
| expected = os.path.normcase(os.path.abspath(os.path.join(windir, "System32", "tar.exe"))) | ||
| if tar != expected: | ||
| return False | ||
|
|
||
| try: | ||
| result = subprocess.run([f"{tar}", "--version"], capture_output=True, text=True, check=True) | ||
| version_str = result.stdout.strip() | ||
| except: | ||
| version_str = None | ||
|
|
||
| if not version_str: | ||
| return False | ||
|
|
||
| # print(f"{tar} --version => {version_str!r}") | ||
|
|
||
| # tar.exe --version (Windows 10, GH windows-2022): | ||
| # bsdtar 3.5.2 - libarchive 3.5.2 zlib/1.2.5.f-ipp | ||
|
|
||
| # tar.exe --version (Windows 11): | ||
| # bsdtar 3.7.7 - libarchive 3.7.7 zlib/1.2.5.f-ipp liblzma/5.4.3 bz2lib/1.0.8 libzstd/1.5.4 | ||
|
|
||
| # tar.exe --version (GH windows-2025): | ||
| # bsdtar 3.7.7 - libarchive 3.7.7 zlib/1.2.13.1-motley liblzma/5.4.3 bz2lib/1.0.8 libzstd/1.5.5 | ||
|
|
||
| version_comps = version_str.split() | ||
| if len(version_comps) < 5: | ||
| return False | ||
|
|
||
| for indx, expected in [ | ||
| (0, "bsdtar"), (2, "-"), (3, "libarchive"), | ||
| ]: | ||
| if version_comps[indx].lower() != expected: | ||
| return False | ||
|
|
||
| # Reasonably confident this is the windows/system32/tar.exe (bsdtar). | ||
|
|
||
| # bsdtar_version = version_comps[1] | ||
| # libarchive_version = version_comps[4] | ||
|
|
||
| for component in version_comps[5:]: | ||
| if component.lower().startswith("liblzma/"): | ||
| # Don't skip test: xz/liblzma appears to be supported. | ||
| return False | ||
|
|
||
| # Skip test: xz/liblzma appears to be unsupported. | ||
| return True | ||
|
|
||
| skip_test = windows_system32_bsdtar_have_xz(tar) | ||
| if skip_test: | ||
| test.skip_test('windows tar found; xz not supported, skipping test\n') | ||
|
|
||
| test.subdir('src') | ||
|
|
||
|
|
@@ -60,14 +124,11 @@ | |
| Program( 'src/main.c' ) | ||
| env=Environment(tools=['packaging', 'filesystem', 'tar']) | ||
|
|
||
| # needed for windows to prevent picking up windows tar and thinking non-windows bzip2 would work. | ||
| env.PrependENVPath('PATH', r'%s') | ||
|
|
||
| env.Package( PACKAGETYPE = 'src_tarxz', | ||
| target = 'src.tar.xz', | ||
| PACKAGEROOT = 'test', | ||
| source = [ 'src/main.c', 'SConstruct' ] ) | ||
| """%xz_path) | ||
| """) | ||
|
|
||
| test.run(arguments='', stderr=None) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
%SystemRoot%/system32/tar.exe.%SystemRoot%/system32/tar.exesubject 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,-libarchiveand 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test, as written prior to changes, only checks the first tar executable found on the path.
There was a problem hiding this comment.
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)
Additional clause added to always test the target executable path against what should be the windows system32 path.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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: