Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

test: fix resolve test #385

Merged
merged 2 commits into from
Feb 19, 2019
Merged

test: fix resolve test #385

merged 2 commits into from
Feb 19, 2019

Conversation

Stebalien
Copy link
Contributor

@Stebalien Stebalien commented Oct 30, 2018

  1. This is an IPLD path.
  2. Resolve really should be resolving up to the last path, not just "failing".

Neither js-ipfs nor go-ipfs pass this test. However, IMO, a PR is the easiest way to highlight this issue. Thoughts? Objections?

ipfs.resolve(path, (err, path) => {
expect(err).to.exist()
expect(err.message).to.equal('found non-link at given path')
const path = `/ipld/${cid.toBaseEncodedString()}/path/to/file`
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK js-ipfs doesn't support /ipld paths (please tell me I'm wrong). This is probably going to be painful to fix, and blocking go-ipfs release on this is less than ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're probaly not wrong.

@alanshaw
Copy link
Contributor

@Stebalien why is it useful for it to partially resolve a path? Could we implement this as an option parital=true to avoid a breaking change?

If we want to merge this, we should have a PR ready for go-ipfs/js-ipfs (or both) as well so that we know the test works!

@Stebalien
Copy link
Contributor Author

why is it useful for it to partially resolve a path?

This does fully resolve the path. It's just that some paths stop in the middle of objects, not at object boundaries. (really, that's why this command returns a path, not a CID).

@Stebalien
Copy link
Contributor Author

Go-ipfs PR: ipfs/kubo#5704

@alanshaw alanshaw removed the blocked label Feb 19, 2019
Stebalien and others added 2 commits February 19, 2019 13:14
1. This is an IPLD path.
2. Resolve really should be resolving up to the last path, not just "failing".
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw merged commit 92fb51b into master Feb 19, 2019
@ghost ghost removed the in progress label Feb 19, 2019
@alanshaw alanshaw deleted the fix/resolve-test branch February 19, 2019 13:16
alanshaw pushed a commit to ipfs/js-ipfs that referenced this pull request Feb 19, 2019
refs ipfs-inactive/interface-js-ipfs-core#385
resolves #1763

BREAKING CHANGE: `ipfs.resolve` now supports resolving to the middle of an IPLD block instead of erroring.

Given:

```js
b = {"c": "some value"}
a = {"b": {"/": cidOf(b) }}
```

`ipfs resolve /ipld/cidOf(a)/b/c` should return `/ipld/cidOf(b)/c`. That is, it resolves the path as much as it can.

Previously it would simply fail with an error.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants