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

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
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/1f242649/attachment.html>

> 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
<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/32d2ca47/attachment.html>

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>