Skip to content

Fix OS condition #5802

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.

Sign up for GitHub

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

Open
jaredpar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix OS condition #5802

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...osoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Microsoft.Azure.Cosmos.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
<ProjectReference Include="..\..\src\Microsoft.Azure.Cosmos.csproj" />
</ItemGroup>

<ItemGroup Condition="$(OS) != 'Linux' AND '$(ProjectRef)' != 'True' ">
<ItemGroup Condition="$(OS) == 'Windows_NT' AND '$(ProjectRef)' != 'True' ">
Copy link
Copy Markdown
Member

@Pilchie Pilchie Apr 22, 2026

Choose a reason for hiding this comment

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

This means that this will no longer run on macOS, right? I think that's correct, but want to call it out explicitly.

Copy link
Copy Markdown
Author

@jaredpar jaredpar Apr 22, 2026

Choose a reason for hiding this comment

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

Correct. That seemed to be the intent here. Also MSBuild doesn't have a great way to differentiate between Linux and MacOS. The $(OS) value for both is "Unix". Can differentiate but you have to use a different type of condition

Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli Apr 23, 2026

Choose a reason for hiding this comment

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

Since the original intent was not honored and so-far not know of any broken/impact. How about remove condition it-self?

Copy link
Copy Markdown
Member

@Pilchie Pilchie Apr 23, 2026

Choose a reason for hiding this comment

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

Well, @jaredpar was trying to build on Linux and it didn't work, right? So presumably something about that doesn't work on Linux?

Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli Apr 23, 2026

Choose a reason for hiding this comment

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

/usr/lib/dotnet/sdk/10.0.106/Microsoft.Common.CurrentVersion.targets(5381,5): error MSB3030: Could not copy the file "/home/kirankk/.nuget/packages/Microsoft.HybridRow/1.1.0-preview3/microsoft.hybridrow.nuspec" because it was not found.

Yup. lets take this PR then.

Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Seems this was primarily driven through the yml files where its set.
One option might be <ItemGroup Condition="$(TargetFramework.Contains('windows'))"> (for build time?)

Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

WSL varified that it works

Copy link
Copy Markdown
Author

@jaredpar jaredpar Apr 23, 2026

Choose a reason for hiding this comment

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

Yeah I was trying to review the other PR and I didn't quite understand how the build laid out files such that the targets could actually work. Opened up the repo in codespaces to play around and the build failed. The fix was essentially just changing the condition after which build worked fine.

<None Include="$(NugetPackageRoot)\Microsoft.HybridRow\$(HybridRowVersion)\microsoft.hybridrow.nuspec">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
Loading