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

Add copyright only once per package (with test) #43

Closed
wants to merge 3 commits into from

Conversation

woky
Copy link
Contributor

@woky woky commented Dec 14, 2022

This is #36 with regression test that depends on #42.

Since this is multi-commit PR that depends on another PR, it must not be squash-merged.

woky and others added 2 commits December 14, 2022 13:04
Currently we only test with base-files deb which is embedded as base64
encoded binary in testutil/pkgdata.go. If we want to change the contents
of the data files, we have to un-ar it, un-tar it, change the data
files, tar it and ar it. Furthermore, these operations modify attributes
based on the workstation on which this process is run. So we end up with
arbitrary user/group ids/names names and dates.

Fix it by creating debs programatically. The new interface provides way
to create debs with arbitrary content of data tarball.
In the current code, a copyright file from a package is added to every
slice of that package. This went unnoticed because the file was silently
overwritten from successive slices of the same package. However, the
openssl's copyright file is a symlink to the libssl3's copyright file,
and creating a symlink on the same path for the second time fails. Fix
it by adding a copyright file to only the first slice of a package.
@woky woky changed the title Add copyright only once per package Add copyright only once per package (with test) Dec 14, 2022
This adds regression test for fix in commit 70f8749 ("Add copyright
only once per package").
@woky woky force-pushed the fix-copyright-symlink-with-test branch from 83d97e0 to 64ea98c Compare December 14, 2022 12:41
@niemeyer
Copy link
Contributor

Thanks for addressing these issues. I'm closing this PR, but just because both #36 and #42 need work, so having this here hanging with the merging of both won't be helpful. We do need testing for this fix, but let's address the review issues on the respective PRs first please.

@niemeyer niemeyer closed this Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants