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

Condition source-build specific configurations on DotNetBuildFromSouce #9672

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

ViktorHofer
Copy link
Member

Contributes to VMR work: dotnet/source-build#3926

Changes are from dotnet/dotnet#46

No functional changes introduced.

Contributes to VMR work: dotnet/source-build#3926

Changes are from dotnet/dotnet#46

No functional changes introduced.
@ViktorHofer ViktorHofer self-assigned this Jan 22, 2024
@ViktorHofer ViktorHofer requested a review from a team as a code owner January 22, 2024 11:02
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Looks good - kust a minor comment on an accidental header removal

eng/Build.props Show resolved Hide resolved
@ViktorHofer
Copy link
Member Author

I assume the failures in this PR are unrelated?

@JanKrivanek
Copy link
Member

I assume the failures in this PR are unrelated?

I do not recall seaing any similar error recently.
Reruning just for sure. Let's have closer look tomorrow if it's persistent

@JanKrivanek
Copy link
Member

Still failing
Microsoft.NETCoreSdk.BundledVersions.props is missing for some reason

image

I'm short on time today - how urgent is this?

@ViktorHofer
Copy link
Member Author

Not too urgent. Getting it in later this week is totally fine.

@JanKrivanek
Copy link
Member

Hi @ViktorHofer - I believe the issue is directly related to the change

The missing Microsoft.NETCoreSdk.BundledVersions.props is imported in SourceBuildIntermediate.proj - that is not part of the failing build:

image

The reason will be likely related to the fact that the conditionaly excluded target (ConfigureInnerBuildArgs) prepares InnerBuildArgs property that is used later on in other targets. The property is now missing any project to run - so likely rest of the configuration that it's serving is lost that way:

image

I haven't dug deeper into what exactly is the purpose of the property and what is skipped as the result of missing project - but hopefully you are equiped to carry on investigation based on the intent of the change

@ViktorHofer
Copy link
Member Author

Thank you for taking a look. This is really weird but let me try something different.

Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

LGTM

@JanKrivanek JanKrivanek merged commit e7a44d7 into main Jan 26, 2024
8 checks passed
@JanKrivanek JanKrivanek deleted the ViktorHofer-patch-1 branch January 26, 2024 09:23
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.

4 participants