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

Implement ipfs refs #2004

Merged
merged 28 commits into from May 16, 2019

Conversation

3 participants
@dirkmc
Copy link
Contributor

commented Apr 22, 2019

Implement the ipfs refs command:

USAGE
  ipfs refs <ipfs-path>... - List links (references) from an object.

SYNOPSIS
  ipfs refs [--format=<format>] [--edges | -e] [--unique | -u] [--recursive | -r] [--max-depth=<max-depth>] [--] <ipfs-path>...
  • implement unique flag
  • implement http-api

Depends on ipfs/js-ipfs-http-client#978

@ghost ghost assigned dirkmc Apr 22, 2019

@ghost ghost added the in progress label Apr 22, 2019

Show resolved Hide resolved src/cli/commands/refs.js Outdated

@dirkmc dirkmc force-pushed the feat/refs branch 2 times, most recently from 24ccb51 to aaf2d3b Apr 22, 2019

@dirkmc dirkmc marked this pull request as ready for review Apr 24, 2019

@dirkmc dirkmc requested a review from alanshaw Apr 24, 2019

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

After talking to @achingbrain I will take a look at making a change to js-ipfs-unixfs-exporter to include the parent CID so that a call to refs can be streamed (instead of collecting all the results, working out each link's parents and only then outputting a result)

@alanshaw

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Is refs only for unixfs nodes or should it work with all IPLD nodes? I was originally thinking you’d be talking directly to IPLD for this...

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2019

hmm that's a good question. I'll take a look at exactly what refs does and whether it makes sense to use unixfs exporter

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2019

Looking at the code it seems like js-ipfs-unixfs-exporter does traverse any kind of IPLD object, but only in order to find unix format ipld objects. I will want to do something very similar, but more generic, ie not just for unix format objects.

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2019

It turns out it's actually simple to traverse the refs DAG in js-ipfs with pull-traverse instead of relying on js-ipfs-unix-exporter, which is specifically aimed at unix format files

@alanshaw
Copy link
Member

left a comment

🚀

const { print } = require('../utils')

module.exports = {
command: 'refs-local',

This comment has been minimized.

Copy link
@alanshaw

alanshaw May 1, 2019

Member

We should be able to use this as ipfs refs local i.e. without the hyphen

This comment has been minimized.

Copy link
@dirkmc

dirkmc May 2, 2019

Author Contributor

@alanshaw it seems like it will be tricky to make the parser work with both
ipfs refs <path>
ipfs refs local

I can probably hack it to make it work, but before I do that do you think it would make more sense to the user to give the commands different names instead?

This comment has been minimized.

Copy link
@alanshaw

alanshaw May 8, 2019

Member

Lets keep compat with go-ipfs - it's not too hacky to detect when path === 'local' and do the right thing in the refs handler.

This comment has been minimized.

Copy link
@dirkmc

dirkmc May 8, 2019

Author Contributor

That part is easy, it's making the command line parser / help text auto-formatter understand it that will be a little trickier, I'll see if I can make it work

This comment has been minimized.

Copy link
@alanshaw

alanshaw May 8, 2019

Member

Good point, please take a look but if it's going to be a big time sink or over complicated then lets forget it.

This comment has been minimized.

Copy link
@dirkmc

dirkmc May 9, 2019

Author Contributor

I added a command aliaser at the beginning of parsing (unfortunately it's not possible to do with yargs middleware): https://github.com/ipfs/js-ipfs/pull/2004/files#diff-71bd1bde77e38a68f76195b8f85ce19c

Show resolved Hide resolved src/cli/commands/refs-local.js Outdated
Show resolved Hide resolved src/cli/commands/refs-local.js Outdated
Show resolved Hide resolved src/cli/commands/refs-local.js Outdated
Show resolved Hide resolved src/cli/commands/refs.js Outdated
Show resolved Hide resolved src/http/api/resources/files-regular.js Outdated
Show resolved Hide resolved src/http/api/resources/files-regular.js Outdated
Show resolved Hide resolved src/http/api/routes/files-regular.js
Show resolved Hide resolved src/http/api/routes/files-regular.js
Show resolved Hide resolved test/utils/ipfs-exec.js Outdated

@dirkmc dirkmc requested a review from alanshaw May 3, 2019

@dirkmc dirkmc referenced this pull request May 8, 2019

Open

Implement Garbage Collection #2022

9 of 11 tasks complete
Show resolved Hide resolved src/core/components/object.js Outdated

@alanshaw alanshaw referenced this pull request May 9, 2019

Closed

⚡️ v0.36.0 RELEASE 🚀 #2024

43 of 44 tasks complete

@dirkmc dirkmc force-pushed the feat/refs branch from b69d378 to ba840f1 May 9, 2019

@momack2 momack2 added this to In Progress in ipfs/js-ipfs May 10, 2019

@momack2 momack2 added this to In Progress in ipfs/js-waffle May 10, 2019

@alanshaw

This comment has been minimized.

Copy link
Member

commented May 13, 2019

@dirkmc I've released ipfs-http-client@31 and updated the dependency version here. What's left to do on this PR? I'd like to get it merged ASAP 😁

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

@alanshaw I think it's good to go 🚀

@dirkmc

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

Oh hmm let me fix that commit message... it's failing commitlint

@dirkmc dirkmc force-pushed the feat/refs branch from bb4356b to 29112bb May 13, 2019

Show resolved Hide resolved src/cli/commands/refs-local.js Outdated
@alanshaw

This comment has been minimized.

Copy link
Member

commented May 13, 2019

Can we get some 💚 CI please @dirkmc?

@dirkmc dirkmc force-pushed the feat/refs branch from bc9b9d6 to d0d577c May 13, 2019

@dirkmc dirkmc force-pushed the feat/refs branch from ce7c41f to c731ddf May 15, 2019

@alanshaw alanshaw merged commit 6dc9075 into master May 16, 2019

4 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
codecov/patch 90.95% of diff hit (target 86.52%)
Details
codecov/project 86.76% (+0.24%) compared to c3ba285
Details

hugomrdias added a commit that referenced this pull request May 16, 2019

git pushMerge branch 'feat/support-file-dom-api' of github.com:ipfs/j…
…s-ipfs into feat/support-file-dom-api

* 'feat/support-file-dom-api' of github.com:ipfs/js-ipfs:
  feat: implement ipfs refs and refs local (#2004)

hugomrdias added a commit that referenced this pull request May 16, 2019

chore: merge
* 'feat/support-file-dom-api' of github.com:ipfs/js-ipfs:
  feat: implement ipfs refs and refs local (#2004)

@alanshaw alanshaw deleted the feat/refs branch May 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.