feat: remove ipld legacy and format globals in ipld/merkledag

[aschmahmann]

Related to #291

[aschmahmann]

@willscott WDYT about explicitly passing the go-ipld-prime linksystem here instead of requiring the default? This might make it easier for people to get out of the "global defaults" game.

[willscott]

is there a reason to have this as a global versus the approach i took in ipfs/go-merkledag#104 and making the decoder in the NewDAGService below?

[aschmahmann]

Initially it was just a stylistic preference that I had since a) it's still a global in 104, it's just a little more subtle b) it's a bit of a DRY thing where I didn't want to accidentally change one instead of the other.

However, I'm now also thinking that given we still have this global we might want to make it settable until we can make relying on a merkledag global registry no longer a thing (especially since it also implicitly hides the go-ipld-prime global registry)

[aschmahmann]

@willscott thanks for doing ipfs/go-merkledag#104. I mostly copied it. Although I left the globals up in the global space as a reminder for us to try and exit globals land when it becomes possible 😅

[willscott]

as a reminder for us to try and exit globals land when it becomes possible

I'm confused by this - in 104 i did exit globals land. - is there a reason not to stay consistent with that in the copy here, since it's the direction we want to go anyway?

[aschmahmann]

I'm confused by this - in 104 i did exit globals land

We're still stuck with globals:

1. The merkledag libraries still have globals. IMO this is bad for the same reasons the other ones were bad, it just turns out the other ones are basically unused independently so for now we can at fairly low cost coalesce here (and solve a problem we currently have)
2. There are still go-ipld-prime globals used implicitly in go-ipld-legacy unless we fix that https://github.com/ipfs/go-ipld-legacy/pull/12/files#r1211104340
   • Ran out of time to do the kubo PR exercising this (chore: update boxo to version with fewer globals kubo#9907). Maybe it'll be fine to leave this merkledag PR mostly alone (aside from passing in something like a linksystem to go-ipld-legacy). However, to start wrestling control away from the globals we might also want to make the legacy decoder used by merkledag publicly settable (albeit with the various warnings about being careful about using it, and trying to only use it in top-level binaries).

is there a reason not to stay consistent with that in the copy here

If we go with making the go-ipld-legacy decoder publicly settable (and therefore not tied to go-ipld-prime's global registry) then this approach looks much better rather than just being a stylistic difference. If it's just a stylistic difference then 🤷.

[codecov]

Codecov Report

Merging #322 (3200229) into main (0927be4) will increase coverage by 0.41%.
The diff coverage is 61.29%.

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   49.26%   49.67%   +0.41%     
==========================================
  Files         280      283       +3     
  Lines       33850    33873      +23     
==========================================
+ Hits        16675    16828     +153     
+ Misses      15398    15287     -111     
+ Partials     1777     1758      -19     

Impacted Files	Coverage Δ
bitswap/bitswap.go	37.20% <ø> (ø)
bitswap/client/client.go	84.58% <ø> (+2.37%)	⬆️
bitswap/client/internal/getter/getter.go	64.10% <ø> (ø)
...tswap/client/internal/messagequeue/messagequeue.go	69.70% <ø> (+5.14%)	⬆️
bitswap/client/internal/peermanager/peermanager.go	65.41% <ø> (ø)
...ernal/providerquerymanager/providerquerymanager.go	64.73% <ø> (ø)
bitswap/client/internal/session/session.go	82.18% <ø> (ø)
.../internal/sessionpeermanager/sessionpeermanager.go	72.50% <ø> (ø)
bitswap/network/ipfs_impl.go	0.00% <ø> (ø)
bitswap/server/internal/decision/engine.go	76.05% <ø> (ø)
... and 18 more

... and 12 files with indirect coverage changes

[Jorropo]

ipfs/kubo#9787 (comment) 🤣 #309 has some merits I see

[aschmahmann]

merging without approval from the other kubo maintainers since I feel like @willscott's review is enough 😄. If someone objects we can fix it up before the boxo release tag.