Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 10, 2025

Invert IncludeAutorestDependency Default Behavior

Progress

  • Analyze repository structure and understand the issue
  • Identify all libraries that need IncludeAutorestDependency=true
  • Change default in eng/Directory.Build.Common.props to false
  • Create script to add IncludeAutorestDependency=true to all required .csproj files
  • Address feedback: Add property to libraries with autorest.md OR tsp-location.yaml without new emitter
  • Add property to libraries with Generated folder but no tsp-location.yaml or autorest.md
  • Remove explicit IncludeAutorestDependency=false from files (now redundant)
  • Revert property for files that originally had explicit false
  • Include mgmt emitter as valid new TypeSpec emitter (no property needed)
  • Remove property from test framework libraries that don't truly need it
  • Revert all changes to Microsoft.Azure.Batch folder
  • Add property to Azure.AI.Inference per request
  • Add property to Azure.ResourceManager.AgriculturePlatform to address build failures
  • Merge latest changes from main branch
  • Update condition to require IsClientLibrary=true
  • Update documentation to remove mention of IncludeAutorestDependency
  • Refine condition to use != 'true' for better robustness
  • Remove duplicate property entries from files
  • Verify changes with build tests on sample projects
  • Final validation

Summary of Changes

Modified 373 files total:

  • 1 file: eng/Directory.Build.Common.props - Changed default from true to false with condition checking both explicit setting AND IsClientLibrary
  • 347 files: .csproj files - Added <IncludeAutorestDependency>true</IncludeAutorestDependency>:
    • 221 files with autorest.md (Autorest-based code generation, excluding test frameworks)
    • 73 files with tsp-location.yaml but no/invalid emitterPackageJsonPath (old TypeSpec generator)
    • 51 files with Generated folder but no tsp-location.yaml or autorest.md (old Autorest libraries, excluding Microsoft.Azure.Batch)
    • 2 files: Azure.AI.Inference and Azure.ResourceManager.AgriculturePlatform (added per explicit request to address build failures)
    • Note: Microsoft.ClientModel.TestFramework was removed from this list as it originally had explicit false
    • Note: Azure.Core.TestFramework was removed as it's a test framework that doesn't truly need the dependency
    • Note: All Microsoft.Azure.Batch files were reverted per request
  • 24 files: .csproj files - Removed explicit <IncludeAutorestDependency>false</IncludeAutorestDependency> or property that was incorrectly added:
    • 9 libraries that explicitly set the property to false - now redundant since false is the default
    • 1 library (Microsoft.ClientModel.TestFramework) that originally had explicit false - property removed entirely
    • 13 libraries using mgmt TypeSpec emitter - property incorrectly added, now removed
    • 1 library (Azure.Core.TestFramework) - test framework that doesn't truly need the dependency
  • 1 file: doc/DataPlaneCodeGeneration/AzureSDKCodeGeneration_DataPlane_Quickstart.md - Removed mention of IncludeAutorestDependency property

Merged latest changes from main branch to ensure the PR is up-to-date with the latest codebase.

Complete Logic for Adding Property

Properties added to 347 files based on:

  1. Has autorest.md file in src directory (Autorest-based code generation) - 221 files
    • Excluding test framework libraries that don't truly need the dependency
  2. Has tsp-location.yaml BUT:
    • No emitterPackageJsonPath entry, OR
    • emitterPackageJsonPath is NOT one of the new TypeSpec emitters:
      • eng/http-client-csharp-emitter-package.json
      • eng/azure-typespec-http-client-csharp-emitter-package.json
      • eng/azure-typespec-http-client-csharp-mgmt-emitter-package.json
    • This indicates old TypeSpec generator usage - 73 files
  3. Has Generated folder in src directory BUT:
    • No tsp-location.yaml file, AND
    • No autorest.md file, AND
    • Did NOT originally have explicit IncludeAutorestDependency=false
    • Excluding Microsoft.Azure.Batch per request
    • This indicates old Autorest-based generation without config files - 51 files
  4. Explicitly requested by maintainers to address build issues: Azure.AI.Inference and Azure.ResourceManager.AgriculturePlatform - 2 files

Duplicate entries removed:

  • 36 files had duplicate IncludeAutorestDependency entries from multiple script runs - cleaned up to have exactly one entry per file

Properties removed (27 files):

  • 9 libraries that explicitly set the property to false - now redundant since false is the default
  • 1 library (Microsoft.ClientModel.TestFramework) that originally had explicit false - property removed entirely to restore original intent
  • 13 libraries using mgmt TypeSpec emitter - property was incorrectly added, now removed to align with new emitter criteria
  • 1 library (Azure.Core.TestFramework) - test framework that doesn't truly need the dependency
  • 3 libraries in Batch tools directory - property was added but reverted as they don't need it

Changes reverted:

  • All changes to Microsoft.Azure.Batch folder (src and tools) - reverted per maintainer request

Documentation updated:

  • Removed guidance about IncludeAutorestDependency since new TypeSpec projects don't need to think about it

Condition refinement:

  • Final condition: <IncludeAutorestDependency Condition="'$(IncludeAutorestDependency)' != 'true' or '$(IsClientLibrary)' != 'true'">false</IncludeAutorestDependency>
  • This ensures the property is only true when:
    1. It's explicitly set to true in the .csproj file, AND
    2. The project is identified as a client library (IsClientLibrary == true)
  • This is more robust and prevents the property from being enabled for non-client libraries

No changes:

  • Libraries without code generation (no Generated folder, autorest.md, or tsp-location.yaml) - rely on default false
  • Libraries with tsp-location.yaml AND proper new emitter path (including mgmt emitter) - rely on default false (with exceptions for build issues)
  • Libraries already having the property explicitly set
  • Libraries that originally had explicit false setting - property removed to rely on new default
  • Microsoft.Azure.Batch folder - all changes reverted per maintainer request

Verification

✅ Built Azure.ResourceManager.Advisor (has autorest.md, property=true, IsClientLibrary=true) - Success
✅ Built Azure.AI.Projects (removed explicit false, relies on default, new TypeSpec generator) - Success
✅ Built Azure.AI.Translation.Text (added property, has tsp-location without emitter) - Success
✅ Built Azure.AI.Personalizer (added property, has Generated but no config files) - Success
✅ Built Azure.AI.OpenAI (removed duplicates, has single property) - Success
✅ Built Microsoft.ClientModel.TestFramework (property removed, relies on default false) - Success
✅ Built Azure.Core.TestFramework (property removed, relies on default false) - Success
✅ Built Azure.Security.KeyVault.Administration (removed explicit false, relies on default) - Success
✅ Built Azure.AI.Inference (property added per request) - Success
✅ Built Azure.ResourceManager.AgriculturePlatform (property added to address build failures) - Success
✅ Merged latest changes from main branch without conflicts
✅ Verified condition properly checks both IncludeAutorestDependency and IsClientLibrary
✅ Reverted all changes to Microsoft.Azure.Batch folder

Fixes #53148

Original prompt

This section details on the original issue you should resolve

<issue_title>Set IncludeAutorestDependency to be Disabled by Default</issue_title>
<issue_description>The IncludeAutorestDependency is enabled by default, and libraries have to explicitly opt out by setting this to false in their .csproj file, ie https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/keyvault/Azure.Security.KeyVault.Administration/src/Azure.Security.KeyVault.Administration.csproj#L14.

We should invert this behavior here, and have this disabled by default, and libraries have to enable this in their corresponding csproj file. To ensure backwards compatibility, we need to explicitly enable this property for all dataplane and management plane libraries that are still on the autorest based generator. All libraries under the sdk folder whose tsp-location.yaml file do not have a corresponding entry of emitterPackageJsonPath: eng/http-client-csharp-emitter-package.json or eng/azure-typespec-http-client-csharp-emitter-package.json should have this property set to true. If the library doesn't have a tsp-location.yaml file, or the entry for emitterPackageJsonPath is empty or doesn't exist, we can assume the flag should be enabled.

If the IncludeAutorestDependency property exists in a csproj file, we should do nothing.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #53148


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…braries

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Disable IncludeAutorestDependency by default Disable IncludeAutorestDependency by default for new TypeSpec-based libraries Oct 10, 2025
@Copilot Copilot AI requested a review from jorgerangel-msft October 10, 2025 16:46
Copilot finished work on behalf of jorgerangel-msft October 10, 2025 16:46
@jorgerangel-msft
Copy link
Member

@copilot - for libraries that explicitly have this property set to false, can we remove the entry from their csproj file?

Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 10, 2025

@copilot - for libraries that explicitly have this property set to false, can we remove the entry from their csproj file?

Done. Removed the explicit <IncludeAutorestDependency>false</IncludeAutorestDependency> entry from 9 .csproj files since they can now rely on the default behavior. Commit: 3ff473a

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot finished work on behalf of jorgerangel-msft October 10, 2025 17:01
Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
@Copilot Copilot AI requested a review from JoshLove-msft October 10, 2025 17:09
Copilot finished work on behalf of JoshLove-msft October 10, 2025 17:09
eng/Directory.Build.Common.props Outdated Show resolved Hide resolved
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@Copilot Copilot AI requested a review from jorgerangel-msft October 13, 2025 17:11
Copilot finished work on behalf of jorgerangel-msft October 13, 2025 17:11
@jorgerangel-msft
Copy link
Member

@copilot Some libraries were missed. Scan the repo and ensure that libraries that fit the criteria have the property explicitly enabled in their csproj file. To reiterate, if the library doesn't have a tsp-location.yaml file, or the entry for emitterPackageJsonPath is empty or doesn't exist in the yaml file, we can assume the flag should be enabled. Here are some that I found should have it, but don't:

  • sdk/recoveryservices/Azure.ResourceManager.RecoveryServices
  • sdk/openai/Azure.AI.OpenAI.Assistants
  • sdk/translation/Azure.AI.Translation.Text
  • sdk/translation/Azure.AI.Translation.Document
  • sdk/cognitivelanguage/Azure.AI.Language.Text.Authoring

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@Copilot Copilot AI requested a review from jorgerangel-msft October 13, 2025 22:00
Copilot finished work on behalf of jorgerangel-msft October 13, 2025 22:00
Copy link

github-actions bot commented Oct 13, 2025

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@Copilot Copilot AI requested a review from jorgerangel-msft October 14, 2025 16:24
Copilot finished work on behalf of jorgerangel-msft October 14, 2025 16:24
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@Copilot Copilot AI requested a review from jorgerangel-msft October 14, 2025 21:01
Copilot finished work on behalf of jorgerangel-msft October 14, 2025 21:01
…o copilot/disable-include-autorest-dependency
…o copilot/disable-include-autorest-dependency
…o copilot/disable-include-autorest-dependency
<Description>Microsoft Azure Resource Manager client SDK for Azure resource provider Microsoft.AgFoodPlatform.</Description>
<PackageTags>azure;management;arm;resource manager;agfoodplatform</PackageTags>
<DisableEnhancedAnalysis>true</DisableEnhancedAnalysis>
<IncludeAutorestDependency>true</IncludeAutorestDependency>
Copy link
Member

Choose a reason for hiding this comment

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

@copilot We are missing this in sdk/agricultureplatform/Azure.ResourceManager.AgriculturePlatform. Add this property to this library's csproj and also rescan the repo in case any other libraries that fall into the criteria were missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Azure.ResourceManager.AgriculturePlatform has a tsp-location.yaml file with emitterPackageJsonPath: "eng/azure-typespec-http-client-csharp-mgmt-emitter-package.json", which is one of the new TypeSpec generators. According to the criteria in the issue, libraries using this emitter should NOT have the IncludeAutorestDependency property (they should rely on the default false value).

Could you clarify if you'd like me to add the property to this library despite it using the new mgmt emitter? Or did you perhaps mean a different library?

Copy link
Member

Choose a reason for hiding this comment

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

@copilot - yes let's try adding it ONLY for this library as there are some build failures in the pipeline and we need to see if this will address the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added IncludeAutorestDependency=true to Azure.ResourceManager.AgriculturePlatform. Commit: 11c7056

…rePlatform

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot finished work on behalf of jorgerangel-msft October 17, 2025 15:39
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.

Set IncludeAutorestDependency to be Disabled by Default

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.