Skip to content

$(RepoRoot) is missing trailing slash so local builds fail #32615

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

Closed
SteveSandersonMS opened this issue May 12, 2021 · 9 comments
Closed

$(RepoRoot) is missing trailing slash so local builds fail #32615

SteveSandersonMS opened this issue May 12, 2021 · 9 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework task
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented May 12, 2021

Not sure if this is a known issue yet, but it seems not to work to run dotnet build in almost any of the project directories under src/Components. Most likely other projects will fail too.

For some reason, $(RepoRoot) evaluates to a location without a trailing slash, even though code in the top-level Directory.Build.props explicitly generates a value with a trailing slash. This makes many project references and <Import> declarations fail to resolve correctly, since they assume there would be a trailing slash after $(RepoRoot). Also for some reason, this doesn't affect build.cmd - perhaps it overrides the reporoot in some way.

As a workaround, you can do dotnet build /p:RepoRoot=C:\your\path\to\aspnetcore\.

Update

I tracked down the source of the bad value: it's the build.cmd scripts, as they set an environment variable called RepoRoot with the non-slash-ended value. Then the MSBuild logic only creates a value with a trailing slash if there isn't already a value, but there is already a value because of the env var.

So the remaining question is: how did this ever work in the past? I see that build.cmd hasn't changed recently. One possible clue is this comment from the top Directory.build.props:

<!-- $(RepoRoot) is normally set globally and Arcade overrides it to ensure a trailing slash. -->

Maybe Arcade no longer has this behavior of overriding it to ensure a trailing slash.


Older report

This seems to be a recent regression. Repro:

  • cd src\Components
  • git checkout main
  • git reset --hard
  • build.cmd
  • cd Samples\BlazorServerApp
  • dotnet build

Expected: builds the project
Actual: fails with errors like this:

C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\Microsoft.NET.Sdk.Razor.SourceGenerators\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\BlazorServerApp.UnifiedAssembly.Info.g.cs(1,30): error CS0234: The type or namespace name 'AspNetCore' does not exist in the namespace 'Microsoft' (are you missing an assembly reference?) [C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\BlazorServerApp.csproj]
C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\Microsoft.NET.Sdk.Razor.SourceGenerators\Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator\_Pages__Host_cshtml.cs(4,30): error CS0234: The type or namespace name 'AspNetCore' does not exist in the namespace 'Microsoft' (are you missing an assembly reference?) [C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\BlazorServerApp.csproj]
C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\Program.cs(6,17): error CS0234: The type or namespace name 'AspNetCore' does not exist in the namespace 'Microsoft' (are you missing an assembly reference?) [C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\BlazorServerApp.csproj]
C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\Program.cs(7,17): error CS0234: The type or namespace name 'AspNetCore' does not exist in the namespace 'Microsoft' (are you missing an assembly reference?) [C:\git\dotnet\aspnetcore\src\Components\Samples\BlazorServerApp\BlazorServerApp.csproj]

Something to do with source generators, it appears.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label May 12, 2021
@SteveSandersonMS SteveSandersonMS changed the title Can no longer build Components\Samples\BlazorServerApp in repo $(RepoRoot) is missing trailing slash so local builds fail May 12, 2021
@SteveSandersonMS SteveSandersonMS added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 12, 2021
@mkArtakMSFT mkArtakMSFT added this to the 6.0-preview5 milestone May 12, 2021
@captainsafia
Copy link
Member

@SteveSandersonMS Can you share the binlogs for the failing dotnet build invocation?

At the expense of being That Person(tm), I couldn't get a repro of this in a fresh sandbox against commit bc5960e.

@SteveSandersonMS
Copy link
Member Author

Sure, will email the binlog. One good way to see the problem in action is by evaluating the RepoRoot environment variable before and after running build.cmd:

  • echo %RepoRoot% (no output)
  • Inside src\Components, run build.cmd (millions of pages of output)
  • echo %RepoRoot% => c:\your\path\to\aspnetcore\src\Components\..\..

Now the presence of this RepoRoot env var breaks other attempts to build:

  • cd Samples\BlazorServerApp
  • dotnet build (loads of errors, which are because $(RepoRoot) evaluates as the string from above with no trailing slash)

... and removing it fixes it:

  • set RepoRoot=
  • dotnet build (works)

This problem didn't occur until recently - not sure what's changed!

@captainsafia
Copy link
Member

So, I tried two scenarios when validating this.

  • Running dotnet build immediately in the Samples/BlazorServerApp

image

  • Running the build.cmd script in src/Components then running dotnet build in Samples/BlazorServerApp

image

The RepoRoot environment variable defined in the build scripts was only intended to be used to resolve the location of the root build script.

Now, MSBuild does support referencing the value of an environment variable in the build. However, when both an environment variable and a build property share the same name, it is meant to favor the build property over the environment variable. Maybe the recent Arcade update brought in some differing behavior?

In any case, although I can't repro it myself, I think it's probably a good hygiene measure to unset the RepoRoot environment variable in the local build scripts to avoid any confusion between the two. Our build script shouldn't leave any variables behind.

@dougbu
Copy link
Contributor

dougbu commented May 13, 2021

this doesn't affect build.cmd - perhaps it overrides the reporoot in some way.

See https://github.com/dotnet/arcade/blob/e7ede87875f41a9b3df898ae08da5ebc96e24f56/src/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj#L37

Our build script shouldn't leave any variables behind.

We absolutely should not do anything w/ the Bash or Powershell RepoRoot settings. As @SteveSandersonMS said, "this doesn't affect build.cmd". And, if we stop setting RepoRoot, builds using the Arcade wrappers will fail. See https://github.com/dotnet/arcade/blob/e7ede87875f41a9b3df898ae08da5ebc96e24f56/src/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj#L83

@dougbu
Copy link
Contributor

dougbu commented May 13, 2021

/btw Build.proj hasn't changed since early January

@captainsafia
Copy link
Member

The Build.proj only comes in to play if you are using the build.cmd script, correct?

When you build with dotnet it uses the properties defined in the Directory.Build.props file, but not the ones in the Arcade project. That's why the behavior is different between the two now.

And, if we stop setting RepoRoot, builds using the Arcade wrappers will fail. See https://github.com/dotnet/arcade/blob/e7ede87875f41a9b3df898ae08da5ebc96e24f56/src/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj#L83

OK -- that makes sense to me.

@dougbu
Copy link
Contributor

dougbu commented May 14, 2021

I'm starting to understand this:

  • Reproduction of this bug requires use of the cmd command line. I can't see it because I always use bash and pwsh. @captainsafia is that true for you as well❔
  • My change in the root Directory.Build.props relied on Arcade doing the right thing (which it still does) and no other messed-up RepoRoot settings.
  • Unfortunately, our build.cmd scripts pollute the environment with a bad $(RepoRoot) value but (somehow) the root Directory.Build.props was previously able to fix that up.

Bottom line, my change in #32552 removed a fix for the "set but w/o a trailing slash" case and we're hitting that here. I don't get why %RepoRoot% is not handled as a global property (which should mean it can't be overridden) but have enough to go on.

@BrennanConroy this likely explains the SignalR failures as well as why I haven't seen that issue locally either.

@dougbu dougbu assigned dougbu and unassigned captainsafia May 14, 2021
@dougbu
Copy link
Contributor

dougbu commented May 14, 2021

Taking this issue over. I'll fix it up in a new PR that reverts some of what I did in #32552 but limits the "can't override a global property" noise.

@dougbu
Copy link
Contributor

dougbu commented May 14, 2021

@rainersigwald one question: Why is our root Directory.Build.props able to override %RepoRoot% in @SteveSandersonMS's scenario above (under "Older report")❔ Are environment variables able to be overridden in msbuild files even though they're otherwise treated as global properties❔

Oops, meant to ask this in #32664

@dougbu dougbu closed this as completed in c53700b May 14, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants