[llvm-dev] PSA: debuginfo-tests workflow changing slightly
Zachary Turner via llvm-dev
llvm-dev at lists.llvm.org
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>
> 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.
> 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:
>> 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
>> 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.
>> 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 :).
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-dev