-
Notifications
You must be signed in to change notification settings - Fork 113
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
Clean headers dependencies in nrn #2389
Changes from all commits
0e77cac
8fa8091
611d6cf
06399e4
5d8b578
8168b4b
e8ce04c
3bd2900
31a7c6d
8116223
3838f15
411eec3
787d3ea
510b87f
4aa5393
a582b42
ccb080a
81f29b3
a40e447
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ UserLDFLAGS = | |
# install dirs | ||
bindir := ${ROOT}/@CMAKE_INSTALL_BINDIR@ | ||
libdir := ${ROOT}/@CMAKE_INSTALL_LIBDIR@ | ||
incdir := ${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@ | ||
incdir := -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@ -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/crout -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/neuron -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/newton -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/nrncvode -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/nrniv -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/nrnmpi -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/nrnoc -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/oc -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/scopmath -I${ROOT}/@CMAKE_INSTALL_INCLUDEDIR@/sparse13 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused by this change. If we are now including a lot more of these directory names in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because mod files still include "raw" files from the old grouped There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose that is widespread in I'm not sure it's a step forwards to introduce two different There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can slowly clean all mod files. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't buy that; the cost/reward for updating all of ModelDB for that is such that it will never happen, in my view. So: this seems like it's creating and locking in a new source of inconsistency between different bits of C++. Is that really worth it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree on the first part that it will never happens. For the second part, I, personnaly, think, it worse it, to clean more the code and know who include what. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts @pramodk @alexsavulescu @nrnhines ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We are on the same page!
I understand the "inconsistency" aspects that Olli mentioned (i.e. having a single way would be better). I don't have recent enough hands-on experience to assess the impact or propose a better solution here. Maybe @olupton you can propose what should be done here? |
||
datadir:= ${ROOT}/@CMAKE_INSTALL_DATADIR@ | ||
datadir_lib := ${ROOT}/@CMAKE_INSTALL_DATADIR@/lib | ||
|
||
|
@@ -47,7 +47,7 @@ OS_NAME := $(shell uname) | |
_cm =, | ||
|
||
# We rebuild the include dirs since a lot of stuff changes place | ||
INCLUDES = -I. $(INCFLAGS) $(UserINCFLAGS) -I$(incdir) | ||
INCLUDES = -I. $(INCFLAGS) $(UserINCFLAGS) $(incdir) | ||
ifeq (@NRN_INCLUDE_MPI_HEADERS@, ON) | ||
INCLUDES += $(if @MPI_C_INCLUDE_PATH@, -I$(subst ;, -I,@MPI_C_INCLUDE_PATH@),) | ||
endif | ||
|
@@ -116,14 +116,14 @@ C_GREEN := \033[32m | |
# RPATH is set for DESTDIR_RPATH and coreneuron lib | ||
special: $(mech_lib) | ||
@printf " => $(C_GREEN)LINKING$(C_RESET) executable $(special) LDFLAGS are: $(LDFLAGS)\n" | ||
$(CXX_LINK_EXE) -I $(incdir) -I $(incdir)/nrncvode -DAUTO_DLOPEN_NRNMECH=0 $(datadir)/nrnmain.cpp -o $(special) \ | ||
$(CXX_LINK_EXE) $(incdir) -DAUTO_DLOPEN_NRNMECH=0 $(datadir)/nrnmain.cpp -o $(special) \ | ||
-L$(OBJS_DIR) -l$(mech_libname) $(NRNLIB_FLAGS) -l$(mech_libname) $(extra_lib_link) -Wl,-rpath,'$(DESTDIR_RPATH)' -Wl,-rpath,$(libdir) $(LDFLAGS) $(EXTRA_LDFLAGS) | ||
|
||
$(mech_lib): $(mech_lib_type) | ||
|
||
mech_lib_shared: mod_func.o $(mod_objs) build_always | ||
@printf " => $(C_GREEN)LINKING$(C_RESET) shared library $(mech_lib)\n" | ||
$(CXX_LINK_SHARED) -I $(incdir) -o ${mech_lib} ${_SONAME} \ | ||
$(CXX_LINK_SHARED) $(incdir) -o ${mech_lib} ${_SONAME} \ | ||
$(mod_func_o) $(mod_objs) $(NRNLIB_FLAGS) $(NRNLIB_RPATH_FLAGS) $(LDFLAGS) | ||
rm -f $(OBJS_DIR)/.libs/libnrnmech.so ; mkdir -p $(OBJS_DIR)/.libs ; cp $(mech_lib) $(OBJS_DIR)/.libs/libnrnmech.so | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
|
||
/* | ||
Copyright (C) 1988 Free Software Foundation | ||
written by Dirk Grunwald (grunwald@cs.uiuc.edu) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#ifdef WIN32 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include "bndedval.h" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include <InterViews/window.h> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include "epsprint.h" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
/* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include <stdio.h> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#define USEGNU 1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include <InterViews/canvas.h> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
#if HAVE_IV // to end of file | ||
|
||
#include <stdio.h> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#include <../../nrnconf.h> | ||
#include <nrnconf.h> | ||
|
||
#include <CodeFragments.h> | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment for other reviewers: I understand the idea here is that
src/dir/header.h
used to get installed asinclude/header.h
, but now it gets installed asinclude/dir/header.h
.