improve portability of reproducible tarballs by replacing external `tar` command with `tarfile` module #4660

lexming · 2024-09-27T13:55:27Z

fixes #4657

~~use more portable --date argument for touch~~
~~catch failed commands inside the pipeline~~
~~move generation of command to make reproducible archives intro its own method~~
~~replace harcoded pattern in tests of reproducible archives command for call to filetools.reproducible_archive_cmd~~
make new implementation of reproducible_archive_cmd using the tarfile module
added new filetools.make_archive() method and related unit test
change handling of filename argument in filetools.get_source_tarball_from_git() to expect filenames without extension
- archive extension is appended as needed by make_archive()
- known archive extensions are automatically removed from filename if it contains one
add required argument to filetools.find_extensions()
checksums of sources from git repos are only verified with Python 3.9+

…ble tarballs

…d in filetools

…o filetools.reproducible_archive_cmd

test/framework/filetools.py

…eate archives of git repos

…_archive addition

…ducible archives

lexming · 2024-11-06T07:50:35Z

@boegel This one is ready. As discussed, archives will be made with tarfile instead of the tar command and checksums of sources from git repos will only be checked with Python 3.9+.

…syBuild version 6.0

boegel · 2024-11-13T10:57:32Z

easybuild/tools/filetools.py


    # cleanup (repo_name dir does not exist in dry run mode)
    remove(tmpdir)

    return archive_path


+def make_archive(dir_path, archive_name=None, archive_dir=None, reproducible=False):


I would rename this to make_tar_xz, since we're creating a very specific archive (a tarball), compressed with a hardcoded compression algorithms (XZ).

Or maybe even make_reproducible_tar_xz

boegel · 2024-11-13T11:09:20Z

easybuild/framework/easyblock.py

        if isinstance(filename, dict):
-            filename = filename['filename']
+            chksum_input = filename['filename']


@lexming Should we be more careful here, and also use filename.get('filename', None) to avoid crashing if the filename key is not set?

boegel · 2024-11-13T11:10:16Z

easybuild/framework/easyblock.py

+            # checksums of tarballs made by EB of git repos cannot be reliably checked prior to Python 3.9
+            if chksum_input_git is not None:
+                self.log.deprecated(
+                    "Reproducible tarballs of git repos are only supported in Python 3.9+. "


Suggested change

"Reproducible tarballs of git repos are only supported in Python 3.9+. "

"Reproducible tarballs of Git repos are only possible when using Python 3.9+ to run EasyBuild. "

boegel · 2024-11-13T11:13:28Z

easybuild/tools/filetools.py

-        raise EasyBuildError("git_config currently only supports filename ending in .tar.gz")
+    file_ext = find_extension(filename, required=False)
+    if file_ext:
+        print_warning(f"Ignoring extension of filename '{filename}' set in git_config parameter")


Isn't this going to require cleaning up a whole bunch of easyconfigs?

Rather than printing a warning, we should enforce the use of .tar.xz as extension here (to "ease" the transition, in some way)

boegel · 2024-11-13T11:16:24Z

easybuild/tools/filetools.py

+    a reproducible archive. Other formats like .gz are not reproducible due to
+    arbitrary strings and timestamps getting added into their metadata.


Hmm, this is new to me, on what source is this based, do we have a reference we can point to?

boegel · 2024-11-13T11:17:22Z

easybuild/tools/filetools.py

+        # reset file permissions by applying go+u,go-w
+        user_mode = tarinfo.mode & stat.S_IRWXU
+        group_mode = (user_mode >> 3) & ~stat.S_IWGRP  # user mode without write
+        other_mode = group_mode >> 3  # user mode without write


Suggested change

other_mode = group_mode >> 3 # user mode without write

other_mode = group_mode >> 3 # same as group mode

boegel · 2024-11-13T11:18:17Z

easybuild/tools/filetools.py

+        tarinfo.mode = (tarinfo.mode & ~0o77) | group_mode | other_mode
+        # reset ownership numeric UID/GID 0
+        tarinfo.uid = tarinfo.gid = 0
+        tarinfo.uname = tarinfo.gname = ""


Hmm, why does empty user/group name make sense, what does this imply?

boegel · 2024-11-13T11:19:45Z

easybuild/framework/easyblock.py

+            return None
+
+        if sys.version_info[0] >= 3 and sys.version_info[1] < 9:
+            # ignore any checksum for given filename due to changes in python/cpython#90021


Suggested change

# ignore any checksum for given filename due to changes in python/cpython#90021

# ignore any checksum for given filename due to changes in https://github.com/python/cpython/issues/90021

boegel · 2024-11-13T11:20:07Z

easybuild/tools/filetools.py

+        return archive_path
+
+    # TODO: replace with TarFile.add(recursive=True) when support for Python 3.6 drops
+    # since Python v3.7 tarfile automatically orders the list of files added to the archive


Do we have a reference for this?

boegel · 2024-11-13T11:21:38Z

easybuild/tools/filetools.py

+            archive.add(filepath, arcname=file_name, recursive=False, filter=archive_filter)
+            _log.debug("File/folder added to archive '%s': %s", archive_filename, filepath)
+
+    _log.info("Archive '%s' created successfully", archive_filename)


Suggested change

_log.info("Archive '%s' created successfully", archive_filename)

_log.info(f"Archive '{archive_path}' created successfully")

boegel · 2024-11-13T13:35:08Z

@lexming We need to make sure that git_config entries that still use .tar.gz as extension don't leading to clone+tar+xz every time, we also need to check whether the source filename with .tar.xz already exists?

boegel · 2024-11-13T13:37:24Z

easybuild/tools/filetools.py

-        raise EasyBuildError("git_config currently only supports filename ending in .tar.gz")
+    file_ext = find_extension(filename, required=False)
+    if file_ext:
+        print_warning(f"Ignoring extension of filename '{filename}' set in git_config parameter")


Rather than printing a warning, we should enforce the use of .tar.xz as extension here (to "ease" the transition, in some way)

lexming added 2 commits September 27, 2024 14:48

use more portable --date argument for touch command used in reproduci…

ec1ede9

…ble tarballs

stop reproducible tarball generation command on any failure in the pipe

e7f3bbd

lexming mentioned this pull request Sep 27, 2024

reproducible tarballs have unexpected checksums on macOS #4657

Open

lexming added EasyBuild-5.0 EasyBuild 5.0 bug fix labels Sep 27, 2024

lexming added 2 commits September 30, 2024 09:39

move command to make reproducible archives to its own generator metho…

ca09f4e

…d in filetools

replace harcoded pattern for reproducible archives command for call t…

d0a55ba

…o filetools.reproducible_archive_cmd

lexming force-pushed the reprod-tarballs-mac branch from 73b39e2 to d0a55ba Compare September 30, 2024 07:39

boegel added this to the 5.0 milestone Oct 2, 2024

boegel changed the title ~~use more portable --date argument for touch command used in reproducible tarballs~~ use more portable --date argument for touch command used in reproducible tarballs Oct 2, 2024

boegel requested changes Oct 2, 2024

View reviewed changes

test/framework/filetools.py Outdated Show resolved Hide resolved

lexming added 6 commits October 8, 2024 09:31

use tarfile module instead of executing external shell commands to cr…

d7195c7

…eate archives of git repos

add required flag to filetools.find_extension() method

87b733a

add support for extended_dry_run mode to filetools.make_archive()

dd28095

add unit test for filetools.make_archive()

a37af5a

make test for github_get_source_tarball_from_git compatible with make…

f2296de

…_archive addition

improve reliability of bit-wise operations setting file mode in repro…

980f618

…ducible archives

lexming changed the title ~~use more portable --date argument for touch command used in reproducible tarballs~~ improve portability of reproducible tarballs by replacing external tar command with tarfile module Oct 9, 2024

lexming added 6 commits November 5, 2024 15:39

Merge branch '5.0.x' into reprod-tarballs-mac

ddb9cae

set reproducible flag of make_archives from a specific variable

a26c71e

simplify logic in EasyBlock.get_checksum_for and improve its logging

3936a6e

ignore checksums of sources from git repos prior to Python 3.9

6188cc7

only run checksum assertions in test_make_archive on Python 3.9+

ee772f7

add test_fetch_sources_git to easyblock suite

27ea1d0

push deprecation of cheksum check of git repo with Python < 3.9 to Ea…

ad47cac

…syBuild version 6.0

boegel changed the title ~~improve portability of reproducible tarballs by replacing external tar command with tarfile module~~ improve portability of reproducible tarballs by replacing external tar command with tarfile module Nov 13, 2024

boegel requested changes Nov 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve portability of reproducible tarballs by replacing external `tar` command with `tarfile` module #4660

improve portability of reproducible tarballs by replacing external `tar` command with `tarfile` module #4660

lexming commented Sep 27, 2024 •

edited

Loading

lexming commented Nov 6, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel Nov 13, 2024

boegel commented Nov 13, 2024

boegel Nov 13, 2024

	"Reproducible tarballs of git repos are only supported in Python 3.9+. "
	"Reproducible tarballs of Git repos are only possible when using Python 3.9+ to run EasyBuild. "

		a reproducible archive. Other formats like .gz are not reproducible due to
		arbitrary strings and timestamps getting added into their metadata.

	other_mode = group_mode >> 3 # user mode without write
	other_mode = group_mode >> 3 # same as group mode

	# ignore any checksum for given filename due to changes in python/cpython#90021
	# ignore any checksum for given filename due to changes in https://github.com/python/cpython/issues/90021

	_log.info("Archive '%s' created successfully", archive_filename)
	_log.info(f"Archive '{archive_path}' created successfully")

improve portability of reproducible tarballs by replacing external tar command with tarfile module #4660

Are you sure you want to change the base?

improve portability of reproducible tarballs by replacing external tar command with tarfile module #4660

Conversation

lexming commented Sep 27, 2024 • edited Loading

lexming commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boegel commented Nov 13, 2024

Choose a reason for hiding this comment

improve portability of reproducible tarballs by replacing external `tar` command with `tarfile` module #4660

improve portability of reproducible tarballs by replacing external `tar` command with `tarfile` module #4660

lexming commented Sep 27, 2024 •

edited

Loading