Add support for /t:Publish /p:NoBuild=true #2111
Conversation
| @@ -36,9 +36,26 @@ Copyright (c) .NET Foundation. All rights reserved. | |||
| The main publish entry point. | |||
| ============================================================ | |||
| --> | |||
|
|
|||
| <PropertyGroup Condition="'$(NoBuild)' != 'true'"> | |||
There was a problem hiding this comment.
Should we use a less generic name for this property? Perhaps PublishWithoutBuild or something?
There was a problem hiding this comment.
This matches pack --no-build so I think it's worth the consistency. Otherwise it should have been PackWithoutBuilding.
| } | ||
|
|
||
| // capture modification time of all binaries before publish | ||
| var modificationTimes = GetLastWriteTimesUtc(buildCommand, "*.exe", "*.dll", "*.resources", "*.pdb"); |
There was a problem hiding this comment.
There should probably be a "sleep until current time advances" loop here.
| var modificationTimes = GetLastWriteTimesUtc(buildCommand, "*.exe", "*.dll", "*.resources", "*.pdb"); | ||
|
|
||
| // publish (with NoBuild set) | ||
| publishCommand.Execute("/p:NoBuild=true").Should().Pass(); |
There was a problem hiding this comment.
As a sanity check for this test, have you tried running it without the /p:NoBuild=true parameter and verified that it fails due to updated modification dates of the output files?
There was a problem hiding this comment.
Yes, I have. :) I also tried various combinations of not having the right targets in _BeforePublishTargets.
| { | ||
| return searchPatterns.SelectMany( | ||
| pattern => Directory.EnumerateFiles( | ||
| Path.Combine(command.ProjectRootPath, ".."), |
There was a problem hiding this comment.
I'm guessing this is going up one level from the ProjectRootPath in order to cover the referenced projects, but that's worth an explanatory comment.
There was a problem hiding this comment.
Correct. I will add a comment.
| }); | ||
|
|
||
| testAsset.Restore(Log, "TestApp"); | ||
| testAsset.Restore(Log, "TestLibrary"); |
There was a problem hiding this comment.
It shouldn't be necessary to restore TestLibrary separately from TestApp. (I know this is old code that you just moved)
There was a problem hiding this comment.
True. I'll fix it.
nguerrera
force-pushed
the
publish-no-build
branch
from
820e9ca to
e9c503a
Compare
nguerrera
force-pushed
the
publish-no-build
branch
from
8dbd0c6 to
6b242c2
Compare
|
@dsplaisted I think I've addressed all of your feedback. Mind taking another look? |
|
@nguerrera looks good, but I just noticed: Did you mean to target master with this PR? |
|
Yes, master is the right branch for this. release/2.1.3xx is for preview2 only at the moment. |
dotnet/sdk half of https://github.com/dotnet/cli/issues/5331
After this, we just need to wire --no-build to /p:NoBuild=true in dotnet/cli