Skip to content
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

fix: Fixed path separator in .tgz output artifacts created on windows. Fixes #10562 #11097

Merged
merged 2 commits into from
May 22, 2023

Conversation

PeterKoegel
Copy link
Contributor

@PeterKoegel PeterKoegel commented May 17, 2023

Fixes #10562

Motivation

Output artifacts in .tgz format that are created on Windows result in wrong directory and file structure when unpacked by 7-zip or on Linux.

Example:

  1. Folder structure Windows is
    A\B\C.txt

  2. An output artifact in .tgz format is generated from this (on Windows)

  3. The resulting .tgz archive is opened in 7zip

(folder) "A\"
(folder) "A\B\"
(file)   "A\B\C.txt"

Modifications

When generating the tar archive the pathes that are written to the tar-file's file header are converted to use forward slashes instead of backslashes independent of the current operation systems path format.

Verification

When the above example is repeaded after this fix then 7zip shows the correct folder structure:

(folder) A
(folder)   B
(file)       C.txt

Signed-off-by: Peter Kögel <peter.koegel@vector.com>
@PeterKoegel PeterKoegel changed the title fix: Fixed path separator in tar files created on windows. Fixes #10562 fix: Fixed path separator in .tgz output artifacts created on windows. Fixes #10562 May 17, 2023
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

@lippertmarkus Would you mind helping us review this change?

@lippertmarkus
Copy link
Member

@terrytangyuan names in tar headers need to use forward slashes for the tar archive to extract correctly on both Linux and Windows, so the fix is fine

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thanks!

@terrytangyuan terrytangyuan marked this pull request as ready for review May 19, 2023 10:54
@terrytangyuan terrytangyuan enabled auto-merge (squash) May 19, 2023 10:54
@terrytangyuan terrytangyuan merged commit 1e4a376 into argoproj:master May 22, 2023
terrytangyuan pushed a commit that referenced this pull request May 25, 2023
…Fixes #10562 (#11097)

Signed-off-by: Peter Kögel <peter.koegel@vector.com>
JPZ13 pushed a commit to pipekit/argo-workflows that referenced this pull request Jul 4, 2023
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
…Fixes argoproj#10562 (argoproj#11097)

Signed-off-by: Peter Kögel <peter.koegel@vector.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
@agilgur5 agilgur5 added area/artifacts S3/GCP/OSS/Git/HDFS etc area/windows Windows Container support labels Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc area/windows Windows Container support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants