[PATCH] D48309: [sanitizer] Unify and generalize Apple platforms in CMake and lit test configs

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added a comment.

For the whole "ios/iossim" naming and `ios_or_iossim`: Let's not try to fix that in this patch. But in general, I don't think this is too terrible. The rule simply is: All embedded platforms claim they are also "ios" because they are derivatives of iOS. Maybe this could be greatly improved with just a bit more documentation? I like the fact that you can disable a test for all embedded platforms with just "DISABLE: ios" and you don't have to list all of them.



================
Comment at: test/lit.common.cfg:130
+
+  prepare_output_json = subprocess.check_output([prepare_script, config.apple_platform, config.clang]).strip().split("\n")[-1]
+  prepare_output = json.loads(prepare_output_json)
----------------
delcypher wrote:
> Should we have more error checking here?
`check_output` will abort if the exit code isn' 0.


================
Comment at: test/lit.common.cfg:132
+  prepare_output = json.loads(prepare_output_json)
+  config.environment.update(prepare_output["env"])
 elif config.android:
----------------
delcypher wrote:
> We should probably check that the JSON data is what we expect before trying to use it.
> 
> ```
> assert 'env' in prepare_output
> assert isinstance(prepare_output['env'], dict)
> ```
> 
> Or something similar.
Hm, this seems very non-Python-y. Just using `prepare_output["env"]` implicitly checks for these things as well.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D48309