Skip to content

Fix output folder for absolute path inputs#296

Open
davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
davispuh:paths
Open

Fix output folder for absolute path inputs#296
davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
davispuh:paths

Conversation

@davispuh
Copy link
Copy Markdown

Currently if you use absolute path for input files then output folder is not respected.

That is, this fails:

$ dstep -o /tmp /usr/include/stdio.h /usr/include/stdlib.h 
dstep: an unknown error occurred: std.file.FileException@std/file.d(840): /usr/include/stdio.d: Permission denied
----------------
/usr/include/dlang/dmd/std/file.d:283 @trusted bool std.file.cenforce!(bool).cenforce(bool, scope const(char)[], scope const(char)*, immutable(char)[], ulong) [0x562fb05e0003]
??:? @trusted void std.file.writeImpl(scope const(char)[], scope const(char)*, scope const(void)[], bool) [0x562fb06dae0b]
/usr/include/dlang/dmd/std/file.d:745 @safe void std.file.write!(immutable(char)[]).write(immutable(char)[], const(void[])) [0x562fb0639917]
dstep/translator/Translator.d:75 void dstep.translator.Translator.Translator.translate() [0x562fb06b35e0]
dstep/driver/Application.d:66 void dstep.driver.Application.Application.run() [0x562fb065a15c]
dstep/driver/CommandLine.d:181 int dstep.driver.CommandLine.run(immutable(char)[][]) [0x562fb066aa61]
dstep/main.d:18 _Dmain [0x562fb066d977]
std.file.FileException@std/file.d(840): /usr/include/stdio.d: Permission denied
----------------
/usr/include/dlang/dmd/std/file.d:283 @trusted bool std.file.cenforce!(bool).cenforce(bool, scope const(char)[], scope const(char)*, immutable(char)[], ulong) [0x562fb05e0003]
??:? @trusted void std.file.writeImpl(scope const(char)[], scope const(char)*, scope const(void)[], bool) [0x562fb06dae0b]
/usr/include/dlang/dmd/std/file.d:745 @safe void std.file.write!(immutable(char)[]).write(immutable(char)[], const(void[])) [0x562fb0639917]
dstep/translator/Translator.d:75 void dstep.translator.Translator.Translator.translate() [0x562fb06b35e0]
dstep/driver/Application.d:66 void dstep.driver.Application.Application.run() [0x562fb065a15c]
dstep/driver/CommandLine.d:181 int dstep.driver.CommandLine.run(immutable(char)[][]) [0x562fb066aa61]
dstep/main.d:18 _Dmain [0x562fb066d977]

It tries to create .d files 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.

Copilot AI review requested due to automatic review settings March 18, 2026 21:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 findBasePath helper to compute a common base directory for absolute inputs.
  • Update multi-input output mapping to build outputs under config.output using relativePath(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.

Comment on lines 133 to 139
{
auto basePath = findBasePath(inputFiles);
alias fmap = file => Path.buildPath(
config.output,
makeDefaultOutputFile(file, false));
makeDefaultOutputFile(relativePath(file, basePath), false));

return inputFiles.map!fmap.array;
Comment thread dstep/driver/Util.d
Comment on lines +23 to +45
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 "/";
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner

@jacob-carlborg jacob-carlborg Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dstep/driver/Util.d Outdated
Copy link
Copy Markdown
Owner

@jacob-carlborg jacob-carlborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no new tests added that shows the change is working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants