[PATCH] D129728: [llvm-dwp] Add SHF_COMPRESSED support and remove .zdebug support

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D129728#3653818 <https://reviews.llvm.org/D129728#3653818>, @MaskRay wrote:

> In D129728#3653781 <https://reviews.llvm.org/D129728#3653781>, @dblaikie wrote:
>
>> In D129728#3653775 <https://reviews.llvm.org/D129728#3653775>, @MaskRay wrote:
>>
>>> In D129728#3653769 <https://reviews.llvm.org/D129728#3653769>, @dblaikie wrote:
>>>
>>>> In D129728#3653748 <https://reviews.llvm.org/D129728#3653748>, @MaskRay wrote:
>>>>
>>>>> In D129728#3653746 <https://reviews.llvm.org/D129728#3653746>, @dyung wrote:
>>>>>
>>>>>> Your change is causing the test 'LLVM :: tools/llvm-dwp/X86/nocompress.test' to fail due to one of the expected input files being deleted by your change. Perhaps you need to delete or fixup this test as well?
>>>>>>
>>>>>> https://lab.llvm.org/buildbot/#/builders/216/builds/7412
>>>>>
>>>>> I've removed it in 33aa374e597260dae739bb949892cff31ae31eb9 <https://reviews.llvm.org/rG33aa374e597260dae739bb949892cff31ae31eb9> . Someone feeling a need to have a coverage can add it.
>>>>
>>>> Please don't remove test coverage like this - this test covers the error path here: https://github.com/llvm/llvm-project/blob/33aa374e597260dae739bb949892cff31ae31eb9/llvm/lib/DWP/DWP.cpp#L288
>>>
>>> It's a fast way to get bots green. It takes time to construct a test.
>>
>> The usual way to do that if this would take to long is to revert the causal patch while addressing the regression - not lower test coverage permanently.
>
> The file will need to be re-added anyway to avoid the precanned binary. Adding a test in one minute, I am not sure it's useful to revert this current patch.

Ah, OK - thanks! Nah, I don't mind fixing forward here, though I think in the future it'd be simpler/clearer if the patch were reverted and recommitted correctly rather than temporarily removing testing like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129728