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

Type conflicts between normal and gx imports of the same package #63

Closed
sahib opened this issue Mar 31, 2016 · 9 comments
Closed

Type conflicts between normal and gx imports of the same package #63

sahib opened this issue Mar 31, 2016 · 9 comments

Comments

@sahib
Copy link

sahib commented Mar 31, 2016

Hey,

Im not using gx myself, but Im trying to use ipfs as a library. I tend to run into the following issuse: When using the same package twice, once as normal import from github and once as gx import, go will spit out an error when mixing those types:

/tmp/mh/mh.go:12: cannot use b (type 
"gx/ipfs/QmYf7ng2hG5XBtJA3tN34DQ2GUN5HNksEw1rLDkmr6vGku/go-multihash".Multihash) 
as type "github.com/jbenet/go-multihash".Multihash in assignment

See this gist for the code producing this error.

This usually happens when using a function from ipfs or libp2p that returns a multihash, multiaddr (etc.) and I need to store those in my own structures. I could probably resolve that by being forced to use gx myself or by brute force: Convert all types manually into my own types. Both solution would not be favorable for me.

I might be missing something, but gx seems to make the usage of ipfs as library harder... :-(

@whyrusleeping
Copy link
Owner

@sahib yeah, This is a problem we've been having even before using gx.

Take a look at this gist: https://gist.github.com/whyrusleeping/a297dc2ae4b3f54daa666fc1f04562a8

Its the same exact problem, and a failing in how go works, if you were using gx for javascript, this wouldnt be an issue.

The solution i'm trying to work towards is twofold, the first is to take a better approach at how we design the interfaces we export. Some discussion of that is here multiformats/go-multiaddr#17

The second part of the solution i mention near the end of that thread is a non-breaking change to how go checks interfaces.

@whyrusleeping
Copy link
Owner

The quick and dirty solution for now is to just replace the imports in your code with the gx imports from the error message.

@sahib
Copy link
Author

sahib commented Apr 2, 2016

Sorry for blaming it on gx then.

The quick and dirty solution for now is to just replace the imports in your code with the gx imports from the error message.

Sadly really a dirty solution that will break again when packages are updated.
I will go with that until a better solution comes the way.

I guess this can be closed then.

@whyrusleeping
Copy link
Owner

@sahib this is a problem i'm very interested in solving. If you come across any good solutions, please let me know

@b5
Copy link

b5 commented Sep 28, 2017

Apologies in advance for a big post on a closed issue. But this exact problem has us in a particular world of pain atm.

We're building on top of the wonderful, stupendous, ipfs/go-ipfs. But not using GX quite yet, this Golang/GX type conflict problem is forcing us into a tough choice:

  1. Convert our entire codebase to using GX and keep lock-step sync with go-ipfs imports (a costly endeavor when we're on tight timelines).
  2. Fork go-ipfs and re-write all imports
  3. (we're currently) wrapping all of the IPFS bits we need in a package & shimming relative imports via interfaces.

In all honesty none of these options are the end of the world, but as someone very excited to build upon IPFS, and who wants others to build on ipfs, I can see how this would be the point where others would be less inclined / confused about how to solve the gx/no-gx issue in a codebase.

I think the spirit of GX is fantastic, but having hashes in me imports makes for terse stack traces, which often overflows my stderr so fast I can't see initial panic messages. Of greater concern is the cognitive overhead foisted onto the developer by having hashes in the import paths. Initial internal code reviews of gx'd code always start with a five minute tangent about what gx is, and then it's 10 minutes later and I'm still grumbling about go's super-duper-specific type system. 20 minutes after that we're in a holy war about weather anything is ever "idiomatic" or not, and all I wanted to do was ask for more unit tests.

Annnnnyway, none of this is to dump on gx specifically. gx seems to have a lot to say about build reproducibility without making us vendor everything all the time, and when looking at a gx'd package.json I'm reminded a lot of yarn lock files which use hashing techniques to solve npm nightmares. I'm wondering if it would be worth exploring a version of gx that obeys GOPATH-relative imports, but leverages package.json as a lock file that manages the vendor directory, with a gx test that could be included as a CI test? Some variation? Maybe gx generates symlinks in vendor to some content-addressed backing store for deduplication? Ok, that last bit sounds bonkers.

Another option would be to explore go's new alias feature, but that's a half-baked idea at best.

I'd love to join any discussion / dev about helping GX overcome this go-specific packaging issue if you'd have me (and by no means do any of the above ideas need to be implemented). Thanks for writing super fantastic awesome code, and please let me know if I can help!

@whyrusleeping
Copy link
Owner

@b5 Could you link me to your codebase? I'd love to have a case study on 'how people interact with gx'. There are a few different ways I can see moving forward, the simplest might be something along the lines of gx-go use github.com/ipfs/go-ipfs, which could automagically set up gx import paths in the project its run in, and maybe even make them point to hashes via symlinks in some way.

On the other hand, youre importing go-ipfs directly. What modules are you using exactly? go-ipfs is comitted with gx'ed deps because we don't want people to build it with incorrect deps accidentally and encounter bugs. Thats just required for the binary (package main), the rest of the packages are slated for removal from the main repo. All our other repos outside of go-ipfs are committed with github import paths, and you should be able to interact with them as you would any other normal go package.

@whyrusleeping
Copy link
Owner

@b5 Also, Just to be clear, if youre importing go-ipfs, you don't necessarily have to use gx for all of your deps. Just the ipfs ones.

@b5
Copy link

b5 commented Oct 3, 2017

Thanks @whyrusleeping! sorry for the delay on this, things have gotten a little nutty on my end. The codebase in question is currently closed, but will be opened up once the lawyers give us the go-ahead (hoping to release white paper on Oct. 31st, code shortly after), I'd be happy to invite you to review our codebase beforehand if you're interested, there are a few approaches in there that I'm very excited to get feedback on and see if there's anything we could work into tools the broader community could make use of.

I had a look at go-go use (I had hoped that tool would be around somewhere, but hadn't found it). In the end I bit the bullet & did the gx conversion for all IPFS related things manually, it was less painful than expected. ended up doing it manually b/c sometimes we're passing around things that exist just to satisfy interfaces, which makes room for gx / no-gx interplay.

I do now see the light on tracking directly with IPFS imports. Part of me thinks this should be filed under being mindful of our dependencies. Thanks again for the great work, and looking forward to upping our contributions to IPFS in the future!

@whyrusleeping
Copy link
Owner

@b5 Thats great to hear :) Any notes you have on how we can make this all easier would be much appreciated, either in the way of docs, tooling, bugfixes or anything else that could possibly help out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants