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

fix: Better functionality for ipfs add --to-files -w #10658

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ProximaNova
Copy link

@ProximaNova ProximaNova commented Jan 10, 2025

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:

  • Without wrap, the final thing is "[CID] [file/folder name]" -> file/folder name added to MFS.
  • With wrap, the final thing is "[CID]" -> CID added to MFS

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.

@ProximaNova ProximaNova requested a review from a team as a code owner January 10, 2025 05:17
@ProximaNova ProximaNova changed the title Better functionality for ipfs add --to-files -w fix: Better functionality for ipfs add --to-files -w Jan 10, 2025
@lidel lidel requested a review from guillaumemichel January 14, 2025 15:49
@guillaumemichel
Copy link

Related to #10611 and #10612

@ProximaNova
Copy link
Author

Related to 10611 and 10612

(yes it is related to those two. I didn't mention that for some reason.)

Some checks were not successful
4 failing and 12 successful checks

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.

Copy link

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

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.

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