-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: initial commit for erofs support #14
Conversation
0abac16
to
2843a67
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
==========================================
- Coverage 16.52% 16.28% -0.25%
==========================================
Files 6 10 +4
Lines 932 1640 +708
==========================================
+ Hits 154 267 +113
- Misses 757 1334 +577
- Partials 21 39 +18 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ramkumar Chinchani <[email protected]>
c27c15d
to
29321f8
Compare
We are going to support multiple underlying filesystems (squash, erofs and maybe more). As long as they are filesystem image blobs, verity data can be appended. Signed-off-by: Ramkumar Chinchani <[email protected]>
erofs/erofs.go
Outdated
type erofsFuseInfoStruct struct { | ||
Path string | ||
Version string | ||
SupportsNotfiy bool |
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 typo is consistent but will be murder to maintain later :)
erofs/erofs.go
Outdated
// everyting underneath is also implicitly excluded. The | ||
// AddExclude()/AddInclude() methods do the math to figure out what is the | ||
// correct set of things to exclude or include based on what paths have been | ||
// previously included or excluded. |
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.
I'm finding this hard to reason about, and therefore finding it hard to figure out whether AddExclude() is doing the right thing.
I think you're mainly thinking in terms of "I am generating a changeset and therefore want to include only files which have changed", is that right?
erofs/erofs.go
Outdated
// If /usr/bin/ls has changed but /usr hasn't, we don't want to list | ||
// /usr in the include paths any more, so let's be sure to only | ||
// add things which aren't prefixes. | ||
if strings.HasPrefix(inc, p) { |
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.
But what if both /usr/bin/cal and /usr/bin/calendar have changed? Will this check end up possibly ignoring /usr/bin/cal?
erofs/erofs.go
Outdated
eps.include = append(eps.include, orig) | ||
} | ||
|
||
func (eps *ExcludePaths) String() (string, error) { |
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.
I assume you are using this only to spit out a file to feed to mkerofs. But because it is generally called String(), I would expect it to be more generally useful, so in particular would expect it to also print the includes.
I'm not sure the best way to handle this. Probably best to just leave a comment above it explaining that some information does get lost and why.
erofs/erofs.go
Outdated
return blob, GenerateErofsMediaType(compression, verity), rootHash, nil | ||
} | ||
|
||
func isMountedAtDir(_, dest string) (bool, error) { |
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.
Is checking that the source is correct a TODO?
PR #25 supersedes this one, so closing. |
No description provided.