Fix buggy support of ethpm-spec submodule

[njgheorghita]

What was wrong?

For developer setup / to run the ethpm tests successfully, developers will have to initialize the ethpm-spec/ submodule.

Todo:

• Add entry to the release notes

Cute Animal Picture

[pipermerriam]

What happens when they don't initialize it. Do they get a reasonable error message? If not I'd advocate for something in the test suite that detects the directory is missing and gives something indicating how it should be fixed.

[wolovim]

Just checked out Trinity for a reference:

Actually, pretty sure the Py-EVM references are a copy pasta bug? But if the recursive flag works to the same effect, might be nice to keep that consistency across libs.

[njgheorghita]

@marcgarreau Good call on the --recursive flag, seems to work great on my end and removes an extra command. I also think it would be a good idea to put a check in the respective tests like @pipermerriam suggested, since it's possible users are just regular cloning / not directly copying the command. I'll write up a check in the morning for the ethpm tests that will tell users what to do if they don't have the submodule available.

[njgheorghita]

@marcgarreau Ok, I feel much better about this now.

• I updated the clone command in the README to have the --recursive flag.
• If users clone this repo without the --recursive flag, and use a function that requires an ethpm-spec asset, there's now a check that will tell them how to import the files if the files are unavailable. I also moved this down from the module-level, so if users don't have the submodule but don't need it for whatever they're doing, then it won't bug them to initialize their submodule.
• I added two lines to MANIFEST.in that ensure the necessary ethpm-spec json assets are included in the builds. (I built the dist locally, and tested importing it into another project and the assets are there, and everything's working nicely)

[pipermerriam]

I think this error message should be expanded since there are two main paths that it seems like a user would find themselves in this situation.

1. pip install web3 with a broken package that doesn't include the assets
2. get clone ... that doesn't pull the submodules.

Maybe lets expand this error message to include sections for both of those. In the case where the problem is get clone based, the user has agency to fix it. In the pip install ... case, they have no agency to fix the problem because it is likely indicative of a broken release....

The current exception messaging would be confusing in the pip install case since it would not actually fix the user's problem.

Alternatively, we could allow use of an environment variable to point at a different spec directory than the default one... but I'm skeptical that is the right approach because it would only serve as a way to make broken web3.py releases work, when we really should be focusing on not issuing broken releases....

[njgheorghita]

@pipermerriam My assumption was that adding the spec json assets in MANIFEST.in (as far as I understand) guarantees that the spec files will always be included in the distribution, so I wasn't really considering case #1 to be a probable scenario. That being said, things can always go wrong I guess, but it just seemed unlikely enough that it doesn't warrant its own message. But, if you feel like this is a case we should guard against, I can definitely update the warning strategy.

[wolovim]

queues up beta.2

[pipermerriam]

What about someone who does pip install GIT-HUB_URL? I suspect that would put us in that position as well?

[njgheorghita]

I just tested a fresh install with pip install git+https://github.com/ethereum/web3.py/ and the ethpm-spec submodule assets are all included in that install. This should mean all the install stories are covered (at least the ones I'm familiar with), but if we want to go with a better safe than sorry approach here, I can update the warning.

[pipermerriam]

Interesting. I would not have expected the submodules assets to make their way in. In fact, I'm surprised enough by that to be suspicious (but not enough to dig in myself).

If you are confident that there are not any obvious cases where a user would find themselves in this position, I don't think the warning/error-messaging change is necessary.

[njgheorghita]

I was also curious, these docs seemed the most helpul to me, but I was unable to glean any relevant/meaningful insight from those docs, my best guess is that it uses the MANIFEST.in file during the installation process.