Publish obj files for PR #127295 modified source files [Just for testing no merge]#128236
Publish obj files for PR #127295 modified source files [Just for testing no merge]#128236moyo1997 wants to merge 3 commits into
Conversation
…ine artifacts Add a post-build step to global-build-job.yml that collects compiled object files (.obj/.o) for the 102 source files modified in PR dotnet#127295 (Remove Unused Includes) and publishes them as pipeline artifacts. - New script: eng/pipelines/collect-obj-files.ps1 Searches artifacts/obj/ for object files matching the 102 source basenames and copies them to a staging directory. - Modified: eng/pipelines/common/global-build-job.yml Added pwsh step + PublishBuildArtifacts task after the build step. Runs on every build leg, publishes per-leg artifacts named ObjFiles_<os>_<arch>_<config>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dotnet-policy-service
Bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a CI step to collect compiled object files (.obj/.o) for 102 specific source files modified in PR #127295 ("Remove Unused Includes") and publishes them as build artifacts for size-comparison analysis.
Changes:
- New PowerShell script
collect-obj-files.ps1that searchesartifacts/objfor object files matching a hardcoded list of 102 source basenames and copies matches to a staging directory. - Updates
global-build-job.ymlto invoke the script and publish the resulting object files viaPublishBuildArtifacts@1, scoped per OS/arch/config.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/collect-obj-files.ps1 | New script enumerating 102 source basenames, recursively scanning obj dir, stripping .cpp/.c + .obj/.o suffixes, and copying matches preserving relative path. |
| eng/pipelines/common/global-build-job.yml | Adds two pipeline steps: invoke the collection script, then publish the staged object files as a per-leg build artifact. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 9
| $sourceBasenames = @( | ||
| "assembly" | ||
| "dacfn" |
| # Deduplicate basenames (some appear more than once across different source dirs) | ||
| $uniqueBasenames = $sourceBasenames | Sort-Object -Unique |
| # Search for .obj and .o files recursively | ||
| $extensions = @('*.obj', '*.o') | ||
| foreach ($ext in $extensions) { | ||
| $files = Get-ChildItem -Path $ArtifactsObjDir -Filter $ext -Recurse -File -ErrorAction SilentlyContinue | ||
| foreach ($file in $files) { | ||
| # Extract the source basename: e.g., "assembly.cpp.obj" -> "assembly" | ||
| # CMake names obj files as <sourcename>.cpp.obj (or .c.obj) on Windows, | ||
| # and <sourcename>.cpp.o (or .c.o) on Linux/macOS. | ||
| $objName = $file.Name | ||
| # Remove the object extension first | ||
| $withoutObjExt = $objName | ||
| if ($objName.EndsWith('.obj')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 4) | ||
| } | ||
| elseif ($objName.EndsWith('.o')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 2) | ||
| } | ||
|
|
||
| # Remove the source extension (.cpp, .c) | ||
| $baseName = $withoutObjExt | ||
| if ($withoutObjExt.EndsWith('.cpp')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 4) | ||
| } | ||
| elseif ($withoutObjExt.EndsWith('.c')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 2) | ||
| } | ||
|
|
||
| if ($basenameSet.Contains($baseName)) { | ||
| # Preserve relative path from artifacts/obj | ||
| $relativePath = $file.FullName.Substring($ArtifactsObjDir.TrimEnd([IO.Path]::DirectorySeparatorChar, '/').Length + 1) | ||
| $destPath = Join-Path $OutputDir $relativePath | ||
| $destDir = Split-Path $destPath -Parent | ||
| if (-not (Test-Path $destDir)) { | ||
| New-Item -ItemType Directory -Path $destDir -Force | Out-Null | ||
| } | ||
| Copy-Item -Path $file.FullName -Destination $destPath -Force | ||
| Write-Host "Copied: $relativePath ($($file.Length) bytes)" | ||
| $totalCopied++ | ||
| } |
| # CMake names obj files as <sourcename>.cpp.obj (or .c.obj) on Windows, | ||
| # and <sourcename>.cpp.o (or .c.o) on Linux/macOS. | ||
| $objName = $file.Name | ||
| # Remove the object extension first | ||
| $withoutObjExt = $objName | ||
| if ($objName.EndsWith('.obj')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 4) | ||
| } | ||
| elseif ($objName.EndsWith('.o')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 2) | ||
| } | ||
|
|
||
| # Remove the source extension (.cpp, .c) | ||
| $baseName = $withoutObjExt | ||
| if ($withoutObjExt.EndsWith('.cpp')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 4) | ||
| } | ||
| elseif ($withoutObjExt.EndsWith('.c')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 2) | ||
| } |
|
|
||
| if ($basenameSet.Contains($baseName)) { | ||
| # Preserve relative path from artifacts/obj | ||
| $relativePath = $file.FullName.Substring($ArtifactsObjDir.TrimEnd([IO.Path]::DirectorySeparatorChar, '/').Length + 1) |
| # Collect and publish .obj/.o files for the 102 source files from PR #127295 | ||
| - pwsh: | | ||
| $(Build.SourcesDirectory)/eng/pipelines/collect-obj-files.ps1 ` | ||
| -ArtifactsObjDir "$(Build.SourcesDirectory)/artifacts/obj" ` | ||
| -OutputDir "$(Build.StagingDirectory)/obj-artifacts" | ||
| displayName: Collect object files for PR #127295 |
| - task: PublishBuildArtifacts@1 | ||
| displayName: Publish object files | ||
| inputs: | ||
| PathtoPublish: '$(Build.StagingDirectory)/obj-artifacts' | ||
| ArtifactName: 'ObjFiles_${{ parameters.osGroup }}${{ parameters.osSubgroup }}_${{ parameters.archType }}_${{ parameters.buildConfig }}' |
| displayName: Collect object files for PR #127295 | ||
| continueOnError: true | ||
| condition: succeededOrFailed() |
| # Search for .obj and .o files recursively | ||
| $extensions = @('*.obj', '*.o') | ||
| foreach ($ext in $extensions) { | ||
| $files = Get-ChildItem -Path $ArtifactsObjDir -Filter $ext -Recurse -File -ErrorAction SilentlyContinue | ||
| foreach ($file in $files) { | ||
| # Extract the source basename: e.g., "assembly.cpp.obj" -> "assembly" | ||
| # CMake names obj files as <sourcename>.cpp.obj (or .c.obj) on Windows, | ||
| # and <sourcename>.cpp.o (or .c.o) on Linux/macOS. | ||
| $objName = $file.Name | ||
| # Remove the object extension first | ||
| $withoutObjExt = $objName | ||
| if ($objName.EndsWith('.obj')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 4) | ||
| } | ||
| elseif ($objName.EndsWith('.o')) { | ||
| $withoutObjExt = $objName.Substring(0, $objName.Length - 2) | ||
| } | ||
|
|
||
| # Remove the source extension (.cpp, .c) | ||
| $baseName = $withoutObjExt | ||
| if ($withoutObjExt.EndsWith('.cpp')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 4) | ||
| } | ||
| elseif ($withoutObjExt.EndsWith('.c')) { | ||
| $baseName = $withoutObjExt.Substring(0, $withoutObjExt.Length - 2) | ||
| } | ||
|
|
||
| if ($basenameSet.Contains($baseName)) { | ||
| # Preserve relative path from artifacts/obj | ||
| $relativePath = $file.FullName.Substring($ArtifactsObjDir.TrimEnd([IO.Path]::DirectorySeparatorChar, '/').Length + 1) | ||
| $destPath = Join-Path $OutputDir $relativePath | ||
| $destDir = Split-Path $destPath -Parent | ||
| if (-not (Test-Path $destDir)) { | ||
| New-Item -ItemType Directory -Path $destDir -Force | Out-Null | ||
| } | ||
| Copy-Item -Path $file.FullName -Destination $destPath -Force | ||
| Write-Host "Copied: $relativePath ($($file.Length) bytes)" | ||
| $totalCopied++ | ||
| } |
|
Please open a discussion at https://github.com/dotnet/runtime/discussions to explain the objectives behind these PRs first. We need to understand the rationale for these changes. As it stands, I don't think we want to publish intermediate objects even for experiments. |
Apply the 102 file changes from dotnet/runtime PR dotnet#127295 which removes unused #include directives from C++ source files across coreclr and native host components. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hi Adeel, I understand your concern. I'm looking to validate the include cleanup changes which is why I need the objs. I was chatting with Jan about the work and it'll be good to find a way to ensure we catch the tricky cases. I see you closed this PR which caused my pipeline with artifacts to get deleted, so I'll need to reopen it and kick off a new run. |
|
As mentioned in #127295, removing headers without familiarity with the codebase generally isn’t safe or particularly beneficial. These changes can easily break compilation on platforms or configurations that aren’t covered by the regular CI matrix. If you’re looking to get started with contributions in runtime repo, it would be more helpful to focus on issues labeled help wanted, where the expected change and impact are clearer: https://github.com/dotnet/runtime/issues?q=is:issue label:"help wanted". |
|
@am11 , Agreed, and that's why I'm seeking review on the PR. The initial removals were automated by a tool and we're looking to validate the accuracy. We've made contributions to other repos as well. If you see any concerning removal, please let us know and we'll be happy to make a change. |
|
I don’t think it’s appropriate to use the full CI matrix to experimentally upload artifacts for speculative include cleanup validation, especially for a new contributor and especially when the original PR has not established any measurable benefit yet. CI resources and artifact storage are shared and limited, and we generally try to use them sparingly. If you want to investigate whether these changes affect binary size, please do the measurements locally. I also doubt you will find any meaningful difference in release builds artifacts since we already build with |
|
This repository is very complicated comparing to others. It's not suitable for unconscious automated tools that aren't build specifically. It's also unsuitable to do such clean up in a scope of entire repository, because different components are having different historical quirks. |
…tifacts
Add a post-build step to global-build-job.yml that collects compiled object files (.obj/.o) for the 102 source files modified in PR #127295 (Remove Unused Includes) and publishes them as pipeline artifacts.