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

Fri Nov 3 21:34:03 PDT 2017

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.
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
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171104/6988146c/attachment.html>