feat: initial commit for erofs support#14
feat: initial commit for erofs support#14rchincha wants to merge 2 commits intoproject-machine:mainfrom
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 <rchincha@cisco.com>
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 <rchincha@cisco.com>
| type erofsFuseInfoStruct struct { | ||
| Path string | ||
| Version string | ||
| SupportsNotfiy bool |
There was a problem hiding this comment.
This typo is consistent but will be murder to maintain later :)
| // 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.
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?
| // 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.
But what if both /usr/bin/cal and /usr/bin/calendar have changed? Will this check end up possibly ignoring /usr/bin/cal?
| eps.include = append(eps.include, orig) | ||
| } | ||
|
|
||
| func (eps *ExcludePaths) String() (string, error) { |
There was a problem hiding this comment.
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.
| return blob, GenerateErofsMediaType(compression, verity), rootHash, nil | ||
| } | ||
|
|
||
| func isMountedAtDir(_, dest string) (bool, error) { |
There was a problem hiding this comment.
Is checking that the source is correct a TODO?
|
PR #25 supersedes this one, so closing. |
No description provided.