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

Enable source-build with arcade #126

Merged
merged 1 commit into from Feb 11, 2021

Conversation

omajid
Copy link
Member

@omajid omajid commented Feb 10, 2021

This enables 'source-build', which makes it easier to build the entire shipping .NET SDK from source.

This is the first and second step of arcade-powered-source-build: https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/README.md

See dotnet/sourcelink#692 for a similar PR, that this is based on.

Aside from arcade-specific changes, it also makes all .sh files executable to make this build under Linux/macOS.

The rename from LICENSE to LICENSE.txt is a workaround for NuGet/Home#7601.

This enables 'source-build', which makes it easier to build the entire
shipping .NET SDK from source.

This is the first and second step of arcade-powered-source-build:
https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/README.md

See dotnet/sourcelink#692 for a similar PR, that this is based on.

Aside from arcade-specific changes, it also makes all `.sh` files
executable to make this build under Linux/macOS.

The rename from LICENSE to LICENSE.txt is a workaround for
NuGet/Home#7601.
@omajid
Copy link
Member Author

omajid commented Feb 10, 2021

cc @crummel @dagood @dseefeld

Is there a source-build champion for this repo?

AfterTargets="PrepareInnerSourceBuildRepoRoot">

<ItemGroup>
<SourceBuildPatchFile Include="$(RepositoryEngineeringDir)source-build-patches\*.patch" />
Copy link
Member Author

Choose a reason for hiding this comment

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

RepositoryEngineeringDir refers to the original/outer repo. This can break badly, right? Is there something that that points to the eng dir of the inner repo?

Copy link
Member

@dagood dagood Feb 10, 2021

Choose a reason for hiding this comment

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

I can't think of anything that would happen differently depending on whether this uses the files in the outer repo in the inner repo. The outer and inner repo are always the same source code. The PrepareInnerSourceBuildRepoRoot target only executes in the context of the outer repo, so the patches will always be there.

I think there is a quirk where if the outer repo has some unstaged file additions, they don't transfer over properly into the inner repo. (This is based on git stash behavior.) But that's just dev workflow, which isn't a priority for ArPow right now--and it's easy to just stage the files to work around this. (Edit: Filed dotnet/source-build#2047 to make this more visible, though, now that I'm thinking about it.)

There wouldn't be any harm in adding a prop in dotnet/arcade to refer to the inner eng dir, though: https://github.com/dotnet/arcade/blob/e4ab5cb034f0c40b4a52a5952b20809daaa2f39b/src/Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/SourceBuildArcade.targets#L19-L20

@dagood
Copy link
Member

dagood commented Feb 10, 2021

Is there a source-build champion for this repo?

Not quite yet, @crummel is on it: dotnet/source-build#2028.

@crummel
Copy link
Contributor

crummel commented Feb 10, 2021

@nohwnd or @Haplois should be able to help review.

@Haplois
Copy link
Contributor

Haplois commented Feb 10, 2021

I'll review this tomorrow morning, CET time.

@Haplois Haplois merged commit 40281be into dotnet:master Feb 11, 2021
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.

None yet

4 participants