llvm dev - Nov 2017 - PSA: debuginfo-tests workflow changing slightly

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
<mailto: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
<mailto:vsk at apple.com>> wrote:
>>> On Nov 3, 2017, at 3:21 PM, Zachary Turner via llvm-dev
<llvm-dev at lists.llvm.org <mailto: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
<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 <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<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/20171103/396ce37b/attachment-0001.html>

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>

> On 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.
Since they are an end-to-end-test that the clang (and the lld) we just built
works with the system debugger, there is no benefit of running them in more than
one clang build configuration. From that point of view that would be ok.

-- adrian
> 
>> 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
>>> 
>> 
>

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>