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

Roger Ferrer Ibanez via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 00:07:24 PST 2019


rogfer01 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
----------------
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)


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

https://reviews.llvm.org/D68863





More information about the llvm-commits mailing list