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

feat: support UnixFS 1.5 file mode and modification times #653

Merged
merged 21 commits into from
Aug 20, 2024
Merged

Conversation

gammazero
Copy link
Contributor

@gammazero gammazero commented Aug 6, 2024

Replaces PR #34 written by @kstuart

Adds support for storing and retrieving file mode and last modification time.

Support added to:

When the TAR archive (headers) include a file mode or modification time, the extractor will restore that metadata when supported for the underlying filesystem.

The Golang runtime currently does not support changing the times on a symlink, for Linux and some BSDs a custom solution has been implemented, for Darwin this is not the case so when copying a symlink to the filesystem the last modification time is not updated. Since for concrete files and directories stored modes and modification times are faithfully restored to the filesystem this should not be a breaking issue, a similar solution to that implemented for Linux/BSDs is likely implementable by a developer with access to a Darwin platform.

Replaces PRs:

Relates to ipfs/kubo#6920

Replaces PR #34 written by @kstuart

Adds support for storing and retrieving file mode and last modification time.

Support added to:
- [X] Files
- [X] LinkFiles
- [X] Webfiles
- [X] Directories
- [X] Tar Archives

When the TAR archive (headers) include a file mode or modification time, the extractor will restore that metadata when supported for the underlying filesystem.

The Golang runtime currently does not support changing the times on a symlink, for Linux and some BSDs a custom solution has been implemented, for Darwin this is not the case so when copying a symlink to the filesystem the last modification time is not updated. Since for concrete files and directories stored modes and modification times are faithfully restored to the filesystem this should not be a breaking issue, a similar solution to that implemented for Linux/BSDs is likely implementable by a developer with access to a Darwin platform.

Replaces PRs:
- ipfs/go-ipfs-files#31
- ipfs/tar-utils#11
- #34

Relates to ipfs/kubo#6920
@gammazero gammazero requested review from lidel and a team as code owners August 6, 2024 21:23
tar/extractor.go Dismissed Show dismissed Hide dismissed
Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 68.31551% with 237 lines in your changes missing coverage. Please review.

Project coverage is 60.10%. Comparing base (88beadf) to head (b2660c5).
Report is 2 commits behind head on main.

Files Patch % Lines
mfs/dir.go 5.55% 51 Missing ⚠️
tar/extractor.go 66.99% 18 Missing and 16 partials ⚠️
mfs/file.go 62.33% 21 Missing and 8 partials ⚠️
ipld/unixfs/pb/unixfs.pb.go 27.77% 26 Missing ⚠️
files/multipartfile.go 69.23% 12 Missing and 4 partials ⚠️
mfs/ops.go 7.69% 12 Missing ⚠️
files/meta.go 42.10% 8 Missing and 3 partials ⚠️
ipld/unixfs/importer/helpers/dagbuilder.go 69.69% 8 Missing and 2 partials ⚠️
gateway/backend_car_files.go 0.00% 8 Missing ⚠️
files/meta_posix.go 68.42% 3 Missing and 3 partials ⚠️
... and 10 more

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
+ Coverage   59.89%   60.10%   +0.21%     
==========================================
  Files         237      241       +4     
  Lines       29955    30600     +645     
==========================================
+ Hits        17942    18393     +451     
- Misses      10398    10563     +165     
- Partials     1615     1644      +29     
Files Coverage Δ
files/linkfile.go 67.85% <100.00%> (+27.85%) ⬆️
files/multifilereader.go 85.84% <100.00%> (+3.04%) ⬆️
files/readerfile.go 74.00% <100.00%> (+26.63%) ⬆️
files/slicedirectory.go 80.95% <100.00%> (+7.61%) ⬆️
files/webfile.go 68.42% <100.00%> (+12.32%) ⬆️
ipld/unixfs/io/dagreader.go 81.22% <100.00%> (+0.77%) ⬆️
mfs/root.go 40.50% <ø> (ø)
files/meta_other.go 75.00% <75.00%> (ø)
files/tarwriter.go 83.52% <93.10%> (+10.91%) ⬆️
files/util.go 75.86% <89.47%> (+25.86%) ⬆️
... and 17 more

... and 13 files with indirect coverage changes

gammazero added a commit to ipfs/kubo that referenced this pull request Aug 13, 2024
Replaces #7754 written by @kstuart

- ipfs/boxo#653
- ipfs/boxo#658

- [X] Can `ipfs add` with preserved mode and/or last modification time
  - [X] on files
  - [X] on directories
- [X] Can `ipfs add` with custom mode and/or last modification time
  - [X] on files
  - [X] on directories
- [X] Can `ipfs get` restoring mode and/or last modification time
  - [X] on files
  - [X] on directories
  - [X] in archives
- [X] Can `ipfs files chmod` to change mode
  - [X] on files
  - [X] on directories
- [X] Can `ipfs files touch` to change last modification time
  - [X] on files
  - [X] on directories
- [X] Automatically update the last modification time when file data is changed or truncated (e.g. `ipfs files write`)
- [X] Can add files and directories with mode and/or modification time using multipart-form data
- [X] `ipfs files stat` reports mode and last modification time

**Note:**
- [X] Adds support to `kubo/core/rpc` (may require additional tests).

- ~ipfs/interface-go-ipfs-core/pull/66~ replace by this PR
- ~ipfs/go-unixfs/pull/85~ replaced by: ipfs/boxo#658
- ~ipfs/go-mfs/pull/93~ replaced by: ipfs/boxo#658
- ~ipfs/go-ipfs-files/pull/31~ replaced by: ipfs/boxo#653
- ~ipfs/tar-utils/pull/11~ replaced by: ipfs/boxo#653

- When adding files and directories without opting to store a mode or modification time the same CIDs are generated that would have been created before this feature was implemented (opt-in).
- The Go runtime currently has no native support for restoring file mode and modification time on symbolic-links, support for restoring the last modification time has been added for Linux distributions and the following BSDs: freebsd, netbsd, openbsd, dragonflybsd.
- Automatically updating a modification time will only occur if a modification time was previously stored.
- When creating an archive, for compatibility, time resolution is to the second; Nanoseconds are not supported.

The `ipfs add` options `--preserve-mode` and `--preserve-mtime` are used to store the original mode and last modified time of the file being added, the options `--mode`, `--mtime` and `--mtime-nsecs` are used to store custom values, a custom value of 0 is a no-op as is providing `--mtime-nsecs` without `--mtime`.

The preserve flags and custom options are mutually exclusive, if both are provided the custom options take precedence.

---

Closes #6920
gammazero and others added 5 commits August 13, 2024 10:10
…658)

* feat: Support UnixFS mode and modification times in ipld dag and mfs

Adds support for storing and retrieving file mode and last modification time.

Support added to:
- Files
- LinkFiles
- Webfiles
- Directories

Tar archives are supported by the parent branch.
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did bunch of testing with ipfs/kubo#10478 and I think we are mostly good to go.

Need to decide if/how we address below issues.

Comment on lines +32 to +44
// mostly copied from proto 3 - with int32 nanos changed to fixed32 for js-ipfs compatibility
// https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/timestamp.proto
message IPFSTimestamp {
// Represents seconds of UTC time since Unix epoch
// 1970-01-01T00:00:00Z. Must be from 0001-01-01T00:00:00Z to
// 9999-12-31T23:59:59Z inclusive.
required int64 seconds = 1;

// Non-negative fractions of a second at nanosecond resolution. Negative
// second values with fractions must still have non-negative nanos values
// that count forward in time. Must be from 0 to 999,999,999
// inclusive.
optional fixed32 nanos = 2;
Copy link
Member

@lidel lidel Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we should have some GO-JS interop tests for nanos somewhere.

Initially planned to do that in gateway-conformance after #659 is done, but realized that the Last-Modified HTTP header in gateway responses will not include nanosecond precision. It only supports second precision with an optional millisecond component.

@achingbrain any suggestions? my initial idea is to add mode and mtime tests (live creation and with static dag-pb fixture) to https://github.com/ipfs/helia/tree/main/packages/interop#readme after we have kubo 0.30.0-rc1 ad updated kubo-rpc-client that works with new RPC in JS, but lmk if there is a better place/way.

Comment on lines +250 to +266
func Chmod(rt *Root, pth string, mode os.FileMode) error {
nd, err := Lookup(rt, pth)
if err != nil {
return err
}

return nd.SetMode(mode)
}

func Touch(rt *Root, pth string, ts time.Time) error {
nd, err := Lookup(rt, pth)
if err != nil {
return err
}

return nd.SetModTime(ts)
}
Copy link
Member

@lidel lidel Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gammazero found a bug: ipfs files touch and ipfs files chmod fail when we execute them against raw blocks.

Repro steps with ipfs/kubo#10478:

$  echo -n "hello path gw" | ipfs add --cid-version 1 -Q  --raw-leaves --to-files /raw-test-1
bafkreiehuhfvxn7w4i5hexlhq4a3y7jf5kfogsvmm5zlfxg5poj4oqozyu

$ ipfs files touch /raw-test-1
Error: expected a ProtoNode as internal node

$ ipfs files chmod 0644 /raw-test-1
Error: expected a ProtoNode as internal node

lmk if you feel this is fixable in this PR before Wednesday, or should we move ahead with RC1, fill bug for this and fix later.

(hopefully we can add code that detects when node is a raw block and upgrade it to dag-pb before setting mode/time. otherwise, might cause a bigger refactor, because MFS hardcodes rawleaves=true for mutating files that use CIDv1)

Copy link
Member

@lidel lidel Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To unblock RC testing, I've extracted it into #660.

Copy link
Contributor Author

@gammazero gammazero Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidel I have fixed this in the commit f4259d6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Bumped and added end-to-end test in Kubo PR as well ipfs/kubo@55dad60

lidel added a commit to ipfs/kubo that referenced this pull request Aug 20, 2024
switching to version with fix from
ipfs/boxo#653 (comment)
and adding tests
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @kstuart for initial version, and @gammazero for refactoring and implementing the missing parts and pushing over finish line.

This should be ready for shipping in kubo 0.30.0-rc1 and getting user feedback from folks that asked for mode and mtime.

We have a bunch of tests here and in ipfs/kubo#10478 (ignore sharness failing here, it passes in kubo PR), and these extra attributes have been around in JS for years, and are always opt-in, so relatively low risk.

@lidel lidel merged commit aa27cd2 into main Aug 20, 2024
15 of 16 checks passed
lidel added a commit to ipfs/kubo that referenced this pull request Aug 20, 2024
@lidel lidel deleted the feat/unixfs15 branch August 21, 2024 00:03
wenyue pushed a commit to wenyue/boxo that referenced this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants