Skip to content

test_tarfile_vs_tar of test_shutil fails on macos Sonoma #109980

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

Closed
sobolevn opened this issue Sep 27, 2023 · 13 comments
Closed

test_tarfile_vs_tar of test_shutil fails on macos Sonoma #109980

sobolevn opened this issue Sep 27, 2023 · 13 comments
Labels
OS-mac tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Sep 27, 2023

Bug report

0:10:50 load avg: 1.97 [343/463/3] test_shutil
test test_shutil failed -- Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_shutil.py", line 1592, in test_tarfile_vs_tar
    self.assertEqual(self._tarinfo(tarball), self._tarinfo(tarball2))
AssertionError: Tuples differ: ('dist', 'dist/file1', 'dist/file2', 'dist/[31 chars]ub2') != ('._dist', 'dist', 'dist/._file1', 'dist/._[122 chars]ub2')

First differing element 0:
'dist'
'._dist'

Second tuple contains 6 additional elements.
First extra element 6:
'dist/file1'

- ('dist', 'dist/file1', 'dist/file2', 'dist/sub', 'dist/sub/file3', 'dist/sub2')
+ ('._dist',
+  'dist',
+  'dist/._file1',
+  'dist/._file2',
+  'dist/._sub',
+  'dist/._sub2',
+  'dist/file1',
+  'dist/file2',
+  'dist/sub',
+  'dist/sub/._file3',
+  'dist/sub/file3',
+  'dist/sub2')

Running locally on macos 14.0

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir OS-mac labels Sep 27, 2023
@ronaldoussoren
Copy link
Contributor

The difference appears to be archiving of extended attributes (given the names starting with "._"). Listing the extended attributes for the source directory could help find what's going on here.

I don't get this when running python3.12 -m test test_shutil (using the current python.org installer). Likewise for running the full test suite, although I do run into #109981 when doing that.

A fresh checkout of main (configure --with-pydebug) also passed ./python.exe -m test.test_shutil, and make test passes without problems (but: I don't have dependencies for C extensions installed).

All of this on an M1 macbook running Sonoma and using Xcode 15.

@sobolevn
Copy link
Member Author

sobolevn commented Sep 28, 2023

@ronaldoussoren I easily repoduce it on a fresh main build (m2):

» ./python.exe -m test test_shutil                         
0:00:00 load avg: 2.00 Run 1 test sequentially
0:00:00 load avg: 2.00 [1/1] test_shutil
test test_shutil failed -- Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Lib/test/test_shutil.py", line 1592, in test_tarfile_vs_tar
    self.assertEqual(self._tarinfo(tarball), self._tarinfo(tarball2))
AssertionError: Tuples differ: ('dist', 'dist/file1', 'dist/file2', 'dist/[31 chars]ub2') != ('._dist', 'dist', 'dist/._file1', 'dist/._[122 chars]ub2')

First differing element 0:
'dist'
'._dist'

Second tuple contains 6 additional elements.
First extra element 6:
'dist/file1'

- ('dist', 'dist/file1', 'dist/file2', 'dist/sub', 'dist/sub/file3', 'dist/sub2')
+ ('._dist',
+  'dist',
+  'dist/._file1',
+  'dist/._file2',
+  'dist/._sub',
+  'dist/._sub2',
+  'dist/file1',
+  'dist/file2',
+  'dist/sub',
+  'dist/sub/._file3',
+  'dist/sub/file3',
+  'dist/sub2')

test_shutil failed (1 failure)

== Tests result: FAILURE ==

1 test failed:
    test_shutil

Total duration: 758 ms
Total tests: run=137 failures=1 skipped=47
Total test files: run=1/1 failed=1
Result: FAILURE

@ronaldoussoren
Copy link
Contributor

Does adding subprocess.check_call(["xattr", "-r", base_dir]) on line 1591 (before the failing checkEqual call, but after checking that the invocation of the tar command) print any extended attributes?

Are there extended attributes on the source tree? (xattr -r /Users/sobolev/Desktop/cpython2)

@ronaldoussoren
Copy link
Contributor

The reason for asking to check for extended attributes: tar on macOS will by default archive extended attributes when creating a tar archive (e.g., "--xattr" is on by default for "tar -c").

This can be disabled by adding "--no-xattr" to the command-line, or possibly "--no-mac-metadata" if the problem is in extended ACLs. Those are Mac-specific arguments though, leading to more complicated test code.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 1, 2023

@ronaldoussoren adding the suggested line to 1591:

diff --git Lib/test/test_shutil.py Lib/test/test_shutil.py
index a2ca4df1358..dadc4735f7e 100644
--- Lib/test/test_shutil.py
+++ Lib/test/test_shutil.py
@@ -1588,6 +1588,7 @@ def test_tarfile_vs_tar(self):
                               stdout=subprocess.DEVNULL)
 
         self.assertTrue(os.path.isfile(tarball2))
+        subprocess.check_call(["xattr", "-r",  base_dir])
         # let's compare both tarballs
         self.assertEqual(self._tarinfo(tarball), self._tarinfo(tarball2))

It produces:

» ./python.exe -m test test_shutil  
0:00:00 load avg: 1.60 Run 1 test sequentially
0:00:00 load avg: 1.60 [1/1] test_shutil
xattr: No such file: dist
test test_shutil failed -- Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_shutil.py", line 1591, in test_tarfile_vs_tar
    subprocess.check_call(["xattr", "-r",  base_dir])
  File "/Users/sobolev/Desktop/cpython/Lib/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['xattr', '-r', 'dist']' returned non-zero exit status 1.

test_shutil failed (1 error)

== Tests result: FAILURE ==

1 test failed:
    test_shutil

Total duration: 753 ms
Total tests: run=137 skipped=47
Total test files: run=1/1 failed=1
Result: FAILURE

@gvanrossum
Copy link
Member

Just ran into this too. Can one of you come up with a fix?

@ronaldoussoren
Copy link
Contributor

Just ran into this too. Can one of you come up with a fix?

Maybe. Is this something you can reproduce?

I can reproduce the error with the following patch:

diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index d7061b2f9d..b5e4c05d74 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -1660,6 +1660,13 @@ def _create_files(self, base_dir='dist'):
     def test_tarfile_vs_tar(self):
         root_dir, base_dir = self._create_files()
         base_name = os.path.join(self.mkdtemp(), 'archive')
+
+        subprocess.check_call([
+            'chmod', '+a', "admin allow write,chown,directory_inherit",
+            f"{root_dir}/dist/sub2"])
+
+        subprocess.check_call(["ls", "-@lRre", root_dir])
+
         with no_chdir:
             tarball = make_archive(base_name, 'gztar', root_dir, base_dir)
 

Running as ./python.exe -m test test_shutil. This adds an ACL to one of the directory and calls ls to list the to be archived directory.

If you can reproduce this at will I'm interested in the output of ls without the check_call of chmod.

@ronaldoussoren
Copy link
Contributor

And if anyone can reproduce the issue with my patch, the following should fix it:

diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index d7061b2f9d..b5e4c05d74 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -1670,9 +1677,13 @@ def test_tarfile_vs_tar(self):
         # now create another tarball using `tar`
         tarball2 = os.path.join(root_dir, 'archive2.tar')
         tar_cmd = ['tar', '-cf', 'archive2.tar', base_dir]
+        if sys.platform == 'darwin':
+            tar_cmd.insert(1, '--no-mac-metadata')
         subprocess.check_call(tar_cmd, cwd=root_dir,
                               stdout=subprocess.DEVNULL)

The line numbers are a bit off, the patch is on top of my previous one. I haven't created a PR yet because I haven't checked yet how far back the --no-mac-metadata flag is valid on macOS.

@sobolevn
Copy link
Member Author

sobolevn commented Dec 9, 2023

I cannot reproduce it anymore.

@ronaldoussoren I believe that this flag exists:

     --mac-metadata
             (c, r, u and x mode only) Mac OS X specific.  Archive or extract extended
             ACLs and extended file attributes using copyfile(3) in AppleDouble format.
             This is the reverse of --no-mac-metadata.  and the default behavior in c, r,
             and u modes or if tar is run in x mode as root.

ronaldoussoren added a commit to ronaldoussoren/cpython that referenced this issue Dec 9, 2023
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
@ronaldoussoren
Copy link
Contributor

The attached PR uses tar --no-mac-metadata on macOS 11 and later. I've checked that macOS 11 has that argument, and that macOS 10.14 doesn't have the argument. That only leaves 10.15 to check, I can do that tomorrow.

@ronaldoussoren
Copy link
Contributor

I cannot reproduce it anymore.

I'd love to find what's causing the test failure. Something must cause additional metadata to be present in the to-be-archived directory, and given the test error this is likely an inherited ACL on the directory used by tempfile.mkdtemp. But why? It is rather annoying I don't get this failure myself 😉

@gvanrossum
Copy link
Member

It happens every time on the unchanged main branch when I run this:

~/cpython$ ./python.exe -m test test_shutil -v -m test_tarfile_vs_tar

This is on a new M3 Macbook Pro running Sonoma 14.1.2. On Monday I can test on an older i86 Mac running macOS 13.something.

@gvanrossum
Copy link
Member

I should add that, while the Mac is brand new, when I configured it I let it copy all the filesystem contents from my old Intel Mac; this was so thorough that initially the compiler seemed to be building i86 code. Possibly this did something to the temp dir ACL so as to cause the problem. Anyway, your PR fixes it.

ronaldoussoren added a commit that referenced this issue Dec 10, 2023
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 10, 2023
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
(cherry picked from commit dd2ebdf)

Co-authored-by: Ronald Oussoren <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 10, 2023
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
(cherry picked from commit dd2ebdf)

Co-authored-by: Ronald Oussoren <[email protected]>
ronaldoussoren added a commit that referenced this issue Dec 10, 2023
gh-109980: Fix test_tarfile_vs_tar on macOS (GH-112905)

On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
(cherry picked from commit dd2ebdf)

Co-authored-by: Ronald Oussoren <[email protected]>
ronaldoussoren added a commit that referenced this issue Dec 10, 2023
gh-109980: Fix test_tarfile_vs_tar on macOS (GH-112905)

On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
(cherry picked from commit dd2ebdf)

Co-authored-by: Ronald Oussoren <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
On recentish macOS versions the system tar
command includes system metadata (ACLs, extended attributes
and resource forks) in the tar archive, which
shutil.make_archive will not do. This can cause
spurious test failures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-mac tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants