Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Oct 27, 2024

Fixes failures in source-build in dotnet/sdk#43015. Same fix was applied to cdac tool in #108946

Fixes failures in source-build in dotnet/sdk#43015. Same fix was applied to cdac tool in dotnet#108946
@MichalStrehovsky
Copy link
Member

CI didn't seem to run so closing and reopening.

As I wrote in #108946 (comment), I don't think this is the last managed tool that we need to run during the build. I don't know how many we have. But we can keep adding rollforwards to discover more if there's pushback to the environment variable.

@am11
Copy link
Member

am11 commented Oct 28, 2024

I think we can just define this property here https://github.com/dotnet/sdk/blob/4a56ead8a89bc74b82f1d7c80507d95d161c1872/src/SourceBuild/content/repo-projects/runtime.proj#L17

<BuildArgs>$(BuildArgs) /p:RollForward=Major</BuildArgs>

I agree with @MichalStrehovsky that defining it broadly (e.g. at runtime.proj level) shouldn't have any risk.

See https://learn.microsoft.com/dotnet/core/versions/selection#control-roll-forward-behavior (emphasis mine)

Major Roll-forward to the next available higher major version, and lowest minor version, if requested major version is missing. If the requested major version is present, then the Minor policy is used.

So if we are using ./build.sh (our only supported entrypoint to build on unix), which calls dotnet.sh, which checks "do we have the requested SDK, if not, download", that small chance that we may pick up the wrong/newer SDK, disappears.

@jkotas
Copy link
Member Author

jkotas commented Oct 28, 2024

Using RollForward to "fix" tools for source build is a hack. It is prone to problems like #106740 . I am not able to reason about consequences of applying this hack globally.

Proper solution would be to set NetCoreAppToolCurrent to the TFM that we are going to run the tool against during source build. What would that condition look like? cc @dotnet/source-build

@MichalStrehovsky
Copy link
Member

Using RollForward to "fix" tools for source build is a hack. It is prone to problems like #106740 . I am not able to reason about consequences of applying this hack globally.

I thought these local RollForwards were just temporary to unblock the SDK build. It felt easier to just do an environment variable and delete it later instead of adding local RollForwards that we're going to forget to revert. But I see SDK might have a different plan now so it might not be needed.

@ViktorHofer
Copy link
Member

Filed #109329 to avoid the hardcoded value alltogether.

@jkotas
Copy link
Member Author

jkotas commented Oct 29, 2024

Superseded by #109331

@jkotas jkotas closed this Oct 29, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2024
@jkotas jkotas deleted the crossgen2-rollforward branch January 21, 2025 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants