yinghuitan added inline comments.

================
Comment at: 
lldb/test/Shell/SymbolFile/OnDemand/shared-lib-globals-hydration.test:4
+
+# REQUIRES: system-linux
+
----------------
labath wrote:
> yinghuitan wrote:
> > labath wrote:
> > > Is this here because there is no portable way to create shared libraries 
> > > in shell tests? Would it be possible to run this test on other systems as 
> > > well if this were written as an API test (where we have a shared lib 
> > > portability layer)?
> > Yeah, that's one reason. Another reason is the name of the shared library 
> > (used for checking global variables/debug info) varies across platform. For 
> > example, it can be libfoo.dylib (Mac) vs libfoo.so (linux) vs *foo*.dll 
> > (windows). I find it cumbersome to support multiple platforms. 
> I'm not saying you have to go out of your way to support other platforms and 
> their idiosyncrasies, but the differences in library names hardly seem like 
> an insurmountable obstacle. We already have abstractions in the API tests 
> (`self.platformContext.{shlib_prefix,shlib_extension}`) which exist precisely 
> for this reason. There's no function to automatically construct the library 
> name (you have to do the concatenation yourself), but I certainly wouldn't be 
> opposed to adding one.
> 
> If it helps, think of it as courtesy towards other developers who may need to 
> debug this feature when they break these tests -- that's much easier to 
> achieve if you can reproduce the failure locally vs. just getting a failure 
> email from a random bot (after you've already submitted your patch).
> self.platformContext.{shlib_prefix,shlib_extension}

Ah, did not know their existence, thanks! Will add API tests for them. Do you 
prefer to keep these linux-only shell tests or completely deprecate them?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to