Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to diagnostics/qt to reflect new cmake behavior. #788

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

KineticTheory
Copy link
Collaborator

@KineticTheory KineticTheory commented Apr 1, 2020

Background

  • After upgrading to cmake-3.17.0, our one Qt example failed to configure or build. I have updated build system logic to match what cmake-3.17+ expects for Qt applications.

Purpose of Pull Request

Description of changes

  • Greatly simplify Qt discovery in vendor_libraries.cmake, but require Qt5 (disallow Qt3 or Qt4).
  • When building diagnostics/qt require cmake-3.17.0+:
    • Begin using CMAKE_AUTOUIC and CMAKE_AUTORCC.
    • CMake no longer requries the use of the macros qt5_wrap_ui or add_resources.
    • .ui files are now treated like source files.
  • Add code to delete move ctors and move assignment operators.

Status

@KineticTheory KineticTheory added this to the Draco-7_7_0 milestone Apr 1, 2020
@KineticTheory KineticTheory self-assigned this Apr 1, 2020
@codecov
Copy link

codecov bot commented Apr 1, 2020

Codecov Report

Merging #788 into develop will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop    #788   +/-   ##
=======================================
  Coverage     94.0%   94.0%           
=======================================
  Files          367     367           
  Lines        17229   17229           
=======================================
  Hits         16197   16197           
  Misses        1032    1032           

@KineticTheory
Copy link
Collaborator Author

clang is generating warnings

CMake Warning at config/vendor_libraries.cmake:319 (find_package):
  By not providing "FindQt5.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Qt5", but
  CMake did not find one.

  Could not find a package configuration file provided by "Qt5" with any of
  the following names:

    Qt5Config.cmake
    qt5-config.cmake

  Add the installation prefix of "Qt5" to CMAKE_PREFIX_PATH or set "Qt5_DIR"
  to a directory containing one of the above files.  If "Qt5" provides a
  separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
  config/vendor_libraries.cmake:635 (setupQt)
  config/vendor_libraries.cmake:797 (setupVendorLibrariesUnix)
  CMakeLists.txt:95 (setupVendorLibraries)

I'm looking into it.

@KineticTheory KineticTheory mentioned this pull request Apr 2, 2020
6 tasks
@KineticTheory
Copy link
Collaborator Author

KineticTheory commented Apr 2, 2020

@keadyk It looks like there is a remaining configure warning for clang and valgrind builds. I'll deal with that as a separate issue. I'm rerunning the snow tests right now. Trinitite is on DST.

@KineticTheory
Copy link
Collaborator Author

Snow builds look okay. There are warnings related to IPO for release builds. I'm working with Doug to fix that (we might just IPO off). Again, let's deal with that later. We need to get the regressions running again.

Copy link
Collaborator

@keadyk keadyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! My comments are minor :)

environment/bashrc/.bashrc_cray Show resolved Hide resolved
config/vendor_libraries.cmake Show resolved Hide resolved
src/diagnostics/qt/diWidget.hh Outdated Show resolved Hide resolved
+ After upgrading to cmake-3.17.0, our one Qt example failed to configure or
  build. I have updated build system logic to match what cmake-3.17+ expects for
  Qt applications.
+ Greatly simplify Qt discovery in `vendor_libraries.cmake`, but require Qt5
  (disallow Qt3 or Qt4).
+ When building `diagnostics/qt` require cmake-3.17.0+:
  + Begin using `CMAKE_AUTOUIC` and `CMAKE_AUTORCC`.
  + CMake no longer requries the use of the macros `qt5_wrap_ui` or
    `add_resources`.
  + `.ui` files are now treated like source files.
+ Add code to delete move ctors and move assignment operators.
+ Update default dev env to load cmake/3.17.0 on CTS-1 and ATS-1.
@keadyk
Copy link
Collaborator

keadyk commented Apr 2, 2020

I will plan to merge when the latest github checks pass, unless you object @KineticTheory

@keadyk keadyk merged commit 85f6c05 into lanl:develop Apr 2, 2020
@KineticTheory KineticTheory deleted the fix_for_cmake3170_plus_qt branch April 3, 2020 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants