fix: Better functionality for ipfs add --to-files -w #10658
Conversation
ProximaNova
changed the title
(yes it is related to those two. I didn't mention that for some reason.)
I see this message in regards to and on this pull request after logging in to GitHub. This is insignificant syntax pickiness such as "should omit type bool from declaration; it will be inferred from the right-hand side (stylecheck)" at https://github.com/ipfs/kubo/actions/runs/12703758495/job/35598233653?pr=10658 . I compiled everything before making this pull request and my modified Go code all worked. It compiled or was built with no errors or warnings showing up in the CLI. The resulting binary worked as I described. I am saying all this to point out that those minor "fails" should not detract much from this idea and this functional code. |
There was a problem hiding this comment.
Now passing all tests except sharness tests that need to be updated to reflect the change of behaviour.
I am not sure of the value of combining -w and --to-files, which results in adding an unnamed directory in MFS. Filesystems usually use human readable names, so I don't understand why it would be desirable to have a directory named after a CID. It would be like calling mkdir without providing a name for the new directory.
|
Triage notes:
@ProximaNova we also may miss the point of your PR. Do you mind sharing exact CLI commands you would use with this PR, and how you have to do things without it? We want to make sure existing UX is acceptable. |
The benefit would be that everything you add to IPFS also easily or quickly (in one command) gets added to MFS in an explicitly/very deduplicated way* with no bad collisions. This is true if you always run the same options. Like if you always run "ipfs add -wrH --to-files=/cid fileOrFolderName". My original thoughts were about doing things in an "archived by design" way. Everything added to ipfs should also be added to mfs (not later, but immediately and by design), even if it's in an imperfect starter-level state. This is because organizing things in a file system setup (mfs) is better than merely having a set of pins. Without this pull request code, there is ways to always run --to-files and never hit annoying collisions (like "readme.txt already exists in the folder"). With Bash, you can always run "time=$(date +%Y)/...; ipfs files mkdir -p /cid/$time; ipfs add -rHw --to-files=/cid/$time fileOrFolderName # $time = 2025/01/09/02/03/04". That "messily" fixes the collision problem, but not the deduplication problem in mfs.
I anticipated such an idea. The idea that whatever added to MFS by --to-files should only be a file name of folder name of something in the OS file system. However, having options is good, and just disabling -w + --to-files is not the best solution. Something has to happen if add + -w + --to-files is used. We could even having a thing in $IPFS_PATH/config JSON which does this:
Without this code, to do what this code does I have only two lame options (Bash):
*Deduplication by unique CIDs, of course you can have two different folders which have the same files (and different files). |
Make ipfs add + --to-files + --wrap-with-directory work by adding the root CID of what was added to MFS. The is different from past official Kubo which did this: added a literal "." folder to MFS (bug). It's also different from current official Kubo: disallow add to be ran with to-files and wrap.
Todo/ideas for this fork: make something like --to-files-cid which only adds root added CID to MFS, whether it's a file, folder, ran with wrap, ran without wrap.(Done: add+to-files+wrap always defaults to adding the CID to MFS and not literal "." and not disabled.)This update makes perfect sense with --to-files; in other words it does the following:
If a user ran add + --to-files + -w then it's expected behavior to add the final CID to MFS. It being disabled would be unexpected. BTW, https://github.com/ipfs/community/blob/master/CONTRIBUTING_GO.md says something about changing branch name to something other than "master"; I didn't do that.