fix: parent directory permissions #128
Conversation
To make the content transparent and facilitate changing the package, this commit creates the deb used for testing programmatically instead of embedding it as a base64-encoded blob directly.
The entries not mentioned explicitly in the slice definition will not be added to the report (e.g. copyright, parent directories). Additionally, we have removed the Globbed option and simplified it.
This commit fixes three bugs regarding parent directory creation. The first one is that implicit folders were overwriting the permissions of explicit ones in slice definitions (e.g. slice A declares /foo/bar and slice B /foo, if slice A was installed after B then the implicit permissions of /foo from A would prevail. The second one is that we were creating parent directories of optional files which did not exist. The third bug is that we were not using the permissions from the tarball for parent directories if the path was a glob.
internal/fsutil/create.go
Outdated
| @@ -16,6 +16,8 @@ type CreateOptions struct { | |||
| Mode fs.FileMode | |||
| Data io.Reader | |||
| Link string | |||
| // If OverrideMode is true and a directory exists, its mode will be modified. | |||
| OverrideMode bool | |||
There was a problem hiding this comment.
This is not looking right, for two reasons:
-
We've added a flag to control behavior that was standard until this commit, meaning every single call site for the Create function was the equivalent of OverrideMode: true. Yet, we're not using this flag anywhere apparently, which means it's false 100% of the time and the behavior has been disabled. That implies to me that the old behavior has turned irrelevant after the last couple of changes, so we can just drop the behavior altogether.
-
This flag doesn't seem to sit well in the API as a whole. It doesn't make sense for this flag to work only on directories, for example.
GIven (1), what's the situation? Can we just delete the behavior altogether?
There was a problem hiding this comment.
Agree with both. I was keeping it for future compatibility but when we need it we can just add it back to the API. Deleting unused code is the best code.
This commit fixes three bugs regarding parent directory creation.
The first one is that implicit folders were overwriting the permissions
of explicit ones in slice definitions (e.g. slice A declares /foo/bar
and slice B /foo, if slice A was installed after B then the implicit
permissions of /foo from A would prevail.
The second one is that we were creating parent directories of optional
files which did not exist.
The third bug is that we were not using the permissions from the tarball
for parent directories if the path was a glob.
This PR depends on #124, the new commit is a2805d0.
I had a discussion with @niemeyer that we need to discuss and agree on way to record parent paths in chisel db. However, I think that meanwhile we should fix these three bugs which are in the code right now.