Skip to content

Build all tasks, regardless of target - #55346

Merged
akoeplinger merged 5 commits into
dotnet:mainfrom
lambdageek:rebuild-tasks-if-target-changed
Jul 9, 2021
Merged

Build all tasks, regardless of target#55346
akoeplinger merged 5 commits into
dotnet:mainfrom
lambdageek:rebuild-tasks-if-target-changed

Conversation

@lambdageek

Copy link
Copy Markdown
Member

In a dirty tree, different architectures require different tasks.

For example,

./build.sh --os iossimulator -c Release
./build.sh --os android -c Release

the second step will complain that the AndroidAppBuilder task does not exist,
because the build-semaphore.txt was created during the first build which built
the AppleAppBuilder, but not the AndroidAppBuilder.

In a dirty tree, different architectures require different tasks.
For example,
```
./build.sh --os iossimulator -c Release
./build.sh --os android -c Release
```
the second step will complain that the AndroidAppBuilder task does not exist,
because the build-semaphore.txt was created during the first build which built
the AppleAppBuilder, but not the AndroidAppBuilder.
@ghost

ghost commented Jul 8, 2021

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost

ghost commented Jul 8, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

In a dirty tree, different architectures require different tasks.

For example,

./build.sh --os iossimulator -c Release
./build.sh --os android -c Release

the second step will complain that the AndroidAppBuilder task does not exist,
because the build-semaphore.txt was created during the first build which built
the AppleAppBuilder, but not the AndroidAppBuilder.

Author:lambdageek
Assignees:-
Labels:

area-Infrastructure

Milestone:-

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@akoeplinger I tried adding @(ProjectReference) to the Input of BuildIncrementally, but it didn't work - @(TasksSrc) already includes all those proj files, I think. I don't think MSBuild keeps track of the input dependencies between runs - it just looks at the timestamps of the files in the current run. So if the second msbuild invocation has different files, but none of them are modified, it won't rebuild the target.

Comment threadsrc/tasks/tasks.proj Outdated
@safern

Copy link
Copy Markdown
Member

Does this actually work? I'm surprised msbuild node reuse is not holding a reference to the installer.tasks dll in the second build. That's why we added the semaphore in the first place.

Use synthetic inputs/outputs to avoid building it all the time. This should let devs build with
MSBuild node reuse enabled (the Arcade default). If it were built every time, it would hit file
locking issues vs. the persistent nodes that loaded the task DLL for the previous build. It
isn't particularly accurate, but better than nothing.

Comment threadsrc/tasks/tasks.proj Outdated
@lambdageek

Copy link
Copy Markdown
MemberAuthor

Does this actually work?

I haven't used this PR extensively yet, but I'm not getting the sort of failures that I've come to associate with msbuild node reuse bugs. (But I'm also on OSX, not Windows, so maybe the locking is a more aggressive problem over there)

I'm surprised msbuild node reuse is not holding a reference to the installer.tasks dll in the second build. That's why we added the semaphore in the first place.

Maybe we should just build all the tasks and not try to filter by target?

@lambdageeklambdageek changed the title Use build-sempahore-TargetOS-TargetArchitecture.txt when building tasksUse build-sempahore-TargetOS.txt when building tasksJul 8, 2021
@safern

Copy link
Copy Markdown
Member

Maybe we should just build all the tasks and not try to filter by target?

I think that makes more sense, building an extra project shouldn't hurt.

Simplifies building for different targets from the same dirty tree. All the
tasks are always built once and then remain unchanges as long as their sources
don't change.
@lambdageeklambdageek changed the title Use build-sempahore-TargetOS.txt when building tasksBuild all tasks for any targetJul 8, 2021
@lambdageeklambdageek changed the title Build all tasks for any targetBuild all tasks, regardless of targetJul 8, 2021
@steveisok
steveisok self-requested a review July 9, 2021 03:43
@ViktorHofer

ViktorHofer commented Jul 28, 2021

Copy link
Copy Markdown
Member

I don't understand why we build all tasks by default now regardless of the OS. Why do I need to build all these tasks upfront? This is slowing down the build because a) tasks that aren't used are built and b) dependencies of such tasks (i.e. for mobile workloads) need to be restored. Can we please revert this change?

On a higher level, I don't think that upfront building these tasks is the right approach. I would expect that components which require certain tasks would build these incrementally.

 installer.tasks -> C:\git\runtime3\artifacts\bin\installer.tasks\Debug\net6.0\installer.tasks.dll
RuntimeConfigParser -> C:\git\runtime3\artifacts\bin\RuntimeConfigParser\Debug\net6.0\RuntimeConfigParser.dll
AndroidAppBuilder -> C:\git\runtime3\artifacts\bin\AndroidAppBuilder\Debug\net6.0\AndroidAppBuilder.dll
AppleAppBuilder -> C:\git\runtime3\artifacts\bin\AppleAppBuilder\Debug\net6.0\AppleAppBuilder.dll
MonoAOTCompiler -> C:\git\runtime3\artifacts\bin\MonoAOTCompiler\Debug\net6.0\MonoAOTCompiler.dll
AndroidAppBuilder -> C:\git\runtime3\artifacts\bin\AndroidAppBuilder\Debug\net6.0\publish\
WasmAppBuilder -> C:\git\runtime3\artifacts\bin\WasmAppBuilder\Debug\net472\WasmAppBuilder.dll
JsonToItemsTaskFactory -> C:\git\runtime3\artifacts\bin\JsonToItemsTaskFactory\Debug\net6.0\JsonToItemsTaskFactory.dll
Crossgen2Tasks -> C:\git\runtime3\artifacts\bin\Crossgen2Tasks\Debug\net6.0\Crossgen2Tasks.dll
WasmAppBuilder -> C:\git\runtime3\artifacts\bin\WasmAppBuilder\Debug\net6.0\WasmAppBuilder.dll
WasmAppBuilder -> C:\git\runtime3\artifacts\bin\WasmAppBuilder\Debug\net6.0\publish\
RuntimeConfigParser -> C:\git\runtime3\artifacts\bin\RuntimeConfigParser\Debug\net472\RuntimeConfigParser.dll
JsonToItemsTaskFactory -> C:\git\runtime3\artifacts\bin\JsonToItemsTaskFactory\Debug\net472\JsonToItemsTaskFactory.dll
WorkloadBuildTasks -> C:\git\runtime3\artifacts\bin\WorkloadBuildTasks\Debug\net6.0\WorkloadBuildTasks.dll
WasmBuildTasks -> C:\git\runtime3\artifacts\bin\WasmBuildTasks\Debug\net6.0\WasmBuildTasks.dll
WorkloadBuildTasks -> C:\git\runtime3\artifacts\bin\WorkloadBuildTasks\Debug\net6.0\publish\
MonoAOTCompiler -> C:\git\runtime3\artifacts\bin\MonoAOTCompiler\Debug\net472\MonoAOTCompiler.dll
WasmBuildTasks -> C:\git\runtime3\artifacts\bin\WasmBuildTasks\Debug\net6.0\publish\
WasmAppBuilder -> C:\git\runtime3\artifacts\bin\WasmAppBuilder\Debug\net472\publish\
installer.tasks -> C:\git\runtime3\artifacts\bin\installer.tasks\Debug\net461\installer.tasks.dll

@lambdageek

lambdageek commented Jul 28, 2021

Copy link
Copy Markdown
MemberAuthor

@ViktorHofer Building all tasks is a workaround for incorrect dependency tracking between the tasks and the platforms that require them. The incorrect dependency tracking is by design due to that build sempahore file.

@ViktorHofer

Copy link
Copy Markdown
Member

When the build semaphore was added, mobile tasks didn't exist and at that point it made sense. The semaphore didn't need to be TargetOS aware. I don't think building all tasks upfront makes sense anymore. I wouldn't expect any Wasm* tasks to be built when targeting desktop configurations.

Also these repo tasks were never intended to be shipping and I believe some do now as part of workloads? I think it makes more sense to just build them when they are required and not anytime upfront.

@ghostghost locked as resolved and limited conversation to collaborators Aug 27, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@safern@ViktorHofer@steveisok@akoeplinger@am11