[PATCH] D59530: [LLD][COFF] Fix /linkrepro with options that take a filename or path

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

In D59530#1434700 <https://reviews.llvm.org/D59530#1434700>, @mstorsjo wrote:

> In D59530#1434691 <https://reviews.llvm.org/D59530#1434691>, @aganea wrote:
>
> > In D59530#1434688 <https://reviews.llvm.org/D59530#1434688>, @mstorsjo wrote:
> >
> > > Aren't all of `/implib`, `/pdb`and `/out` output files? In that sense it doesn't matter where they are written in the repro case (and writing them locally instead of somewhere else with an absolute path probably is more convenient?).
> >
> >
> > Yes, however our build system generates cmd-lines with full or relative paths for these options, which makes the response unusable, unless you edit it (because the paths aren't there on the reproducer's machine)
> >
> > Are you suggesting that we strip those options from their path, and only kept the filename? That would be a good alternative.
>
>
> That sounds like a good alternative to me, yes.


I agree, stripping directory names from a path seems a better approach. In particular, making it relative path as you proposed originally has an issue that you need to add an empty directory

  F/other/path

to a generated tar file for

  /out:F/other/path/file.exe

or otherwise the linker would fail when trying to create the output file. And adding an empty directory to an archive file is a little tricky -- I'm not 100% sure if all tar implementations restores an empty directory (a common idiom is to add an empty file `.keep` to an empty directory, so that the directory is not empty, but that's yet another hack.) All in all, keeping the only filename seems like the best option here.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59530/new/

https://reviews.llvm.org/D59530