Fix output folder for absolute path inputs#296
Fix output folder for absolute path inputs#296davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes output path generation so that using -o <dir> with absolute input header paths places generated .d files under the requested output directory instead of attempting to write next to the input headers (often unwritable system locations).
Changes:
- Add
findBasePathhelper to compute a common base directory for absolute inputs. - Update multi-input output mapping to build outputs under
config.outputusingrelativePath(file, basePath).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dstep/driver/Util.d | Adds findBasePath to derive a base directory from absolute input paths. |
| dstep/driver/Application.d | Uses findBasePath + relativePath when building output paths for multiple inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| { | ||
| auto basePath = findBasePath(inputFiles); | ||
| alias fmap = file => Path.buildPath( | ||
| config.output, | ||
| makeDefaultOutputFile(file, false)); | ||
| makeDefaultOutputFile(relativePath(file, basePath), false)); | ||
|
|
||
| return inputFiles.map!fmap.array; |
| import std.path : isAbsolute, dirName; | ||
| import std.string : lastIndexOf; | ||
|
|
||
| auto absolutePaths = paths.filter!isAbsolute.array; | ||
| if (absolutePaths.empty) | ||
| { | ||
| return "/"; | ||
| } | ||
| if (absolutePaths.length == 1) | ||
| { | ||
| return dirName(absolutePaths[0]); | ||
| } | ||
| auto prefix = commonPrefix(absolutePaths[0], absolutePaths[1]); | ||
| foreach (path; absolutePaths[1..$]) | ||
| { | ||
| prefix = commonPrefix(prefix, path); | ||
| } | ||
| auto pos = lastIndexOf(prefix, '/'); | ||
| if (pos > 0) | ||
| { | ||
| return prefix[0..pos]; | ||
| } | ||
| return "/"; |
There was a problem hiding this comment.
That would be great if there actually were functions like commonPath, rootPath but it's hallucination. They don't exist and I don't have access to Windows system to test how it should be implemented there to support this case.
There was a problem hiding this comment.
I think Unix style paths work on Windows too. None of the functions used in findBasePath touches the file system. Perhaps you can cross-compile? findBasePath should work at CTFE so can test that in isolation without having to run it on Windows.
jacob-carlborg
left a comment
There was a problem hiding this comment.
There are no new tests added that shows the change is working.
Currently if you use absolute path for input files then output folder is not respected.
That is, this fails:
It tries to create
.dfiles in same directory as input file without respecting output folder.This PR fixes that so that it will correctly use output folder for such case.