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

CMake: Fix export with AMReX_INSTALL=OFF #2838

Merged

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Jun 14, 2022

Summary

When introducing AMReX_INSTALL #1831, the export of targets #1149 broke for OFF. This generalizes this to work in either case.

Additional background

Check the change without white space changes to see the changed logic more cleanly:
https://github.com/AMReX-Codes/amrex/pull/2838/files?diff=unified&w=1

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

When introducing `AMReX_INSTALL`, the `export` of targets
broke for `OFF`. This generalizes this to work in either case.

#
# Export build-tree
#
export( EXPORT AMReXTargets NAMESPACE AMReX::
Copy link
Member Author

Choose a reason for hiding this comment

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

This line only works if an equivalent install() line exists for the same target.

@WeiqunZhang WeiqunZhang merged commit e39b964 into AMReX-Codes:development Jun 14, 2022
@ax3l ax3l deleted the cmake-fix-export-wo-install branch June 14, 2022 18:26
@ax3l
Copy link
Member Author

ax3l commented Jun 21, 2022

@jmusser304 let's coordinate here on your regression in MFIX

Quote:
MFIX complains about CUDA::curand not found

CMake Error at subprojects/amrex/builddir/lib/cmake/AMReX/AMReXTargets.cmake:51 (set_target_properties):
  The link interface of target "AMReX::amrex" contains:

    CUDA::curand

if I run the same command echoed by the regtest, it works
oddly, MFIX link line reported by cmake from regtest contains curand

--    MFIX link line        = -pthread /.nfs/misc/apps/MPI/openmpi/4.0.4/gnu/9.3.0/cuda/10.2/lib/libmpi.so CUDA::curand /nfs/apps/Compilers/NVIDIA/CUDA/11.3/targets/x86_64-linux/lib/stubs/libcurand.so 

@jmusser304
Copy link
Contributor

It appears that I can get it to build if edit the file <path to amrex builddir>/lib/cmake/AMReX/AMReXTargets.cmake and remove the CUDA::curand from the line

INTERFACE_LINK_LIBRARIES "Threads::Threads;MPI::MPI_C;MPI::MPI_CXX;CUDA::curand;AMReX::Flags_CXX"

ax3l added a commit to ax3l/impactx that referenced this pull request Jun 22, 2022
ax3l added a commit to ECP-WarpX/impactx that referenced this pull request Jun 22, 2022
@ax3l
Copy link
Member Author

ax3l commented Jun 28, 2022

Fixed via #2849

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants