Rework workflow design to allow opting in to SplitBuild feature, and use Windows-compatible tar options#1103
Rework workflow design to allow opting in to SplitBuild feature, and use Windows-compatible tar options#1103
Conversation
| - name: Refresh artifact timestamps | ||
| shell: powershell | ||
| run: | | ||
| # Touch all extracted build outputs so ninja sees them as newer | ||
| # than source files and skips rebuilding. | ||
| Get-ChildItem -Recurse -File DXC\build\bin, llvm-project\build | | ||
| ForEach-Object { $_.LastWriteTime = Get-Date } |
There was a problem hiding this comment.
Can this really get away with being windows only?
There was a problem hiding this comment.
Yes, on line 230 we guard this code against macOS runs.
MacOS will not do any of this artifact download / upload stuff, since it is so fast it doesn't need this optimization, and I think this whole thing will be simpler if we exclude MacOS.
There was a problem hiding this comment.
See my post-commit review here: #1067 (review) - I have some misgivings about these architectural choices.
bob80905
changed the title
| --exclude='*.obj' \ | ||
| --exclude='*.o' \ | ||
| --exclude='*.ilk' \ | ||
| --exclude='*.pdb' \ | ||
| --exclude='CMakeFiles' \ |
There was a problem hiding this comment.
Do we need all of these, or is CMakeFiles sufficient? AFAIK the object files all end up within the CMakeFiles/SomeTarget.dir directories.
There was a problem hiding this comment.
I can confirm that .pdbs should also be excluded, as they do exist in the bin directory of the runner. We don't want to zip up and upload any more than we need to, all we need is enough to execute the tests, to optimize zip/upload artifact speed and the unzipping step as well.
For example, right now these paths exists on the runner:
"C:\actions-runner\_work\offload-test-suite\offload-test-suite\llvm-project\build\bin\api-query.pdb"
"C:\actions-runner\_work\offload-test-suite\offload-test-suite\llvm-project\build\bin\offloader.exe"
This is not under the CMakeFiles dir, and we don't want the pdb. I believe the other file types will be useful to avoid in other OS cases as well. But we do want the .exe, and it is not under CMakeFiles.
2 participants
--touch is not a supported option on the windows version of tar.
This PR changes tar to focus on extraction, and introduces a later step to update the timestamps after extraction in a cross-platform way, so that ninja doesn't rebuild the binaries.
Separately, there were other architectural / design concerns with the way the workflow was organized.
This PR parameterizes the SplitBuild option, so the current workflows can be manually dispatched to opt in to the split build run type.
But, this change will not affect current workflows, except just adding 2 skipped steps at the bottom, for uploading artifacts.
Addresses #1081