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 build does not generate QL_PACKAGE_NAME #1505

Open
tom91136 opened this issue Oct 31, 2022 · 5 comments
Open

CMake build does not generate QL_PACKAGE_NAME #1505

tom91136 opened this issue Oct 31, 2022 · 5 comments

Comments

@tom91136
Copy link

When using CMake, we get a define for PACKAGE_NAME instead of the QL_PACKAGE_NAME when using configure and make.
Looking at qt/Makefile.am, there's this intall-data-hook rule which rewrites a few things on install:

# ...
install-data-hook:
	$(SED) -e "s,HAVE_CONFIG_H,QL_HAVE_CONFIG_H," \
           -e "s,/\* install-hook \*/,#define QL_HAVE_CONFIG_H," \
           $(DESTDIR)$(this_includedir)/qldefines.hpp > .qldefines.hpp
	$(INSTALL_DATA) .qldefines.hpp $(DESTDIR)$(this_includedir)/qldefines.hpp
	rm .qldefines.hpp
	$(SED) -e "s,PACKAGE,QL_PACKAGE," \
           -e "s,STDC,QL_STDC," \
           -e "s, HAVE, QL_HAVE," \
           -e "s, VERSION, QL_AC_VERSION," \
           $(DESTDIR)$(this_includedir)/config.hpp > .config.hpp
	$(INSTALL_DATA) .config.hpp $(DESTDIR)$(this_includedir)/config.hpp
	rm .config.hpp
# ...

This logic isn't present in the CMake build.

There's a few libraries that depend on QL_PACKAGE_NAME (e.g https://github.com/eddelbuettel/rquantlib) so I think the CMake build should try to match that.

I'm happy to give this a stab, as the CMake build is also missing the pkg-config file generation logic that some libraries need.

@boring-cyborg
Copy link

boring-cyborg bot commented Oct 31, 2022

Thanks for posting! It might take a while before we look at your issue, so don't worry if there seems to be no feedback. We'll get to it.

@tom91136
Copy link
Author

Forgot to mention, I'm aware QL_VERSION exists in ql/version.hpp but since we have dependencies already using the undocumented one, I think it's best to keep both.

@lballabio
Copy link
Owner

Sure, go ahead. Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2023

This issue was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks.

@github-actions github-actions bot added the stale label Jan 7, 2023
@lballabio lballabio removed the stale label Jan 13, 2023
@github-actions
Copy link
Contributor

This issue was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks.

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

No branches or pull requests

2 participants