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

CLI cannot process multiple passed files with the same name #8264

Open
bmwiedemann opened this issue Jul 16, 2021 · 19 comments · Fixed by ipfs/go-ipfs-cmds#220 or #8493
Open

CLI cannot process multiple passed files with the same name #8264

bmwiedemann opened this issue Jul 16, 2021 · 19 comments · Fixed by ipfs/go-ipfs-cmds#220 or #8493
Assignees
Labels
effort/days Estimated to take multiple days, but less than a week exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@bmwiedemann
Copy link
Contributor

bmwiedemann commented Jul 16, 2021

Version information:

go-ipfs version: 0.9.0
Repo version: 11
System version: amd64/linux
Golang version: go1.16

Description:

For https://github.com/bmwiedemann/ipfs-iso-jigsaw/blob/prehashnodirs/prehash.py#L53 I am trying to add files without directories into ipfs and noticed that directories are not printed in output, but what is worse, files are silently not added (or at least not reported)

mkdir -p /tmp/x/{1,2}
echo x > /tmp/x/1/x
echo y > /tmp/x/2/x
ipfs add -n --raw-leaves -- /tmp/x/1/x /tmp/x/2/x
added bafkreib3wkv3nhv3e7574y6hmolcjrxmlyzrxba2lpemh26bbojil2iio4 x
@bmwiedemann bmwiedemann added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Jul 16, 2021
@aschmahmann
Copy link
Contributor

Thanks for the report, it looks like the CLI command parser (https://github.com/ipfs/go-ipfs-cmds/blob/4ade007405e5d3befb14184290576c63cc43a6a3/cli/parse.go#L239) bundles up all the files into a map keyed by the file name (not path, inode, etc.) of the files leading to the described issue.

Perhaps just keying on path name instead of file name (i.e. removing https://github.com/ipfs/go-ipfs-cmds/blob/4ade007405e5d3befb14184290576c63cc43a6a3/cli/parse.go#L335) would do the job.

@bmwiedemann
Copy link
Contributor Author

I tested that L335 patch and indeed, it helps to add both files and also helps to get distinct path names in the output:

ipfs add -n --raw-leaves -- /tmp/x/1/x /tmp/x/2/x
added bafkreidtzm4frjuhvbeuzizsgbjqcyuc6pnnhhkcz5rmuttz3wrkvr6zvq /tmp/x/1/x
added bafkreib3wkv3nhv3e7574y6hmolcjrxmlyzrxba2lpemh26bbojil2iio4 /tmp/x/2/x
cd /tmp/
ipfs add -n --raw-leaves -- x/1/x x/2/x
added bafkreidtzm4frjuhvbeuzizsgbjqcyuc6pnnhhkcz5rmuttz3wrkvr6zvq x/1/x
added bafkreib3wkv3nhv3e7574y6hmolcjrxmlyzrxba2lpemh26bbojil2iio4 x/2/x

Thanks.

@bmwiedemann
Copy link
Contributor Author

It seems, the patch broke something else:

ipfs --upgrade-cidv0-in-output dag put --pin --input-enc raw --format dag-pb /path/to/openSUSE-Tumbleweed-DVD-i586-Snapshot20210714-Media.iso.dag-pb
Error: expected a regular file

And the other problem I encountered was exceeding the number of open files (1024 by default). The DVD has 5260 files.

@aschmahmann
Copy link
Contributor

aschmahmann commented Jul 18, 2021

It seems, the patch broke something else

@bmwiedemann are you sure? Does that work without the patch?

I tried this and it worked for me:

C:\Users\adin\workspace\test\example>echo "a" | ipfs add -q
QmXNiF7qYu82aXfuVsmAyNd7Eku7psgBu7eXKMNhcGZjfN

C:\Users\adin\workspace\test\example>ipfs block get QmRJAPrEuBZyezSe5zKGsfh9KT91v2Dyj6DqHMRiFTXoS4 > block.dagpb

C:\Users\adin\workspace\test\example>ipfs --upgrade-cidv0-in-output dag put --pin --input-enc raw --format dag-pb block.dagpb
bafybeibl57rc754ayntwyrj52oolpnmc472d5x4x73r3r5sprhmsponvou

I'll put up a PR to go-ipfs-cmds and go-ipfs to see how tests look.


A couple of side notes that we can discuss further on discuss.ipfs.io if you have additional questions (just tag me):

ipfs dag put is meant for putting individual blocks of data and all blocks of data should be kept under 1MiB or they will not be transferable by Bitswap.

So it seems pretty strange to me that:

  1. How you're getting a single dagpb block that's an ISO (a UnixFS DAG is what you'd get from ipfs add myIso)
  2. Why you are using ipfs dag put at all here. Perhaps you're looking for ipfs dag import/export for moving around DAGs as files.
  3. Why you'd need multiple file descriptors to open a single file being used for import (ISOs like zip and tar files are just one file to the file system no matter how many internal objects they represent internally)

@aschmahmann aschmahmann changed the title ipfs forgets to add files CLI cannot process multiple passed files with the same name Jul 18, 2021
@bmwiedemann
Copy link
Contributor Author

bmwiedemann commented Jul 18, 2021

Why you are using ipfs dag put at all here

The idea is to have https://github.com/bmwiedemann/ipfs-iso-jigsaw/ create a custom UnixFS PB object such as openSUSE-Tumbleweed-DVD-x86_64-Snapshot20210715-Media.iso = bafybeiby7fd6dp37qaqvre2dypadzg2yxlw7voeoxno7cwe3qt32seu5xe (537533 byte) that re-uses file CIDs that I already store independently plus approx 20 MB for non-file data for iso bootloader and directory data.

https://www.reddit.com/r/ipfs/comments/ohbqgv/efficiently_store_daily_iso_images/ explains it a bit.

Why you'd need multiple file descriptors to open a single file being used for import

I use fuseiso to mount the iso and call ipfs add with a list of those 5000 files in the iso.
One workaround would be to call it multiple times with a part of the file list, but would be nicer to have it working out of the box.

@aschmahmann aschmahmann added P1 High: Likely tackled by core team if no one steps up effort/days Estimated to take multiple days, but less than a week exp/novice Someone with a little familiarity can pick up and removed need/triage Needs initial labeling and prioritization labels Jul 30, 2021
@Zanda256
Copy link

Zanda256 commented Aug 28, 2021

Hullo team,
Can I pick this up? If no PR is in review currently or no one is working on it. @aschmahmann

@BigLep
Copy link
Contributor

BigLep commented Sep 10, 2021

@Zanda256 : it looks like PRs were created for this, and we're waiting to get this merged.

@aschmahmann : do you have anyone in mind for the PRs so we can get this merged?

@BigLep
Copy link
Contributor

BigLep commented Sep 24, 2021

@schomatis : this would be a good candidate to take on. The existing PRs weren't exactly what's needed. @aschmahmann will add more clarity on how to do this right.

@BigLep
Copy link
Contributor

BigLep commented Mar 3, 2022

Internal note: this is something we'd like for go-ipfs 0.13

@Zanda256
Copy link

Zanda256 commented Mar 3, 2022

@BigLep , so can I carry on with the implementation?

@BigLep
Copy link
Contributor

BigLep commented Mar 4, 2022

@Zanda256 : do you have an implementation separate from the PRs that are linked to this issue?

@Zanda256
Copy link

I had figured something out, but that was a long time ago. I just need to re-implement it. I'll submit a PR for review when it's ready.

@schomatis
Copy link
Contributor

@Zanda256 This is already implemented in #8493 (we're just polishing final details) linked above. Sorry for the noise.

@BigLep
Copy link
Contributor

BigLep commented Mar 23, 2022

@schomatis : I'm reopening because there is a linked PR that is still open. Feel free to close if that's wrong.

@BigLep BigLep reopened this Mar 23, 2022
@schomatis
Copy link
Contributor

Yes, sorry, we need to land the main PR in go-ipfs to close this: #8493.

@BigLep BigLep reopened this Apr 26, 2022
@BigLep
Copy link
Contributor

BigLep commented Apr 26, 2022

Reopening because #8850 is still open.

@BigLep
Copy link
Contributor

BigLep commented Apr 28, 2022

2022-04-28 conversation:

Remaining work:
If you add multiple items it should work (all files are added to the repo).
Previous: it didn't work and it was silent.
Now (as of #8850): it doesn't work but errors
Future: it works

@schomatis
Copy link
Contributor

@BigLep I've been over this issue too many times to the point of losing confidence in my ability to understand exactly what is being requested. If you can provide the exact commands and outputs (either success or error, and what error) I'll give it one last shot. Otherwise, I'll un-assign myself here and someone closer to the request can implement the final details with as much support from me as needed, of course. Blocking until then.

@bmwiedemann
Copy link
Contributor Author

I don't mind closing this issue. It is not that important.

The original idea was that I could do a single ipfs add call to add all files from a directory tree, without adding the directory objects (as would happen with ipfs add -r).

For that, the output would need to be something like this:

mkdir -p 1 2
echo 1 > 1/x
echo 2 > 2/x

ipfs add 1/x 2/x
added QmdytmR4wULMd3SLo6ePF4s3WcRHWcpnJZ7bHhoj3QB13v 1/x
added QmSFxnK675wQ9Kc1uqWKyJUaNxvSc2BP5DbXCD3x93oq61 2/x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/days Estimated to take multiple days, but less than a week exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up
Projects
No open projects
Status: 🛑 Blocked
5 participants