[Lldb-commits] [PATCH] D112212: [lldb/test] Print build commands in trace mode

Sat Oct 23 12:07:44 PDT 2021

JDevlieghere added a comment.

In D112212#3081935 <https://reviews.llvm.org/D112212#3081935>, @dblaikie wrote:

> In D112212#3081828 <https://reviews.llvm.org/D112212#3081828>, @JDevlieghere wrote:
>
>> In D112212#3080491 <https://reviews.llvm.org/D112212#3080491>, @teemperor wrote:
>>
>>> This LGTM, but `shlex.join` is actually Py3 exclusive and I don't think there is a good Py2 replacement. I think we're just in time for the Py2->3 migration according to the timeline Jonas posted last year <https://lists.llvm.org/pipermail/lldb-dev/2020-August/016388.html>, so let's use this patch to actually do that? Then we can also get rid of all the `six` stuff etc.
>>>
>>> Let's see if Jonas has any objections against dropping Py2 with this, otherwise this is good to go.
>>
>> We're planning to branch from open source on October 26th. If there's no urgency, it would really be great if we can hold off breaking Py2 until then.
>>
>> I'm all in favor for getting rid of Python 2 support, but sweeping changes like dropping the `six` stuff will introduce a lot of headaches (merge conflicts) for us. If we could postpone that for another release that would save us a bunch of engineering time.
>
> No judgment (I think it's a reasonable request to punt a patch like this a few days if it helps out major contributors) - but I'm curious/just not quite wrapping my head around: Why would it be easier if this sort of patch went in after you branch? I'd have thought it'd be easier if it goes in before the branch. That way when you're backporting patches from upstream after the branch there will be fewer unrelated changes/merge conflicts, yeah?

The patch introduces a dependency on Python 3 and unfortunately we still have a small (but important) group of users that haven't fully migrated yet. If the patch were to land before the branch, I'd have to revert it (same result) or find a way to do what `shlex.join` does in Python 2. I did a quick search yesterday and didn't immediately find a good alternative and with the timeline I've given in the past, I also don't think the burden should be on the patch author (Pavel). So that's why I suggested holding off on landing it. If it does turn out to cause a lot of conflicts, I can always reconsider.

But yes, backporting is a real concern, which is the main reason I'm asking not to start making big mechanical changes like replacing all the `six` stuff unless there's a pressing reason to do so.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112212