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

Fix intellisense package xml file failing up-to-date check in libs projects #105243

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Jul 22, 2024

Fixes #92940

Tested locally and verified that the correct xml file lands in the intermediate and in the final output folder, with the correct time stamps. Also tested that this really fixes the broken fast up-to-date check.

@ViktorHofer ViktorHofer requested a review from carlossanlop July 22, 2024 14:07
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 22, 2024
@ViktorHofer ViktorHofer requested a review from ericstj July 22, 2024 14:07
@ViktorHofer ViktorHofer added area-Infrastructure-libraries and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 22, 2024
Copy link
Contributor

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

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some questions.

eng/intellisense.targets Show resolved Hide resolved
<Target Name="PrepareIntellisensePackageXmlFile"
Inputs="$(IntellisensePackageXmlFilePath)"
Outputs="$(IntermediateDocFileItemFromIntellisensePackage)">
<Copy SourceFiles="$(IntellisensePackageXmlFilePath)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the copy be enough to give the new file a new creation timestamp? Why is the touch below needed?

Copy link
Member Author

@ViktorHofer ViktorHofer Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is about modification date&time which doesn't get updated by the Copy task.

eng/intellisense.targets Show resolved Hide resolved
BeforeTargets="CopyFilesToOutputDirectory;DocumentationProjectOutputGroup"
Condition="'$(UseCompilerGeneratedDocXmlFile)' != 'true' and '$(IntellisensePackageXmlFilePath)' != ''">
<ItemGroup>
<DocFileItem Remove="@(DocFileItem)" />
<DocFileItem Include="$(IntellisensePackageXmlFilePath)" />
<DocFileItem Include="$(IntermediateDocFileItemFromIntellisensePackage)" />
Copy link
Member

@carlossanlop carlossanlop Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to revert the name to the original after this? I wonder if the new suffix *.intellisense-package* would affect our process. It was my understanding that the intellisense file needs to match the assembly name, except for the extension.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just the name of the intermediate file in the obj directory. We need a different name as we also have the compiler generated xml file. The final output path is the expected one: AssemblyName.xml.

@ViktorHofer ViktorHofer merged commit 9477e94 into main Jul 23, 2024
147 of 149 checks passed
@ViktorHofer ViktorHofer deleted the ViktorHofer-patch-2 branch July 23, 2024 08:35
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to 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.

ChangeDocumentationFileForPackaging causes incremental build issues inside VS
2 participants