Michael137 added a comment. In D134344#3806509 <https://reviews.llvm.org/D134344#3806509>, @aprantl wrote:
> In D134344#3805953 <https://reviews.llvm.org/D134344#3805953>, @Michael137 > wrote: > >>> teach the debug info replication to ignore tests with the gmodules category >>> (just like it does for @no_debug_info_test_case tests). This step wouldn't >>> be necessary if we made debug info replication opt-in instead of opt-out, >>> as discussed on one of the previous patches (@JDevlieghere might remember >>> which one it was) >> >> That's an interesting idea. @JDevlieghere @aprantl How much appetite is >> there for changing the replication to be opt-in (that would require an audit >> of each API test right?). Otherwise, an alternative that comes to mind >> without hard-coding a `category == gmodules` into the replication logic >> would be to make `debug_info_categories` a `dictionary<category: string, >> replicable: bool>` and keep `gmodules` in there. Then we wouldn't need to >> make changes to `getBuildCommand` either. > > That's such a big change (we also need to make the change in all downstream > branches like swift-lldb) that I probably wouldn't want to roll it into this > patch series right now, but I'm open to having a separate discussion about > it. But I'm also missing the context as to why this would be desirable, so if > there's a good reason, let me know! Just to clarify, any solution will have to support the following points (which Pavel mentioned in his suggestion): (1) Make sure we *don't* run a test with all debug-info variants if we only want `gmodules` (2) Be able to explicitly compile with `gmodules` support when we add the `gmodules` category to a test Currently the options are: (A) Currently the test-replication works by checking all the categories on a test-case and if it finds no debug_info categories it will replicate the test once for each variant. If we made `gmodules` not a debug_info category this means tagging a test with `add_test_categories(["gmodules"])` would not work as expected. Pavel suggests making it opt-in, in which case this loop would never do the unexpected thing. We would still have to solve (2), but we would get (1) as a consequence of the opt-in (Correct me if I misunderstood @labath). (B) Keep the `gmodules` category in the debug_info categories but add an indicator (e.g., by making the `debug_info_categories` a dictionary) that will skip replication if set. That would solve (1) and (2) would work as it does today without changes. (C) The approach in this patch. Remove the `gmodules` category entirely and add a decorator that is responsible for skipping the test and adding a debug_info category to the decorated test-case (to prevent replication from kicking in). Unfortunately this also requires the user to add `MAKE_GMODULES`/`MAKE_DSYM` to the test's Makefile; that's how this patch addresses the issue of (2), i.e., it bypasses it entirely Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134344/new/ https://reviews.llvm.org/D134344 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
