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: processing symlinks in directories #453

Merged
merged 3 commits into from Feb 12, 2022
Merged

Conversation

Felixoid
Copy link
Contributor

@Felixoid Felixoid commented Feb 9, 2022

This PR fixes #428 and depends on goreleaser/fileglob#24

appendGlobbedFiles now distinguish symlinks and add a proper Content object.

Tests in deb and rpm are removed because covered by files now.

I'll update the go.mod and convert it from the draft after the mentioned PR is merged.

@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #453 (ec46922) into main (53a3fc2) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head ec46922 differs from pull request most recent head bcac98a. Consider uploading reports for the commit bcac98a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #453      +/-   ##
==========================================
+ Coverage   65.89%   65.93%   +0.03%     
==========================================
  Files          16       16              
  Lines        1903     1905       +2     
==========================================
+ Hits         1254     1256       +2     
  Misses        510      510              
  Partials      139      139              
Impacted Files Coverage Δ
files/files.go 88.28% <100.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53a3fc2...bcac98a. Read the comment docs.

@Felixoid Felixoid marked this pull request as ready for review February 10, 2022 00:07
@Felixoid
Copy link
Contributor Author

Woop-woop, it's green :-j

go.mod Outdated Show resolved Hide resolved
@caarlos0 caarlos0 added the bug Something isn't working label Feb 10, 2022
@caarlos0 caarlos0 changed the title Fix processing symlinks in directories fix: processing symlinks in directories Feb 10, 2022
Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

looks good, but I would keep the symlink tests in each packager too

@Felixoid
Copy link
Contributor Author

Felixoid commented Feb 10, 2022

Then I'd rather just put the static links and see if it's there, without helpers and creating. On one hand.

On another hand, there are still tests though https://github.com/goreleaser/nfpm/blob/main/apk/apk_test.go#L477, https://github.com/goreleaser/nfpm/blob/main/rpm/rpm_test.go#L641, and https://github.com/goreleaser/nfpm/blob/main/deb/deb_test.go#L685

Removed tests were there for particular checking if the destination files in archives are the same as the target files for symlinks. And this logic is completely dead now.

upd: the coverage report shows, that I haven't removed any tests that did check for anything

@Felixoid
Copy link
Contributor Author

Dear @erikgeiser and @djgilcrease, can you review it too, please? Would be very helpful to have it merged!

Copy link
Contributor

@djgilcrease djgilcrease left a comment

Choose a reason for hiding this comment

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

Overall looks good to me,

@caarlos0 caarlos0 merged commit c049e1c into goreleaser:main Feb 12, 2022
@caarlos0
Copy link
Member

Thanks everyone!

@github-actions github-actions bot added this to the 2.12.0 milestone Feb 12, 2022
@Felixoid
Copy link
Contributor Author

Thank you, awesome!

I am looking forward to a new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

handle symlinks correctly when doing dir copy
3 participants