[PATCH] D85024: [llvm-objdump] Implement --prefix option

[Vinicius Tinti via Phabricator via llvm-commits]

tinti marked 4 inline comments as done.
tinti added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/source-interleave-absolute-paths.test:2
+; Test --prefix option.
+; UNSUPPORTED: system-windows
+
----------------
jhenderson wrote:
> tinti wrote:
> > jhenderson wrote:
> > > Is this option not intended to be supported on Windows? I couldn't figure out GNU objdump's behaviour for Windows, so it's possible it doesn't work, but it would be nice, if we could get it to. One option would be to insert the prefix after the drive letter. Thus `C:\foo` would become `C:\prefix\foo`. It might be necessary to look at GNU's source code to identify its Windows behaviour.
> > > Is this option not intended to be supported on Windows? I couldn't figure out GNU objdump's behaviour for Windows, so it's possible it doesn't work, but it would be nice, if we could get it to. One option would be to insert the prefix after the drive letter. Thus `C:\foo` would become `C:\prefix\foo`. It might be necessary to look at GNU's source code to identify its Windows behaviour.
> > 
> > It is intended to be supported on Windows. The behavior implemented up to now is correct and matches GNU objdump. On Windows path `C:\foo` with prefix `bar` is supposed and will to become `barC:\foo`. It looks wrong  but it is not. Now I understand why it is supposed to be that way. Please read below.
> > 
> > There is another option that I have already implemented but not submitted called `--prefix-strip`. `--prefix-strip` receives an integer and strip parts by the separators from the original absolute path. So to make `C:\foo\filename.c` become `C:\prefix\foo\filename.c` one need to use `--prefix C:\prefix\` and `--prefix-strip 1`.
> Thanks, that makes sense to me. If you remove the `UNSUPPORTED` line, the per-merge bot might tell you whether it works on Windows. I also primarily develop on Windows, so can double-check too if you want.
> 
> My suspicion is that this might not work, due to how --prefix and absolute paths work on Windows. Once you've implemented --prefix-strip, that one can be supported in a separate test, which will work on Windows then hopefully.
As far as I understood it should pass.

Would be awesome if you could test and remove `UNSUPPORTED` if it pass.


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

https://reviews.llvm.org/D85024