Compiler task is passing `/generatedfilesout` even though it didn't create the directory

[RikkiGibson]

Could not find a part of the path makes it sound like the output directory is not present. But the compiler and compiler tasks should create the directory (both the /generated directory and the individual generator output directories within it).

https://github.com/dotnet/roslyn/blob/d5b9aaa7e984890c9a8d967bde68d1c62d4858fa/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets#L339-L340

https://github.com/dotnet/roslyn/blob/d5b9aaa7e984890c9a8d967bde68d1c62d4858fa/src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs#L1173-L1176

However it looks like the task linked above didn't run in the cdacreader build. binlog says:

Target Name=CreateCompilerGeneratedFilesOutputPath Project=cdacreader.csproj Target "CreateCompilerGeneratedFilesOutputPath" skipped, due to false condition; ('$(EmitCompilerGeneratedFiles)' == 'true' and !('$(DesignTimeBuild)' == 'true' OR '$(BuildingProject)' != 'true')) was evaluated as ('true' == 'true' and !('' == 'true' OR 'false' != 'true')).

I'm confused by this :) as 'true' == 'true' and !('' == 'true' OR 'false' != 'true') seems like it ought to evaluate to true. false, my conditional logic brain wasn't working. I think the BuildingProject property should be true instead of false in order for the directory to be created here. What controls that?

I think maybe the compiler task has a bug here. Basically it feels like we should only be passing a /generatedfilesout argument to the compiler if we also ran target CreateCompilerGeneratedFilesOutputPath. But it looks like the current build task permits us to pass that argument without running the target.

Originally posted by @RikkiGibson in https://github.com/dotnet/source-build/issues/4335#issuecomment-2062537783

[chsienki]

But it looks like the current build task permits us to pass that argument without running the target.

@RikkiGibson Yeah, I think this is the issue here. The condition here:

https://github.com/dotnet/roslyn/blob/d5b9aaa7e984890c9a8d967bde68d1c62d4858fa/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets#L329

Should probably include the building project check. Although it's slightly weird because the user is asking us to emit files, we just don't in that scenario.

[RikkiGibson]

It feels like it would be simplifying for compiler to be in charge of creating all the generated out directories, not the task. Then there wouldn't be the possiblity of this mismatch.

[jaredpar]

It feels like it would be simplifying for compiler to be in charge of creating all the generated out directories, not the task. Then there wouldn't be the possiblity of this mismatch.

The history of the compiler is that it does not create output directories, only output files. Source generators are just following the existing pattern here.

At the same time I've always found the behavior puzzling. There are no determinism issues as it's clear from the command line what all needs to be created, it's just that the build task usually does the work. There are no other issues I can really think of for why the compiler shouldn't just create the directories but it's pretty clear it was the intended behavior (not an accident of design). @rainersigwald do you have any idea why this separation was done?

I'm not completely opposed to making a change here but it should be consistent across all our output paths, not just for source generators.

[rainersigwald]

At the same time I've always found the behavior puzzling. There are no determinism issues as it's clear from the command line what all needs to be created, it's just that the build task usually does the work. There are no other issues I can really think of for why the compiler shouldn't just create the directories but it's pretty clear it was the intended behavior (not an accident of design). @rainersigwald do you have any idea why this separation was done?

I do not--@danmoseley do you?

Though @jaredpar what makes you think it's fully intentional? Don't most of the "open file for write" APIs not create directories, so it's (small, easy) extra work to ensure it exists?

[jaredpar]

Though @jaredpar what makes you think it's fully intentional?

There are unit tests that very the behavior that point back to old old TFS Work Item. So old that I can't figure them out at the moment

http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/545247

Possible the bug is just recording "native compiler did it this way so we will to". Will have to do some TFS archeology later to get access to the bug.

[jaredpar]

@jjonescz pointed me in the right direction for the newer version of the work item link. As suspected it's just recording what the native compiler did. The bug also mentions we could just create the directory and be done with it.

So if we want to fix this by just creating the dirs in the compiler I'd be fine with it. But I do want it to be consistent across all the different places that we emit files: PE, PDB, XML, source generated files, etc ...

[rainersigwald]

So if we want to fix this by just creating the dirs in the compiler I'd be fine with it. But I do want it to be consistent across all the different places that we emit files: PE, PDB, XML, source generated files, etc ...

I support this.

[RikkiGibson]

So, it sounds like the resolution for this bug will be to make a change to compiler and tasks such that:

1. Compiler tasks generally do not create directories
2. Compiler automatically creates the containing directory for all kinds of files that it outputs

And, to review the logic of the CreateCompilerGeneratedFilesOutputPath, and decide how to move that logic over to control whether to pass /generatedfilesout rather than whether to create a directory.

[jaredpar]

Compiler tasks generally do not create directories

I'd likely leave it in cause who knows what part of build depends on this. For example do these get created when we do design time builds? What breaks if we stop doing that???