mattd added a comment.

In D74917#1885621 <https://reviews.llvm.org/D74917#1885621>, @JDevlieghere 
wrote:

> In D74917#1885615 <https://reviews.llvm.org/D74917#1885615>, @mattd wrote:
>
> > In D74917#1885590 <https://reviews.llvm.org/D74917#1885590>, @JDevlieghere 
> > wrote:
> >
> > > It should never be possible for `LLDB_ENABLE_PYTHON` to be true but 
> > > `SWIG_FOUND` to be false. The modules `FindPythonInterpAndLibs.cmake` and 
> > > `FindLuaAndSwig.cmake` should fail early when SWIG isn't found.
> >
> >
> > Thanks for taking a look.  Would you be okay with making the status 
> > `message()` in `FindPythonInterpAndLibs.cmake` and `FindLuaAndSwig.cmake` 
> > fatal, that way the dependency error is clearer?
>
>
> That would defeat the purpose of auto-detecting these dependencies. Please 
> take a look at D71306 <https://reviews.llvm.org/D71306> for all the details. 
> The summary is that all optional dependencies default to `Auto`: where we 
> enable them if we can find them. You can override this behavior by passing 
> `LLDB_ENABLE_PYTHON=ON` to CMake, in which case not finding Python (or SWIG) 
> will be a fatal error.


Thanks for the clarification there.

> I agree that it can be confusing to figure out that Python got disabled 
> because SWIG wasn't found. Currently we call `find_package` with `QUIET` from 
> `FindPythonInterpAndLibs.cmake` and `FindLuaAndSwig.cmake`. I think we should 
> remove that so that the user gets more information in CMake's configuration 
> output. Would that address your concerns?

Ah, yes, dropping the `QUIET` produces meaningful insight as to what might be 
missing. I think others, who might not have all the dependencies, might find 
this level of verbosity clearer, so that they can more quickly understand what 
is missing in their build environment.


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

https://reviews.llvm.org/D74917



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

Reply via email to