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

ProjectInstance based build fails with a "Target not found exception" when the instance is mutated #1726

Closed
cdmihai opened this issue Feb 17, 2017 · 7 comments · Fixed by #2018
Assignees
Labels
Milestone

Comments

@cdmihai
Copy link
Contributor

cdmihai commented Feb 17, 2017

        private const string projectContents =
@"<Project>
  <PropertyGroup>
    <ImportIt>true</ImportIt>
  </PropertyGroup>

  <Import Project=""{0}"" Condition=""'$(ImportIt)' == 'true'""/>

  <Target Name=""Bazz"">
    <Message Text=""Buzz"" Importance=""High"" />
  </Target>

</Project>";

        private const string projectImport =
@"<Project>
  <Target Name=""Foo"">
    <Message Text=""Bar"" Importance=""High"" />
  </Target>
</Project>";

        [Fact]
        public void Test1()
        {
            string importPath = Path.GetTempFileName();
            File.WriteAllText(importPath, projectImport);

            var collection = new ProjectCollection();
            var root = ProjectRootElement.Create(XmlReader.Create(new StringReader(string.Format(projectContents, importPath))), collection);

            root.FullPath = Path.GetTempFileName();
            root.Save();

            var project = new Project(root, new Dictionary<string, string>(), MSBuildConstants.CurrentToolsVersion, collection);
            var instance = project.CreateProjectInstance(ProjectInstanceSettings.Immutable).DeepCopy(isImmutable: false);

            var manager = new BuildManager();

            var request = new BuildRequestData(instance, new[] { "Foo" });
            var parameters = new BuildParameters()
            {
                DisableInProcNode = true,
            };

            manager.BeginBuild(parameters);
            var submission = manager.PendBuildRequest(request);

            var results = submission.Execute();
            Assert.True(results.OverallResult == BuildResultCode.Success);

            manager.EndBuild();
            manager.ResetCaches();

            project.SetProperty("ImportIt", "false");
            project.Save();

            manager.BeginBuild(parameters);
            request = new BuildRequestData(instance, new[] { "Foo" });
            submission = manager.PendBuildRequest(request);
            results = submission.Execute();
            Assert.True(results.OverallResult == BuildResultCode.Success);

            manager.EndBuild();
        }

The second builds fails with a "target not found exception".

There seem to be two weird things happening here:

  1. The build is failing with a target not found exception, even if the build is started on a ProjectInstance. This is probably because a target is getting conditioned to false. It is my understanding that if you start a build on a project instance, msbuild shouldn’t re-evaluate and just reuse the evaluated state from the project instance.

  2. MSBuild is ignoring the properties that CPS set on a project instance after the project instance info is serialized and sent to the out of proc node.

@cdmihai
Copy link
Contributor Author

cdmihai commented Feb 18, 2017

Mkay, so this behaviour appears to be “by design”. When a build is requested on a ProjectInstance and an out of proc node is used, the out of proc node uses two ProjectInstance objects to recover the state. ProjectInstance1 is the deserialized one. Apparently serializing a ProjectInstance only keeps part of the state: global properties, properties, and items. Everything else (targets, using tasks, etc) is discarded. To recover the missing part of the state, the out of proc node Re-evaluates the entry project form disk into ProjectInstance2. It then merges the two by keeping the properties and items from ProjectInstance1 and everything else from ProjectInstance2.

If the properties and items from ProjectInstance1 influence what targets get loaded, and what imports get imported, and if the project files change while the build is running in a way that changes those properties and items, then weird stuff like this failure happens (target not found exception).

I don’t quite understand why msbuild is doing this state merging as opposed to also serializing target and task information.

So, the fix is to change ProjectInstance to serialize the complete evaluated state, and the out of proc node to reuse it entirely without re-evaluation from disk. This is a big change which would affect all out of proc builds, so probably too risky for RTW.
As a workaround, CPS could discard the failed design time build.

@davkean
Copy link
Member

davkean commented Feb 20, 2017

The above description does not match my debugging, I was finding that MSBuild was throwing away properties from ProjectInstance1 and hence dotnet/project-system#1554 was occurring. Secondly, it's not viable to retry - as all the builds were failing.

@cdmihai
Copy link
Contributor Author

cdmihai commented Apr 7, 2017

Potential manifestation: dotnet/project-system#1939

@cdmihai cdmihai self-assigned this Apr 7, 2017
@davkean
Copy link
Member

davkean commented Apr 27, 2017

What's going on with this? This is still being run into.

@davkean
Copy link
Member

davkean commented Apr 27, 2017

Here's another hit: dotnet/project-system#2076

@cdmihai
Copy link
Contributor Author

cdmihai commented Apr 27, 2017

@davkean High chance of inserting the fix on Friday

cdmihai added a commit that referenced this issue May 2, 2017
Resolves #1726 

From issue comments:

Previous to this change, when a build is requested on a ProjectInstance and an out of proc node is used, the out of proc node uses two ProjectInstance objects to recover the state. ProjectInstance1 is the deserialized one. Apparently serializing a ProjectInstance only keeps part of the state: global properties, properties, and items. Everything else (targets, using tasks, etc) is discarded. To recover the missing part of the state, the out of proc node Re-evaluates the entry project form disk into ProjectInstance2. It then merges the two by keeping the properties and items from ProjectInstance1 and everything else from ProjectInstance2.

If the properties and items from ProjectInstance1 influence what targets get loaded, and what imports get imported, and if the project files change while the build is running in a way that changes those properties and items, then weird stuff like this failure happens (target not found exception).

The fix is to change ProjectInstance to serialize the complete evaluated state, and the out of proc node to reuse it entirely without re-evaluation from disk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants