Skip to content

CMake: Fix export with AMReX_INSTALL=OFF#2838

Merged
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
ax3l:cmake-fix-export-wo-install
Jun 14, 2022
Merged

CMake: Fix export with AMReX_INSTALL=OFF#2838
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
ax3l:cmake-fix-export-wo-install

Conversation

@ax3l
Copy link
Copy Markdown
Member

@ax3l ax3l commented Jun 14, 2022 •
edited
Loading

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

@ax3l
CMake: Fix export with AMReX_INSTALL=OFF
6898adc 
When introducing `AMReX_INSTALL`, the `export` of targets
broke for `OFF`. This generalizes this to work in either case.
@ax3l ax3l requested review from atmyers, etpalmer63, mic84 and sayerhs June 14, 2022 13:01
ax3l
ax3l commented Jun 14, 2022
View reviewed changes
Tools/CMake/AMReXInstallHelpers.cmake
#
# Export build-tree
#
export( EXPORT AMReXTargets NAMESPACE AMReX::
Copy link
Copy Markdown
Member Author

@ax3l ax3l Jun 14, 2022

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
Copy Markdown
Member Author

ax3l commented Jun 21, 2022 •
edited
Loading

@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
Copy Markdown
Contributor

jmusser304 commented Jun 21, 2022

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
Update ABLASTR: AMReX/Python
5f5580c 
Various Python related updates:
- AMReX-Codes/amrex#2842
- AMReX-Codes/amrex#2844
- AMReX-Codes/amrex#2838

Fix:
- AMReX-Codes/amrex#2837
@ax3l ax3l mentioned this pull request Jun 22, 2022
Merged
ax3l added a commit to BLAST-ImpactX/impactx that referenced this pull request Jun 22, 2022
@ax3l
Update ABLASTR: AMReX/Python (#148)
1421304 
Various Python related updates:
- AMReX-Codes/amrex#2842
- AMReX-Codes/amrex#2844
- AMReX-Codes/amrex#2838

Fix:
- AMReX-Codes/amrex#2837
@ax3l
Copy link
Copy Markdown
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

Reviewers

@atmyers atmyers Awaiting requested review from atmyers

@etpalmer63 etpalmer63 Awaiting requested review from etpalmer63

@mic84 mic84 Awaiting requested review from mic84

1 more reviewer

@sayerhs sayerhs sayerhs approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug install

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ax3l @jmusser304 @sayerhs @WeiqunZhang