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

Remove IsDir from ipfs object #907

Closed
jbenet opened this issue Mar 9, 2015 · 5 comments
Closed

Remove IsDir from ipfs object #907

jbenet opened this issue Mar 9, 2015 · 5 comments

Comments

@jbenet
Copy link
Member

jbenet commented Mar 9, 2015

currently ipfs object has an IsDir portion:

> ipfs object get QmWNj1pTSjbauDHpdyg5HQ26vYcNWnubg1JehmwAE9NnU9
{
  "Links": [
    {
      "Name": "",
      "Hash": "QmPHPs1P3JaWi53q5qqiNauPhiTqa3S1mbszcVPHKGNWRh",
      "Size": 262158,
      "IsDir": false
    },
    {
      "Name": "",
      "Hash": "QmPCuqUTNb21VDqtp5b8VsNzKEMtUsZCCVsEUBrjhERRSR",
      "Size": 262158,
      "IsDir": false
    },
    {
      "Name": "",
      "Hash": "QmS7zrNSHEt5GpcaKrwdbnv1nckBreUxWnLaV4qivjaNr3",
      "Size": 262158,
      "IsDir": false
    },
    {
      "Name": "",
      "Hash": "QmQQhY1syuqo9Sq6wLFAupHBEeqfB8jNnzYUSgZGARJrYa",
      "Size": 76151,
      "IsDir": false
    }
  ],
  "Data": "\u0008\u0002\u0018��4 ��\u0010 ��\u0010 ��\u0010 ��\u0004"
}

ipfs object is a plumbing command for the merkledag. it must not be aware of (or make assumptions about) datastructures on top, like unixfs. IsDir is unixfs specific. An ipfs unix (or maybe ipfs files) command is planned that will be unixfs specific. IsDir this belongs there, not here.

Sorry @cryptix -- this is my fault for doing lousy CR :( -- i should've explained this then.

@cryptix
Copy link
Contributor

cryptix commented Mar 9, 2015

I only added the IsDir field to the Link type in core/commands because it was needed to implement the trailing slash portion of #833. I didn't see a better way for transferring this information from the commands Run() to the Marshalers, especially in the case where Run() executes on the daemon, fetching this information on the client again seemed impractical. If you can show me a better Solution, i'd be happy to fix it.

@jbenet
Copy link
Member Author

jbenet commented Mar 9, 2015

Ah, this is tricky. the trailing slash is only on ls, correct? Also, this means that ls is now pulling out the objects themselves to check their type, right? this makes ls more expensive. maybe unixfs should include the type of each link in the Dir entries

@cryptix
Copy link
Contributor

cryptix commented Mar 9, 2015

The trailing slash is only on ls, correct?

Effectivly, yes. The changed mashalLinks() function is also used by object stat but there, the additional data isn't fetched, so IsDir is always false. (A quick fix might be to add json:",omitempty")

Also, this means that ls is now pulling out the objects themselves to check their type, right?

Indeed.

maybe unixfs should include the type of each link in the Dir entries

SGTM 😄

@jbenet
Copy link
Member Author

jbenet commented Mar 9, 2015

we can just make object and ls use different functions. ls can use its own, with the extra type information.

@cryptix
Copy link
Contributor

cryptix commented Mar 10, 2015

fixed by 1f1f9f9

@cryptix cryptix closed this as completed Mar 10, 2015
@Stebalien Stebalien mentioned this issue May 26, 2020
77 tasks
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

No branches or pull requests

2 participants