Skip to content

Conversation

@Stebalien
Copy link
Member

This isn't the correct fix but it (mostly) restores the previous behavior.

Note 1: It also fixes resolution of /ipld paths by avoiding path.FromCid.
Note 2: This does not preserve the error. That was a DagCBOR specific error that has nothing to do with the issue.

This isn't the *correct* fix but it (mostly) restores the previous behavior.

Note 1: It *also* fixes resolution of `/ipld` paths by avoiding `path.FromCid`.
Note 2: This does not preserve the error. That was a DagCBOR specific error that
has nothing to do with the issue.

License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien requested a review from Kubuxu as a code owner October 30, 2018 19:39
@ghost ghost assigned Stebalien Oct 30, 2018
@ghost ghost added the status/in-progress In progress label Oct 30, 2018
@Stebalien
Copy link
Member Author

Alternative to #5704 that mostly preserves the current behavior. In retrospect, I actually prefer just breaking this now (we're not going to pass the js-ipfs-api tests either way).


c := rp.Cid()
if rp.Remainder() != "" {
return fmt.Errorf("path does not end on a dag-node boundary")
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

i.e. found non-link at given path

Copy link
Member Author

Choose a reason for hiding this comment

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

That error message is just a side-effect of DagCBOR. But yeah, we can keep it for now.

License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien merged commit d5c8fa5 into master Oct 30, 2018
@ghost ghost removed the status/in-progress In progress label Oct 30, 2018
@Stebalien Stebalien deleted the fix/resolve-non-terminal branch February 28, 2019 22:47
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.

3 participants