CHANGE: docs tools update - #2460
Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Harness Review
The recipe source update is directionally fine, but the checked-in Yamato configuration has not been regenerated, so the CI jobs this repository actually runs still use the old docs tooling.
The recipe changes here do not take effect until the generated .yamato/input-system-*.yml files are refreshed and committed. The checked-in job definitions that Yamato consumes still contain the old commands (git clone --branch "2.3.0-preview" ... everywhere, plus %GSUDO% choco install netfx-4.7.1-devpack ... on Windows; see .yamato/input-system-editor-functional-tests.yml / .yamato/input-system-standalone-functional-tests.yml), and .yamato/triggers.yml points at those generated files directly. As committed, this PR will merge without actually switching CI to PMDT 3.14 or installing DocFX 2.70 anywhere. Please regenerate the job YAML and include the updated .yamato/input-system-*.yml outputs in the PR.
Reviewed commit 7efc128
🤖 Helpful? 👍/👎
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #2460 +/- ##
========================================
Coverage 78.95% 78.96%
========================================
Files 767 767
Lines 140780 140811 +31
========================================
+ Hits 111153 111187 +34
+ Misses 29627 29624 -3 Flags with carried forward coverage won't be shown. Click here to find out more.
|
jfreire-unity
left a comment
jfreire-unity
left a comment
There was a problem hiding this comment.
The change looks like it upgrades the PMDT but CI still fails, so we should circle back to the docs engineering team and let us know it's not fixing the problem.
Also, have you tried locally if this change generates the documentation?
| if (!File.Exists($"{projectName}.sln")) | ||
| { | ||
| var assemblies = UnityEditor.Compilation.CompilationPipeline.GetAssemblies( | ||
| UnityEditor.Compilation.AssembliesType.Editor); | ||
| foreach (var asm in assemblies) | ||
| { | ||
| if (asm.sourceFiles.Length == 0 || | ||
| !asm.sourceFiles.Any(f => f.Replace("\\", "/").Contains("Packages/com.unity.inputsystem/"))) | ||
| continue; | ||
| var csprojPath = $"{asm.name}.csproj"; | ||
| if (File.Exists(csprojPath)) | ||
| continue; | ||
| var csprojContent = new StringBuilder(); | ||
| csprojContent.AppendLine("<?xml version=\"1.0\" encoding=\"utf-8\"?>"); | ||
| csprojContent.AppendLine("<Project ToolsVersion=\"4.0\" DefaultTargets=\"Build\" " + | ||
| "xmlns=\"http://schemas.microsoft.com/developer/msbuild/2003\">"); | ||
| // DefineConstants must be in an unconditional PropertyGroup so DocFX reads them | ||
| // regardless of whether MSBuild's Platform property is set. | ||
| csprojContent.AppendLine(" <PropertyGroup>"); | ||
| csprojContent.AppendLine($" <DefineConstants>{string.Join(";", asm.defines)}</DefineConstants>"); | ||
| csprojContent.AppendLine(" </PropertyGroup>"); | ||
| csprojContent.AppendLine(" <PropertyGroup Condition=\" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' \">"); | ||
| csprojContent.AppendLine($" <AssemblyName>{asm.name}</AssemblyName>"); | ||
| csprojContent.AppendLine(" <TargetFrameworkVersion>v4.7.1</TargetFrameworkVersion>"); | ||
| csprojContent.AppendLine(" <OutputType>Library</OutputType>"); | ||
| csprojContent.AppendLine(" <AllowUnsafeBlocks>True</AllowUnsafeBlocks>"); | ||
| csprojContent.AppendLine(" <LangVersion>9.0</LangVersion>"); | ||
| csprojContent.AppendLine(" <NoConfig>true</NoConfig>"); | ||
| csprojContent.AppendLine(" <NoStdLib>true</NoStdLib>"); | ||
| csprojContent.AppendLine(" </PropertyGroup>"); | ||
| csprojContent.AppendLine(" <ItemGroup>"); | ||
| foreach (var src in asm.sourceFiles) | ||
| csprojContent.AppendLine($" <Compile Include=\"{src}\" />"); | ||
| csprojContent.AppendLine(" </ItemGroup>"); | ||
| csprojContent.AppendLine(" <ItemGroup>"); | ||
| foreach (var refPath in asm.compiledAssemblyReferences) | ||
| csprojContent.AppendLine( | ||
| $" <Reference Include=\"{Path.GetFileNameWithoutExtension(refPath)}\">" + | ||
| $"<HintPath>{refPath}</HintPath></Reference>"); | ||
| csprojContent.AppendLine(" </ItemGroup>"); | ||
| csprojContent.AppendLine("</Project>"); | ||
| File.WriteAllText(csprojPath, csprojContent.ToString(), Encoding.UTF8); | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this strictly necessary? Is there no better way to avoid this or generate this XML without essentially concatenating strings of XML together? I'd report this to the docs engineering team if this is the only because it feels bad one if one has to do this to make docs work on CI.
There was a problem hiding this comment.
To my knowledge state it is necessary because the .csproj file is needed for the doc tooling in version 3x. It's generated by code for two main reasons:
- DefineConstants — includes Unity version defines (UNITY_6000_0_52),
platform defines (PLATFORM_STANDALONE_OSX), and project-specific feature
flags. These change per Unity version, OS, and project. DocFX needs accurate
defines to parse #if blocks correctly, so hardcoding them would cause wrong
API members to be included/excluded. - Reference HintPaths (lines 87-90) — the compiled assembly references use
absolute paths to the machine's Library/ScriptAssemblies/ folder. Inherently
machine-specific.
…logies/InputSystem into update-docs-tooling
2 participants
Description
Upgrading docs package tools. DocFX is no longer part of the package and needs to be installed separately.
Testing status & QA
None.
Overall Product Risks
Comments to reviewers
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.