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

fix(mcpack): check manifest include value before resolving #1446

Open
wants to merge 1 commit into
base: public
Choose a base branch
from

Conversation

HipsterBrown
Copy link
Contributor

While trying to build a project targeting the Raspberry Pi Pico with mcpack, I ran into an error:

### TypeError: call: not a function

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 mcpack with the mcmanifest tool, I noticed the latter has support for git sources that is missing from mcpack.

This PR fixes the current error by checking the type of the include field'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).

@phoddie
Copy link
Collaborator

phoddie commented Jan 2, 2025

Interesting find. The problem is very real. mcpack isn't expecting to find a Git repository in the includes. I'm not 100% sure what it should do here. I think the right approach is to make sure that the include gets passed through to mcconfig to handle. Will take a look.

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

Successfully merging this pull request may close these issues.

2 participants