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: Project Names with Spaces Break the Build #953

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

kiapanahi
Copy link
Contributor

@kiapanahi kiapanahi commented Nov 19, 2023

Fixes: #945.

I however couldn't find a solution for projects with (.) in their names.
I've figured out that the generated IServiceMetadata is correct but the actual name-of-project.AppHost template is outputting faulty template.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Nov 19, 2023
@DamianEdwards
Copy link
Member

DamianEdwards commented Nov 20, 2023

The issue when using a . in the project name seems like a regression from #510 but I'm not sure what's causing it. We might need help from the template engine folks because the name form we're using now should be indicating that we want the class form.

That said, you could try changing "sourceName": "AspireStarterApplication-1" to "sourceName": "AspireStarterApplication.1" and then change the reference to it in Program.cs to Projects.AspireStarterApplication__1_ApiService

If this works, it would be great if you could update it in the empty template too, and ensure the directory names, project files, etc. are updated to match.

@kiapanahi
Copy link
Contributor Author

@DamianEdwards

Thank you for the pointers.
I'd love for you to take a look at the updated templates.

@DamianEdwards
Copy link
Member

@kiapanahi have you tested the templates locally? After calling build.cmd -pack in the repo root, you should have a package like ./artifacts/packages/Release/Shipping/Aspire.ProjectTemplates.8.0.0-dev.nupkg that has the templates in it. You can install that locally by first deleting your template cache (~/.templateengine/packages directory and ~/.templateengine/packages.json) and then calling dotnet new -i <path-to-package.nupkg>. Then you can create some projects using your updated templates and verify the relevant scenarios (name with . in it, name with spaces in it, etc.).

@DamianEdwards
Copy link
Member

/azp run

Copy link

No pipelines are associated with this pull request.

@mmitche mmitche closed this Nov 21, 2023
@mmitche mmitche reopened this Nov 21, 2023
@kiapanahi
Copy link
Contributor Author

kiapanahi commented Nov 21, 2023

@DamianEdwards I did. The thing that I didn't test (to be honest didn't know how to test without much hassle) was the fix for replacing the single whitespace character with underlines which this PR fixes as well.

I think I know how to test it though. If I'm not wrong, I need to somehow reference my own build of Aspire.Hosting package instead of the !!REPLACE_WITH_LATEST_VERSION!! one which still contains the whitespace bug.

but the . part is fully tested locally.

@DamianEdwards
Copy link
Member

I think I know how to test it though. I I'm not wrong, I need to somehow reference my own build of Aspire.Hosting package instead of the !!REPLACE_WITH_LATEST_VERSION!! one which still contains the whitespace bug.

You'll need to create a local NuGet feed that points at the artifacts directory of the repo (./artifacts/packages/Release/Shipping/). Then you can reference the -dev suffixed version which is what you built.

@kiapanahi
Copy link
Contributor Author

Alright, I managed to test with the locally built -dev NuGet packages and I can confirm that the issue with the replacing of whitespace with underline is fixed.

Copy link
Member

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@DamianEdwards DamianEdwards merged commit 69077e9 into dotnet:main Nov 21, 2023
4 checks passed
@kiapanahi kiapanahi deleted the fix/projects-with-space-in-names branch November 21, 2023 22:06
@kiapanahi
Copy link
Contributor Author

Thank you for all of your pointers and helps

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project Names with Spaces Break the Build
3 participants