Use relative ResXFileRef paths in translated resx#17077
Draft
mwiemer-microsoft wants to merge 1 commit into
Draft
Use relative ResXFileRef paths in translated resx#17077mwiemer-microsoft wants to merge 1 commit into
mwiemer-microsoft wants to merge 1 commit into
Conversation
TranslateSource writes translated .resx files under the XLIFF intermediate output directory. ResXFileRef values were intentionally rewritten from source-relative paths to absolute paths so generated files could still resolve external resources after moving away from the source directory, but that bakes the repo location into cached intermediate files. Add an output-path-aware rewrite for resx file references so the generated file points back to the resource relative to the translated .resx location. Keep the existing absolute rewrite API as the default for existing callers and tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts XliffTasks’ path-rewrite behavior so translated .resx files written under the XLIFF intermediate output directory do not embed absolute ResXFileRef paths tied to a specific repo clone location, improving incremental build reliability after moving/renaming a repo.
Changes:
- Introduces an output-aware rewrite hook (
RewriteRelativePathsForOutputPath(sourceFullPath, outputFullPath)) onTranslatableDocument. - Updates
TranslateSourceto call the output-aware hook when generating translated outputs. - Overrides the hook for
.resxto writeResXFileRefvalues relative to the translated.resxoutput directory, and adds a regression test.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.DotNet.XliffTasks/Tasks/TranslateSource.cs | Calls the new output-aware rewrite hook prior to saving the translated document. |
| src/Microsoft.DotNet.XliffTasks/Model/TranslatableDocument.cs | Adds the virtual output-aware path rewrite hook with a default implementation. |
| src/Microsoft.DotNet.XliffTasks/Model/ResxDocument.cs | Implements .resx-specific output-relative rewriting of ResXFileRef values (and adds helper logic). |
| src/Microsoft.DotNet.XliffTasks.Tests/ResxDocumentTests.cs | Adds a regression test asserting output-relative ResXFileRef paths and no baked-in absolute source paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+118
| private static string MakeRelativePath(string fromDirectory, string toPath) | ||
| { | ||
| string fromFullPath = Path.GetFullPath(fromDirectory); | ||
| string toFullPath = Path.GetFullPath(toPath); | ||
| string fromRoot = Path.GetPathRoot(fromFullPath); | ||
| string toRoot = Path.GetPathRoot(toFullPath); | ||
| StringComparison comparison = Path.DirectorySeparatorChar == '\\' | ||
| ? StringComparison.OrdinalIgnoreCase | ||
| : StringComparison.Ordinal; | ||
|
|
||
| if (!string.Equals(fromRoot, toRoot, comparison)) | ||
| { | ||
| return toFullPath; | ||
| } | ||
|
|
||
| Uri fromUri = new(AppendDirectorySeparator(fromFullPath)); | ||
| Uri toUri = new(toFullPath); | ||
| string relativePath = Uri.UnescapeDataString(fromUri.MakeRelativeUri(toUri).ToString()); | ||
| return relativePath.Replace('/', Path.DirectorySeparatorChar); | ||
| } |
Comment on lines
+87
to
+95
| XElement valueNodeOfFileRef = node.Element("value"); | ||
| string[] splitRelativePathAndSerializedType = valueNodeOfFileRef.Value.Split(';'); | ||
| string resourceRelativePath = splitRelativePathAndSerializedType[0].Replace('\\', Path.DirectorySeparatorChar); | ||
| string absoluteResourcePath = Path.GetFullPath(Path.Combine(sourceDirectory, resourceRelativePath)); | ||
|
|
||
| splitRelativePathAndSerializedType[0] = MakeRelativePath(outputDirectory, absoluteResourcePath); | ||
|
|
||
| valueNodeOfFileRef.Value = string.Join(";", splitRelativePathAndSerializedType); | ||
| } |
mmitche
requested changes
Jul 13, 2026
mmitche
left a comment
mmitche
left a comment
Member
There was a problem hiding this comment.
The fact that that it's already outputting absolute paths makes me wonder whether this is a requirement in some way. I don't know enough to say.
My guess is that scenarios like this break in other ways, and that we generally don't support moving a repo around and getting a working incremental build.
Any thoughts @ViktorHofer?
Member
|
I want to defer to @tmat as he knows most about this code base. |
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.
Summary
Treats the absolute
ResXFileRefrewrite as an intentional design choice, but adjusts the design for translated.resxoutputs so cached intermediate files do not bake in the repo clone path.TranslateSourcewrites translated files under the XLIFF intermediate output directory. The existingRewriteRelativePathsToAbsolutebehavior appears intentional: the code comment says external-file references, often icons, have relative paths adjusted to absolute paths, and the existing unit test asserts the absolute rewrite. I did not find separate docs or issue/PR discussion explaining more than that; the behavior arrived in Arcade with the 2023 import fromdotnet/xliff-tasks.Changes
RewriteRelativePathsForOutputPath(sourceFullPath, outputFullPath)as the output-aware hook, defaulting to the existing absolute rewrite for non-.resxdocument types..resxsoResXFileRefvalues are written relative to the translated.resxoutput directory.TranslateSourceto call the output-aware hook..resxcontains an output-relative path, not the absolute source clone path.Fixes #17061.
Validation
./eng/common/dotnet.cmd test ./src/Microsoft.DotNet.XliffTasks.Tests/Microsoft.DotNet.XliffTasks.Tests.csproj --configuration Debug