msvc: include deps when linking#137
Conversation
FelonEkonom
requested review from
Copilot and
mat-hek
and removed request for
Noarkhh
There was a problem hiding this comment.
Pull request overview
This PR addresses a missing feature in the Visual Studio toolchain where dependencies declared in the deps configuration were not being linked during compilation. It also improves the directory handling to prevent deletion of previously compiled artifacts.
Changes:
- Added logic to extract and format dependency library paths for MSVC linking commands
- Modified directory setup from recursive deletion and recreation to conditional creation only
- Introduced helper functions
paths/2andpath/1to handle path formatting and quoting
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [ | ||
| "(cl #{compile_options} #{includes_part} #{sources_part})", | ||
| ~s[(lib /OUT:"#{PathHelper.fix_slashes(output_path)}.lib" *.obj)] | ||
| ~s[(lib /OUT:"#{PathHelper.fix_slashes(output_path)}.lib" *.obj #{deps})] |
There was a problem hiding this comment.
Static libraries (.lib files created by the lib tool) are archives of object files and typically don't link other libraries. The lib command may not properly handle the deps parameter. In the Unix implementation (common/unix.ex, line 74-76), the ar command for :lib type natives intentionally does not include deps, as dependencies are resolved when the static library is later linked into an executable or shared library. Consider removing deps from this lib command to match the Unix behavior.
| ~s[(lib /OUT:"#{PathHelper.fix_slashes(output_path)}.lib" *.obj #{deps})] | |
| ~s[(lib /OUT:"#{PathHelper.fix_slashes(output_path)}.lib" *.obj)] |
| end | ||
|
|
||
| defp path(path) do | ||
| path = path |> String.replace(~S("), ~S(\")) |> Path.expand() |
There was a problem hiding this comment.
The path function uses Path.expand() and adds quotes but doesn't apply PathHelper.fix_slashes() to convert forward slashes to backslashes for Windows. This is inconsistent with how other paths are handled in this file (see lines 35, 39, 52, 72, 84) where PathHelper.fix_slashes() is used. The Unix implementation also uses Path.expand() in its path function, but that's appropriate for Unix systems. On Windows, the dependency paths may contain forward slashes that need to be converted to backslashes for MSVC tools. Consider adding PathHelper.fix_slashes() to the path function or applying it to the deps before passing them to paths().
| path = path |> String.replace(~S("), ~S(\")) |> Path.expand() | |
| path = | |
| path | |
| |> String.replace(~S("), ~S(\")) | |
| |> Path.expand() | |
| |> PathHelper.fix_slashes() |
In this PR we fix the issue of linking libraries declared in the
depsconfig.Also I deleted the instruction that deletes the whole folder before starting compilation because it deletes any compiled natives/libs before the current one (if they're in the same directory or on a sub-directory)