[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

Wed Nov 28 15:37:02 PST 2018

davide added a comment.

In D53818#1312133 <https://reviews.llvm.org/D53818#1312133>, @martong wrote:

> > I reverted this change because it breaks a bunch of lldb tests (on MacOS, and probably on other platforms too). To be clear, part of the reason I'm reacting strongly here is that this is not the first patch this has come up on. I'm worried about the broader trend of landing patches to ASTImporter without proper reviews and testing as I am of this very instance. I really think you should consider asking reviews to somebody who's familiar with clang and test lldb as it's the main client of this functionality we have in tree.
>
> @davide I am terribly sorry about this issue. I just applied this patch on my Linux box, and it did not failed those tests which failed on the green lab buildbots. Could you please refer to the Linux buildbots which failed? Seems like there is a significant difference between the testsuites on Linux and MacOS. Our primary target is Linux and we do run the LLDB tests before we accept any commit into our fork and only then we continue with upstreaming to Phabricator.

I have no Linux bots access, this fails on macos (looks on http://lab.llvm.org:8080/green/). Although now it's too late because the logs are gone.

> Our platform in our CI is Linux only, but we are in the middle of getting a hosted MacOS to support our CI. I do hope that in the near future there will be no such issues. In the meantime, is there a way for us to execute a green lab MacOS build bot with a specific patch before a commit? If that is not possible, then we can just promise that we will monitor your buildbots and we will do the revert.

I'm not aware of a way of pre-commit testing this hosted on llvm.org. I generally run on my machine and check the bots output.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53818/new/

https://reviews.llvm.org/D53818