Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
Hi Zach, I think nobody is using the "if __name__ == '__main__'" block as executing a test file directly isn't working at the moment (the "import lldb" command fails). If you plan to change all test file then I would prefer to remove the reference to unittest2 from them for simplicity if nobody have an objection against it. Tamas On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > *TL;DR - Nobody has to do anything, this is just a heads up that a 400+ > file CL is coming.* > > IANAL, but I've been told by one that I need to move all third party code > used by LLDB to lldb/third_party. Currently there is only one thing there: > the Python `six` module used for creating code that is portable across > Python 2 and Python 3. > > The only other 2 instances that I'm aware of are pexpect and unittest2, > which are under lldb/test. I've got some patches locally which move > pexpect and unittest2 to lldb/third_party. I'll hold off on checking them > in for a bit to give people a chance to see this message first, because > otherwise you might be surprised when you see a CL with 400 files being > checked in. > > Nobody will have to do anything after this CL goes in, and everything > should continue to work exactly as it currently does. > > The main reason for the churn is that pretty much every single test in > LLDB does something like this: > > *import unittest2* > > ... > > if __name__ == '__main__': > import atexit > lldb.SBDebugger.Initialize() > atexit.register(lambda: lldb.SBDebugger.Terminate()) > *unittest2.main()* > > This worked when unittest2 was a subfolder of test, but not when it's > somewhere else. Since LLDB's python code is not organized into a standard > python package and we treat the scripts like dotest etc as standalone > scripts, the way I've made this work is by introducing a module called > *lldb_shared > *under test which, when you import it, fixes up sys.path to correctly add > all the right locations under lldb/third_party. > > So, every single test now needs a line at the top to import lldb_shared. > > TBH I don't even know if we need this unittest2 stuff anymore (does anyone > even use it?) but even if the answer is no, then that still means changing > every file to delete the import statement and the if __name__ == > '__main__': block. > > If there are no major concerns I plan to check this in by the end of the > day, or tomorrow. > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
Todd, Greg, can you guys confirm this is true? The import lldb would succeed if someone had their PYTHONPATH set up just right, but if really none of us care about it, I'm with Tamas in that I'd rather remove it. On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer wrote: > Hi Zach, > > I think nobody is using the "if __name__ == '__main__'" block as > executing a test file directly isn't working at the moment (the "import > lldb" command fails). If you plan to change all test file then I would > prefer to remove the reference to unittest2 from them for simplicity if > nobody have an objection against it. > > Tamas > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > >> *TL;DR - Nobody has to do anything, this is just a heads up that a 400+ >> file CL is coming.* >> >> IANAL, but I've been told by one that I need to move all third party code >> used by LLDB to lldb/third_party. Currently there is only one thing there: >> the Python `six` module used for creating code that is portable across >> Python 2 and Python 3. >> >> The only other 2 instances that I'm aware of are pexpect and unittest2, >> which are under lldb/test. I've got some patches locally which move >> pexpect and unittest2 to lldb/third_party. I'll hold off on checking them >> in for a bit to give people a chance to see this message first, because >> otherwise you might be surprised when you see a CL with 400 files being >> checked in. >> >> Nobody will have to do anything after this CL goes in, and everything >> should continue to work exactly as it currently does. >> >> The main reason for the churn is that pretty much every single test in >> LLDB does something like this: >> >> *import unittest2* >> >> ... >> >> if __name__ == '__main__': >> import atexit >> lldb.SBDebugger.Initialize() >> atexit.register(lambda: lldb.SBDebugger.Terminate()) >> *unittest2.main()* >> >> This worked when unittest2 was a subfolder of test, but not when it's >> somewhere else. Since LLDB's python code is not organized into a standard >> python package and we treat the scripts like dotest etc as standalone >> scripts, the way I've made this work is by introducing a module called >> *lldb_shared >> *under test which, when you import it, fixes up sys.path to correctly >> add all the right locations under lldb/third_party. >> >> So, every single test now needs a line at the top to import lldb_shared. >> >> TBH I don't even know if we need this unittest2 stuff anymore (does >> anyone even use it?) but even if the answer is no, then that still means >> changing every file to delete the import statement and the if __name__ == >> '__main__': block. >> >> If there are no major concerns I plan to check this in by the end of the >> day, or tomorrow. >> > ___ >> lldb-dev mailing list >> lldb-dev@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
+todd and greg On Thu, Oct 22, 2015 at 9:26 AM Zachary Turner wrote: > Todd, Greg, can you guys confirm this is true? The import lldb would > succeed if someone had their PYTHONPATH set up just right, but if really > none of us care about it, I'm with Tamas in that I'd rather remove it. > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer > wrote: > >> Hi Zach, >> >> I think nobody is using the "if __name__ == '__main__'" block as >> executing a test file directly isn't working at the moment (the "import >> lldb" command fails). If you plan to change all test file then I would >> prefer to remove the reference to unittest2 from them for simplicity if >> nobody have an objection against it. >> >> Tamas >> >> On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> >>> *TL;DR - Nobody has to do anything, this is just a heads up that a 400+ >>> file CL is coming.* >>> >>> IANAL, but I've been told by one that I need to move all third party >>> code used by LLDB to lldb/third_party. Currently there is only one thing >>> there: the Python `six` module used for creating code that is portable >>> across Python 2 and Python 3. >>> >>> The only other 2 instances that I'm aware of are pexpect and unittest2, >>> which are under lldb/test. I've got some patches locally which move >>> pexpect and unittest2 to lldb/third_party. I'll hold off on checking them >>> in for a bit to give people a chance to see this message first, because >>> otherwise you might be surprised when you see a CL with 400 files being >>> checked in. >>> >>> Nobody will have to do anything after this CL goes in, and everything >>> should continue to work exactly as it currently does. >>> >>> The main reason for the churn is that pretty much every single test in >>> LLDB does something like this: >>> >>> *import unittest2* >>> >>> ... >>> >>> if __name__ == '__main__': >>> import atexit >>> lldb.SBDebugger.Initialize() >>> atexit.register(lambda: lldb.SBDebugger.Terminate()) >>> *unittest2.main()* >>> >>> This worked when unittest2 was a subfolder of test, but not when it's >>> somewhere else. Since LLDB's python code is not organized into a standard >>> python package and we treat the scripts like dotest etc as standalone >>> scripts, the way I've made this work is by introducing a module called >>> *lldb_shared >>> *under test which, when you import it, fixes up sys.path to correctly >>> add all the right locations under lldb/third_party. >>> >>> So, every single test now needs a line at the top to import lldb_shared. >>> >>> >>> TBH I don't even know if we need this unittest2 stuff anymore (does >>> anyone even use it?) but even if the answer is no, then that still means >>> changing every file to delete the import statement and the if __name__ == >>> '__main__': block. >>> >>> If there are no major concerns I plan to check this in by the end of the >>> day, or tomorrow. >>> >> ___ >>> lldb-dev mailing list >>> lldb-dev@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >>> >> ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
[lldb-dev] Help with MSVC build issues
Hi! I broke couple of times MSVC builds when committed minor code cleanups. Looks like problem occurred because of changed order of headers. I could not investigated problems myself, because I don't have access to MSVC. Changes in question: r250872: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h r250925: source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp source/Plugins/OperatingSystem/Go/OperatingSystemGo.h Eugene. ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
I'm taking a look now. In the future please revert changes that break a buildbot. If you think you might know what the fix is (i.e. you guess that including a header will fix it), then you can just check it in as a test. If after a few attempts you still can't figure it out, then revert the change out again. But definitely please don't leave broken changes in. For example, the MSVC buildbot is now failing due to changes in LLVM. The reason is because it broke last night due to some unrelated changes which weren't reverted, and subsequent breaks did not result in an email to other people so they didn't know. We need to avoid this situation, so the bot has to stay green. anyway I'm working on fixing it now On Thu, Oct 22, 2015 at 9:28 AM Eugene Zelenko via lldb-dev < lldb-dev@lists.llvm.org> wrote: > Hi! > > I broke couple of times MSVC builds when committed minor code > cleanups. Looks like problem occurred because of changed order of > headers. > > I could not investigated problems myself, because I don't have access to > MSVC. > > Changes in question: > > r250872: > > source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp > source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h > > r250925: > > source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp > source/Plugins/OperatingSystem/Go/OperatingSystemGo.h > > Eugene. > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
I believe it would import lldb correctly. I don't tend to run the tests individually, but if it did work, I would use it more. > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev > wrote: > > Todd, Greg, can you guys confirm this is true? The import lldb would > succeed if someone had their PYTHONPATH set up just right, but if really none > of us care about it, I'm with Tamas in that I'd rather remove it. > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer > wrote: > Hi Zach, > > I think nobody is using the "if __name__ == '__main__'" block as executing a > test file directly isn't working at the moment (the "import lldb" command > fails). If you plan to change all test file then I would prefer to remove the > reference to unittest2 from them for simplicity if nobody have an objection > against it. > > Tamas > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev > wrote: > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ file > CL is coming. > > IANAL, but I've been told by one that I need to move all third party code > used by LLDB to lldb/third_party. Currently there is only one thing there: > the Python `six` module used for creating code that is portable across Python > 2 and Python 3. > > The only other 2 instances that I'm aware of are pexpect and unittest2, which > are under lldb/test. I've got some patches locally which move pexpect and > unittest2 to lldb/third_party. I'll hold off on checking them in for a bit > to give people a chance to see this message first, because otherwise you > might be surprised when you see a CL with 400 files being checked in. > > Nobody will have to do anything after this CL goes in, and everything should > continue to work exactly as it currently does. > > The main reason for the churn is that pretty much every single test in LLDB > does something like this: > > import unittest2 > > ... > > if __name__ == '__main__': > import atexit > lldb.SBDebugger.Initialize() > atexit.register(lambda: lldb.SBDebugger.Terminate()) > unittest2.main() > > This worked when unittest2 was a subfolder of test, but not when it's > somewhere else. Since LLDB's python code is not organized into a standard > python package and we treat the scripts like dotest etc as standalone > scripts, the way I've made this work is by introducing a module called > lldb_shared under test which, when you import it, fixes up sys.path to > correctly add all the right locations under lldb/third_party. > > So, every single test now needs a line at the top to import lldb_shared. > > TBH I don't even know if we need this unittest2 stuff anymore (does anyone > even use it?) but even if the answer is no, then that still means changing > every file to delete the import statement and the if __name__ == '__main__': > block. > > If there are no major concerns I plan to check this in by the end of the day, > or tomorrow. > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
You can get pretty much the same effect though by just running dotest and passing it the folder that the .py file is in. Then it only runs tests in that folder. You can specify the filename too if you want to limit it to one name. Sure, it's a few keystrokes less to just type TestMultithreaded.py or something, but given the extra complexity and the fact that it's running a totally different codepath, I wonder if the maintenance burden is worth it (I'm guessing no, since apparently it doesn't work well enough right now for anyone to use it) On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: > I believe it would import lldb correctly. I don't tend to run the tests > individually, but if it did work, I would use it more. > > > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > > > Todd, Greg, can you guys confirm this is true? The import lldb would > succeed if someone had their PYTHONPATH set up just right, but if really > none of us care about it, I'm with Tamas in that I'd rather remove it. > > > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer > wrote: > > Hi Zach, > > > > I think nobody is using the "if __name__ == '__main__'" block as > executing a test file directly isn't working at the moment (the "import > lldb" command fails). If you plan to change all test file then I would > prefer to remove the reference to unittest2 from them for simplicity if > nobody have an objection against it. > > > > Tamas > > > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ > file CL is coming. > > > > IANAL, but I've been told by one that I need to move all third party > code used by LLDB to lldb/third_party. Currently there is only one thing > there: the Python `six` module used for creating code that is portable > across Python 2 and Python 3. > > > > The only other 2 instances that I'm aware of are pexpect and unittest2, > which are under lldb/test. I've got some patches locally which move > pexpect and unittest2 to lldb/third_party. I'll hold off on checking them > in for a bit to give people a chance to see this message first, because > otherwise you might be surprised when you see a CL with 400 files being > checked in. > > > > Nobody will have to do anything after this CL goes in, and everything > should continue to work exactly as it currently does. > > > > The main reason for the churn is that pretty much every single test in > LLDB does something like this: > > > > import unittest2 > > > > ... > > > > if __name__ == '__main__': > > import atexit > > lldb.SBDebugger.Initialize() > > atexit.register(lambda: lldb.SBDebugger.Terminate()) > > unittest2.main() > > > > This worked when unittest2 was a subfolder of test, but not when it's > somewhere else. Since LLDB's python code is not organized into a standard > python package and we treat the scripts like dotest etc as standalone > scripts, the way I've made this work is by introducing a module called > lldb_shared under test which, when you import it, fixes up sys.path to > correctly add all the right locations under lldb/third_party. > > > > So, every single test now needs a line at the top to import lldb_shared. > > > > TBH I don't even know if we need this unittest2 stuff anymore (does > anyone even use it?) but even if the answer is no, then that still means > changing every file to delete the import statement and the if __name__ == > '__main__': block. > > > > If there are no major concerns I plan to check this in by the end of the > day, or tomorrow. > > ___ > > lldb-dev mailing list > > lldb-dev@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > ___ > > lldb-dev mailing list > > lldb-dev@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
I plan to put this in today. Greg, should I just go ahead and delete all the unittest2 stuff entirely? TBH I'm all for anything that reduces the complexity of the test suite. It's got a couple hundred options that nobody uses, seems like we should start whittling away at stuff that doesn't get any use. If you prefer I leave it in that's less work for me since I have that patch ready to go, but TBH I'd rather remove it if that's ok. On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner wrote: > You can get pretty much the same effect though by just running dotest and > passing it the folder that the .py file is in. Then it only runs tests in > that folder. You can specify the filename too if you want to limit it to > one name. Sure, it's a few keystrokes less to just type > TestMultithreaded.py or something, but given the extra complexity and the > fact that it's running a totally different codepath, I wonder if the > maintenance burden is worth it (I'm guessing no, since apparently it > doesn't work well enough right now for anyone to use it) > > On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: > >> I believe it would import lldb correctly. I don't tend to run the tests >> individually, but if it did work, I would use it more. >> >> > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > >> > Todd, Greg, can you guys confirm this is true? The import lldb would >> succeed if someone had their PYTHONPATH set up just right, but if really >> none of us care about it, I'm with Tamas in that I'd rather remove it. >> > >> > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < >> tbergham...@google.com> wrote: >> > Hi Zach, >> > >> > I think nobody is using the "if __name__ == '__main__'" block as >> executing a test file directly isn't working at the moment (the "import >> lldb" command fails). If you plan to change all test file then I would >> prefer to remove the reference to unittest2 from them for simplicity if >> nobody have an objection against it. >> > >> > Tamas >> > >> > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ >> file CL is coming. >> > >> > IANAL, but I've been told by one that I need to move all third party >> code used by LLDB to lldb/third_party. Currently there is only one thing >> there: the Python `six` module used for creating code that is portable >> across Python 2 and Python 3. >> > >> > The only other 2 instances that I'm aware of are pexpect and unittest2, >> which are under lldb/test. I've got some patches locally which move >> pexpect and unittest2 to lldb/third_party. I'll hold off on checking them >> in for a bit to give people a chance to see this message first, because >> otherwise you might be surprised when you see a CL with 400 files being >> checked in. >> > >> > Nobody will have to do anything after this CL goes in, and everything >> should continue to work exactly as it currently does. >> > >> > The main reason for the churn is that pretty much every single test in >> LLDB does something like this: >> > >> > import unittest2 >> > >> > ... >> > >> > if __name__ == '__main__': >> > import atexit >> > lldb.SBDebugger.Initialize() >> > atexit.register(lambda: lldb.SBDebugger.Terminate()) >> > unittest2.main() >> > >> > This worked when unittest2 was a subfolder of test, but not when it's >> somewhere else. Since LLDB's python code is not organized into a standard >> python package and we treat the scripts like dotest etc as standalone >> scripts, the way I've made this work is by introducing a module called >> lldb_shared under test which, when you import it, fixes up sys.path to >> correctly add all the right locations under lldb/third_party. >> > >> > So, every single test now needs a line at the top to import lldb_shared. >> > >> > TBH I don't even know if we need this unittest2 stuff anymore (does >> anyone even use it?) but even if the answer is no, then that still means >> changing every file to delete the import statement and the if __name__ == >> '__main__': block. >> > >> > If there are no major concerns I plan to check this in by the end of >> the day, or tomorrow. >> > ___ >> > lldb-dev mailing list >> > lldb-dev@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> > ___ >> > lldb-dev mailing list >> > lldb-dev@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> >> ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
It looks like you're generating these diffs from an SVN repo, so I'm seeing lldb/trunk in the paths. Because of that, I can't apply it on my git repo which doesn't have the "trunk" folder. Can you generate the diffs for the 2 aforementioned CLs by cding into the trunk directory and generating the diff there? On Thu, Oct 22, 2015 at 9:34 AM Zachary Turner wrote: > I'm taking a look now. In the future please revert changes that break a > buildbot. If you think you might know what the fix is (i.e. you guess that > including a header will fix it), then you can just check it in as a test. > If after a few attempts you still can't figure it out, then revert the > change out again. > > But definitely please don't leave broken changes in. For example, the > MSVC buildbot is now failing due to changes in LLVM. The reason is because > it broke last night due to some unrelated changes which weren't reverted, > and subsequent breaks did not result in an email to other people so they > didn't know. We need to avoid this situation, so the bot has to stay green. > > anyway I'm working on fixing it now > > On Thu, Oct 22, 2015 at 9:28 AM Eugene Zelenko via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > >> Hi! >> >> I broke couple of times MSVC builds when committed minor code >> cleanups. Looks like problem occurred because of changed order of >> headers. >> >> I could not investigated problems myself, because I don't have access to >> MSVC. >> >> Changes in question: >> >> r250872: >> >> source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp >> source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h >> >> r250925: >> >> source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp >> source/Plugins/OperatingSystem/Go/OperatingSystemGo.h >> >> Eugene. >> ___ >> lldb-dev mailing list >> lldb-dev@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
Hi, Zachary! On Thu, Oct 22, 2015 at 9:34 AM, Zachary Turner wrote: > I'm taking a look now. In the future please revert changes that break a > buildbot. If you think you might know what the fix is (i.e. you guess that > including a header will fix it), then you can just check it in as a test. > If after a few attempts you still can't figure it out, then revert the > change out again. > > But definitely please don't leave broken changes in. For example, the MSVC > buildbot is now failing due to changes in LLVM. The reason is because it > broke last night due to some unrelated changes which weren't reverted, and > subsequent breaks did not result in an email to other people so they didn't > know. We need to avoid this situation, so the bot has to stay green. > > anyway I'm working on fixing it now At first I want to apologize for broken builds. In two previous cases were reason was more or less clear, I reverted my changes after receiving buildbot notifications. I was not sure about last case, since fix should be made in file which I didn't change in previous commit. Sure, I'll try to bu more careful in future. Eugene. ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
For the disassembler patch, the problem is you've defaulted the destructor but you've got a unique_ptr to a forward declared class. MSVC is actually correct in failing to compile this, and I'm not sure why other compilers are accepting it. My guess is something to do with the order of includes from the cpp file. To fix this you need to remove the `default` keyword from the destructor and provide an empty implementation of the destructor in DisassemblerLLVMC.cpp This problem occurs in other places too. In OperatingSystemGo.h the same problem exists with DynamicRegisterInfo. That's the one causing the operating system patch to fail. What compiler are you testing this with? This should fail under clang as well, I'm surprised it doesn't. Are you using GCC by chance? In any case, you'll need to fix all the occurrences of having a defaulted destructor in a class with a std::unique_ptr of an incomplete type. On Thu, Oct 22, 2015 at 10:48 AM Eugene Zelenko wrote: > On Thu, Oct 22, 2015 at 10:09 AM, Zachary Turner > wrote: > > It looks like you're generating these diffs from an SVN repo, so I'm > seeing > > lldb/trunk in the paths. Because of that, I can't apply it on my git > repo > > which doesn't have the "trunk" folder. Can you generate the diffs for > the 2 > > aforementioned CLs by cding into the trunk directory and generating the > diff > > there? > > See attached files. I did them in same way as diffs for Differential. > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
On Thu, Oct 22, 2015 at 11:15 AM, Zachary Turner wrote: > For the disassembler patch, the problem is you've defaulted the destructor > but you've got a unique_ptr to a forward declared class. MSVC is actually > correct in failing to compile this, and I'm not sure why other compilers are > accepting it. My guess is something to do with the order of includes from > the cpp file. > > To fix this you need to remove the `default` keyword from the destructor and > provide an empty implementation of the destructor in DisassemblerLLVMC.cpp > > This problem occurs in other places too. In OperatingSystemGo.h the same > problem exists with DynamicRegisterInfo. That's the one causing the > operating system patch to fail. > > What compiler are you testing this with? This should fail under clang as > well, I'm surprised it doesn't. Are you using GCC by chance? > > In any case, you'll need to fix all the occurrences of having a defaulted > destructor in a class with a std::unique_ptr of an incomplete type. I think including missing file will be better solution. I use trunk (~ week old) Clang. Eugene. ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Help with MSVC build issues
The classes appear to be all forward declared intentionally, so you'd need to include a lot of missing files. Just moving the destructor to cpp file should be pretty easy and still allow to keep the header footprint down, which speeds up build times. On Thu, Oct 22, 2015 at 11:22 AM Eugene Zelenko wrote: > On Thu, Oct 22, 2015 at 11:15 AM, Zachary Turner > wrote: > > For the disassembler patch, the problem is you've defaulted the > destructor > > but you've got a unique_ptr to a forward declared class. MSVC is > actually > > correct in failing to compile this, and I'm not sure why other compilers > are > > accepting it. My guess is something to do with the order of includes > from > > the cpp file. > > > > To fix this you need to remove the `default` keyword from the destructor > and > > provide an empty implementation of the destructor in > DisassemblerLLVMC.cpp > > > > This problem occurs in other places too. In OperatingSystemGo.h the same > > problem exists with DynamicRegisterInfo. That's the one causing the > > operating system patch to fail. > > > > What compiler are you testing this with? This should fail under clang as > > well, I'm surprised it doesn't. Are you using GCC by chance? > > > > In any case, you'll need to fix all the occurrences of having a defaulted > > destructor in a class with a std::unique_ptr of an incomplete type. > > I think including missing file will be better solution. > > I use trunk (~ week old) Clang. > > Eugene. > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
I'd be okay with that. The unittest2 stuff looks like it was a vestige of being incorporated before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should have a unitest included that is effectively what we use as unittest2. -Todd On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > I plan to put this in today. Greg, should I just go ahead and delete all > the unittest2 stuff entirely? TBH I'm all for anything that reduces the > complexity of the test suite. It's got a couple hundred options that > nobody uses, seems like we should start whittling away at stuff that > doesn't get any use. > > If you prefer I leave it in that's less work for me since I have that > patch ready to go, but TBH I'd rather remove it if that's ok. > > On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner wrote: > >> You can get pretty much the same effect though by just running dotest and >> passing it the folder that the .py file is in. Then it only runs tests in >> that folder. You can specify the filename too if you want to limit it to >> one name. Sure, it's a few keystrokes less to just type >> TestMultithreaded.py or something, but given the extra complexity and the >> fact that it's running a totally different codepath, I wonder if the >> maintenance burden is worth it (I'm guessing no, since apparently it >> doesn't work well enough right now for anyone to use it) >> >> On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: >> >>> I believe it would import lldb correctly. I don't tend to run the tests >>> individually, but if it did work, I would use it more. >>> >>> > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < >>> lldb-dev@lists.llvm.org> wrote: >>> > >>> > Todd, Greg, can you guys confirm this is true? The import lldb would >>> succeed if someone had their PYTHONPATH set up just right, but if really >>> none of us care about it, I'm with Tamas in that I'd rather remove it. >>> > >>> > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < >>> tbergham...@google.com> wrote: >>> > Hi Zach, >>> > >>> > I think nobody is using the "if __name__ == '__main__'" block as >>> executing a test file directly isn't working at the moment (the "import >>> lldb" command fails). If you plan to change all test file then I would >>> prefer to remove the reference to unittest2 from them for simplicity if >>> nobody have an objection against it. >>> > >>> > Tamas >>> > >>> > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < >>> lldb-dev@lists.llvm.org> wrote: >>> > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ >>> file CL is coming. >>> > >>> > IANAL, but I've been told by one that I need to move all third party >>> code used by LLDB to lldb/third_party. Currently there is only one thing >>> there: the Python `six` module used for creating code that is portable >>> across Python 2 and Python 3. >>> > >>> > The only other 2 instances that I'm aware of are pexpect and >>> unittest2, which are under lldb/test. I've got some patches locally which >>> move pexpect and unittest2 to lldb/third_party. I'll hold off on checking >>> them in for a bit to give people a chance to see this message first, >>> because otherwise you might be surprised when you see a CL with 400 files >>> being checked in. >>> > >>> > Nobody will have to do anything after this CL goes in, and everything >>> should continue to work exactly as it currently does. >>> > >>> > The main reason for the churn is that pretty much every single test in >>> LLDB does something like this: >>> > >>> > import unittest2 >>> > >>> > ... >>> > >>> > if __name__ == '__main__': >>> > import atexit >>> > lldb.SBDebugger.Initialize() >>> > atexit.register(lambda: lldb.SBDebugger.Terminate()) >>> > unittest2.main() >>> > >>> > This worked when unittest2 was a subfolder of test, but not when it's >>> somewhere else. Since LLDB's python code is not organized into a standard >>> python package and we treat the scripts like dotest etc as standalone >>> scripts, the way I've made this work is by introducing a module called >>> lldb_shared under test which, when you import it, fixes up sys.path to >>> correctly add all the right locations under lldb/third_party. >>> > >>> > So, every single test now needs a line at the top to import >>> lldb_shared. >>> > >>> > TBH I don't even know if we need this unittest2 stuff anymore (does >>> anyone even use it?) but even if the answer is no, then that still means >>> changing every file to delete the import statement and the if __name__ == >>> '__main__': block. >>> > >>> > If there are no major concerns I plan to check this in by the end of >>> the day, or tomorrow. >>> > ___ >>> > lldb-dev mailing list >>> > lldb-dev@lists.llvm.org >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >>> > ___ >>> > lldb-dev mailing list >>> > lldb-dev
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
We could also then remove unittest2 from inclusion in the lldb repo. On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala wrote: > I'd be okay with that. > > The unittest2 stuff looks like it was a vestige of being incorporated > before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should > have a unitest included that is effectively what we use as unittest2. > > -Todd > > On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > >> I plan to put this in today. Greg, should I just go ahead and delete all >> the unittest2 stuff entirely? TBH I'm all for anything that reduces the >> complexity of the test suite. It's got a couple hundred options that >> nobody uses, seems like we should start whittling away at stuff that >> doesn't get any use. >> >> If you prefer I leave it in that's less work for me since I have that >> patch ready to go, but TBH I'd rather remove it if that's ok. >> >> On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner >> wrote: >> >>> You can get pretty much the same effect though by just running dotest >>> and passing it the folder that the .py file is in. Then it only runs >>> tests in that folder. You can specify the filename too if you want to >>> limit it to one name. Sure, it's a few keystrokes less to just type >>> TestMultithreaded.py or something, but given the extra complexity and the >>> fact that it's running a totally different codepath, I wonder if the >>> maintenance burden is worth it (I'm guessing no, since apparently it >>> doesn't work well enough right now for anyone to use it) >>> >>> On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: >>> I believe it would import lldb correctly. I don't tend to run the tests individually, but if it did work, I would use it more. > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > > Todd, Greg, can you guys confirm this is true? The import lldb would succeed if someone had their PYTHONPATH set up just right, but if really none of us care about it, I'm with Tamas in that I'd rather remove it. > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < tbergham...@google.com> wrote: > Hi Zach, > > I think nobody is using the "if __name__ == '__main__'" block as executing a test file directly isn't working at the moment (the "import lldb" command fails). If you plan to change all test file then I would prefer to remove the reference to unittest2 from them for simplicity if nobody have an objection against it. > > Tamas > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ file CL is coming. > > IANAL, but I've been told by one that I need to move all third party code used by LLDB to lldb/third_party. Currently there is only one thing there: the Python `six` module used for creating code that is portable across Python 2 and Python 3. > > The only other 2 instances that I'm aware of are pexpect and unittest2, which are under lldb/test. I've got some patches locally which move pexpect and unittest2 to lldb/third_party. I'll hold off on checking them in for a bit to give people a chance to see this message first, because otherwise you might be surprised when you see a CL with 400 files being checked in. > > Nobody will have to do anything after this CL goes in, and everything should continue to work exactly as it currently does. > > The main reason for the churn is that pretty much every single test in LLDB does something like this: > > import unittest2 > > ... > > if __name__ == '__main__': > import atexit > lldb.SBDebugger.Initialize() > atexit.register(lambda: lldb.SBDebugger.Terminate()) > unittest2.main() > > This worked when unittest2 was a subfolder of test, but not when it's somewhere else. Since LLDB's python code is not organized into a standard python package and we treat the scripts like dotest etc as standalone scripts, the way I've made this work is by introducing a module called lldb_shared under test which, when you import it, fixes up sys.path to correctly add all the right locations under lldb/third_party. > > So, every single test now needs a line at the top to import lldb_shared. > > TBH I don't even know if we need this unittest2 stuff anymore (does anyone even use it?) but even if the answer is no, then that still means changing every file to delete the import statement and the if __name__ == '__main__': block. > > If there are no major concerns I plan to check this in by the end of the day, or tomorrow. > __
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
(I was eventually going to do this at some point after I verified it was indeed true). It should just be called unittest in a stock distribution. On Thu, Oct 22, 2015 at 11:29 AM, Todd Fiala wrote: > We could also then remove unittest2 from inclusion in the lldb repo. > > > On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala wrote: > >> I'd be okay with that. >> >> The unittest2 stuff looks like it was a vestige of being incorporated >> before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should >> have a unitest included that is effectively what we use as unittest2. >> >> -Todd >> >> On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> >>> I plan to put this in today. Greg, should I just go ahead and delete >>> all the unittest2 stuff entirely? TBH I'm all for anything that reduces >>> the complexity of the test suite. It's got a couple hundred options that >>> nobody uses, seems like we should start whittling away at stuff that >>> doesn't get any use. >>> >>> If you prefer I leave it in that's less work for me since I have that >>> patch ready to go, but TBH I'd rather remove it if that's ok. >>> >>> On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner >>> wrote: >>> You can get pretty much the same effect though by just running dotest and passing it the folder that the .py file is in. Then it only runs tests in that folder. You can specify the filename too if you want to limit it to one name. Sure, it's a few keystrokes less to just type TestMultithreaded.py or something, but given the extra complexity and the fact that it's running a totally different codepath, I wonder if the maintenance burden is worth it (I'm guessing no, since apparently it doesn't work well enough right now for anyone to use it) On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: > I believe it would import lldb correctly. I don't tend to run the > tests individually, but if it did work, I would use it more. > > > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > > > Todd, Greg, can you guys confirm this is true? The import lldb > would succeed if someone had their PYTHONPATH set up just right, but if > really none of us care about it, I'm with Tamas in that I'd rather remove > it. > > > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < > tbergham...@google.com> wrote: > > Hi Zach, > > > > I think nobody is using the "if __name__ == '__main__'" block as > executing a test file directly isn't working at the moment (the "import > lldb" command fails). If you plan to change all test file then I would > prefer to remove the reference to unittest2 from them for simplicity if > nobody have an objection against it. > > > > Tamas > > > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > TL;DR - Nobody has to do anything, this is just a heads up that a > 400+ file CL is coming. > > > > IANAL, but I've been told by one that I need to move all third party > code used by LLDB to lldb/third_party. Currently there is only one thing > there: the Python `six` module used for creating code that is portable > across Python 2 and Python 3. > > > > The only other 2 instances that I'm aware of are pexpect and > unittest2, which are under lldb/test. I've got some patches locally which > move pexpect and unittest2 to lldb/third_party. I'll hold off on checking > them in for a bit to give people a chance to see this message first, > because otherwise you might be surprised when you see a CL with 400 files > being checked in. > > > > Nobody will have to do anything after this CL goes in, and > everything should continue to work exactly as it currently does. > > > > The main reason for the churn is that pretty much every single test > in LLDB does something like this: > > > > import unittest2 > > > > ... > > > > if __name__ == '__main__': > > import atexit > > lldb.SBDebugger.Initialize() > > atexit.register(lambda: lldb.SBDebugger.Terminate()) > > unittest2.main() > > > > This worked when unittest2 was a subfolder of test, but not when > it's somewhere else. Since LLDB's python code is not organized into a > standard python package and we treat the scripts like dotest etc as > standalone scripts, the way I've made this work is by introducing a module > called lldb_shared under test which, when you import it, fixes up sys.path > to correctly add all the right locations under lldb/third_party. > > > > So, every single test now needs a line at the top to import > lldb_shared. > > > > TBH I don't even know if we need this unittest2 stuff anymore (does >>
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
Cool! I probably won't delete it from the repo entirely, because that entails mucking with the command line options of dotest to remove any related options, and any initialization code in dotest.py or TestBase subclasses related to unittest2. For now I'll just delete the imports from each individual test and remove the if __name__ == "main" blocks. After that though it should be a fairly straightforward follow-up to remove it entirely if anyone wants to. On Thu, Oct 22, 2015 at 11:30 AM Todd Fiala wrote: > (I was eventually going to do this at some point after I verified it was > indeed true). It should just be called unittest in a stock distribution. > > On Thu, Oct 22, 2015 at 11:29 AM, Todd Fiala wrote: > >> We could also then remove unittest2 from inclusion in the lldb repo. >> >> >> On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala >> wrote: >> >>> I'd be okay with that. >>> >>> The unittest2 stuff looks like it was a vestige of being incorporated >>> before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should >>> have a unitest included that is effectively what we use as unittest2. >>> >>> -Todd >>> >>> On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < >>> lldb-dev@lists.llvm.org> wrote: >>> I plan to put this in today. Greg, should I just go ahead and delete all the unittest2 stuff entirely? TBH I'm all for anything that reduces the complexity of the test suite. It's got a couple hundred options that nobody uses, seems like we should start whittling away at stuff that doesn't get any use. If you prefer I leave it in that's less work for me since I have that patch ready to go, but TBH I'd rather remove it if that's ok. On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner wrote: > You can get pretty much the same effect though by just running dotest > and passing it the folder that the .py file is in. Then it only runs > tests in that folder. You can specify the filename too if you want to > limit it to one name. Sure, it's a few keystrokes less to just type > TestMultithreaded.py or something, but given the extra complexity and the > fact that it's running a totally different codepath, I wonder if the > maintenance burden is worth it (I'm guessing no, since apparently it > doesn't work well enough right now for anyone to use it) > > On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton > wrote: > >> I believe it would import lldb correctly. I don't tend to run the >> tests individually, but if it did work, I would use it more. >> >> > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > >> > Todd, Greg, can you guys confirm this is true? The import lldb >> would succeed if someone had their PYTHONPATH set up just right, but if >> really none of us care about it, I'm with Tamas in that I'd rather remove >> it. >> > >> > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < >> tbergham...@google.com> wrote: >> > Hi Zach, >> > >> > I think nobody is using the "if __name__ == '__main__'" block as >> executing a test file directly isn't working at the moment (the "import >> lldb" command fails). If you plan to change all test file then I would >> prefer to remove the reference to unittest2 from them for simplicity if >> nobody have an objection against it. >> > >> > Tamas >> > >> > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > TL;DR - Nobody has to do anything, this is just a heads up that a >> 400+ file CL is coming. >> > >> > IANAL, but I've been told by one that I need to move all third >> party code used by LLDB to lldb/third_party. Currently there is only one >> thing there: the Python `six` module used for creating code that is >> portable across Python 2 and Python 3. >> > >> > The only other 2 instances that I'm aware of are pexpect and >> unittest2, which are under lldb/test. I've got some patches locally >> which >> move pexpect and unittest2 to lldb/third_party. I'll hold off on >> checking >> them in for a bit to give people a chance to see this message first, >> because otherwise you might be surprised when you see a CL with 400 files >> being checked in. >> > >> > Nobody will have to do anything after this CL goes in, and >> everything should continue to work exactly as it currently does. >> > >> > The main reason for the churn is that pretty much every single test >> in LLDB does something like this: >> > >> > import unittest2 >> > >> > ... >> > >> > if __name__ == '__main__': >> > import atexit >> > lldb.SBDebugger.Initialize() >> > atexit.register(lambda: lldb.SBDebugger.Terminate()) >> > unittest2.main() >> >
Re: [lldb-dev] lldb tests and tear down hooks
(And side note: if you're pushing a "lambda: self.foo()" with no arguments, the lambda is unneeded and you can just push "self.foo" --- that cleanup hook pushed on most tests at the end of the file is a perfect example of an unneeded level of lambda indirection). On Wed, Oct 21, 2015 at 12:04 PM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > Well you can see them getting added via self.addTearDownHook(), so that > means they're called through an instance. Specifically, it happens in > Base.tearDown(self), so it's basically identical (in concept) to if the > relevant handlers were called in the implementation of MyTest.tearDown(), > but different in order. > > I agree that it's useful in principle to be able to do what you suggest in > your example, but there's just no way to guarantee the right ordering if > you let the base class run the handlers. If there actually *were* a > tearDown() function in your example, to be correct it would need to look > like this: > > class MyTest(TestBase): > def tearDown(self): > # run the teardown hooks > # Do the inverse of setUp() > super.tearDown() > > def test_1(self): > self.addTearDownHook(lambda: self.foo()) > raise ValueError > def test_2(self): > self.addTearDownHook(lambda: self.bar()) > raise ValueError > > One possible solution is to shift burden of maintaining the hooks list to > the individual test case. E.g. > > class MyTest(TestBase): > self.hooks = [] > def tearDown(self): > self.runTearDownHooks(self.hooks) # This could be implemented in > TestBase, since now we can call it with our list at the right time. > # Do the inverse of setUp() > super.tearDown() > > def test_1(self): > self.hooks.append(lambda: self.foo()) > raise ValueError > def test_2(self): > self.hooks.append(lambda: self.bar()) > raise ValueError > > Almost no extra code to write, and should be bulletproof. > > On Wed, Oct 21, 2015 at 11:41 AM Greg Clayton wrote: > >> I think it was mostly done to provide an exception safe way to cleanup >> stuff without having to override TestBase.tearDown(). I am guessing this >> cleanup happens on TestCase.tearDown() and not after the current test case >> right? >> >> I could see it being used to cleanup after a single test case in case you >> have: >> >> class MyTest(TestBase): >> def test_1(self): >> self.addTearDownHook(lambda: self.foo()) >> raise ValueError >> def test_2(self): >> self.addTearDownHook(lambda: self.bar()) >> raise ValueError >> >> >> Are these tearDowns happening per test function, or during class >> setup/teardown? >> >> > On Oct 21, 2015, at 9:33 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > >> > Yea, that's what I think too. I think this mechanism was probably >> invented to just to save some code and promote reusability, but in practice >> leads to these kinds of problems. >> > >> > On Wed, Oct 21, 2015 at 2:07 AM Pavel Labath wrote: >> > I think we can remove these, provided there is a way to mimic the >> > functionality they are used for now, which I think shouldn't be hard. >> > Anything which was set up in the setUp() method should be undone in >> > tearDown(). Anything which was set up in the test method, can be >> > undone using a try-finally block. Is there a use case not covered by >> > this? >> > >> > pl >> > >> > On 21 October 2015 at 04:47, Zachary Turner via lldb-dev >> > wrote: >> > > There's a subtle bug that is pervasive throughout the test suite. >> Consider >> > > the following seemingly innocent test class. >> > > >> > > class MyTest(TestBase); >> > > def setUp(): >> > > TestBase.setUp()#1 >> > > >> > > # Do some stuff #2 >> > > self.addTearDownHook(lambda: self.foo()) #3 >> > > >> > > def test_interesting_stuff(): >> > > pass >> > > >> > > Here's the problem. As a general principle, cleanup needs to happen >> in >> > > reverse order from initialization. That's why, if we had a tearDown() >> > > method, it would probably look something like this: >> > > >> > > def tearDown(): >> > > # Clean up some stuff #2 >> > > >> > > TestBase.tearDown()#1 >> > > >> > > This follows the pattern in other languages like C++, for example, >> where >> > > construction goes from base -> derived, but destruction goes from >> derived -> >> > > base. >> > > >> > > But if you add these tear down hooks into the mix, it violates that. >> tear >> > > down hooks get invoked as part of TestBase.tearDown(), so in the above >> > > example the initialization order is 1 -> 2 -> 3 but the teardown >> order is 2 >> > > -> 1 -> 3 (or 2 -> 3 -> 1, or none of the above depending on where >> inside >> > > of TestBase.tearDown() hook the hooks get invoked). >> > > >> > > To make matters worse, tear down hooks can be added from arbitrary >>
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
Yeah I think the biggest thing I wanted to check there was that there wasn't any unittest2 behavior present in that cut of unittest2 that didn't make it into the revamped version brought into the python distributions when they upgraded unittest. Then it's just a big rename exercise on replacing unittest2 with unittest (again, after making sure that my expectations here are correct on this being valid). On Thu, Oct 22, 2015 at 11:33 AM, Zachary Turner wrote: > Cool! I probably won't delete it from the repo entirely, because that > entails mucking with the command line options of dotest to remove any > related options, and any initialization code in dotest.py or TestBase > subclasses related to unittest2. For now I'll just delete the imports from > each individual test and remove the if __name__ == "main" blocks. After > that though it should be a fairly straightforward follow-up to remove it > entirely if anyone wants to. > > On Thu, Oct 22, 2015 at 11:30 AM Todd Fiala wrote: > >> (I was eventually going to do this at some point after I verified it was >> indeed true). It should just be called unittest in a stock distribution. >> >> On Thu, Oct 22, 2015 at 11:29 AM, Todd Fiala >> wrote: >> >>> We could also then remove unittest2 from inclusion in the lldb repo. >>> >>> >>> On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala >>> wrote: >>> I'd be okay with that. The unittest2 stuff looks like it was a vestige of being incorporated before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should have a unitest included that is effectively what we use as unittest2. -Todd On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > I plan to put this in today. Greg, should I just go ahead and delete > all the unittest2 stuff entirely? TBH I'm all for anything that reduces > the complexity of the test suite. It's got a couple hundred options that > nobody uses, seems like we should start whittling away at stuff that > doesn't get any use. > > If you prefer I leave it in that's less work for me since I have that > patch ready to go, but TBH I'd rather remove it if that's ok. > > On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner > wrote: > >> You can get pretty much the same effect though by just running dotest >> and passing it the folder that the .py file is in. Then it only runs >> tests in that folder. You can specify the filename too if you want to >> limit it to one name. Sure, it's a few keystrokes less to just type >> TestMultithreaded.py or something, but given the extra complexity and the >> fact that it's running a totally different codepath, I wonder if the >> maintenance burden is worth it (I'm guessing no, since apparently it >> doesn't work well enough right now for anyone to use it) >> >> On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton >> wrote: >> >>> I believe it would import lldb correctly. I don't tend to run the >>> tests individually, but if it did work, I would use it more. >>> >>> > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < >>> lldb-dev@lists.llvm.org> wrote: >>> > >>> > Todd, Greg, can you guys confirm this is true? The import lldb >>> would succeed if someone had their PYTHONPATH set up just right, but if >>> really none of us care about it, I'm with Tamas in that I'd rather >>> remove >>> it. >>> > >>> > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < >>> tbergham...@google.com> wrote: >>> > Hi Zach, >>> > >>> > I think nobody is using the "if __name__ == '__main__'" block as >>> executing a test file directly isn't working at the moment (the "import >>> lldb" command fails). If you plan to change all test file then I would >>> prefer to remove the reference to unittest2 from them for simplicity if >>> nobody have an objection against it. >>> > >>> > Tamas >>> > >>> > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < >>> lldb-dev@lists.llvm.org> wrote: >>> > TL;DR - Nobody has to do anything, this is just a heads up that a >>> 400+ file CL is coming. >>> > >>> > IANAL, but I've been told by one that I need to move all third >>> party code used by LLDB to lldb/third_party. Currently there is only >>> one >>> thing there: the Python `six` module used for creating code that is >>> portable across Python 2 and Python 3. >>> > >>> > The only other 2 instances that I'm aware of are pexpect and >>> unittest2, which are under lldb/test. I've got some patches locally >>> which >>> move pexpect and unittest2 to lldb/third_party. I'll hold off on >>> checking >>> them in for a bit to give people a chance to see this message first, >>> because otherwise you might be surprised when you see a CL
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
This is going in right now. As it is a fairly large change, it wouldn't surprise me if someone encounters an issue. I tested this everywhere I can and it seems fine, so please let me know if anyone encounters anything. On Thu, Oct 22, 2015 at 11:39 AM Todd Fiala wrote: > Yeah I think the biggest thing I wanted to check there was that there > wasn't any unittest2 behavior present in that cut of unittest2 that didn't > make it into the revamped version brought into the python distributions > when they upgraded unittest. Then it's just a big rename exercise on > replacing unittest2 with unittest (again, after making sure that my > expectations here are correct on this being valid). > > On Thu, Oct 22, 2015 at 11:33 AM, Zachary Turner > wrote: > >> Cool! I probably won't delete it from the repo entirely, because that >> entails mucking with the command line options of dotest to remove any >> related options, and any initialization code in dotest.py or TestBase >> subclasses related to unittest2. For now I'll just delete the imports from >> each individual test and remove the if __name__ == "main" blocks. After >> that though it should be a fairly straightforward follow-up to remove it >> entirely if anyone wants to. >> >> On Thu, Oct 22, 2015 at 11:30 AM Todd Fiala wrote: >> >>> (I was eventually going to do this at some point after I verified it was >>> indeed true). It should just be called unittest in a stock distribution. >>> >>> On Thu, Oct 22, 2015 at 11:29 AM, Todd Fiala >>> wrote: >>> We could also then remove unittest2 from inclusion in the lldb repo. On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala wrote: > I'd be okay with that. > > The unittest2 stuff looks like it was a vestige of being incorporated > before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should > have a unitest included that is effectively what we use as unittest2. > > -Todd > > On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > >> I plan to put this in today. Greg, should I just go ahead and delete >> all the unittest2 stuff entirely? TBH I'm all for anything that reduces >> the complexity of the test suite. It's got a couple hundred options that >> nobody uses, seems like we should start whittling away at stuff that >> doesn't get any use. >> >> If you prefer I leave it in that's less work for me since I have that >> patch ready to go, but TBH I'd rather remove it if that's ok. >> >> On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner >> wrote: >> >>> You can get pretty much the same effect though by just running >>> dotest and passing it the folder that the .py file is in. Then it only >>> runs tests in that folder. You can specify the filename too if you >>> want to >>> limit it to one name. Sure, it's a few keystrokes less to just type >>> TestMultithreaded.py or something, but given the extra complexity and >>> the >>> fact that it's running a totally different codepath, I wonder if the >>> maintenance burden is worth it (I'm guessing no, since apparently it >>> doesn't work well enough right now for anyone to use it) >>> >>> On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton >>> wrote: >>> I believe it would import lldb correctly. I don't tend to run the tests individually, but if it did work, I would use it more. > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > > Todd, Greg, can you guys confirm this is true? The import lldb would succeed if someone had their PYTHONPATH set up just right, but if really none of us care about it, I'm with Tamas in that I'd rather remove it. > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < tbergham...@google.com> wrote: > Hi Zach, > > I think nobody is using the "if __name__ == '__main__'" block as executing a test file directly isn't working at the moment (the "import lldb" command fails). If you plan to change all test file then I would prefer to remove the reference to unittest2 from them for simplicity if nobody have an objection against it. > > Tamas > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > TL;DR - Nobody has to do anything, this is just a heads up that a 400+ file CL is coming. > > IANAL, but I've been told by one that I need to move all third party code used by LLDB to lldb/third_party. Currently there is only one thing there: the Python `six` module used for creating code that is portable across Python 2 and Python 3. >>>
Re: [lldb-dev] Moving pexpect and unittest2 to lldb/third_party
Okay, will do. On Thu, Oct 22, 2015 at 12:56 PM, Zachary Turner wrote: > This is going in right now. As it is a fairly large change, it wouldn't > surprise me if someone encounters an issue. I tested this everywhere I can > and it seems fine, so please let me know if anyone encounters anything. > > On Thu, Oct 22, 2015 at 11:39 AM Todd Fiala wrote: > >> Yeah I think the biggest thing I wanted to check there was that there >> wasn't any unittest2 behavior present in that cut of unittest2 that didn't >> make it into the revamped version brought into the python distributions >> when they upgraded unittest. Then it's just a big rename exercise on >> replacing unittest2 with unittest (again, after making sure that my >> expectations here are correct on this being valid). >> >> On Thu, Oct 22, 2015 at 11:33 AM, Zachary Turner >> wrote: >> >>> Cool! I probably won't delete it from the repo entirely, because that >>> entails mucking with the command line options of dotest to remove any >>> related options, and any initialization code in dotest.py or TestBase >>> subclasses related to unittest2. For now I'll just delete the imports from >>> each individual test and remove the if __name__ == "main" blocks. After >>> that though it should be a fairly straightforward follow-up to remove it >>> entirely if anyone wants to. >>> >>> On Thu, Oct 22, 2015 at 11:30 AM Todd Fiala >>> wrote: >>> (I was eventually going to do this at some point after I verified it was indeed true). It should just be called unittest in a stock distribution. On Thu, Oct 22, 2015 at 11:29 AM, Todd Fiala wrote: > We could also then remove unittest2 from inclusion in the lldb repo. > > > On Thu, Oct 22, 2015 at 11:28 AM, Todd Fiala > wrote: > >> I'd be okay with that. >> >> The unittest2 stuff looks like it was a vestige of being incorporated >> before unittest2 was stock (unitest) on Python 2.[6,7]?. Everyone should >> have a unitest included that is effectively what we use as unittest2. >> >> -Todd >> >> On Thu, Oct 22, 2015 at 10:05 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> >>> I plan to put this in today. Greg, should I just go ahead and >>> delete all the unittest2 stuff entirely? TBH I'm all for anything that >>> reduces the complexity of the test suite. It's got a couple hundred >>> options that nobody uses, seems like we should start whittling away at >>> stuff that doesn't get any use. >>> >>> If you prefer I leave it in that's less work for me since I have >>> that patch ready to go, but TBH I'd rather remove it if that's ok. >>> >>> On Thu, Oct 22, 2015 at 9:50 AM Zachary Turner >>> wrote: >>> You can get pretty much the same effect though by just running dotest and passing it the folder that the .py file is in. Then it only runs tests in that folder. You can specify the filename too if you want to limit it to one name. Sure, it's a few keystrokes less to just type TestMultithreaded.py or something, but given the extra complexity and the fact that it's running a totally different codepath, I wonder if the maintenance burden is worth it (I'm guessing no, since apparently it doesn't work well enough right now for anyone to use it) On Thu, Oct 22, 2015 at 9:47 AM Greg Clayton wrote: > I believe it would import lldb correctly. I don't tend to run the > tests individually, but if it did work, I would use it more. > > > On Oct 22, 2015, at 9:26 AM, Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > > > Todd, Greg, can you guys confirm this is true? The import lldb > would succeed if someone had their PYTHONPATH set up just right, but > if > really none of us care about it, I'm with Tamas in that I'd rather > remove > it. > > > > On Thu, Oct 22, 2015 at 2:55 AM Tamas Berghammer < > tbergham...@google.com> wrote: > > Hi Zach, > > > > I think nobody is using the "if __name__ == '__main__'" block as > executing a test file directly isn't working at the moment (the > "import > lldb" command fails). If you plan to change all test file then I would > prefer to remove the reference to unittest2 from them for simplicity > if > nobody have an objection against it. > > > > Tamas > > > > On Wed, Oct 21, 2015 at 8:57 PM Zachary Turner via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > TL;DR - Nobody has to do anything, this is just a heads up that > a 400+ file CL is coming. > > > > IANAL, but I've been told by one that I need to move
