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

put USE_HDF5 in the GNUMake build system #1047

Merged
merged 5 commits into from
Jun 24, 2020

Conversation

atmyers
Copy link
Member

@atmyers atmyers commented Jun 23, 2020

No description provided.

@atmyers atmyers requested review from WeiqunZhang and mic84 June 23, 2020 23:28
HDF5_ABSPATH = $(abspath $(AMREX_HDF5_HOME))
INCLUDE_LOCATIONS += $(HDF5_ABSPATH)/include
LIBRARY_LOCATIONS += $(HDF5_ABSPATH)/lib
LIBRARIES += -lhdf5 -lz -ldl -L$(HDF5_ABSPATH)/lib
Copy link
Member

Choose a reason for hiding this comment

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

Even when AMREX_HDF5_HOME is not defined, we should have those -ls.

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean these three: -lhdf5 -lz -ldl?

Copy link
Member

Choose a reason for hiding this comment

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

yes, they might be found without -L.

Copy link
Member

Choose a reason for hiding this comment

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

And we probably also want to pass -Xlinker=-rpath -Xlinker=$(HDF5_ABSPATH)/lib.

HDF5_ABSPATH = $(abspath $(AMREX_HDF5_HOME))
INCLUDE_LOCATIONS += $(HDF5_ABSPATH)/include
LIBRARY_LOCATIONS += $(HDF5_ABSPATH)/lib
LIBRARIES += -L$(HDF5_ABSPATH)/lib
Copy link
Member

Choose a reason for hiding this comment

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

This line is not needed because the path has already been added to LIBRARY_LOCATIONS. In Make.defs, we have

LDFLAGS += -L. $(addprefix -L, $(LIBRARY_LOCATIONS))

Copy link
Member Author

Choose a reason for hiding this comment

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

We also have:

ifeq ($(USE_RPATH),TRUE)
  LDFLAGS += -Xlinker -rpath -Xlinker '$(abspath .)' $(addprefix -Xlinker -rpath -Xlinker , $(abspath $(LIBRARY_LOCATIONS)))
endif

in Make.defs.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but USE_RPATH is not defaulted to TRUE.

@WeiqunZhang WeiqunZhang self-requested a review June 24, 2020 00:54
@atmyers atmyers merged commit a8b4d5c into AMReX-Codes:development Jun 24, 2020
dwillcox pushed a commit to dwillcox/amrex that referenced this pull request Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants