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

Make spfs diff only check sizes on blobs #999

Merged
merged 1 commit into from
Mar 6, 2024
Merged

Conversation

jrray
Copy link
Collaborator

@jrray jrray commented Mar 6, 2024

The main issue is the size of directories. If a path is committed on one machine with a different filesystem type or with/without fuse, the directory sizes captured into the manifest are platform-dependent, and then rendering that manifest on a different system may cause spfs diff to report changes.

One easy way to make this happen is to spk build a package while fuse is enabled and then spk env the built package with fuse disabled.

$ spk env stdfs -- spfs diff
~/spfs/bin [size {0=>6}]
~/spfs/etc [size {1=>18}]
~/spfs/etc/spfs [size {1=>23}]
~/spfs/etc/spfs/startup.d [size {2=>53}]
~/spfs/lib [size {0=>6}]
~/spfs/spk [size {1=>17}]
~/spfs/spk/pkg [size {1=>19}]
~/spfs/spk/pkg/stdfs [size {1=>19}]
~/spfs/spk/pkg/stdfs/1.1.0 [size {1=>22}]
~/spfs/spk/pkg/stdfs/1.1.0/PF6B6TLF [size {5=>77}]

@jrray jrray added the bug Something isn't working label Mar 6, 2024
@jrray jrray self-assigned this Mar 6, 2024
@jrray
Copy link
Collaborator Author

jrray commented Mar 6, 2024

Fun fact, no tests were harmed in the making of this PR.

This is affecting us when launching jobs on our render farm because spawn sees these changes from spfs diff and thinks it needs to create an spfs snapshot and capture the changes.

IMO this should be taken a step further and change the Entry type(s) to only contain sizes for blob-type entries. I have a branch with this change but the problem is that it affects the digest calculation so existing manifests that contain sizes for non-blobs would be readable but then encode back out into an object with a different digest from what it originally had.

The main issue is the size of directories. If a path is committed on one
machine with a different filesystem type or with/without fuse, the
directory sizes captured into the manifest are platform-dependent, and
then rendering that manifest on a different system may cause `spfs diff`
to report changes.

One easy way to make this happen is to `spk build` a package while fuse
is enabled and then `spk env` the built package with fuse disabled.

```
$ spk env stdfs -- spfs diff
~/spfs/bin [size {0=>6}]
~/spfs/etc [size {1=>18}]
~/spfs/etc/spfs [size {1=>23}]
~/spfs/etc/spfs/startup.d [size {2=>53}]
~/spfs/lib [size {0=>6}]
~/spfs/spk [size {1=>17}]
~/spfs/spk/pkg [size {1=>19}]
~/spfs/spk/pkg/stdfs [size {1=>19}]
~/spfs/spk/pkg/stdfs/1.1.0 [size {1=>22}]
~/spfs/spk/pkg/stdfs/1.1.0/PF6B6TLF [size {5=>77}]
```

Signed-off-by: J Robert Ray <jrray@jrray.org>
@jrray jrray force-pushed the spfs-diff-size-differences branch from cd4eb7e to 3291948 Compare March 6, 2024 00:40
@jrray jrray requested a review from rydrman March 6, 2024 01:03
@jrray jrray merged commit b9b87a1 into main Mar 6, 2024
7 checks passed
@jrray jrray deleted the spfs-diff-size-differences branch March 6, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants