-
Notifications
You must be signed in to change notification settings - Fork 43
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
Handle duplicate symlink in multiple slices of a package #36
Conversation
@niemeyer There's not any new test to test this regression simply because I don't think that adding more packages as base64 is sustainable ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
What happens if one actual symlink is referred by two different slices?
When a symlink is directly specified in two different slices of a same package, this happens:
This was fixed by Rafid in d2099b9 but the PR was closed. Do you want to cherry-pick that commit into new PR? |
We might have logic to not attempt to create if we have matching content. With that said, that change from Rafid looks like a fine and simple, and it goes towards the conversation we've been having of preserving content that is already in place rather than overwritng it, so +1. Either option needs testing, though. |
05c8aa9
to
60ccc04
Compare
This doesn't apply to this PR though, does it? The mistake being fixed here is that copyright file was included in multiple slices of a same package. In other words, the following could be called more than once with same
Fixing it by making the symlink function ignore existing symlink would just hide the mistake.
I've rebased this PR on top of #42 and added tests. |
As I mentioned in the history above, that "mistake" can happen in normal circumstances by simply referring to the same symlink in two different slices, so the problem has nothing to do with copyrights. The fix should be to not make the general mechanism fail in such a normal scenario, which it looks like you believe Rafid's fix addresses, and which I mentioned above seems okay too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has gone through many updates, but the issue remains the same since the first conversation long ago: this is a general issue that must be fixed generally.
60ccc04
to
499858c
Compare
499858c
to
d5bf3a2
Compare
Thanks. Can you please push it again once the pre-req is in? There are some easy comments there. |
c93d4f5
to
af22d98
Compare
@niemeyer I've updated the linked PR #42 and rebased this one. (TIL |
In bf2a16e copyright files are added to every selected slice of a package. Check that it's OK even when copyright file is a symlink.
af22d98
to
e554571
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
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.