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

Problems with installation of header files #49

Closed
tkralphs opened this issue Sep 2, 2019 · 10 comments
Closed

Problems with installation of header files #49

tkralphs opened this issue Sep 2, 2019 · 10 comments

Comments

@tkralphs
Copy link
Member

tkralphs commented Sep 2, 2019

There are two problems with the installation of header files using the autoconf set-up (I know you do not like it and that it's deprecated, but it is what we need to build the OS and the Optimization Suite). The first problem that is relatively easy to fix is that in makefile.am here, you have

nobase_myinclude_HEADERS =  \
	include/cppad/base_require.hpp \
	include/cppad/CMakeLists.txt \
	include/cppad/configure.hpp.in \
...

the nobase means that these files get installed with the full prefix (including the include, see here) , so they end up in

<prefix>/include/include/cppad`

which I assume is not what you intend. I'm not actually sure what the right fix is here. I guess there must be a way to say "strip all prefixes and replace them with just cppad/" (headers are already put into the include directory by default).

A second issue is that you are installing configure.hpp.in rather than configure.hpp (see here).

Typically, these auto-generated headers are not installed (they are for internal use in building the project), but you are pulling it into a number of the other user-facing headers and source files and this is why it needs to be installed. I'm not sure if you want or need to address this. In the the other COIN-OR projects, we generate two different configuration headers---one with internal symbols only needed to build the libraries and one with symbols meant to be exported.

I know you probably want a PR, but I can't easily re-generate makefile.in for reasons we've discussed. It should be straightforward to fix these two issues and then I can test to see if everything is OK.

@bradbell
Copy link
Contributor

bradbell commented Sep 9, 2019

After configure does it work, the install files are best described by the command
INSTALL(
DIRECTORY "${CMAKE_SOURCE_DIR}/include/cppad/"
DESTINATION ${cppad_abs_includedir}/cppad
FILES_MATCHING PATTERN "*.hpp"
)
which appears in the file
https://github.com/coin-or/CppAD/blob/master/CMakeLists.txt

Would you please try making the corresponding change to
https://github.com/coin-or/CppAD/blob/master/makefile.am
testing it and if it works for you inform me of the change and I will try it.

@tkralphs
Copy link
Member Author

tkralphs commented Sep 9, 2019

Sure, I understood your intention. As I've explained many times, I cannot easily do the testing. It is vastly easier for you to do it. As far as I can tell, the appropriate change to be made to Makefile.am is to change this line

nobase_myinclude_HEADERS =  \

to this

myincludedir = $(includedir)/cppad
myinclude_HEADERS =  \

I would greatly appreciate it if you could try this. As for the installation of configure.hpp, that is up to you, I was just pointing out that these kinds of headers are not usually installed.

@bradbell
Copy link
Contributor

bradbell commented Sep 12, 2019

I have not been running the autotools tests:

   bin/autotools.sh automake
   bin/autotools.sh configure
   biin/autotooils.sh test

is now failing before I make and changes. I need to fix this first.

@bradbell
Copy link
Contributor

bradbell commented Sep 12, 2019

I have the test running in a sandbox, but when I go to install as you suggested, it tries to put all the include files in one directory. What I need is to copy the directory and sub-directory structure like the following bash command

cp -r include/cppad  $includedir/cppad

but only need include the *.hpp files.

Note sure how to do this with automake, It is easy in cmake where I use the following:

#
# During install copy all the cppad include files to
# ${cppad_abs_includedir}/cppad
INSTALL(
    DIRECTORY "${CMAKE_SOURCE_DIR}/include/cppad/"
    DESTINATION ${cppad_abs_includedir}/cppad
    FILES_MATCHING PATTERN "*.hpp"
)

@bradbell
Copy link
Contributor

I think that I have fixed this with the commits from fb5d0a4 to 30d3903; see the heading 09-14 on
https://coin-or.github.io/CppAD/doc/whats_new_19.htm

The real fix to the install is moving the nobase include to the include subdirectory; see
https://github.com/coin-or/CppAD/blob/master/include/makefile.am

Most of the work was getting the tests to run using the automake configuration.

I want to mention again that using automake is deprecate
and that using cmake does a better job of configuring and testing CppAD.

@tkralphs
Copy link
Member Author

tkralphs commented Sep 16, 2019

This seems to work for in-place builds, but fails for VPATH builds. Supporting VPATH builds is critical and requires some extra care, but I don't recall now exactly what. To replicate, do

git checkout master
mkdir build
cd build
../configure --prefix=$PWD/xxx
make install

and the should end in

make[1]: *** No rule to make target 'include/cppad/local/sparse/list.hpp', needed by 'all-am'.  Stop.
make[1]: Leaving directory '/mnt/c/Users/tkral/Documents/Projects/cppad/build'
makefile:960: recipe for target 'install-recursive' failed
make: *** [install-recursive] Error 1

I will try to poke around a little as I have time.

@tkralphs
Copy link
Member Author

Oh, sorry, I was not on the most recent commit. Now everything seems to work except that the auto-generated headers (include/cappad/configure.hpp) are still not installed. What is installed is include/cppad/configure.hpp.in. Same for local/is_pod.hpp. I'm actually not 100% sure how auto-generated things are supposed to get installed. We have these hooks in Makefile.am that install the headers for the COIN projects I maintain, but there's some more complicated stuff going on around this.

install-exec-local:
	$(install_sh_DATA) config_cbc.h $(DESTDIR)$(includecoindir)/CbcConfig.h

uninstall-local:
	rm -f $(DESTDIR)$(includecoindir)/CbcConfig.h

@svigerske
Copy link
Member

That's for renaming while installing.
If we had a CbcConfig.h generated in the build dir, then it would just be

includecoindir = $(includedir)/coin
installcoin_HEADERS = CbcConfig.h

So something like that should work installing CppAD's configure.hpp and is_pod.hpp.

@bradbell
Copy link
Contributor

There was a bug in include/makefile.am and I was installing the the .hpp.in files instead of the corresponding .hpp files.

In addition, the program bin/check_makefile.sh was checking for the hpp.in files instead of check for the .hpp files.

This has been fixed in commit
4fccfa7

As for installing in $(includedir)/coin, this is the purpose of the postfix_dir option to the configure script; see the heading postfix_dir on
https://coin-or.github.io/CppAD/doc/autotools.htm#postfix_dir

@tkralphs
Copy link
Member Author

tkralphs commented Sep 17, 2019

OK, now it works. I didn't remember that the installation of headers that end up in the build directory was so straightforward. I thought there was an additional step. I'm doing a final full build of the Optimization Suite to be sure, but the problems seem to be fixed, so I'll close this issue.

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

No branches or pull requests

3 participants