Skip to content

Fix path to tool settings file in build #47788

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 21, 2025
Merged

Fix path to tool settings file in build #47788

merged 1 commit into from
Mar 21, 2025

Conversation

dsplaisted
Copy link
Member

Fixes #17454

@Copilot Copilot AI review requested due to automatic review settings March 21, 2025 14:58
Copilot
Copilot AI reviewed Mar 21, 2025
View reviewed changes
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.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets: Language not supported

@ghost ghost added Area-NetSDK untriaged Request triage from a team member labels Mar 21, 2025
@dsplaisted dsplaisted requested a review from a team March 21, 2025 17:49
@dsplaisted dsplaisted merged commit 578f7a1 into dotnet:main Mar 21, 2025
41 checks passed
@shyamnamboodiripad
Copy link

@dsplaisted @baronfel

Thanks for fixing this issue! This addresses the race described in #47696 that we were running into in the dotnet/extensions repo's build. We are currently working around this race by targeting only net8.0 for our dotnet tool. (It previously used to target both net8.0 and net9.0.)

We would like to add back the net9.0 TFM since some users have reported problems with running the net8.0 version of our tool on machines where dotnet 8 is not installed (dotnet/extensions#6388 contains details about the error they are seeing). However, since the dotnet/extensions repo needs to use dotnet 9 SDK, we can't add back the net9.0 TFM until the fix in this PR is backported to the dotnet 9 SDK.

The fix seems low risk, and looks like other repos are also running into this (see @andrewlock's comment in #17454 (comment)). Could you please help back port this?

If that is not possible, is there a different workaround you could recommend that would allow us to add back the multi-targeting and the net9.0 TFM while still avoiding the race?

Thanks much!

cc @peterwald @RussKie

@baronfel
Copy link
Member

baronfel commented May 8, 2025

I'd be ok with a backport - normally .NET Tactics wants to see a) evidence of the fix working in a released version, and b) justification from a customer. Your feedback helps satisfy both!

@shyamnamboodiripad
Copy link

I'd be ok with a backport - normally .NET Tactics wants to see a) evidence of the fix working in a released version, and b) justification from a customer. Your feedback helps satisfy both!

Awesome thank you @baronfel! We have not had a chance to try out the fix in a released version for our specific repro - but I am assuming that evidence of the fix working is already available from other sources.

Also, just saw @andrewlock's work around in DataDog/dd-trace-dotnet#6905 which disables parallel build while preserving the multi-targeting. I'll look into updating the workaround we used for our tool above to do the same while we wait for the backported fix. (Thank you @andrewlock!)

shyamnamboodiripad added a commit to shyamnamboodiripad/extensions that referenced this pull request May 8, 2025
@shyamnamboodiripad
Add back net9.0 version of the aieval dotnet tool
2c7e8f9 
In dotnet#6148, we disabled net9.0 TFM for the aieval tool to work around the race described in dotnet/sdk#47696. The underlying issue was subsequently fixed in the SDK (via dotnet/sdk#47788). However, this fix has not been backported to the dotnet 9 SDK yet.

The SDK team is working on backporting the fix (see discussion in dotnet/sdk#47788 (comment)). But in the meanwhile, we can add back the net9.0 TFM and continue to work around the race by disabling parallel build.

This would help users of the aieval tool sidestep errors such as the ones described in dotnet#6388 when they dont have dotnet8 installed on the machine.

We can remove this workaround, once the backported fix is available in the dotnet 9 SDK.
@shyamnamboodiripad shyamnamboodiripad mentioned this pull request May 8, 2025
Merged
shyamnamboodiripad added a commit to dotnet/extensions that referenced this pull request May 8, 2025
@shyamnamboodiripad
Add back net9.0 version of the aieval dotnet tool (#6396)
91eabd6 
In #6148, we disabled net9.0 TFM for the aieval tool to work around the race described in dotnet/sdk#47696. The underlying issue was subsequently fixed in the SDK (via dotnet/sdk#47788). However, this fix has not been backported to the dotnet 9 SDK yet.

The SDK team is working on backporting the fix (see discussion in dotnet/sdk#47788 (comment)). But in the meanwhile, we can add back the net9.0 TFM and continue to work around the race by disabling parallel build.

This would help users of the aieval tool sidestep errors such as the ones described in #6388 when they dont have dotnet8 installed on the machine.

We can remove this workaround, once the backported fix is available in the dotnet 9 SDK.

Fixes #6388
@baronfel
Copy link
Member

baronfel commented May 8, 2025

/backport to release/9.0.1xx

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2025

Started backporting to release/9.0.1xx: https://github.com/dotnet/sdk/actions/runs/14915596777

@github-actions github-actions bot mentioned this pull request May 8, 2025
Open
jeffhandley pushed a commit to dotnet/extensions that referenced this pull request May 9, 2025
@shyamnamboodiripad @jeffhandley
Add back net9.0 version of the aieval dotnet tool (#6396)
8cdbd2e 
In #6148, we disabled net9.0 TFM for the aieval tool to work around the race described in dotnet/sdk#47696. The underlying issue was subsequently fixed in the SDK (via dotnet/sdk#47788). However, this fix has not been backported to the dotnet 9 SDK yet.

The SDK team is working on backporting the fix (see discussion in dotnet/sdk#47788 (comment)). But in the meanwhile, we can add back the net9.0 TFM and continue to work around the race by disabling parallel build.

This would help users of the aieval tool sidestep errors such as the ones described in #6388 when they dont have dotnet8 installed on the machine.

We can remove this workaround, once the backported fix is available in the dotnet 9 SDK.

Fixes #6388
shyamnamboodiripad added a commit to shyamnamboodiripad/extensions that referenced this pull request May 9, 2025
@shyamnamboodiripad
Add back net9.0 version of the aieval dotnet tool (dotnet#6396)
90c4fa9 
In dotnet#6148, we disabled net9.0 TFM for the aieval tool to work around the race described in dotnet/sdk#47696. The underlying issue was subsequently fixed in the SDK (via dotnet/sdk#47788). However, this fix has not been backported to the dotnet 9 SDK yet.

The SDK team is working on backporting the fix (see discussion in dotnet/sdk#47788 (comment)). But in the meanwhile, we can add back the net9.0 TFM and continue to work around the race by disabling parallel build.

This would help users of the aieval tool sidestep errors such as the ones described in dotnet#6388 when they dont have dotnet8 installed on the machine.

We can remove this workaround, once the backported fix is available in the dotnet 9 SDK.

Fixes dotnet#6388
jeffhandley added a commit to dotnet/extensions that referenced this pull request May 9, 2025
@jeffhandley @stephentoub
@MackinnonBuck @SteveSandersonMS @gewarren @eiriktsarpalis @jozkee @shyamnamboodiripad @peterwald @dependabot
Prepare 9.4.x update for Microsoft.Extensions.AI (#6409)
61dcce6 
* Update CHANGELOGS for M.E.AI libs (#6359)

* Add project template build tests + CG reporting (#6355)

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>

* Use ConversationId instead of ChatThreadId (#6367)

* Update OpenTelemetryChatClient/EmbeddingGenerator to 1.33 (#6366)

Also adds the Enable SensitiveData property to OpenTelemetryEmbeddingGenerators. This was missed when adding it to OpenTelemetryChatClient.

* Add AsIEmbeddingGenerator for Azure.AI.Inference ImageEmbeddingsClient (#6363)

* Add DataContent.Base64Data (#6365)

* Fix AIFunctionFactory handling of default struct arguments (#6381)

* Fix AIFunctionFactory handling of default struct arguments

* Extend testing to custom structs

* Use GetUninitializedObject instead of Activator

* Add JS dependency update instructions to chat template README (#6376)

* Add see also links to conceptual docs (#6368)

* Translate OpenAI refusals to ErrorContent (#6393)

Refusals in OpenAI are errors reported when the service can't generate an output that matches the requested schema. Translate refusals to ErrorContent now that we have it.

* Rename useJsonSchema parameter (#6394)

* Rename useJsonSchema parameter

The `GetResponseAsync<T>` methods accept a `bool?` parameter currently called `useJsonSchema`. This is confusing, because the whole point of the method is to create and use a JSON schema from the `T`. The parameter actually controls _how_ that schema is used, whether it's included as part of the messages (false), as part of a ChatResponseFormat in the ChatOptions (true), or up to the system to decide.

I've clarified it by renaming it from `useJsonSchema` to `useJsonSchemaResponseFormat`. It's wordier, but it disambiguates the intent.

* Update src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ChatClientStructuredOutputExtensions.cs

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

* Add JSON schema transformation functionality to `AIJsonUtilities` (#6383)

* Add initial schema transformation functionality and incorporate into the OpenAI leaf client.

* Update all leaf client implementions, improve naming, add testing.

* Remove redundant suppressions

* Address feedback.

* Add ChatOptions.RawRepresentationFactory (#6319)

* Look for OpenAI.ChatCompletionOptions in top-level additional properties and stop looking for individually specific additional properties

* Add RawRepresentation to ChatOptions and use it in OpenAI and AzureAIInference

* Remove now unused locals

* Add [JsonIgnore] and update roundtrip tests

* Overwirte properties only if the underlying model don't specify it already

* Clone RawRepresentation

* Reflection workaround for ToolChoice not being cloned

* Style changes

* AI.Inference: Bring back propagation of additional properties

* Don't use 0.1f, it doesn't roundtrip properly in .NET Framework

* Add RawRepresentationFactory instead of object? property

* Augment remarks to discourage returning shared instances

* Documentation feedback

* AI.Inference: keep passing TopK as AdditionalProperty if not already there

* Avoid caching in CachingChatClient when ConversationId is set (#6400)

* Add comment LoggingChatClient et al trace-level logging (#6391)

Also fixed the name of the LoggingSpeechToTextClientBuilderExtensions type to conform to patterns used elsewhere in the library.

* Fix test validation of aggregate usage counts (#6401)

* Add BinaryEmbedding (#6398)

* Add BinaryEmbedding

Also:
- Renames the polymorphic discriminators to conform with typical lingo for these types.
- Adds an Embedding.Dimensions virtual property.

* Fix keys for EvaluationResult.Metrics dictionary to reflect the correct metric names for Safety evaluators (#6361)

This was an unfortunate regression that was introduced during a recent refactoring.

The metrics returned from the Azure AI Foundry Evaluation service have different names than the ones we use in the Safety library. We translate the EvaluationMetric.Name of the metrics returned by the service to the more display friendly names used in the library before returning the metrics to the caller.

While the returned metrics were correctly patched up, the EvaluationResult.Metrics dictionary still stored these metrics by the original names returned from the service. Unfortunately, this meant EvaluationResult.Get would throw an exception when trying to fetch metric with name ViolenceEvaluator.ViolenceMetricName.

The fix in this commit fixes the keys in the dictionary as well. This commit also updates tests to cover the case being fixed.

Fixes #6360

* Update branding for Azure service used by Safety evaluators (#6362)

`Azure AI Content Safety service` -> `Azure AI Foundry Evaluation service`

* Remove CacheOptions from DiskBasedResponseCache (#6395)

Details for this change are available in #6387.

Fixes #6387

* Add back net9.0 version of the aieval dotnet tool (#6396)

In #6148, we disabled net9.0 TFM for the aieval tool to work around the race described in dotnet/sdk#47696. The underlying issue was subsequently fixed in the SDK (via dotnet/sdk#47788). However, this fix has not been backported to the dotnet 9 SDK yet.

The SDK team is working on backporting the fix (see discussion in dotnet/sdk#47788 (comment)). But in the meanwhile, we can add back the net9.0 TFM and continue to work around the race by disabling parallel build.

This would help users of the aieval tool sidestep errors such as the ones described in #6388 when they dont have dotnet8 installed on the machine.

We can remove this workaround, once the backported fix is available in the dotnet 9 SDK.

Fixes #6388

* Add some additional documentation around usage of cache, and CSP properties on report (#6377)

* Add documentation around proper usage of IDistributedCache

* Add Content-Security-Policy to prevent page from calling into other sites.

* Remove remark about IDistributedCache usage

* Fix package-lock.json

* Remove <remarks> start tag.

* Some API related fixes for the evaluation libraries (#6402)

* Rename IResultStore and IResponseCacheProvider

IResultStore -> IEvaluationResultStore and
IResponseCacheProvider -> IEvaluationResponseCacheProvider

* Include missing EvaluationContextConverter in AzureStorageJsonUtilities

Also use linked files to avoid the need to duplicate code.

* Reorder enum members

The new order goes from least desirable rating to most desirable.

* Refactor extension method overloads

Implement overloads that take ChatMessage by calling corresponding overloads that take ChatResponse.

* Refactor AddTurnDetails to support adding details for multiple turns

Adding single turns continues to be supported via a params array overload.

* Add missing parameter for timeToLiveForCacheEntries to DiskBasedReportingConfiguration

This was missed in an earlier PR that introduced the timeToLiveForCacheEntries on the constructor of DiskBasedResponseCacheProvider.

Also reorder constructor parameters for AzureStorageReportingConfiguration so that the parameters for caching apear next to each other and so that the parameter ordering is aligned with DiskBasedReportingConfiguration.

* Minor formatting changes

* Bump vite from 6.2.6 to 6.3.4 in /src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/TypeScript (#6354)

* Bump vite

Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 6.2.6 to 6.3.4.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v6.3.4/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-version: 6.3.4
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Shyam Namboodiripad <gnamboo@microsoft.com>

* Allow image rendering in evaluation report (#6407)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Stephen Toub <stoub@microsoft.com>
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Co-authored-by: David Cantú <dacantu@microsoft.com>
Co-authored-by: Shyam N <shyamnamboodiripad@users.noreply.github.com>
Co-authored-by: Peter Waldschmidt <pewaldsc@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Shyam Namboodiripad <gnamboo@microsoft.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@baronfel baronfel baronfel approved these changes

Assignees
No one assigned
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GenerateToolsSettingsFile causes incremental build issues
3 participants
@dsplaisted @shyamnamboodiripad @baronfel