[Enhancement] Prevent restore for already restored libs when building all projects

[WolfspiritM]

With this change 684aa49 the call to dotnet restore has been moved behind the cwd to only restore the project that is being build.

This breaks dockers cache and will always restore libs again for each project when building one after another, which is very suboptimal with many libs and dependencies.
Before that change docker was calling restore only once and was reusing that layer for all the other projects.

I think it would be better to remove the dcproj file from .dockerignore as 2.1-sdk now supports dcproj and move all the restores back to the original position to restore dockers cache. (As I assume the reason was the "dcproj missing" error which prevent building)

This would still fail with a:
MSBUILD : error MSB1011: Specify which project or solution file to use because this folder contains more than one project or solution file.

So maybe something like that will work instead:
dotnet restore *.sln (as all non-docker solutions are ignored in .dockerignore anyway)

As the MSB3202 is resolved with that change (I'm not sure about nu1503 as I've not faced that error yet) the workaround with -nowarn can be removed aswell.

Here is a Dockerfile I use (also note the --no-restore for publish aswell as build):

FROM microsoft/dotnet:2.1-aspnetcore-runtime AS base
WORKDIR /app
EXPOSE 80

FROM microsoft/dotnet:2.1-sdk AS build
WORKDIR /src
COPY . .
RUN dotnet restore *.sln
WORKDIR /src/src/Services/Some.Project
RUN dotnet build --no-restore -c Release -o /app

FROM build AS publish
RUN dotnet publish --no-restore -c Release -o /app

FROM base AS final
WORKDIR /app
COPY --from=publish /app .
ENTRYPOINT ["dotnet", "Some.Project.dll"]

[mvelosop]

Hi @WolfspiritM,

Thanks for the suggestion, it looks like it could save some building time! 👍

I was actually trying your suggestion to time the difference but just couldn't get it working, perhaps you could submit a PR with the suggested changes applied.

Thanks.

@eiximenis could you take a look at this, please?

[mvelosop]

Hi @WolfspiritM,

I've been exploring your suggestion, but found that dotnet restore doesn't support *.sln as a parameter.

Am I missing something?

[WolfspiritM]

Hi @mvelosop !

Dotnet restore itself does not support *.sln as a parameter but it's the bash autocompletion that expands the command to the correct file. As far as I understand even the windows docker-compose uses the Linux images (microsoft/dotnet:2.1-sdk) at build time, correct?

I made a fork and a branch for it here.
https://github.com/WolfspiritM/eShopOnContainers/tree/fix-docker-cache
Here is the commit with the changes: WolfspiritM@1839fd1
Maybe you can give it a try before I create a pull request from it?
I wasn't able to test yet if the javascript is copied correctly as I had to change the order of the statements for the ones using node.

Also I have to say I've not tested it with Visual Studio yet.

I had to make sure that all dockerfiles do restore first:

FROM microsoft/dotnet:2.1-sdk as dotnet-build
WORKDIR /src
COPY . .
RUN dotnet restore *.sln

Also I added a dotnet build before dotnet publish for WebSPA and WebMVC as I think it's in general better to build before publishing. At least that's what all other Dockerfiles for ASP.NET Core do.

Also I had to remove "test" from the .dockerignore otherwise the restore would fail cause of missing projects.

And I added Dockerfiles to the .dockerignore so changes to the dockerfile itself aren't breaking the cache for "COPY . ."

[mvelosop]

Hi @WolfspiritM,

Thanks for the clarification, I was definitely missing something important, but precisely that rules out this nice suggestion, because one of the requirements of Dockerfiles (that @eiximenis has been able to achieve thanks to some convoluted workarounds) is that Dockerfiles work in Windows containers as well as in Linux containers.

So, until bash commands are directly supported in Windows, or some other similar workaround, we won't be able to use this solution.

Anyway, have you measured how much time difference does it make to use this solution?

[mvelosop]

However (I still like the idea), you could use this PowerShell command to achieve the same thing:

ls *.csproj -Recurse | %{dotnet restore $_.FullName}

As long as PowerShell is installed in the base Linux images (haven't tested it though)!

I know this might end summoning some Linux daemons on me, but it might just work. 😉

Hey @eiximenis, how do you like the idea? (I know you don't, just poking 😉)

[mvelosop]

Hummm, I think this could pose a problem with Docker's cache, right?

[WolfspiritM]

I don't think that the linux docker image contains powershell.

But what we can do is remove the dcproj file from the Docker context again (.dockerignore) and just use the -nowarn as before. This however needs another workaround that prevents it from failing in the case of a missing dcproj:
aspnet/aspnet-docker#389

So either call it like that:

FROM microsoft/dotnet:2.1-sdk as dotnet-build
ENV RestoreUseSkipNonexistentTargets false
WORKDIR /src
COPY . .
RUN dotnet restore -nowarn:msb3202,nu1503

Or like that:

FROM microsoft/dotnet:2.1-sdk as dotnet-build
WORKDIR /src
COPY . .
RUN dotnet restore -nowarn:msb3202,nu1503 -p:RestoreUseSkipNonexistentTargets=false

I build identity.api successfully with windows containers that way.

Maybe it would be a good idea to open a ticket for dotnet-cli (or better nuget) to ignore dcproj files from the dotnet restore command as there is nothing to restore for dcproj anyway.

[WolfspiritM]

@mvelosop I think I've found a solution here:
https://github.com/Microsoft/msbuild/blob/6f9d40db9811c3af036cde532d62c547af91a532/src/MSBuild/XMake.cs#L2504

It seems msbuild (dotnet restore) supports a switch /ignoreprojectextensions:.dcproj
To me this looks like a good solution for this problem!
We will just ignore the dcproj.
That way it picks up the sln to use itself.

FROM microsoft/dotnet:2.1-sdk as dotnet-build
ENV RestoreUseSkipNonexistentTargets false
WORKDIR /src
COPY . .
RUN dotnet restore /ignoreprojectextensions:.dcproj

I will add that change to my branch 😄

[mvelosop]

Hi @WolfspiritM,

That looks good for Linux containers, lucky you!

But the issue that dotnet restore when run in Windows (containers) needs a .csproj file parameter or needs to find a single one on the current folder to run, makes this a no-go general solution, for the reason I mentioned before. ☹

And it looks like PowerShell for Alpine will not be ready for a while: PowerShell/PowerShell#4605 ☹☹

Would you measure the build time difference in Linux and let us know?

[WolfspiritM]

Hi @mvelosop

I don't really understand. I've build both linux and windows containers that way without issues. There is only one sln available for docker as all other solutions are removed in the docker context via .dockerignore.
https://github.com/dotnet-architecture/eShopOnContainers/blob/dev/.dockerignore#L10

The only problem that was causing the error before was that there is also the dcproj available now otherwise the solution fails to build. The above switch makes dotnet restore ignore that dcproj and it uses the only sln available in the root folder then.

I will measure the time and let you know.

[mvelosop]

That's it!

There are six .sln files in the repo's root folder and dotnet restore doesn't know which one to use!

I guess you must have deleted the ones you don't use.

Actually, dotnet restore .\eShopOnContainers-ServicesAndWebApps.sln does work but that's probably not good, because it creates a "dependency" on the solution file that doesn't make sense from the microservice's (Dockerfile) point of view!

Perhaps @eiximenis could give us his opinion on this at some point. 😁

[WolfspiritM]

No. I did not delete anything. 😃
The .dockerignore in the root folder of the project specifies which files are not available for the Dockerfile (similar to .gitignore)
In there all *.sln files are ignored except for a single one (the ! is an exception).

*.sln
!eShopOnContainers-ServicesAndWebApps.sln

That means within the Dockerfile (the build context) only that single eShopOnContainers-ServicesAndWebApps.sln is available:

See for linux:

Step 1/24 : FROM microsoft/dotnet:2.1-aspnetcore-runtime AS base
 ---> 083ca6a642ea
Step 2/24 : WORKDIR /app
Removing intermediate container 9c55139e7644
 ---> dd2d105e0f09
Step 3/24 : COPY . .
 ---> d0344de242c1
Step 4/24 : RUN ls -la *.sln
 ---> Running in df4fc7fb4fd0
-rwxr-xr-x 1 root root 129227 Jul  3 20:55 eShopOnContainers-ServicesAndWebApps.sln
Removing intermediate container df4fc7fb4fd0

And here the same for windows containers:

Step 1/24 : FROM microsoft/dotnet:2.1-aspnetcore-runtime AS base
 ---> 2564be47e2d5
Step 2/24 : WORKDIR /app
Removing intermediate container cd9298afe23e
 ---> a7a3fba39af6
Step 3/24 : COPY . .
 ---> 25781d9c2f12
Step 4/24 : RUN dir *.sln
 ---> Running in bc58c00bbd9f
 Volume in drive C has no label.
 Volume Serial Number is 92DF-9987

 Directory of C:\app

07/03/2018  10:55 PM           129,227 eShopOnContainers-ServicesAndWebApps.sln
               1 File(s)        129,227 bytes
               0 Dir(s)  136,130,621,440 bytes free
Removing intermediate container bc58c00bbd9f

That way only the single sln is available to the dotnet restore command and it should work as expected.

I'm currently trying to messure the difference...might take some time

[WolfspiritM]

I finally measured the difference.
For a clean build with linux containers (both times using "docker system prune" to clear the cache before the build) I get the following result measured via Measure-Command { docker-compose build } :

Without my change:

Minutes           : 14
Seconds           : 5
Milliseconds      : 712

With my change:

Minutes           : 6
Seconds           : 49
Milliseconds      : 718

[mvelosop]

Great! that's quite a difference! less than half the time, congrats! 👍😊

I found I wasn't testing it properly, but I have to tweak a couple more things.

But will continue tomorrow, I'm having fun but it's too late now!

Thanks @WolfspiritM!

[mvelosop]

Hi @WolfspiritM,

I'm sorry, I had to get off-line for a few days.

I've been working on this issue and have found some other improvement opportunities.

I checked out your branch in my repo (https://github.com/mvelosop/eShopOnContainers/tree/WolfspiritM-fix-docker-cache) and made some changes:

1. Remove all build commands. All of them come just before publish and publish builds again, so no need for the first one.
2. .dockerignore'd all test folders except the .csproj

Everything seems to be working fine, I have to test Windows containers, will post results here and will let you know so you can submit the PR.

Best.