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: referencing symbolic links with tar archive #20

Merged
merged 2 commits into from
Aug 12, 2020
Merged

Conversation

jbrockopp
Copy link
Contributor

Intended to resolve go-vela/community#30

Currently, there is a known bug in the library we use to create the archives we upload to the cache:

mholt/archiver#202

A PR exists to resolve this bug but the maintainers were unwilling to merge until adequate tests have been written:

mholt/archiver#201

In an attempt to fix the issue, we've forked the upstream repo to this org:

https://github.com/go-vela/archiver

And have made the changes reflected in the PR to see if it resolves the underlying issue:

mholt/archiver@master...go-vela:master

@jbrockopp jbrockopp added bug Indicates a bug dependencies Indicates a change to dependencies labels Aug 11, 2020
@jbrockopp jbrockopp self-assigned this Aug 11, 2020
@JordanSussman
Copy link
Collaborator

Why not just use go mod edit -replace instead of updating the import to be github.com/go-vela/archiver?

@jbrockopp
Copy link
Contributor Author

@JordanSussman essentially they are functionally equivalent 🤷

I think each way has it's own pros/cons but none of us on our side really care so we'll update it 👍

@codecov
Copy link

codecov bot commented Aug 12, 2020

Codecov Report

Merging #20 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   11.21%   11.21%           
=======================================
  Files           6        6           
  Lines         419      419           
=======================================
  Hits           47       47           
  Misses        369      369           
  Partials        3        3           

@jbrockopp jbrockopp marked this pull request as ready for review August 12, 2020 19:54
@jbrockopp jbrockopp requested a review from a team as a code owner August 12, 2020 19:54
Copy link
Contributor

@kneal kneal left a comment

Choose a reason for hiding this comment

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

LGTM 🐬

@wass3r wass3r merged commit a6e8fb8 into master Aug 12, 2020
@wass3r wass3r deleted the fix/symbolic_links branch August 12, 2020 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates a bug dependencies Indicates a change to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vela-s3-cache restore issue with symbolic links
4 participants