Skip to content

Commit

Permalink
[C++] Add definition for __PROJECT_SOURCE_DIR__ to aeron_client's pub…
Browse files Browse the repository at this point in the history 
…lic interface.

This is required in case there is an upper level directory that includes
Aeron. In that case the added definitions do not reach the source files
that may include Aeron headers and __PROJECT_SOURCE_DIR__ will be
undefined to them.
  • Loading branch information
@denizevrenci
denizevrenci committed Dec 17, 2018
1 parent eabd7d6 commit 435e276
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
3 changes: 0 additions & 3 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build" FORCE)
endif(NOT CMAKE_BUILD_TYPE)

# relative file paths for use in exceptions
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D__PROJECT_SOURCE_DIR__='\"${PROJECT_SOURCE_DIR}\"'")

set(AERON_THIRDPARTY_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/thirdparty")

##########################################################
Expand Down
5 changes: 5 additions & 0 deletions aeron-client/src/main/cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ target_include_directories(aeron_client
PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}
)

# relative file paths for use in exceptions
target_compile_definitions(aeron_client
PUBLIC "__PROJECT_SOURCE_DIR__=\"${PROJECT_SOURCE_DIR}\""
)

if (NOT WIN32)
set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
set(THREADS_PREFER_PTHREAD_FLAG TRUE)
Expand Down

8 comments on commit 435e276

@LordNacho
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I'm not sure this has fixed the issue. At least I'm coming up with the missing PROJECT_SOURCE_DIR error in my recent compiles. I'm pulling a mirror as an external_project in my root CMakeLists.txt

@denizevrenci
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi. I presume you use ExternalProject_Add for your project. Are you linking against aeron_client using target_link_libraries?

@Spuddles
Copy link

Choose a reason for hiding this comment

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

Hi, I am doing as your mentioned above with ExternalProject_Add and using target_link_libraries to link with aeron_client. Is there a work around to get my app building again?

@lukepalmer
Copy link
Contributor

@lukepalmer lukepalmer commented on 435e276 Dec 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If folks are having issues would it be possible to reproduce your issue in the travis build, or at least post details about how to reproduce the problem?

@Spuddles
Copy link

Choose a reason for hiding this comment

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

Hi LukePalmer,

I believe the issue is that the new PROJECT_SOURCE_DIR value is defined when the aeron_client library is built but when I try to include a header to use the library the value is no longer defined.

Building your library and code works fine but trying to use the resulting library by including a header file causes the error when I build my application.

The compiler error I get is:

In file included from /home/Code/thirdparty_external/aeron/aeron-client/src/main/cpp/Aeron.h:20:0,
/home/Code/thirdparty_external/aeron/aeron-client/src/main/cpp/util/StringUtil.h: In function ‘valueType aeron::util::parse(const string&)’:
/home/Code/thirdparty_external/aeron/aeron-client/src/main/cpp/util/Exceptions.h:34:49: error: ‘PROJECT_SOURCE_DIR’ was not declared in this scope
#define SHORT_FILE aeron::util::past_prefix(PROJECT_SOURCE_DIR,FILE)

@lukepalmer
Copy link
Contributor

@lukepalmer lukepalmer commented on 435e276 Dec 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Ok that makes sense.

Haven't actually tried this yet but how about something like this in Exceptions.h:

#ifdef __PROJECT_SOURCE_DIR__
     #define __SHORT_FILE__ aeron::util::past_prefix(__PROJECT_SOURCE_DIR__,__FILE__)
#else
     #define __SHORT_FILE__ __FILE__
#endif

@denizevrenci
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me test it with ExternalProject_Add, it runs in build time so potentially target level definitions are not passed to your code that links against it.

@denizevrenci
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, importing a library at build time does not import its target level definitions or its include directories. What you may do to alleviate it if you want the relative paths in your exceptions is to manually add the definition.

Here is a sample project that uses ExternalProject_Add with Aeron.
https://github.com/denizevrenci/aeron_as_external_proj

But I suggest switching to https://github.com/Crascit/DownloadProject if you have CMake < 3.11 or https://cmake.org/cmake/help/v3.11/module/FetchContent.html otherwise. They run at configure time so you are able to add Aeron project root with add_subdirectory. That way you won't have to import the archive file or manually add include directories, libraries or definitions.

Please # to comment.