fix(driver): flatten build artifact paths - #1894
Open
ghaith wants to merge 4 commits into
Open
Conversation
Problem: Every object, bitcode and IR artifact was named after the path of its source and created under that path inside the build directory, so a source outside of the project mirrored its whole absolute path. The directory of the target was added twice on top of that. The resulting paths go past the 260 character limit of the Windows file system, which breaks file operations on the workspace. Solution: Name artifacts `<file name>-<digest of the unit>.<extension>` and keep all of them directly in the directory of the target. The digest keeps two units with the same file name apart now that they share a directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The path length fix was implemented and tested on Linux, while the file system it targets is Windows. The note lists the invariants the tests hold, the open points to validate on Windows and the tests worth adding there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The naming of the build artifacts was implemented and tested on Linux, while the file system it targets is Windows. Reverting the fix and running the suite on Windows reproduces the broken state there: the directory of the target appears twice, the absolute path of an external source is mirrored below it, and the scenario of the path limit reaches 374 characters. Closes the open points of the validation note: - The relative form of a key is reached on Windows. `canonicalize` returns a verbatim path for a unit, so a project root that is not canonicalized never prefixes it; the new end-to-end test through the `build` subcommand, where the root comes from the `plc.json`, names its artifacts after the path relative to the project. - Casing is normalized by the canonicalization of the key, not by the digest. A source referenced with another casing keeps its one artifact. - A UNC path, a mapped drive, a reserved device name and a source name outside of ASCII all keep the readable part of the name and build. - A build directory that is already past 260 characters needs no diagnostic: the standard library hands absolute paths to the file system in their verbatim form, so the compiler builds through it. What fails on such a path are the tools around the compiler, which is what the fix is for. Left open, as before: nothing removes the mirrored tree that an older `plc` wrote into a build directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A library is pulled in with one glob per extension, `include/*.st` and `include/*.pli` for the standard library, so two units can share a stem and differ only in their extension. Once the stem reaches the limit of the readable part, the cut falls on the dot of the source extension and both names lose it, which leaves the digest as the only thing telling the two artifacts apart. The existing tests covered the two halves of this separately, a cut stem and an extension that differs, but not the combination the standard library can actually produce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes PRG-4448.
Problem
Every object, bitcode and IR artifact was named after the path of its source and created
under that path inside the build directory, so a source outside the project mirrored its
whole absolute path. The directory of the target was added twice on top of that, once by the
driver and once by
GeneratedModule::persist.On Windows a second problem compounded it:
fs::canonicalizereturns a verbatim path(
\?\C:\...) while the project root does not, so the comparison that selects the relativename never matched and the absolute path was mirrored even for sources inside the project.
The result goes past the 260 characters the Windows file system allows without long path
support. The compiler itself does not fail, because the standard library hands absolute paths
to the file system in verbatim form, so the damage lands on everything else: the reporter of
PRG-4448 hit it copying their workspace in File Explorer.
Solution
Artifacts get a flat name of the form
<file name>-<digest>.<extension>, for examplemain.st-116b35a8828869ac.o, and all of them land directly in<build>/<target>/.on every run, process and platform. It is what keeps two units with the same file name in
different directories apart now that they share one directory.
outside the project. The project root is canonicalized before the comparison, so the
relative form is also reached on Windows.
GeneratedModule::persistwritesto the directory it is given.
The compiler now adds at most 70 characters to the build directory. Unchanged: the final
artifact of a link still lands in
<build>/<target>/<output>, and the header generator stillwrites next to the source or into the requested directory.
Validation on Windows
Ran the Eclipse tool's own compile command unchanged, varying only the compiler binary, with
the standard library at a 202-character delivery path outside the workspace:
xcopyof the build outputThe old path reproduces the shape reported in the ticket, doubled target directory included.
The 3-of-31 copy is the reported failure: silent truncation with a success exit code.
Tests
compiler/plc_driver/src/tests/artifact_names.rscovers the naming, with platform dependentscenarios mirrored between a
linuxand awindowsmodule.tests/integration/build_artifacts.rscovers the layout end to end, including a build through the
buildsubcommand where theproject root comes from
plc.json. Both suites already run in the Windows job.The flat assertion, the character budget and the 260-character limit were each confirmed red
against the pre-fix code on Windows before this went in.
🤖 Generated with Claude Code