Change vendor imports so that it will use system wide modules if the the import from vendor directory fails. #412

Merged
merged 1 commit into from Dec 12, 2016

Projects

None yet

3 participants

@athmane
Contributor
athmane commented Dec 11, 2016

This will help maintaining pyinvoke package in most distro that does not accept bundled libraries (in my case Fedora).

The plan is to clear vendor/ and let pyinvoke use system provided modules.

@athmane athmane Change vendor imports so that it will use system wide modules if the
the import from vendor directory fails.
3327964
@codecov-io
codecov-io commented Dec 11, 2016 edited

Current coverage is 91.92% (diff: 48.14%)

Merging #412 into master will decrease coverage by 1.23%

@@             master       #412   diff @@
==========================================
  Files            21         21          
  Lines          1930       1968    +38   
  Methods           0          0          
  Messages          0          0          
  Branches        329        330     +1   
==========================================
+ Hits           1798       1809    +11   
- Misses           93        120    +27   
  Partials         39         39          

Powered by Codecov. Last update 753e487...3327964

@bitprophet
Member
bitprophet commented Dec 12, 2016 edited

Follow-up on #204 I take it? :D thanks!

This is an interesting reversal of my earlier assumption from that ticket, re: what a common solution might have been - namely to prefer global when present, with vendored as a fallback. In this case, you're preferring vendored, falling back to global.

I think that's good, because it means the only time the vendor copies will be skipped is when the installer/packager is explicitly choosing to forego the vendoring (& is thus implicitly responsible for ensuring dependencies are compatible versions). In that situation I'd assume the installer/packager knows what they're doing (vs a random user not realizing that installing some conflicting dependency is going to break their Invoke.)

Just curious, are you planning to pair this with a build script / .spec / whatever, which just does rm -rf invoke/vendor && (pip|yum|dnf|etc) install <all the deps>?

@bitprophet bitprophet merged commit 07cad51 into pyinvoke:master Dec 12, 2016

2 of 4 checks passed

codecov/patch 48.14% of diff hit (target 93.16%)
Details
codecov/project 91.92% (-1.24%) compared to 753e487
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bitprophet bitprophet added a commit that referenced this pull request Dec 12, 2016
@bitprophet bitprophet Changelog re #204, re #412, closes #204 dcd0ef0
@athmane
Contributor
athmane commented Dec 13, 2016

Just curious, are you planning to pair this with a build script / .spec / whatever, which just does rm -rf invoke/vendor && (pip|yum|dnf|etc) install ?

Yes (close enough), the only issue I think of is if the vedorized library is forked from upstream, in that case the changes should be merged upstream or downstream package.

@bitprophet
Member

Of course. IIRC my vendored copies are untouched aside from occasional hacks required to be vendorized, or (in the case of fluidity, though this was years ago so I'd hope it's released now) unreleased-but-public commits.

If you identify spots where that isn't the case, certainly let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment