fix(mcpack): check manifest include value before resolving#1446
fix(mcpack): check manifest include value before resolving#1446HipsterBrown wants to merge 2 commits intoModdable-OpenSource:publicfrom
Conversation
|
Interesting find. The problem is very real. |
7e94774 to
17e939e
Compare
|
Bumping this PR since I'm running into the issue again while working on some new starting guides for the xs-dev docs and testing the example projects on the Raspberry Pi Pico. I also ran into an issue related to the PICO_EXTRAS_DIR and PICO_EXAMPLES_DIR paths being set in the pico's device manifest.json, which throws an error when trying to build with |
|
Thanks for putting this back on the active list and the additional fixes. mcpack exists to allow you to work in the npm way. The git support is kind of a parallel world. But, they can collide. mcpack eventually outputs a manifest that mcconfig runs (at least that's my memory at the moment...) but obviously the contents of any git repos are going to be opaque to mcpack. Maybe that's ok though. |
|
Yes, this patch basically passes through the include config for that resource to mcconfig since it is unlikely to be a |
While trying to build a project targeting the Raspberry Pi Pico with
mcpack, I ran into an error:All other platform targets were working, so I started exploring the cause.
After digging around the source code for
mcpack, I narrowed down the issue to how the tool resolves modules from included manifest.json files. This led to the arducam module for the ECMA-419 image/in implementation, which uses a git include for the Pico.When comparing
mcpackwith themcmanifesttool, I noticed the latter has support for git sources that is missing frommcpack.This PR fixes the current error by checking the type of the
includefield's value before attempting to resolve the expected string path to the included module.I'm not sure if git support should be added to mcpack if the evaluated manifest from mcpack still goes through mcconfig (a subclass of mcmanifest).