[PATCH] D68863: [LNT] Python 3 support: don't assume order of cmake args

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: tests/runtest/test_suite-cache.shtest:18
+# RUN: FileCheck  --check-prefix CHECK-CACHE1 < %t.cmake-cache.log %s
+# CHECK-CACHE1: Execute: {{.*}}cmake {{(.+ )?}}-DCMAKE_CXX_COMPILER:FILEPATH={{.*}}/FakeCompilers/clang++-r154331
+# RUN: FileCheck  --check-prefix CHECK-CACHE2 < %t.cmake-cache.log %s
----------------
rogfer01 wrote:
> thopre wrote:
> > rogfer01 wrote:
> > > My reading from this
> > > 
> > > ```lang=python
> > > for key in ['CMAKE_C_COMPILER:FILEPATH',                          
> > >             'CMAKE_CXX_COMPILER:FILEPATH']:                       
> > >     value = defs.pop(key, None)                                   
> > >     if value is not None:                                         
> > >         early_defs[key] = value                                   
> > >                                                                   
> > > cmake_cmd = ([cmake_cmd] +                                        
> > >              ['-D%s=%s' % (k, v) for k, v in early_defs.items()] +
> > >              cmake_flags + [self._test_suite_dir()] +             
> > >              ['-D%s=%s' % (k, v) for k, v in defs.items()])       
> > > ```
> > > 
> > > is that we may still want to check that the compilers and the cache appear in the specific order (right after the `cmake`). 
> > > 
> > > The other ones, whose order is unpredictable, (`-DFOO=BAR`, `-DBAR=BAZ`) we do want to check them individually,
> > > 
> > > Does this make sense?
> > It does, good catch! How about now?
> Would it be possible to ensure the `-C flag` does appear right after the compilers?
> 
> It occurs to me that either we make Python 2 deterministic here using an `OrderedDict` for `early_defs` (and then we can write a simples check: first C, then CXX, then the cache file) or we write a check like the one below (untested!) where we allow any order and then the cache file
> 
> ```
> # We allow compilers in either order for Python 2 compat
> # CHECK-CACHE1: Execute: {{.*}}cmake {{(-DCMAKE_CXX_COMPILER:FILEPATH=.*/FakeCompilers/clang\+\+-r154331 -DCMAKE_C_COMPILER:FILEPATH=.*/FakeCompilers/clang-r154331 )|(-DCMAKE_C_COMPILER:FILEPATH=.*/FakeCompilers/clang-r154331 -DCMAKE_CXX_COMPILER:FILEPATH=.*/FakeCompilers/clang\+\+-r154331 )}} -C {{.*}}/Release.cmake
> ```
> 
> (cmake documentation does not seem clear to me regarding the positional behaviour of `-C` and `-D` but I imagine `early_defs` exists for a reason)
Oh I see, early_defs, then cmake_flags (which includes -C option), then test_suite_dir then macro definitions.


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

https://reviews.llvm.org/D68863