feat: add 'ipfs multibase' commands #8180

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add 'ipfs multibase' commands #8180

feat: add 'ipfs multibase' commands #8180

coryschwartz commented Jun 8, 2021 •

edited by lidel

Loading

lidel left a comment •

edited

Loading

lidel commented Jul 1, 2021 •

edited

Loading

lidel commented Jul 15, 2021

aschmahmann commented Jul 15, 2021 •

edited

Loading

gammazero left a comment

gammazero Jul 16, 2021 •

edited

Loading

lidel Aug 11, 2021

Stebalien commented Jul 22, 2021

gammazero commented Jul 22, 2021

lidel commented Aug 11, 2021

aschmahmann Aug 18, 2021

lidel Jan 13, 2022

feat: add 'ipfs multibase' commands #8180

feat: add 'ipfs multibase' commands #8180

Conversation

coryschwartz commented Jun 8, 2021 • edited by lidel Loading

Usage examples

lidel left a comment • edited Loading

Choose a reason for hiding this comment

lidel commented Jul 1, 2021 • edited Loading

ipfs multibase

ipfs multibase decode

ipfs multibase encode

ipfs multibase list

lidel commented Jul 15, 2021

aschmahmann commented Jul 15, 2021 • edited Loading

gammazero left a comment

Choose a reason for hiding this comment

gammazero Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

lidel Aug 11, 2021

Choose a reason for hiding this comment

Stebalien commented Jul 22, 2021

gammazero commented Jul 22, 2021

lidel commented Aug 11, 2021

aschmahmann Aug 18, 2021

Choose a reason for hiding this comment

lidel Jan 13, 2022

Choose a reason for hiding this comment

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