Skip to content

Align TransferOptions defaults to the union of DEFAULT_PUSH_OPTIONS and DEFAULT_PULL_OPTIONS #4940

@LecrisUT

Description

@LecrisUT

It is a fun surprise when you forget that the defaults in TransferOptions are not applied if you do not apply because DEFAULT_*_OPTIONS are actually used which are different

tmt/tmt/guest/__init__.py

Lines 353 to 388 in bae12b9

@container
class TransferOptions:
"""Options for transferring files to/from the guest."""
#: Apply permissions to the destination files
chmod: Optional[int] = None
#: Enable compression during transfer
compress: bool = False
#: Delete extraneous files from destination directory
delete: bool = False
#: Exclude files matching any of these patterns
exclude: list[str] = field(default_factory=list, normalize=tmt.utils.normalize_string_list)
#: Copy symlinks as symlinks
links: bool = False
#: Preserve file permissions
preserve_perms: bool = False
#: Protect file and directory names from interpretation
protect_args: bool = False
#: Recurse into directories
recursive: bool = False
#: Use relative paths
relative: bool = False
#: Ignore symlinks that point outside the source tree
safe_links: bool = False
#: Run a ``mkdir -p`` of the destination before doing transfer
create_destination: bool = False

tmt/tmt/guest/__init__.py

Lines 424 to 441 in bae12b9

DEFAULT_PUSH_OPTIONS = TransferOptions(
protect_args=True,
relative=True,
recursive=True,
compress=True,
links=True,
safe_links=True,
delete=True,
)
DEFAULT_PULL_OPTIONS = TransferOptions(
protect_args=True,
relative=True,
recursive=True,
compress=True,
links=True,
safe_links=True,
)

Some flags that I suggest to revisit:

  • compress: enable by default everywhere

  • relative: disable everywhere, seems very dangerous particularly when you use it with a destination

    For example, if you used this command:

     rsync -av /foo/bar/baz.c remote:/tmp/
    

    would create a file named baz.c in /tmp/ on the remote machine. If instead you used

     rsync -avR /foo/bar/baz.c remote:/tmp/
    

    then a file named /tmp/foo/bar/baz.c would be created on the remote machine, preserving its full path. These extra path elements are called "implied directories" (i.e. the "foo" and the "foo/bar" directories in the above example).

    i.e. destination there means destination root, which is unintuitive. On top of that guest.push on podman does not properly account for it!

  • recursive: disable by default. Although safe for rsync, it is not safe for podman cp (/. suffix fails on non-directories). Alternatively we can make podman be recursive safe

  • links: disable by default. Keeping the links makes sense for the copying of the whole tree, but I do not think it would make sense more generally, e.g. when accessing files outside of the user tree or copying to a destination with different structure. Copying a whole tree is a more specialized operation where turning it on makes sense and operates the same with relative

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No fields configured for Task.

    Projects

    Status
    backlog
    Status
    triaged

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions