msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

Items returned by tasks not considered for EmbedInBinlog

Open KirillOsenkov opened this issue 2 years ago • 10 comments

I'd expect this to embed C:\temp\1.txt into the binlog, but it doesn't:

<Project>

  <Target Name="Build">
    <MSBuild Projects="$(MSBuildThisFileFullPath)" Targets="GetItems">
      <Output TaskParameter="TargetOutputs" ItemName="EmbedInBinlog" />
    </MSBuild>
  </Target>

  <Target Name="GetItems" Returns="@(ResultItem)">
    <ItemGroup>
      <ResultItem Include="C:\temp\1.txt" />
    </ItemGroup>
  </Target>
  
</Project>

As a workaround to fix this, also need to add an explicit ItemGroup:

<Project>

  <Target Name="Build">
    <MSBuild Projects="$(MSBuildThisFileFullPath)" Targets="GetItems">
      <Output TaskParameter="TargetOutputs" ItemName="EmbedInBinlog" />
    </MSBuild>
    <ItemGroup>
      <EmbedInBinlog Include="@(EmbedInBinlog)" />
    </ItemGroup>
  </Target>

  <Target Name="GetItems" Returns="@(ResultItem)">
    <ItemGroup>
      <ResultItem Include="C:\temp\1.txt" />
    </ItemGroup>
  </Target>
  
</Project>

I think this line: https://github.com/dotnet/msbuild/blob/f1dae6ab690483458d37b8900f1d1e4a5fc72851/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs#L521

should also allow for TaskParameterMessageKind.TaskOutput

KirillOsenkov avatar Jun 01 '22 19:06 KirillOsenkov

Y'all can assign me this one - just have to do the dunkin' after ⬆️

MeikTranel avatar Jun 01 '22 19:06 MeikTranel

hey btw i've noticed this last week - does anyone else have issues with dependabot running on forks recently?https://github.com/MeikTranel/msbuild/pull/1/

MeikTranel avatar Jun 01 '22 20:06 MeikTranel

@MeikTranel it has happened on mine before but I was assuming/hoping that was a weird vestige of me having tested it there before configuring it here. Can you check this setting on your fork? https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuring-dependabot-version-updates#enabling-version-updates-on-forks

rainersigwald avatar Jun 01 '22 20:06 rainersigwald

Is it possible this dependabot.csproj is responsible? image

There's no disable there though.

Even under settings dependabot seems disabled: image

MeikTranel avatar Jun 01 '22 20:06 MeikTranel

Well good news is @KirillOsenkov was absolutely right - it really just was that TaskOutput that needed to be added to the filter.

image

On the other hand i'm having trouble writing a test for this one and i really think the binary log deserves some love there (only a single test confirming parallel and console loggers are roundtripping through binary logs - otherwise no test coverage for binary log contents).

I'd like some guidance regarding the BinaryLogs created by the TestEnvironment / ObjectModelHelper in the following code:

BinaryLogger_Tests.cs
[Fact]
public void BinaryLoggerShouldEmbedFilesViaTaskOutput()
{
  using var buildManager = new BuildManager();
  var binaryLogger = new BinaryLogger()
  {
      Parameters = $"LogFile={_logFile}",
      CollectProjectImports = BinaryLogger.ProjectImportsCollectionMode.Embed
  };

  var testProject = @"
<Project>
<Target Name=""Build"">
  <MSBuild Projects=""$(MSBuildThisFileFullPath)"" Targets=""GetItems"" >
    <Output TaskParameter=""TargetOutputs"" ItemName=""EmbedInBinlog"" />
  </MSBuild>
</Target>

<Target Name=""GetItems"" Returns=""@(ResultItem)"">
  <CreateItem Include=""../**/*"">
      <Output TaskParameter=""Include"" ItemName=""ResultItem"" />
  </CreateItem>
</Target>
</Project>";
  ObjectModelHelpers.BuildProjectExpectSuccess(testProject, binaryLogger);

  //Replay and verify file entries
  var logReader = new BinaryLogReplayEventSource();
  var files = new List<string>();
  logReader.AnyEventRaised += (object sender, BuildEventArgs e) =>
  {
      if (e is ProjectImportedEventArgs importArgs)
      {
          files.Add(importArgs.File);
      }
  };
  logReader.Replay(_logFile);
}

When i open the log written by the following code the entire thing looks like a light binlog - no parameter logging nothing.

image

If i run dotnet build /bl over the same temporary project file everything is where it is expected - see above.

MeikTranel avatar Jun 01 '22 22:06 MeikTranel

could it be we need to set the verbosity to diagnostic inside BuildProjectExpectSuccess?

KirillOsenkov avatar Jun 02 '22 00:06 KirillOsenkov

I already tried setting BinaryLogger.Verbosity to Diagnostic to no avail - is there another verbosity setting somewhere i'm missing? I'm also not sure which of the APIs i'm supposed to use for running a simple project build with a binarylogger attached that i can parse for events later on. Maybe @rainersigwald has some guidance?

MeikTranel avatar Jun 02 '22 08:06 MeikTranel

There's no disable there though.

Even under settings dependabot seems disabled:

Asked internally, and got pointed to dependabot/dependabot-core#2804 which sure sounds related. The workaround suggested there is delete/recreate the fork, which is pretty heavyweight.

rainersigwald avatar Jun 02 '22 14:06 rainersigwald

Jesus somebody is pretty petty under that issue - how did they manage to link 1500 PRs to that issue :D:D

Have you looked at the problem regarding tests and binary logs ? Wanted to give it another look after work today.

Make sure to expand the spoiler section: https://github.com/dotnet/msbuild/issues/7665#issuecomment-1144206481

MeikTranel avatar Jun 02 '22 14:06 MeikTranel

I think i found a good solution to have some test coverage - sorry that it took so long - got super distracted and kind of forgot that i still had this laying around.

MeikTranel avatar Aug 08 '22 22:08 MeikTranel