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

feat: Use BlobFormat and properly support adding raw blobs #1518

Merged
merged 12 commits into from
Sep 28, 2023

Conversation

Frando
Copy link
Member

@Frando Frando commented Sep 25, 2023

Description

With #1496 we also got a BlobFormat in more parts of the code base. This extends this to the CLI, bringing with it a few features:

  • Stricter types about adding STDIN vs files
  • Support adding from STDIN with iroh blobs add
  • Support adding single blobs without a wrapping collection
  • Support BlobFormat in tickets
  • Preparations to have a seperate iroh provide or iroh send command that would start a temp node (similar to get)

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

@Frando Frando requested a review from rklaehn September 25, 2023 11:57
wrap_in_collection: bool,
},
/// Add a directory recursively.
Directory {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not for now, but isn't the question whether to include the directory name basically the same thing as wrap_in_collection? E.g. you add a directory , you might want to preserve the file name.

Copy link
Contributor

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

This should allow adding bare blobs without a wrapping collection. But how do I actually do it, from the CLI?

I tried adding a single file, and it always produces a wrapping collection. I also don't see a CLI option to disable that.

Tried this:

cargo run blob add ../testdir_large_files/test1

@Frando
Copy link
Member Author

Frando commented Sep 28, 2023

Alright:

  • Rebased on main
  • Combined the add options into a struct for more code reuse:
  • Added a --wrap option and support it throughout the interfaces From the docs:
    /// Wrap the added file or directory in a collection.
    ///
    /// When adding a single file, without `wrap` the file is added as a single blob and no
    /// collection is created. When enabling `wrap` it also creates a collection with a
    /// single entry, where the entry's name is the filename and the entry's content is blob.
    ///
    /// When adding a directory, a collection is always created.
    /// Without `wrap`, the collection directly contains the entries from the added direcory.
    /// With `wrap`, the directory will be nested so that all names in the collection are
    /// prefixed with the directory name, thus preserving the name of the directory.
    ///
    /// When adding content from STDIN and setting `wrap` you also need to set `filename` to name
    /// the entry pointing to the content from STDIN.

From my point this is as intuitive as it gets and ready to merge.

@Frando Frando requested a review from rklaehn September 28, 2023 09:59
@rklaehn
Copy link
Contributor

rklaehn commented Sep 28, 2023

~So what does this do if you give the path to a single file when doing iroh start? It seems that it wraps in a collection, which is
inconsistent with the default of add.

Other than that, seems to work just fine.~

Nah, all good. Seems to work.

@Frando Frando enabled auto-merge September 28, 2023 11:25
@Frando Frando added this pull request to the merge queue Sep 28, 2023
Merged via the queue into main with commit f3ed0ba Sep 28, 2023
@dignifiedquire dignifiedquire deleted the cli-format-add-get branch November 1, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants