Install Move Tools to `shared/amrex` #3599

ax3l · 2023-10-18T17:02:13Z

Summary

Packaging AMReX for Conda, we realized that we break with common conventions on Unix systems by installing a top-level Tools/ directory.

This moves its content in both the build directory and the install destination to share/amrex/ and <CMakePkgRoot>/AMReXCMakeModules/, respectively.

Additional background

conda-forge/staged-recipes#24294

Checklist

The proposed changes:

fix a bug or incorrect behavior in AMReX
add new capabilities to AMReX
changes answers in the test suite to more than roundoff level
are likely to significantly affect the results of downstream AMReX users
include documentation in the code and/or rst files, if appropriate

AMReX-Codes/amrex#3599

ax3l · 2023-10-19T01:15:25Z

Oh dang, the Windows modules for CMake are still a bit all over the place:

-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/cmake/AMReXConfig.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/cmake/AMReXConfigVersion.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/lib/cmake/AMReX/AMReXTargets.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/lib/cmake/AMReX/AMReXTargets-release.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/cmake/Modules/FindFilesystem.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/cmake/Modules/FindHYPRE.cmake
-- Installing: C:/bld/amrex_1697676564380/_h_env/Library/cmake/Modules/FindPETSc.cmake

ax3l · 2023-10-19T01:22:18Z

Fixed

AMReX-Codes/amrex#3599

Packaging AMReX for Conda, we realized that we break with common conventions on Unix systems by installing a top-level `Tools/` directory. This moves its content in both the build directory and the install destination to `share/amrex/` and `<CMakePkgRoot>/AMReXCMakeModules/`, respectively.

AMReX-Codes/amrex#3599

WeiqunZhang · 2023-10-19T03:19:27Z

We probably need this too.

diff --git a/Tools/CMake/AMReXTypecheck.cmake b/Tools/CMake/AMReXTypecheck.cmake
index 0b68fb8c27..69e0bb9aad 100644
--- a/Tools/CMake/AMReXTypecheck.cmake
+++ b/Tools/CMake/AMReXTypecheck.cmake
@@ -298,7 +298,7 @@ function( add_typecheck_target _target)
 
    # Find typechecker 
    find_file(_typechecker "typechecker.py"
-      HINTS ${AMReX_SOURCE_DIR} ${AMReX_ROOT} ENV AMReX_ROOT PATH_SUFFIXES Tools/typechecker)
+      HINTS ${AMReX_SOURCE_DIR} ${AMReX_ROOT} ENV AMReX_ROOT PATH_SUFFIXES share/amrex/typechecker)
 
    add_custom_target( typecheck_${_target}
       COMMAND python3  ${_typechecker}

WeiqunZhang · 2023-10-19T04:35:44Z

This breaks codes (e.g., mfix) that use amrex as a standalone library and use AMReXBuildInfo.cmake. We may need something like

diff --git a/Tools/CMake/AMReXBuildInfo.cmake b/Tools/CMake/AMReXBuildInfo.cmake
index 1679511065..78d1751f91 100644
--- a/Tools/CMake/AMReXBuildInfo.cmake
+++ b/Tools/CMake/AMReXBuildInfo.cmake
@@ -38,15 +38,23 @@ include(AMReXTargetHelpers)
 #
 # Set paths
 #
-string(REPLACE "/Tools/CMake" "" AMREX_TOP_DIR ${CMAKE_CURRENT_LIST_DIR})
+if (AMReX_FOUND) # AMReX used as a library
+   string(REPLACE "/lib/cmake/AMReX/AMReXCMakeModules" "" AMREX_TOP_DIR
+                 ${CMAKE_CURRENT_LIST_DIR})
+   set( AMREX_C_SCRIPTS_DIR "${AMREX_TOP_DIR}/share/amrex/C_scripts"
+      CACHE INTERNAL "Path to AMReX' C_scripts dir")
+else ()
+   string(REPLACE "/Tools/CMake" "" AMREX_TOP_DIR ${CMAKE_CURRENT_LIST_DIR})
+   set( AMREX_C_SCRIPTS_DIR "${AMREX_TOP_DIR}/Tools/C_scripts"
+      CACHE INTERNAL "Path to AMReX' C_scripts dir")
+endif ()
+
 set( AMREX_TOP_DIR ${AMREX_TOP_DIR} CACHE INTERNAL "Top level AMReX directory")

+
 set( AMREX_BUILDINFO_IFILE ${CMAKE_CURRENT_LIST_DIR}/AMReX_buildInfo.cpp.in
    CACHE INTERNAL "Full path and name of AMReX_buildInfo.cpp.in")

-set( AMREX_C_SCRIPTS_DIR "${AMREX_TOP_DIR}/Tools/C_scripts"
-   CACHE INTERNAL "Path to AMReX' C_scripts dir")
-
 set(AMREX_BUILD_DATETIME "" CACHE STRING
    "User defined build date and time. Set ONLY for reproducibly built binary distributions")

ax3l · 2023-10-19T16:21:38Z

Thanks, just saw something similar in conda-forge/staged-recipes#24298

CMake Error at /home/conda/staged-recipes/build_artifacts/pyamrex_1697732247175/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib/cmake/AMReX/AMReXCMakeModules/AMReXBuildInfo.cmake:201 (target_sources):
  Cannot find source file:

    /home/conda/staged-recipes/build_artifacts/pyamrex_1697732247175/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib/cmake/AMReX/AMReXCMakeModules/Tools/C_scripts/AMReX_buildInfo.H
Call Stack (most recent call first):
  CMakeLists.txt:107 (generate_buildinfo)

ax3l · 2023-10-19T16:35:33Z

I will push here in a bit.

AMReX-Codes/amrex#3599

Co-authored-by: Weiqun Zhang <[email protected]>

ax3l · 2023-10-19T22:13:34Z

Tools/CMake/AMReXBuildInfo.cmake

-set( AMREX_BUILDINFO_IFILE ${CMAKE_CURRENT_LIST_DIR}/AMReX_buildInfo.cpp.in
+if (AMReX_FOUND)
+   # AMReX is pre-installed and used as a library
+   string(REPLACE "/lib/cmake/AMReX/AMReXCMakeModules" "" AMREX_TOP_DIR_DEFAULT


Ooops, this is different for Unix vs. Windows :)
Will post a follow-up :)

## Summary Account for different CMake path on Unix vs. Windows. https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure ## Additional background Follow-up to #3599 ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate

ax3l · 2023-10-26T10:05:30Z

I see an issue with a pre-installed AMReX when used/found through a superbuild.... I thijnk replacing AMReX_FOUND with AMReX_DIR could be better.

#3609

## Summary The `AMReX_DIR` points if set to the CMake module path root. The old logic did not work for me in a situation (ImpactX) where: - AMReX is pre-installed but - found through a superbuild of another transient lib (ABLASTR) ## Additional background Follow-up to #3599 ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate

## Summary Packaging AMReX for Conda, we realized that we break with common conventions on Unix systems by installing a top-level `Tools/` directory. This moves its content in both the build directory and the install destination to `share/amrex/` and `<CMakePkgRoot>/AMReXCMakeModules/`, respectively. ## Additional background conda-forge/staged-recipes#24294 ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate --------- Co-authored-by: Weiqun Zhang <[email protected]>

## Summary Account for different CMake path on Unix vs. Windows. https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure ## Additional background Follow-up to AMReX-Codes#3599 ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate

## Summary The `AMReX_DIR` points if set to the CMake module path root. The old logic did not work for me in a situation (ImpactX) where: - AMReX is pre-installed but - found through a superbuild of another transient lib (ABLASTR) ## Additional background Follow-up to AMReX-Codes#3599 ## Checklist The proposed changes: - [x] fix a bug or incorrect behavior in AMReX - [ ] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate

ax3l added the install label Oct 18, 2023

ax3l requested a review from WeiqunZhang October 18, 2023 17:02

ax3l assigned WeiqunZhang Oct 18, 2023

ax3l added a commit to ax3l/staged-recipes that referenced this pull request Oct 18, 2023

[Patch] Install Move Tools to shared/amrex

da096fc

AMReX-Codes/amrex#3599

ax3l added a commit to ax3l/staged-recipes that referenced this pull request Oct 19, 2023

[Patch] Install Move Tools to shared/amrex

dffad5a

AMReX-Codes/amrex#3599

ax3l added a commit to ax3l/staged-recipes that referenced this pull request Oct 19, 2023

[Patch] Install Move Tools to shared/amrex

916982e

AMReX-Codes/amrex#3599

ax3l force-pushed the install-shared-tools branch from b4a5091 to a1accf5 Compare October 19, 2023 01:58

ax3l added a commit to ax3l/staged-recipes that referenced this pull request Oct 19, 2023

[Patch] Install Move Tools to shared/amrex

54f40e3

AMReX-Codes/amrex#3599

ax3l mentioned this pull request Oct 19, 2023

Architectures: PPC64le, AARCH64/ARM64 conda-forge/amrex-feedstock#1

Merged

7 tasks

ax3l force-pushed the install-shared-tools branch from 69b7a2f to ab5556d Compare October 19, 2023 17:00

ax3l added a commit to ax3l/amrex-feedstock that referenced this pull request Oct 19, 2023

[Patch] Update 3599.patch

c40cfdb

AMReX-Codes/amrex#3599

Fix: AMReXBuildInfo & AMReXTypecheck Paths

984edec

Co-authored-by: Weiqun Zhang <[email protected]>

ax3l force-pushed the install-shared-tools branch from ab5556d to 984edec Compare October 19, 2023 17:02

WeiqunZhang approved these changes Oct 19, 2023

View reviewed changes

WeiqunZhang merged commit c7b1ac7 into AMReX-Codes:development Oct 19, 2023
69 checks passed

ax3l deleted the install-shared-tools branch October 19, 2023 22:12

ax3l commented Oct 19, 2023

View reviewed changes

ax3l mentioned this pull request Oct 19, 2023

Windows: Fix Installed AMReXBuildInfo.cmake #3606

Merged

5 tasks

ax3l mentioned this pull request Oct 26, 2023

AMReXBuildInfo.cmake: AMReX_DIR #3609

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install Move Tools to `shared/amrex` #3599

Install Move Tools to `shared/amrex` #3599

ax3l commented Oct 18, 2023 •

edited

Loading

ax3l commented Oct 19, 2023 •

edited

Loading

ax3l commented Oct 19, 2023

WeiqunZhang commented Oct 19, 2023

WeiqunZhang commented Oct 19, 2023

ax3l commented Oct 19, 2023 •

edited

Loading

ax3l commented Oct 19, 2023

ax3l Oct 19, 2023

ax3l Oct 19, 2023

ax3l commented Oct 26, 2023 •

edited

Loading

Install Move Tools to shared/amrex #3599

Install Move Tools to shared/amrex #3599

Conversation

ax3l commented Oct 18, 2023 • edited Loading

Summary

Additional background

Checklist

ax3l commented Oct 19, 2023 • edited Loading

ax3l commented Oct 19, 2023

WeiqunZhang commented Oct 19, 2023

WeiqunZhang commented Oct 19, 2023

ax3l commented Oct 19, 2023 • edited Loading

ax3l commented Oct 19, 2023

ax3l Oct 19, 2023

Choose a reason for hiding this comment

ax3l Oct 19, 2023

Choose a reason for hiding this comment

ax3l commented Oct 26, 2023 • edited Loading

Install Move Tools to `shared/amrex` #3599

Install Move Tools to `shared/amrex` #3599

ax3l commented Oct 18, 2023 •

edited

Loading

ax3l commented Oct 19, 2023 •

edited

Loading

ax3l commented Oct 19, 2023 •

edited

Loading

ax3l commented Oct 26, 2023 •

edited

Loading