Skip to content

Conversation

@huulinhnguyen-dev
Copy link
Contributor

Fixes #
Fix the issue: Don't emit duplicate generated files into the binlog #11884

Context

The CodeTaskFactory was generating multiple .tmp files in the binlog for each invocation of a task, even when the task used the same source code. The goal is to emit only one .tmp file per unique task source code after its first successful compilation.

Changes Made

Added a HashSet _loggedCodeFiles to track task specifications already logged to the binlog.

Updated the logic to check _loggedCodeFiles before emitting a .tmp file, ensuring only one file is logged per unique FullTaskSpecification

Testing

Created a test project that invokes CustomTask multiple times with parameters (InputParameter="Foo" and InputParameter="Bar") via Another.proj and directly.

Verified that only one .tmp file is generated in the binlog for tasks sharing the same source code (from Import.targets), confirming duplicate suppression.

Created an additional project (Another1.proj) with a different source code (Import1.targets) to confirm two .tmp files are generated when source codes differ, validating that unique FullTaskSpecification instances produce separate files.

Notes

@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as ready for review June 6, 2025 08:43
Copy link
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

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

  1. please make PRs with titles that indicate what the PR does, so we can orient in PR list view, not just issue reference (e.g. copy issue title/description "Fix duplicate logging of code from CodeTaskFactories")
  2. the original issue asks to fix RoslynCodeTaskFactory too, please add that
  3. I added comments about the implementation

@huulinhnguyen-dev huulinhnguyen-dev changed the title Fix issue 11884 Fix issue 11884: Don't emit duplicate generated files into the binlog Jun 6, 2025
@huulinhnguyen-dev huulinhnguyen-dev changed the title Fix issue 11884: Don't emit duplicate generated files into the binlog Fix issue 11884: Don't emit duplicate generated files into the binlog. Change from adding multiple copies of it for each invocation to unique file Jun 6, 2025
@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as draft June 6, 2025 09:24
@huulinhnguyen-dev huulinhnguyen-dev force-pushed the dev/huulinhnguyen/issue11884 branch from a8883f4 to be1eca0 Compare June 9, 2025 03:03
@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as ready for review June 9, 2025 03:07
@huulinhnguyen-dev
Copy link
Contributor Author

Hi @JanProvaznik , about the RoslynCodeTaskFactory, its already implement the logic for use the compile code from the cache, so we no need to implement in RoslynCodeTaskFactory

@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as draft June 9, 2025 03:51
@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as ready for review June 9, 2025 06:22
@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as draft June 9, 2025 06:36
@huulinhnguyen-dev huulinhnguyen-dev changed the title Fix issue 11884: Don't emit duplicate generated files into the binlog. Change from adding multiple copies of it for each invocation to unique file Fix issue 11884: Change from adding multiple copies of it for each invocation to unique file Jun 9, 2025
@JanProvaznik JanProvaznik changed the title Fix issue 11884: Change from adding multiple copies of it for each invocation to unique file Don't emit duplicate src files to binlog in CodeTaskFactory Jun 9, 2025
@AR-May AR-May assigned huulinhnguyen-dev and AR-May and unassigned AR-May Jun 9, 2025
@AR-May AR-May self-requested a review June 9, 2025 11:49
add logic to generate file even when it compile error

Add unit test for new code changes

Add unit test for the code changes

Remove unused code

Add unit test for code change
@huulinhnguyen-dev huulinhnguyen-dev force-pushed the dev/huulinhnguyen/issue11884 branch from 8304e82 to d2edc20 Compare June 10, 2025 01:42
@huulinhnguyen-dev huulinhnguyen-dev marked this pull request as ready for review June 10, 2025 07:17
Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

LGTM

@JanProvaznik JanProvaznik merged commit 2b01314 into dotnet:main Jun 11, 2025
10 checks passed
@GangWang01 GangWang01 linked an issue Jun 12, 2025 that may be closed by this pull request
@KirillOsenkov
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't emit duplicate generated files into the binlog

5 participants