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

Wrap with directory option with uint8array data returns empty root CID #176

Open
vasco-santos opened this issue Sep 28, 2021 · 2 comments
Open
Labels
exp/beginner Can be confidently tackled by newcomers kind/maintenance Work required to avoid breaking changes or harm to project's status quo P3 Low: Not priority right now

Comments

@vasco-santos
Copy link
Member

vasco-santos commented Sep 28, 2021

Code snipped replicating the issue:

import IPFS from 'ipfs-core'

const ipfs = await IPFS.create()
const { cid } = await ipfs.add(new Uint8Array([1, 2, 3]), {
  cidVersion: 1,
  wrapWithDirectory: true
})
console.info(cid) // CID(bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354)

The expectation would be to either throw an error or wrap with a directory. From my debugging, it looks that the underlying reason for this is that a path is not provided and unixfs-importer will not be able to properly wrap with a directory. We can make this work with:

import IPFS from 'ipfs-core'

const ipfs = await IPFS.create()
const { cid } = await ipfs.add({ content: new Uint8Array([1, 2, 3]), path: '/path/to/file' }, {
  cidVersion: 1,
  wrapWithDirectory: true
})
console.info(cid)

We should either just thrown an error in such cases, or have a path inferred. @achingbrain what do you think to be the expectation here?

Related to storacha/ipfs-car#88

@vasco-santos vasco-santos added the need/triage Needs initial labeling and prioritization label Sep 28, 2021
@achingbrain
Copy link
Member

My preference would be to remove the wrapWithDirectory option - if you want to wrap the output in a directory, specify a path with a directory component.

Failing that, this should probably throw an error as otherwise we have to guess what the filename of the content should be inside the directory. We could use the CID of the content but that's hard for the user to predict ahead of time.

@lidel lidel added need/author-input Needs input from the original author kind/maintenance Work required to avoid breaking changes or harm to project's status quo P2 Medium: Good to have, but can wait until someone steps up P3 Low: Not priority right now and removed need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization P2 Medium: Good to have, but can wait until someone steps up labels Oct 1, 2021
@vasco-santos
Copy link
Member Author

My preference would be to remove the wrapWithDirectory option

Yeah, I think this would remove "magic" from the user and is a good call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/beginner Can be confidently tackled by newcomers kind/maintenance Work required to avoid breaking changes or harm to project's status quo P3 Low: Not priority right now
Projects
No open projects
Development

No branches or pull requests

4 participants