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

v9 Fix build required before initial publish #11211

Merged
merged 1 commit into from
Oct 21, 2021

Conversation

p-m-j
Copy link
Contributor

@p-m-j p-m-j commented Sep 28, 2021

With huge thanks to the comments in dotnet/sdk#960

@OzoneNZ
Copy link
Contributor

OzoneNZ commented Sep 28, 2021

Any chance #11215 can be resolved in this PR too?

Believe it's just a case of updating the CopyUmbracoAssets target to specify BeforeTargets="Rebuild;Build"

@p-m-j p-m-j marked this pull request as draft September 29, 2021 10:06
@p-m-j
Copy link
Contributor Author

p-m-j commented Sep 29, 2021

@OzoneNZ potentially, although this is slightly experimental, I'm hoping we can resolve the build+publish problem in a nicer way then I have here so it may make sense to resolve #11215 elsewhere with higher priority as it sounds more problematic.

@OzoneNZ
Copy link
Contributor

OzoneNZ commented Sep 29, 2021

@OzoneNZ potentially, although this is slightly experimental, I'm hoping we can resolve the build+publish problem in a nicer way then I have here so it may make sense to resolve #11215 elsewhere with higher priority as it sounds more problematic.

No worries, cheers for explaining - have opened another PR to resolve that issue

@p-m-j
Copy link
Contributor Author

p-m-j commented Sep 29, 2021

Do believe a5c5f69 will also nicely close #11215

@p-m-j
Copy link
Contributor Author

p-m-j commented Sep 29, 2021

related reading
NuGet/Home#6659
NuGet/Home#4942

@p-m-j p-m-j force-pushed the v9/bugfix/fix-build-required-before-initial-publish branch 2 times, most recently from 8b6eaa7 to 84adc64 Compare September 30, 2021 07:13
@p-m-j p-m-j marked this pull request as ready for review September 30, 2021 07:19
@OzoneNZ
Copy link
Contributor

OzoneNZ commented Oct 20, 2021

@p-m-j I have a similar issue blocking our move to V9, where the first fresh Build of a 9.0.1 project does not copy the umbraco folder into bin

Only the second Build run actually copies the umbraco folder into bin, shown below with a fresh/empty 9.0.1 project

ejH1Wv3U37

I suspect this is something that also requires changes to Umbraco.Cms.StaticAssets.targets but I've been unable to figure out what is causing the files not to be included on first build

Sounds very similar to this issue however: https://stackoverflow.com/a/606658

Also note that the issue remains even when applying this PR patch

@OzoneNZ
Copy link
Contributor

OzoneNZ commented Oct 20, 2021

Managed to solve the above issue with some changes to build/templates/UmbracoProject/UmbracoProject.csproj

I think what's going on is that msbuild is evaluating the line below too early, prior to the execution of the CopyUmbracoAssets target which copies the static assets, so msbuild thinks there is nothing to include:

<Content Include="umbraco/**" CopyToOutputDirectory="Always" />

Which would explain why the second-time build has no issues copying the files, because they exist from the very start having been copied during the first build attempt - the stack overflow link above confirms this msbuild behaviour

My workaround is to wrap the <Content Include="..."> within a target that is guaranteed to execute after CopyUmbracoAssets:

<Target Name="IncludeUmbracoAssets" AfterTargets="CopyUmbracoAssets">
  <ItemGroup>
    <Content Include="umbraco/**" CopyToOutputDirectory="Always" CopyToPublishDirectory="Always" />
  </ItemGroup>
</Target>

@p-m-j p-m-j marked this pull request as draft October 20, 2021 10:30
@p-m-j
Copy link
Contributor Author

p-m-j commented Oct 20, 2021

Yep that's annoying, I called it quits when the published output was sorted, will have a look later on in the week.

@p-m-j p-m-j force-pushed the v9/bugfix/fix-build-required-before-initial-publish branch 8 times, most recently from d5ea79f to f4610af Compare October 21, 2021 00:22
@p-m-j p-m-j marked this pull request as ready for review October 21, 2021 00:23
@p-m-j p-m-j force-pushed the v9/bugfix/fix-build-required-before-initial-publish branch from f4610af to b0ad3a2 Compare October 21, 2021 00:27
@p-m-j p-m-j force-pushed the v9/bugfix/fix-build-required-before-initial-publish branch from b0ad3a2 to 7a92976 Compare October 21, 2021 01:05
@p-m-j p-m-j requested a review from AndyButland October 21, 2021 01:05
@p-m-j p-m-j merged commit c585f77 into v9/dev Oct 21, 2021
@p-m-j p-m-j deleted the v9/bugfix/fix-build-required-before-initial-publish branch October 21, 2021 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants