Skip to content

Conversation

SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Oct 16, 2025

PR Summary

This change allows for a single .dsc.manifests.json (or yaml) file to contain multiple resource or extension manifests so you no longer need a separate file per resource/extension.

No new test added as the dsctest resource now has a single manifest for all of its resources.

This turned out to be more complicated than I expected due to a known limitation in serde for handling HashMap<i32, String> which was also embedded in an enum which is now the case. To workaround this, the schema now has exitCodes be HashMap<String, String> which is what it actually is in the JSON and at runtime, we parse the key to be a i32 otherwise return an error.

Also some general code cleanup in the modified areas putting things in alpha order and modifying some code so that you get a specific error if there's invalid JSON/YAML instead of a generic error.

PR Context

FIx #1160

@SteveL-MSFT SteveL-MSFT requested review from Copilot and tgauth October 16, 2025 00:38
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR consolidates multiple resource manifests into a single .dsc.resourcelist.json file, eliminating the need for separate files per resource. The change introduces support for resource list files that can contain an array of resource manifests, while maintaining backward compatibility with single-resource manifest files.

Key changes:

  • Added ResourceManifestList struct to support arrays of resource manifests
  • Updated discovery logic to handle both single and list manifest files
  • Consolidated 18 separate dsctest resource manifest files into one dsctest.dsc.resourcelist.json

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/dsctest/dsctest.dsc.resourcelist.json New consolidated manifest file containing all dsctest resources
tools/dsctest/*.dsc.resource.json (multiple) Removed individual resource manifest files
lib/dsc-lib/src/discovery/command_discovery.rs Updated discovery and loading logic to support resource lists
lib/dsc-lib/src/dscresources/resource_manifest.rs Added ResourceManifestList struct
lib/dsc-lib/src/dscerror.rs Added InvalidManifest error variant
lib/dsc-lib/src/extensions/discover.rs Updated extension discovery to handle manifest lists
build.ps1 Updated file copy patterns to include resourcelist files

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@SteveL-MSFT
Copy link
Member Author

I'm going to update this PR based on recent discussion:

  • a single container format allowing any type of valid DSC manifest in the array
  • file to be called .dsc.manifestlist.json that contains a property manifests which is an array of manifests

@ThomasNieto
Copy link
Contributor

ThomasNieto commented Oct 17, 2025

Do we need to have list in the name of the file?

What about just having it be plural? dsc.manifests.json

Actually looks like that was the name of the file in the comment thread not sure why the new comment has a different name.

Was thinking that there's no reason to have separate lists for adapters, resources, extensions, etc... we could just have a single format that's an array of manifests. Perhaps .dsc.manifests.json?

@SteveL-MSFT
Copy link
Member Author

@ThomasNieto the original addition for list was to make it more obvious between resource and resources, but since we've moved on from that, I think just manifests is fine. Will change.

@SteveL-MSFT SteveL-MSFT changed the title Enable a single file to contain multiple resource manifests Enable a single '.dsc.manifests.json' file to contain many resource/extension manifests Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable multiple resources in single resource manifest

3 participants