feat: Support storing UnixFS 1.5 Mode and ModTime #10478

gammazero · 2024-08-13T04:45:06Z

Replaces #7754 written by @kstuart

Depends on:

Feature Progress

Remaining Work before Merging

CI sharness needs to pass (details)
including mode/mtime while raw leaves are enabled should force use of dag-pb envelope and never produce a single raw block (details)
- needs explicit test with --cid-version 0 and --cid-version 1

Related PRs

~~support file mode and last modified time for ipfs add interface-go-ipfs-core#66~~ replace by this PR
~~Support for storing file mode and modification times go-unixfs#85~~ replaced by: feat: Support UnixFS mode and modification times in ipld dag and mfs boxo#658
~~initial support for setting/updating file modes and modification times go-mfs#93~~ replaced by: feat: Support UnixFS mode and modification times in ipld dag and mfs boxo#658
~~initial support for Mode and ModTime on Node interface go-ipfs-files#31~~ replaced by: feat: support UnixFS 1.5 file mode and modification times boxo#653
~~support restoring permissions and last modification times tar-utils#11~~ replaced by: feat: support UnixFS 1.5 file mode and modification times boxo#653

Implementation Notes

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.

IPFS Add (CLI)

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
cc #10436

This commit introduces initial Mode and ModTime support for single filesystem files and webfiles. 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. The preserve flags and custom options are mutually exclusive, if both are provided the custom options take precedence. Majority of of the code was from #7754 written by kstuart Replaces #7753 Closes #6920

probably safer to error here, than printing line - if command was executed over http it might be printed on the server, or break cli tools that expect specific stdout rather than this line

was missing executable flag, so was skipped on ci this should re-enable it

lidel · 2024-08-13T22:44:25Z

test/sharness/t0047-add-mode-mtime.sh

@gammazero we seem to miss CIDv1 test here. CIDV1 uses --raw-leaves=true by default, and there is an edge case with small files that produce a single block that is both root and raw leave that was missed.

To reproduce the bug, below does not seem to preserve mode and mtime (small file produces raw block):

$ touch ./file # create empty file $ chmod 654 ./file # custom mode $ ipfs add --preserve-mode --preserve-mtime --cid-version 1 -Q ./file bafkreihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku $ ipfs files stat /ipfs/bafkreihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku Size: 0 CumulativeSize: 0 ChildBlocks: 0 Type: file Mode: not set (not set) Mtime: not set

I think if user passes --preserve-* option, it should take precedence over --raw-leaves if the produced DAG is a single block.

Rationale:

A raw block has no surface for storing mode and mtime. Only dag-pb can store it, so --preserve-* options should make the root block dag-pb instead raw.

Alternative is erroring when both --raw-leaves=true and --preserve-* are passed and root block of one of files is raw, but that is a very bad ux: if someone imports a big directory, and has a small file deep inside, we don't want to error in the middle of a long import.

we've discussed this today, my understanding of next steps here is to implement below:

if the file is small enough to fit in a single block

and if both raw-leaves=true is set (likely implicitly due to CIDv1) and storing mode/mtime was requested by user (always explicit opt-in)

the default behavior will be to wrap block in dag-pb to create surface for storing mode/mtime

Done: Explicitly disable raw leaves if preserving mode or modification time.

@lidel Please check if my latest update does what is asked for above, without having to explicitly look at block sizes.

Thank you @gammazero. I think this is sensible UX to unblock 0.30.0-rc1.
We can refine if we get some feedback from users who requested mod/mtime that its not enough.

FYSA I've adjusted UX a bit in 01e6823, making it error in specific case when user explicitly passed raw-leaves flag to CLI/RPC, rather than having it enabled as implicit behavior of --cid-version 1.

test/sharness/t0047-add-mode-mtime.sh

this will show us what values are read by 'stat' and compared by 'test' when run on CI (tests fail there, but pass locally, so we need more verbose log to debug this)

#10478

mountdir was used for both in and out, this isolates things to ensure we dont mix inputs with outputs

turns out some github workers run with umask 002 which changes the default mode of files created with 'touch' https://github.com/orgs/community/discussions/40876 #10478

Previously it only worked for a single object or top-level directory. This was only for rpc, as used by ipget, as it was already working for normal ipfs get.

just setting expectations

this follows 'no surprises' rule. if user provided --raw-leaves=true explicitly, means they care, and we error to let them correct parameter manually if user added file without raw-leaves flag, but we have cidv1 which implies raw-leaves, we assume user does not mind changing implicit raw-leaves behavior

switching to version with fix from ipfs/boxo#653 (comment) and adding tests

commit after merging ipfs/boxo#653

lidel

Thank you @kstuart and @gammazero.

Switched to boxo main with ipfs/boxo#653, identified touch|chmod bugs are fixed. CI is green.

Merging to start release of kubo 0.30.0-rc1 tomorrow and get some user feedback from folks that asked for mode and mtime.

gammazero requested a review from a team as a code owner August 13, 2024 04:45

gammazero and others added 4 commits August 12, 2024 21:59

mod tidy examples

60f3470

lint fix

beae0b3

Update changelog

633f28f

test(mode): expect empty octal as well

31b8024

lidel mentioned this pull request Aug 13, 2024

Support storing UnixFS 1.5 Mode and ModTime #7754

Closed

19 tasks

chore: latest commit

b170c90

lidel force-pushed the feat/unixfs1.5-mode-mtime branch from 89de255 to 65ef19c Compare August 13, 2024 22:40

lidel added 3 commits August 14, 2024 01:35

docs(changelong): mode/mtime cli example

8e0308b

fix(rpc): cli error instead of daemon println

4817ac7

probably safer to error here, than printing line - if command was executed over http it might be printed on the server, or break cli tools that expect specific stdout rather than this line

chore(files): explicit 'not set' in missing stats

f0fea76

lidel force-pushed the feat/unixfs1.5-mode-mtime branch 2 times, most recently from dfd32ed to f0fea76 Compare August 13, 2024 23:37

lidel and others added 3 commits August 14, 2024 01:54

test(ci): enable running t0047-add-mode-mtime.sh

0c55944

was missing executable flag, so was skipped on ci this should re-enable it

Fix RPC decoding

988597a

Use latest boxo PR version

8d75eb2

lidel reviewed Aug 14, 2024

View reviewed changes

lidel mentioned this pull request Aug 14, 2024

Release 0.30 #10436

Closed

32 tasks

lidel and others added 11 commits August 14, 2024 16:50

Merge branch 'master' into feat/unixfs1.5-mode-mtime

97e10db

chore(ci): verbose log of mod tests

b47e56a

this will show us what values are read by 'stat' and compared by 'test' when run on CI (tests fail there, but pass locally, so we need more verbose log to debug this)

chore(ci): log ci filesystem attributes

60fd454

#10478

chore(ci): inspect stat of unecpected permissions

2e8afc0

Implement symlink support in client/rpc

0312a99

refactor(test): isolate outputs from fixtures

0987496

mountdir was used for both in and out, this isolates things to ensure we dont mix inputs with outputs

fix(ci): force umask 022

5d23bcb

turns out some github workers run with umask 002 which changes the default mode of files created with 'touch' https://github.com/orgs/community/discussions/40876 #10478

Made mode and mod time work for items within a directory

d6c6999

Previously it only worked for a single object or top-level directory. This was only for rpc, as used by ipget, as it was already working for normal ipfs get.

Explicitly disable raw leaves if preserving mode or modification time

7ac1c81

Merge branch 'master' into feat/unixfs1.5-mode-mtime

b549434

docs: note raw-leaves impact, mark experimental

577677c

just setting expectations

This was referenced Aug 20, 2024

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

Merged

mfs: unixfs 1.5 touch and chmod error on raw block ipfs/boxo#660

Closed

lidel and others added 6 commits August 20, 2024 23:12

chore: hardcode import defaults used for tests

1c3cc4c

test: mode/mtime roundtrip with cidv1

32c6a1c

test: files chmod|touch with cidv1

55dad60

switching to version with fix from ipfs/boxo#653 (comment) and adding tests

update boxo

826aae1

Update boxo to latest unixfs PR version

2235051

chore: latest boxo main

9043c55

commit after merging ipfs/boxo#653

lidel approved these changes Aug 21, 2024

View reviewed changes

lidel merged commit 263edb2 into master Aug 21, 2024
14 checks passed

lidel deleted the feat/unixfs1.5-mode-mtime branch August 21, 2024 00:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support storing UnixFS 1.5 Mode and ModTime #10478

feat: Support storing UnixFS 1.5 Mode and ModTime #10478

gammazero commented Aug 13, 2024 •

edited by lidel

Loading

lidel Aug 13, 2024 •

edited

Loading

lidel Aug 19, 2024 •

edited

Loading

gammazero Aug 19, 2024 •

edited

Loading

lidel Aug 19, 2024 •

edited

Loading

lidel left a comment

feat: Support storing UnixFS 1.5 Mode and ModTime #10478

feat: Support storing UnixFS 1.5 Mode and ModTime #10478

Conversation

gammazero commented Aug 13, 2024 • edited by lidel Loading

Depends on:

Feature Progress

Remaining Work before Merging

Related PRs

Implementation Notes

IPFS Add (CLI)

lidel Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

lidel Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

gammazero Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

lidel Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

lidel left a comment

Choose a reason for hiding this comment

gammazero commented Aug 13, 2024 •

edited by lidel

Loading

lidel Aug 13, 2024 •

edited

Loading

lidel Aug 19, 2024 •

edited

Loading

gammazero Aug 19, 2024 •

edited

Loading

lidel Aug 19, 2024 •

edited

Loading