Skip to content

dotnet publish --no-build over-publishes from referenced projects #43835

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

Open
dougbu opened this issue Feb 6, 2022 · 4 comments
Open

dotnet publish --no-build over-publishes from referenced projects #43835

dougbu opened this issue Feb 6, 2022 · 4 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug.
Milestone

Comments

@dougbu
Copy link
Contributor

dougbu commented Feb 6, 2022

Describe the bug

The Web SDK does not normally publish *.cshtml files when using dotnet publish (because they're precompiled into the web site assembly and aren't needed). These files are published from referenced projects if $(NoBuild) is true e.g. using dotnet publish --no-build. These files increase deployment payloads and can confuse tests.

To Reproduce

  1. In a clone of the dotnet/aspnetcore repo, run eng\build.cmd -noBuildManaged -noBuildNative
  2. dotnet build src\Mvc\test\WebSites\RazorPagesWebSite\
  3. dotnet publish --no-build src\Mvc\test\WebSites\RazorPagesWebSite\
  4. Observe the artifacts\bin\RazorPagesWebSite\Debug\net7.0\publish\Pages\ClassLibraryPages folder as well as the *.cshtml files within it
  5. Reset using rm -Recurse artifacts\bin\RazorPagesWebSite\ and rm -Recurse artifacts\obj\RazorPagesWebSite\
  6. dotnet build src\Mvc\test\WebSites\RazorPagesWebSite\
  7. dotnet publish src\Mvc\test\WebSites\RazorPagesWebSite\
  8. Observe the lack of an artifacts\bin\RazorPagesWebSite\Debug\net7.0\publish\Pages folder

More detail

Using /p:BuildProjectReferences=false instead of --no-build in step 3 results in the same over-publication.

The ClassLibraryPages folder comes from the referenced src\Mvc\test\WebSites\RazorPagesClassLibrary\ project. Note the many *.cshtml files from the RazorPagesWebSite project are not published in either (3) or (7).

Moving up a project dependency level and doing the same with the src\Mvc\test\Mvc.FunctionalTests\ project is worse because src\Mvc\test\WebSites\RazorPagesWebSite\ contains a lot of *.cshtml files and other referenced projects add to that; all are copied into artifacts\bin\Microsoft.AspNetCore.Mvc.FunctionalTests\Debug\net7.0\publish\Pages and …\Views. This breaks the functional tests because a few test sites use <RazorCompileOnPublish>false</RazorCompileOnPublish> and pick up incorrect / unexpected files.

Binary logging

The AssignTargetPaths target in the RazorPagesClassLibrary project adds@(ContentWithTargetPath) for Pages\ClassLibraryPages\*.cshtml in all cases. The items are removed before the GetCopyToPublishDirectoryItems target is reached but not when using --no-build or /p:BuildProjectReferences=false. It's not clear where the removals happen because item removals are not consistently logged.

Versions

I've only tested with the 7.0.100-preview.2.22102.6 SDK but this is likely a long-standing issue.

dotnet details
> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   7.0.100-preview.2.22102.6
 Commit:    6b77f292d5

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\dd\dnx\aspnetcore\.dotnet\sdk\7.0.100-preview.2.22102.6\

Host (useful for support):
  Version: 7.0.0-preview.2.22103.2
  Commit:  0ae04e5b33

.NET SDKs installed:
  7.0.100-preview.2.22102.6 [C:\dd\dnx\aspnetcore\.dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.21 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0-dev [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0-preview.2.22080.2 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.21 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.2.22102.1 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.2.22102.17 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.2.22103.2 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 7.0.0-preview.2.22101.13 [C:\dd\dnx\aspnetcore\.dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download
@ghost ghost added the untriaged label Feb 6, 2022
dougbu referenced this issue in dougbu/aspnetcore Feb 8, 2022
- building again in Publish target adds nothing in most cases
- use `-noBuildRepoTasks` more consistently
  - again, building the repo tasks wastefully repeated work done in previous build step
- use `-noBuildJava` more consistently in quarantined pipelines
  - Java tests aren't worth testing _or building_ in these pipelines because they cannot be quarantined
- remove `-noRestore` and `-noBuildDeps` when `-noBuild` specified; redundant
- work around https://github.com/dotnet/sdk/issues/23777
  - do not publish .cshtml files of most test asset projects when `$(NoBuild)` is set
- update FunctionalTestWithAssets.targets to help Publish succeed when `$(NoBuild)` is set
  - fixes build jobs in quarantined-pr pipeline where `--no-build` was already specified
- for IIS test assets, perform all builds needed for their profiles within Build target
  - move `ReferenceTestTasks=false` and comments about it to where property matters
  - merge separate targets in FunctionalTest.props to make items available during (now separate) Publish

nits:
- touch up RunHelix.ps1 console output
- reorder command line arguments for consistency in changed parts of the YAML files
- move `NoBuild=true` setting out of FunctionalTestWithAssets.props
- use `BeforeTargets` in FunctionalTest.props to improve determinism
dougbu referenced this issue Feb 8, 2022
* Use `-noBuild` in second build steps of test jobs
- building again in Publish target adds nothing in most cases
- use `-noBuildRepoTasks` more consistently
  - again, building the repo tasks wastefully repeated work done in previous build step
- use `-noBuildJava` more consistently in quarantined pipelines
  - Java tests aren't worth testing _or building_ in these pipelines because they cannot be quarantined
- remove `-noRestore` and `-noBuildDeps` when `-noBuild` specified; redundant
- work around https://github.com/dotnet/sdk/issues/23777
  - do not publish .cshtml files of most test asset projects when `$(NoBuild)` is set
- update FunctionalTestWithAssets.targets to help Publish succeed when `$(NoBuild)` is set
  - fixes build jobs in quarantined-pr pipeline where `--no-build` was already specified
- for IIS test assets, perform all builds needed for their profiles within Build target
  - move `ReferenceTestTasks=false` and comments about it to where property matters
  - merge separate targets in FunctionalTest.props to make items available during (now separate) Publish

nits:
- touch up RunHelix.ps1 console output
- reorder command line arguments for consistency in changed parts of the YAML files
- move `NoBuild=true` setting out of FunctionalTestWithAssets.props
- use `BeforeTargets` in FunctionalTest.props to improve determinism
@dsplaisted
Copy link
Member

Here's a standalone repro projects (and binlogs in the RazorPages project directory): RazorOverPublish23777.zip

During a normal build, CopyToPublishDirectory metadata on .cshtml Content items is set to Never in the ResolveRazorGenerateInputs Target.

However, when using --no-build, most targets in the referenced project aren't run. The primary one that is run is GetCopyToPublishDirectoryItems.

To fix this, probably the ResolveRazorGenerateInputs target or something like it needs to be set to run before GetCopyToPublishDirectoryItems.

/cc @captainsafia, @pranavkm, @mkArtakMSFT as Razor SDK area owners.

@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/sdk Sep 8, 2022
@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Sep 8, 2022
@mkArtakMSFT mkArtakMSFT added this to the .NET 8 Planning milestone Sep 8, 2022
@ghost
Copy link

ghost commented Sep 8, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@mkArtakMSFT
Copy link
Contributor

@dougbu why is this important? Can we just not do this work or is there a push to get this resolved? so far we've seen no customer impact from this.

@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, .NET 8 Planning Oct 25, 2022
@dotnet dotnet deleted a comment Oct 25, 2022
@dotnet dotnet deleted a comment Oct 25, 2022
@dougbu
Copy link
Contributor Author

dougbu commented Oct 25, 2022

@mkArtakMSFT this issue is important because

  1. It necessitates hacks like the RemoveCshtmlFiles target in our build
  2. Customers could easily over-publish their projects, reducing any benefit of trimming et cetera to keep their package sizes down

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug.
Projects
None yet
Development

No branches or pull requests

5 participants
@dsplaisted @captainsafia @dougbu @mkArtakMSFT and others