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

Manifest fields override for cargo dev targets (e.g. examples) #354

Closed
boozook opened this issue May 9, 2024 · 11 comments · Fixed by #366
Closed

Manifest fields override for cargo dev targets (e.g. examples) #354

boozook opened this issue May 9, 2024 · 11 comments · Fixed by #366
Assignees
Labels
build-utils enhancement New feature or request manifest Package manifest -related things

Comments

@boozook
Copy link
Owner

boozook commented May 9, 2024

Could be great to get option to set (override) manifest's fields specially for concrete example. BundleID is mostly important.

Related message in the matrix room by @paulstraw.

@boozook boozook added enhancement New feature or request build-utils manifest Package manifest -related things labels May 9, 2024
@boozook boozook moved this to Todo in Playdate Development May 9, 2024
@boozook boozook changed the title Manifest fields override of cargo dev targets (e.g. examples) Manifest fields override for cargo dev targets (e.g. examples) May 9, 2024
@boozook boozook self-assigned this May 9, 2024
@paulstraw
Copy link

Thanks for considering this. The name would also be nice, because otherwise all the cards on device will still be indistinguishable by eye.

@boozook
Copy link
Owner Author

boozook commented May 10, 2024

I see this as table in package.metadata.playdate.dev."{example-name}" with same manifest fields as in main table, but all fields should be optional there.

And I see consistency problem here. Look, we have package.metadata.playdate.dev-assets that inherits by package.metadata.playdate.assets. To make this all consistent, it should be look like one of following two options:

  1. just add new fields
  • package.metadata.playdate.{manifest fields} - manifest
  • package.metadata.playdate.assets - main package assets
  • package.metadata.playdate.dev-assets - for all examples, inherited by main assets
  • new: package.metadata.playdate.dev.{example-name}.{manifest fields} - example's manifest, inherited by main manifest
    • or may be: package.metadata.playdate.dev-{example-name}.{manifest fields} to look like dev-assets
  1. change existing structure (also change toml scheme 😬), add new fields
  • package.metadata.playdate.{manifest fields} - manifest
  • package.metadata.playdate.assets - main package assets
  • change: package.metadata.playdate.dev.{example-name}.assets - for all examples, inherited by main assets
  • new: package.metadata.playdate.dev.{example-name}.{manifest fields} - example's manifest, inherited by main manifest

I prefer first one because it's simpler with less changes and without breaking changes.
But second option is more beautiful and consistent.

Also, this ☝🏻 + #209 together could look pretty awesome ❤️‍🔥

@paulstraw
Copy link

paulstraw commented May 10, 2024

This makes sense to me. For my particular use case, having the ability to specify assets at the individual example level would be great, but definitely not as important as the other metadata.

If pursuing the second route, maybe package.metadata.playdate.dev-assets could still be supported (deprecated?) to avoid having it be an immediate braking change?

I'd be happy to help out with a PR if you'd like, though I'd need a bit of a nudge in the right direction since I'm new to the codebase (and Rust itself).

@boozook
Copy link
Owner Author

boozook commented May 14, 2024

🤔 Let's say, okay. But here's another open question:
One powerful cargo-playdate feature that may become main-killer-feature in the future is compiling optimized binaries without gcc. ([[bin]]) There can be a lot of binaries. It turns out that for each of them we should be able to set manifest fields too.

@boozook
Copy link
Owner Author

boozook commented May 14, 2024

Also I have issue about workspace metadata and inheritance like package-fields - #26.

@boozook
Copy link
Owner Author

boozook commented May 14, 2024

@paulstraw, I'll do a little refactoring before. Because there is a lot of unnecessary stuff left there, e.g. for cargo-metabuild support and other currently unnecessary junk. That's easy and fast 🤔

@boozook
Copy link
Owner Author

boozook commented May 14, 2024

@paulstraw
I don't think this whole thing is a good idea. The complexity of the system grows catastrophically. It is no longer a chain of inheritance, but a reverse graph, where each node has several parents (read as sources).

Why don't we just name examples by the name of the example ([[example]].name or filename instead of package name?

@paulstraw
Copy link

That's fair enough. It doesn't really matter to me how the configuration happens, as long as it's possible to provide custom configuration for examples. As long as the bundle ID and name can be changed for examples, I agree that the main issue will have been resolved.

@boozook
Copy link
Owner Author

boozook commented May 19, 2024

@paulstraw: In the #360 is a fix that you waiting for.
But that's a temporary solution and point for change in future.
So, I'm not closing this issue until there is a full implementation of the feature.

@boozook
Copy link
Owner Author

boozook commented Jun 4, 2024

#360 published as beta now. I'll close this issue if all is ok there.

@boozook boozook moved this from Todo to Ready in Playdate Development Jun 4, 2024
@boozook
Copy link
Owner Author

boozook commented Jun 15, 2024

@paulstraw, do you have any comments or remarks on the implementation?

@boozook boozook closed this as completed Jun 18, 2024
@github-project-automation github-project-automation bot moved this from Ready to Done in Playdate Development Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-utils enhancement New feature or request manifest Package manifest -related things
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants