fix(driver): flatten build artifact paths (#1894) - #1895
Draft
ghaith wants to merge 3 commits into
Draft
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 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.
Draft on purpose. We are in feature freeze, so this needs a team decision before it goes
anywhere. Opening it now so the assessment below is on the record.
Backport of #1894 to
release/1.0.x. Fixes PRG-4448 on the v1 line.What it does
Intermediate artifacts get a flat name,
<file name>-<digest>.<extension>, all directly in<build>/<target>/, instead of being named after the source path and nested under it. Thetarget directory is also no longer added twice. See #1894 for the full rationale.
Why it is wanted on v1
PRG-4448 is reported against a shipped workspace. With the standard library delivered outside
the workspace at a realistic path, the longest artifact path measured 445 characters, and an
xcopyof the build output copied 3 of 31 files while returning exit code 0. The user-visiblesymptom is a File Explorer copy of the workspace that silently loses files.
Port risk
Low. The port is mechanical:
pipelines.rs,participant.rs,codegen.rs, and both testharnesses.
src/codegen.rsis byte-identical betweenmasterandrelease/1.0.x.compiler/plc_driver/Cargo.toml, over theplc_utilversion string only.Resolved by keeping
1.0.4.siphasheris already a workspace dependency on this branch; themanifest change only moves it to
[workspace.dependencies]soplc_drivercan use it.Verified on this branch on Windows: clean build, 2742 lib tests, 83 integration, 352
correctness, all green, including
debug_pathsand the 31 new tests.A release build of this branch, run against the Eclipse tool's real compile command, produces
output byte-identical to #1894 — same 233-character longest path and the same digests. So
a v1 and a v1.1 toolchain name artifacts identically, and switching between them against a
shared build directory causes no churn.
What reviewers should weigh
.opath would break. The Eclipse builder resolves only
<compile>/<target>/<output>.soandusr/lib/..., both unchanged and confirmed unchanged in a real build, but packaging ordebug tooling elsewhere is worth a second look.
🤖 Generated with Claude Code