feat: add 'ipfs multibase' commands by coryschwartz · Pull Request #8180 · ipfs/kubo

coryschwartz · 2021-06-08T00:17:37Z

This is intended to implement part 1 of #7939 (comment)

Usage examples

See #8180 (comment)

lidel

Thank you @coryschwartz, looks good, some cosmetic asks in comments inline.

Before we merge this PR, it needs tests similar to test/sharness/t0290-cid.sh.

Tests should cover:

encoding and decoding round (echo foo | ipfs multibase encode -b base32 | ipfs multibase decode and expect "foo" back)
- for a file
- for stdin and stdout
error behaviors when decoder finds
- unknown base prefix
- character outside expected base dictionary

core/commands/multibase.go

core/commands/root.go

core/commands/multibase.go

+ golint fix

lidel · 2021-07-01T23:50:53Z

Cleaned up and added tests.

@ribasushi @Stebalien @aschmahmann lmk if we can improve CLI ergonomics/docs here in any way for ipfs multibase commands.

Usage examples for quick eyeballing:

ipfs multibase

> ipfs multibase  --help
USAGE
  ipfs multibase - Encode and decode files or stdin with multibase format

SYNOPSIS
  ipfs multibase

SUBCOMMANDS
  ipfs multibase decode <encoded_file> - Decode multibase string
  ipfs multibase encode <file>         - Encode data into multibase string
  ipfs multibase list                  - List available multibase encodings.

ipfs multibase decode

> ipfs multibase decode --help
USAGE
  ipfs multibase decode <encoded_file> - Decode multibase string

SYNOPSIS
  ipfs multibase decode [--] <encoded_file>

ARGUMENTS

  <encoded_file> - encoded data to decode

DESCRIPTION

  This command expects multibase inside of a file or via stdin:

    > echo -n hello | ipfs multibase encode -b base16 > file
    > cat file
    f68656c6c6f

    > ipfs multibase decode file
    hello

    > cat file | ipfs multibase decode
    hello

ipfs multibase encode

> ipfs multibase encode --help
USAGE
  ipfs multibase encode <file> - Encode data into multibase string

SYNOPSIS
  ipfs multibase encode [-b=<b>] [--] <file>

ARGUMENTS

  <file> - data to encode

OPTIONS

  -b  string - multibase encoding. Default: base64url.

DESCRIPTION

  This command expects a file name or data provided via stdin.

  By default it will use URL-safe base64url encoding,
  but one can customize used base with -b:

    > echo hello | ipfs multibase encode -b base16 > output_file
    > cat output_file
    f68656c6c6f0a

    > echo hello > input_file
    > ipfs multibase encode -b base16 input_file
    f68656c6c6f0a

ipfs multibase list

This is alias/same command as pre-existing ipfs cid bases.
Decided to add it here as well for completeness and better UX.

> ipfs multibase list --help
USAGE
  ipfs multibase list - List available multibase encodings.

SYNOPSIS
  ipfs multibase list [--prefix] [--numeric]

OPTIONS

  --prefix   bool - also include the single letter prefixes in addition to the code.
  --numeric  bool - also include numeric codes.

> ipfs multibase list --prefix --numeric
      0  identity
0    48  base2
b    98  base32
B    66  base32upper
c    99  base32pad
C    67  base32padupper
f   102  base16
F    70  base16upper
k   107  base36
K    75  base36upper
m   109  base64
M    77  base64pad
t   116  base32hexpad
T    84  base32hexpadupper
u   117  base64url
U    85  base64urlpad
v   118  base32hex
V    86  base32hexupper
z   122  base58btc
Z    90  base58flickr

lidel · 2021-07-15T15:10:05Z

Note from sync discussion:
- ok to merge as-is
- nice to have: add -o for outputing to file without piping (better UX in Powershell). Could be this PR or separate one.

aschmahmann · 2021-07-15T16:28:39Z

@lidel another thing we may want to add here is a transcode function so that you don't have to do ipfs multibase decode | ipfs multibase encode -b=base16, but can instead do ipfs multibase transcode -b=base16. Not having this makes life pretty painful for environments that don't tolerate binary outputs well (e.g. PowerShell, but I suspect there are others)

gammazero

Please consider my comment. I think the correct way to do this is to have a stream decoder, instead of reading the entirety of the (possibly huge) file into memory.

gammazero · 2021-07-16T22:12:05Z

core/commands/multibase.go

+		if err != nil {
+			return fmt.Errorf("failed to access file: %w", err)
+		}
+		encoded_data, err := ioutil.ReadAll(file)


Would it be worth implementing a multibase Decoder that would take an io.Reader and return one as well, with a signature like NewDecoder(io.Reader) io.Reader

Then this could all be done without having to read the entire file into memory.

decReader := mbase.NewDecoder(file) return resp.emit(decReader)

@lidel Seems like this is something that should exist, or am I missing an obvious reason it does not? Seems like a NewDecode function could start reading the stream, and then construct the correct decoder and return it as an io.Reader.

Good question! I filled upstream issue multiformats/go-multibase#44 to discuss this, but out of scope for this PR.

Stebalien · 2021-07-22T15:21:11Z

Are any of the requested changes here blocking or just improvements? If they're just improvements, can we merge and iterate?

gammazero · 2021-07-22T15:50:31Z

Not blocking. Only asking implementor to take note before merging.

lidel · 2021-08-11T22:11:18Z

For the sake of shipping basic ipfs multibase encode|decode in go-ipfs 0.10,
I've extracted nice-to-haves to separate issues that can be tackled separately:

ipfs multibase transcode – Add "ipfs multibase transcode" command #8336
streaming ipfs multibase encode|decode instead of reading the entirety of the (possibly huge) file into memory – filled Streaming multibase decoder multiformats/go-multibase#44 to discuss adding a streaming API to go-multibase, but I also feel it might overlap with generic thing already tracked in go-ipfs-cmds can support passing stateful encoders #5780
-o --output <file> as alternative to stdout for betetr ergonomics in powershell – Support "--output=<file>" in cmds that return data to stdout #8335

@aschmahmann this PR is ready for your final 👀 and a squash-merge.

aschmahmann · 2021-08-18T20:47:58Z

core/commands/multibase.go

+		"decode": mbaseDecodeCmd,
+		"list":   basesCmd,
+	},
+	Extra: CreateCmdExtras(SetDoesNotUseRepo(true)),


Going to merge for now as this seems like it wouldn't hurt and matches what ipfs cid does.

However, I don't think this really works properly without applying the flag to each subcommand.

@lidel let's revisit during the RC process what we want to do here and figure out if ipfs cid has similar problems.

Continued in #8375 (comment)

Cory Schwartz added 5 commits June 7, 2021 17:16

add multibase helper command

c41b68a

handle files

82baaa3

descriptions

8d09010

test

36e60d2

files

0ccf9b2

coryschwartz marked this pull request as ready for review June 9, 2021 23:55

coryschwartz linked an issue Jun 10, 2021 that may be closed by this pull request

Pubsub subscription data encoding #7939

Closed

aschmahmann requested a review from lidel June 14, 2021 15:33

ipfs deleted a comment from welcome bot Jun 15, 2021

lidel requested changes Jun 15, 2021

View reviewed changes

core/commands/multibase.go Outdated Show resolved Hide resolved

core/commands/root.go Outdated Show resolved Hide resolved

core/commands/multibase.go Outdated Show resolved Hide resolved

lidel mentioned this pull request Jun 15, 2021

Pubsub subscription data encoding #7939

Closed

lidel added this to the go-ipfs 0.10 milestone Jun 30, 2021

BigLep assigned lidel and coryschwartz Jul 1, 2021

refactor: use base64url as default multibase

cb997aa

lidel unassigned coryschwartz Jul 1, 2021

lidel changed the title ~~add multibase helper command~~ feat: add 'ipfs multibase' commands Jul 1, 2021

lidel added 3 commits July 1, 2021 18:24

docs(cli): add multibase next to cid command

509b2a0

+ golint fix

refactor: improved ux of multibase command

27bf745

test: sharness for multibase command

1c8c234

lidel force-pushed the feat/multibase-cmd branch from 0af6c8b to 1c8c234 Compare July 1, 2021 23:32

lidel self-requested a review July 1, 2021 23:51

lidel approved these changes Jul 8, 2021

View reviewed changes

aschmahmann assigned aschmahmann and unassigned aschmahmann Jul 9, 2021

aschmahmann self-requested a review July 9, 2021 15:11

BigLep requested review from gammazero and removed request for aschmahmann July 15, 2021 15:17

BigLep mentioned this pull request Jul 15, 2021

Restarting node with invalid pin index takes excessive time #8149

Closed

gammazero reviewed Jul 16, 2021

View reviewed changes

This was referenced Aug 11, 2021

Add "ipfs multibase transcode" command #8336

Closed

Streaming multibase decoder multiformats/go-multibase#44

Open

BigLep mentioned this pull request Aug 13, 2021

[WIP] add multibase binary multiformats/go-multibase#42

Draft

BigLep removed this from the go-ipfs 0.10 milestone Aug 14, 2021

aschmahmann reviewed Aug 18, 2021

View reviewed changes

aschmahmann merged commit 7db27bf into ipfs:master Aug 18, 2021

This was referenced Aug 23, 2021

Release v0.10 #8176

Closed

SetDoesNotUseRepo(true) is not being used correctly #8375

Open

andey-robins mentioned this pull request Sep 3, 2021

feat: multibase transcode command #8403

Merged

Uh oh!

Conversation

coryschwartz commented Jun 8, 2021 • edited by lidel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage examples

Uh oh!

lidel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lidel commented Jul 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ipfs multibase

ipfs multibase decode

ipfs multibase encode

ipfs multibase list

Uh oh!

lidel commented Jul 15, 2021

Uh oh!

aschmahmann commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gammazero left a comment

Choose a reason for hiding this comment

Uh oh!

gammazero Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidel Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

Stebalien commented Jul 22, 2021

Uh oh!

gammazero commented Jul 22, 2021

Uh oh!

lidel commented Aug 11, 2021

Uh oh!

aschmahmann Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

lidel Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coryschwartz commented Jun 8, 2021 •

edited by lidel

Loading

lidel left a comment •

edited

Loading

lidel commented Jul 1, 2021 •

edited

Loading

aschmahmann commented Jul 15, 2021 •

edited

Loading

gammazero Jul 16, 2021 •

edited

Loading