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

Importer with empty source should fail with wrap with directory? #159

Open
vasco-santos opened this issue Jul 13, 2021 · 2 comments
Open
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/discussion Topical discussion; usually not changes to codebase P2 Medium: Good to have, but can wait until someone steps up topic/docs Documentation

Comments

@vasco-santos
Copy link
Member

When using a wrapWithDirectory: false, nothing will be yielded from the unixfs importer when providing an empty import candidate. However, when wrapWithDirectory: true the empty directory is returned.

Is this the expected behaviour?

import normalizeAddInput from 'ipfs-core-utils/src/files/normalise-input/index.js'
import { importer } from 'ipfs-unixfs-importer'

const input = []

const rootEntry = await last(pipe(
    normalizeAddInput(input),
    (source: any) => importer(source, blockstore, {
      cidVersion: 1,
      chunker: 'fixed',
      maxChunkSize: 262144,
      hasher: sha256,
      rawLeaves: true,
      wrapWithDirectory: true
    })
  ))
@vasco-santos vasco-santos added the need/triage Needs initial labeling and prioritization label Jul 13, 2021
@rvagg
Copy link
Member

rvagg commented Jul 14, 2021

This doesn't seem unreasonable behaviour to me - you're essentially switching between "wrapping this nothing in nothing yields nothing " and "wrapping this nothing in something yields something".

Do you think it should error? Or the wrapped version to return nothing? Seems like a squishy edge case where the behaviour just needs to be well defined and documented.

@vasco-santos
Copy link
Member Author

Initially, I was expecting this to error, or at least not yield anything.

But, perhaps we should allow to wrap nothing with a directory 🤷🏼 I agree that we just need to be well defined and documented. I am ok with this behaviour if we document it

@lidel lidel added exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/discussion Topical discussion; usually not changes to codebase P2 Medium: Good to have, but can wait until someone steps up topic/docs Documentation and removed need/triage Needs initial labeling and prioritization labels Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/discussion Topical discussion; usually not changes to codebase P2 Medium: Good to have, but can wait until someone steps up topic/docs Documentation
Projects
None yet
Development

No branches or pull requests

3 participants