This is an archive of the discontinued LLVM Phabricator instance.

Break some more dependencies in lldbUtility
ClosedPublic

Authored by zturner on Feb 13 2017, 1:30 PM.

Details

Summary

This completely removes the dependency from lldbUtility -> lldbCore and lldbTarget. This was done with the following restructure:

  1. ProcessStructReader: Utility -> Target
  2. ModuleCache: Utility -> Target
  3. RegisterNumber: Utility -> Target
  1. Endian: Host -> Utility
  2. Flags: Core -> Utility

This is enforced in the CMake by removing lldbCore and lldbUtility as linker inputs to both the lldbUtility target as well as the UtilityTests gtest target.

Unfortunately we are still requiring the dependency on lldbHost, which in turn depends on everything else, so for now we are still linking against Core and Utility transitively. After the dependency on Host is broken, this effort will be complete. But that is the only remaining step.

Diff Detail

Event Timeline

zturner created this revision.Feb 13 2017, 1:30 PM
clayborg accepted this revision.Feb 13 2017, 2:59 PM

Looks fine.

This revision is now accepted and ready to land.Feb 13 2017, 2:59 PM

Hopefully we are maintaining the SVN history for these files? Hard to tell from the patch.

I'm not sure, it's a bit of black magic :-/ I use git with mono-repo, and there is a bunch of magic that happens at the git-svn layer. When I run git status it shows me that the files are renames / moves, so I hope that means that it preserves the history when it goes to SVN, but I don't know how to check.

git log --follow seems to understand. Is that sufficient to guarantee that it will be retained on the SVN side?

D:\src\llvm-mono>git log --follow lldb/source/Target/ModuleCache.cpp
commit 07bf36c627f3a4304dbb8ea3f8347110a025bd93
Author: Zachary Turner <zturner@google.com>
Date:   Mon Feb 13 13:21:39 2017 -0800

    Break some more dependencies in lldbUtility.

commit 2b9af9a9efe04af53a63e56f35dd835c814f000a
Author: Mehdi Amini <mehdi.amini@apple.com>
Date:   Fri Nov 11 04:29:25 2016 +0000

    Prevent at compile time converting from Error::success() to Expected<T>

    This would trigger an assertion at runtime otherwise.

    Differential Revision: https://reviews.llvm.org/D26482

commit 990debc1558714a5eda436d1e9bb4786b55bb2d5
Author: Mehdi Amini <mehdi.amini@apple.com>
Date:   Fri Nov 11 04:28:40 2016 +0000

    Make the Error class constructor protected

    This is forcing to use Error::success(), which is in a wide majority
    of cases a lot more readable.

    Differential Revision: https://reviews.llvm.org/D26481

I don't know enough to say for sure. Go ahead and try it with this patch as the files are pretty simple and losing the history wouldn't be too bad on these files. If it fails to have history, then you should use an SVN workflow for any future moves.

git log --follow seems to understand. Is that sufficient to guarantee that it will be retained on the SVN side?

On SVN there is need to explicitly rename files to retain history. (svn mv old new or similar syntax)

git log --follow seems to understand. Is that sufficient to guarantee that it will be retained on the SVN side?

On SVN there is need to explicitly rename files to retain history. (svn mv old new or similar syntax)

I know, but there is the question of whether git-svn does this automatically on the backend.

I guess in the future it doesn't matter much, as it looks like an eventual move to git-only is inevitable.

labath accepted this revision.Feb 14 2017, 2:58 AM

looks good

This revision was automatically updated to reflect the committed changes.

FYI.
After r295088 commit Xcode project fails with "no such file or directory: '/Users/boris/ws/lldb/lldb/source/Utility/ModuleCache.cpp'" message.
I think lldb.xcodeproj/project.pbxproj should be updated.

lldb/unittests/Utility/CMakeLists.txt