Skip to content
This repository has been archived by the owner on Jun 5, 2019. It is now read-only.

use trezor-common as a submodule #248

Merged
merged 15 commits into from
Apr 10, 2018
Merged

Conversation

matejcik
Copy link
Contributor

@matejcik matejcik commented Apr 5, 2018

  • Attempt to build protobuf messages from trezor-common as part of setup.py. This falls back to in-tree versions if build_protobuf fails, e.g. if you don't have protoc or are on Windows where shell scripts don't work
  • make Travis check that in-tree protobuf messages are really generated from trezor-common
  • put a note in README saying how to update the submodule
  • install coins.json from trezor-common as part of the package
  • WIP - use coins.json in coins.py instead of static dictionaries

also flake8 is better and stricter here

@matejcik matejcik added this to the v0.9.2 milestone Apr 5, 2018
@matejcik matejcik force-pushed the matejcik/with-trezor-common branch 3 times, most recently from a3ed6b1 to c9e4574 Compare April 10, 2018 10:19
@prusnak
Copy link
Member

prusnak commented Apr 10, 2018

Looks great! ACK 12ea96a

matejcik added 15 commits April 10, 2018 15:58
This enforces presence of the trezor-common submodule, copies coins.json to the
package directory (from where we can install it with bdist) and if possible,
regenerates protobuf messages.

That currently doesn't work on Windows, because it's a shell script. Also it
relies on presence of `protoc` protobuf compiler. Therefore the regeneration
step is optional and converted protobuf messages should still be commited to
this repo.

coins.json, OTOH, is gitignored in trezorlib, and must be copied from
trezor-common every time. This works because sdist includes the vendor
directory.
This way, if the process fails, the files in trezorlib/messages remain
untouched. This is important because "setup.py build" now runs the
build_protobuf tool, and it can easily fail on a system without protoc.
also exclude vendor subdir over which we possibly don't have control
this removes some unused variables and also catches a couple bugs
@matejcik matejcik force-pushed the matejcik/with-trezor-common branch from 12ea96a to 928498c Compare April 10, 2018 14:00
@matejcik matejcik merged commit 928498c into master Apr 10, 2018
@matejcik matejcik deleted the matejcik/with-trezor-common branch April 10, 2018 14:02
@prusnak
Copy link
Member

prusnak commented Apr 10, 2018

🎉

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

Successfully merging this pull request may close these issues.

2 participants