Add support for generating a dsymutil reproducer, which is a folder containing all the object files for linking. When --gen-reproducer is passed, dsymutil uses a FileCollectorFileSystem which keeps track of all the files used by dsymutil. These files are copied into a temporary directory when dsymutil exists. When this path is passed to --use-reproducer, dsymutil uses a RedirectingFileSystem that will use the files from the reproducer directory instead of the actual paths. This means you don't need to mess with the OSO path prefix.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This needs tests and some polish but please let me know what you think of the general approach.
generally it looks good for me, I only have a couple of inline comments.
| llvm/tools/dsymutil/Reproducer.cpp | ||
|---|---|---|
| 42 | It looks like it is not an error, but just logging. Thus it should probably be put under Verbose flag? if (Options.Verbose)
outs() << "Reproducer written" | |
| llvm/tools/dsymutil/dsymutil.cpp | ||
| 101 | it would be good to initialize this: | |
| llvm/tools/dsymutil/Options.td | ||
|---|---|---|
| 167 | I did it for consistently with clang, which has the same flag. | |
| 171 | It's funny you say this because that's what I had originally. It would certainly be more consistent with lldb. I changed it because I don't think "replay" is accurate for dsymutil. I don't plan on capturing anything beyond the input files, so really there's nothing being "replayed". The functionality is more closely related to what clang does, which is why I went with "generate" instead of "capture". | |
| llvm/tools/dsymutil/Reproducer.cpp | ||
| 42 | It's more than logging, it's the only way to find out where the reproducer has been written. I'll change it to outs(), but I don't think it should be behind the Verbose flag. | |
| llvm/tools/dsymutil/Reproducer.cpp | ||
|---|---|---|
| 42 | I see. Let it be just outs() then. | |
| llvm/test/tools/dsymutil/X86/reproducer.test | ||
|---|---|---|
| 28 | most of these should be CHECK-NEXT? | |
| llvm/test/tools/dsymutil/cmdline.test | ||
| 5 | What do you think about adding RUN: cat dsymutil.rst | FileCheck --check-prefix=HELP here to ensure all options are documented? | |
| llvm/tools/dsymutil/Options.td | ||
| 171 | Can you add these to the man page as well? | |
I don't know enough about the VFS to do a thorough review, but the high level logic SGTM
My bots fail with:
../../llvm/tools/dsymutil/Reproducer.cpp:10:10: error: 'Reproducer.h' file not found with <angled> include; use "quotes" instead
#include <Reproducer.h>
^~~~~~~~~~~~~~
"Reproducer.h"Any reason for the <> include style?
No, must have been unconscious/unintentional. Fixed in d395eacca579858972fd4f79f4c9637db7666392.
most of these should be CHECK-NEXT?