Skip to content

Perl Hosting Integration - Unique Resource Installer Names#1286

Open
Omnideth wants to merge 1 commit intoCommunityToolkit:mainfrom
Omnideth:feature-updating-resource-installers
Open

Perl Hosting Integration - Unique Resource Installer Names#1286
Omnideth wants to merge 1 commit intoCommunityToolkit:mainfrom
Omnideth:feature-updating-resource-installers

Conversation

@Omnideth
Copy link
Copy Markdown
Contributor

I didn't enforce uniqueness on their names. The Python extension adds the parent resource name and I felt like that was an appropriate solution.

PR Checklist

Closes #1285

Basically just adding the parent name to installer resources. Nothing fancy. A few tests to validate the installer count and names expected for various installer types. Validated in the example project for perl as well it now shows the parent name on the mojolicious installer.

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

…cause I didn't enforce uniqueness on their names. The Python extension adds the parent resource name and I felt like that was an appropriate solution.
Copilot AI review requested due to automatic review settings April 18, 2026 02:26
@github-actions
Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1286

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1286"

Copy link
Copy Markdown
Contributor

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 addresses Aspire resource installer name collisions in the Perl hosting integration by making per-module installer resource names unique per parent Perl app resource, and adds tests to validate the new naming/uniqueness behavior.

Changes:

  • Prefix per-module installer resource names with the parent resource name to prevent collisions across multiple Perl resources.
  • Update existing tests to match the new installer naming scheme.
  • Add new tests covering multiple resources sharing the same packages, ensuring installer uniqueness and correct parent relationships.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/CommunityToolkit.Aspire.Hosting.Perl/PerlAppResourceBuilderExtensions.PackageManager.cs Changes per-module installer naming to include the parent resource name.
tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/WithPackageTests.cs Updates expected installer names and adds multi-resource uniqueness/parenting tests.
tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/WithLocalLibTests.cs Updates expected installer name to include parent resource name.
tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/InstallerIntegrationTests.cs Adds a Linux-only test to ensure perlbrew + shared package still yields separate installers.
tests/CommunityToolkit.Aspire.Hosting.Perl.Tests/ApplicationModelCompositionTests.cs Adds a composition test ensuring shared packages across resources build successfully with unique installer names.
Comments suppressed due to low confidence (1)

src/CommunityToolkit.Aspire.Hosting.Perl/PerlAppResourceBuilderExtensions.PackageManager.cs:397

  • AddPackageInstaller uses the unsanitized installerName (which can contain : from module names like OpenTelemetry::SDK) when calling TryCreateResourceBuilder, but the actual PerlModuleInstallerResource is created with a sanitized name (Replace(":", "8")). This breaks the “installer already exists” check for modules containing : and can lead to duplicate installer resources. Consider computing a single sanitized installer resource name (e.g., via SanitizeInstallerResourceName) and using it consistently for both the lookup and the resource constructor.
        var installerName = $"{resource.Resource.Name}-{packageName}-installer";
        resource.ApplicationBuilder.TryCreateResourceBuilder<PerlModuleInstallerResource>(
            installerName, out var existingResource);

        if (existingResource is not null)
        {
            // Installer already exists for this package
            return;
        }

        var installer = new PerlModuleInstallerResource(
            installerName.Replace(":", "8"), // replace colons with '8' for the installer resource name (Aspire resource names cannot contain ':')
            packageName,

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.

Perl Hosting Integration - Resource Installer Name Collision

2 participants