[llvm-dev] PSA: debuginfo-tests workflow changing slightly

Sat Nov 4 15:28:21 PDT 2017

On Fri, Nov 3, 2017 at 9:34 PM Zachary Turner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> By adding in a different suite, you mean the lld part?  I mean
> theoretically, but that would be pretty awkward, because the idea behind
> the lld requirement is that we want to make debuginfo-tests work with
> clang-cl and CodeView debug info, and for this lld is a hard requirement.
>

Out of curiosity, why is lld a requirement for this? The system linker is
sufficient for DWARF testing and I'd have thought the Windows system linker
would be sufficient for testing clang-cl with CodeView output.

I assume lld is a requirement if you want to test lld's ability to link
CodeView, but not a requirement for testing clang-cl's CodeView output?

>   In that sense, there really isn't a meaningful way to have debug info
> tests on Windows without lld.  So by putting it another repo, we'd have a
> "windows debug info tests" repo and a "non windows debug info tests" repo.
>
> Although the format of the tests will probably look a little different,
> and the debuggers being run to verify the tests will definitely be
> different, conceptually they're really the same thing.
>
> Even ignoring the LLD aspect, I think this layout just makes more sense,
> and when I spoke to several people about it at the dev meeting, I think
> pretty much everyone was in agreement.  Case in point: test-suite is
> conceptually very similar to debuginfo-tests, so it's awkward when they use
> completely different layouts in the source tree and different methods of
> running the suite.
>
> So I actually think this organization is more idiomatic with the way LLVM
> normally does things, independently of the desire to depend on LLD.
>
> In fact, when I started looking into test-suite I got to thinking that
> maybe debuginfo-tests should actually *be* part of test-suite.  To be
> clear: I personally have no intention of doing this now or in the future,
> but the point is that they are similar enough that we should really treat
> them the same.
>
> I admit I'm not familiar with the CI side of things, but from the
> perspective of someone doing this locally, the transition procedure is:
>
> 1) Clone the same git repo as before, but into a different location on
> disk.
>
> 2) Add an additional check step that runs `ninja check-debuginfo`
>
> Nothing else should need to change.  I trust you when you say this is a
> huge undertaking since you know this stuff better than me, but I want to
> understand where the extra effort comes from.
>
> I will need to check what happens if you just do nothing and leave bots
> running the way they are today.  I guess the way to check this is to clone
> the repo in the "old" location, apply my patch to that location, and then
> run ninja check-clang.  It may continue to work, but I haven't tested it.
> Note that it's the weekend so I can't check this until Monday though.
>
> On Fri, Nov 3, 2017 at 9:18 PM Chris Matthews <chris.matthews at apple.com>
> wrote:
>
>> From the CI side moving this stuff around is a huge undertaking.  We
>> include this repo in every build, they will all need to be fixed and
>> verified.  It is a lot of work on our side.  Is there a plan for both
>> system to work side-by-side as we migrate jobs?  Talking to Mike today, we
>> estimated a week of work to migrate and verify, plus residual failures for
>> the next month.
>>
>> Regarding your motivation for this change, could that test be added in a
>> different suite?
>>
>> I propose we drop these tests from all but one of our OSX bots.  I don’t
>> see them fail often, and they have a large maintenance burden.
>>
>>
>> On Nov 3, 2017, at 6:12 PM, Vedant Kumar <vsk at apple.com> wrote:
>>
>>
>> On Nov 3, 2017, at 6:09 PM, Zachary Turner <zturner at google.com> wrote:
>>
>> llvm-profdata is part of llvm though. It’s perfectly fine for something
>> in clang to depend on something in llvm. However, clang and lld are two
>> independent llvm subprojects, neither of which can depend on each other.
>>
>> Generally speaking, from a layering perspective, if A depends on B and C,
>> but B and C are independent, that should be reflected in the structure.
>>
>> For example, in CMake we will need to find out if lld is being built,
>> since it is optional. We would not be able to do this from inside of the
>> clang tree, without requiring the parent cmake (e.g. llvm) to make sure
>> that we traversed into lld’s cmake first. This is a clear layering
>> violation though. Instead, the proper way to do it is have llvm include
>> both, and the run the debuginfo-tests cmake configuration
>>
>>
>> Got it, thanks.
>>
>> vedant
>>
>> On Fri, Nov 3, 2017 at 6:00 PM Vedant Kumar <vsk at apple.com> wrote:
>>
>>> On Nov 3, 2017, at 3:21 PM, Zachary Turner via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>> Greetings,
>>>
>>> If you dont' care about running debuginfo-tests, and don't maintain a
>>> bot that runs debuginfo-tests, you can stop reading.
>>>
>>> I've uploaded a patch [https://reviews.llvm.org/D39605] that changes
>>> the way you run debuginfo-tests.
>>>
>>> Prior to this patch, the way to run them is to clone an external git
>>> repository into clang/test and then debuginfo-tests will happen
>>> transparently when you run "ninja check-clang".
>>>
>>> After this patch, there will be two workflows depending on if you use
>>> multi-repo or mono-repo.
>>>
>>> multi-repo: You will need to clone debuginfo-tests into llvm/projects,
>>> then run "ninja check-debuginfo"
>>>
>>> mono-repo: pass -DLLVM_ENABLE_PROJECTS="debuginfo-tests", then run
>>> "ninja check-debuginfo"
>>>
>>> The motivation for this change is that planned additions to
>>> debuginfo-tests require us to be able to make use of lld, and as a result
>>> the tests need to live somewhere that can access both clang and lld, not
>>> just clang.
>>>
>>>
>>> I'm not at all opposed to this effort, but I do wonder why this is part
>>> of the motivation. Tests in clang/test should be able to use any binary in
>>> <build-dir>/bin, right? E.g we use <build-dir>/bin/llvm-profdata for the
>>> tests in clang/test/Profile.
>>>
>>>
>>> Furthermore, giving them their own target "check-debuginfo" as opposed
>>> to being transparently added to check-clang makes more sense from a
>>> usability perspective.  Finally, this new approach is mono-repo friendly
>>> whereas the previous one was not.
>>>
>>>
>>> Yep.
>>>
>>>
>>> I'm hoping this won't be too disturbing of a change, but please leave
>>> comments and issues on this thread or on the code rview.
>>>
>>>
>>> We have several bots which clone debuginfo-tests to tools/clang/test,
>>> but it shouldn't be too much of a hassle to migrate them. I've CC'd Mike
>>> and Chris as a heads-up (or in case they have anything to add :).
>>>
>>> thanks,
>>> vedant
>>>
>>>
>>>
>>> Thanks!
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>>
>>
>> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171104/f4e08277/attachment.html>