[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