Skip to content

Rename src/core/common to src/core/openjph#236

Closed
cary-ilm wants to merge 1 commit into
aous72:masterfrom
cary-ilm:common-openjph
Closed

Rename src/core/common to src/core/openjph#236
cary-ilm wants to merge 1 commit into
aous72:masterfrom
cary-ilm:common-openjph

Conversation

@cary-ilm
Copy link
Copy Markdown
Contributor

OpenJPH headers are included in application code via #include <openjph/ojph_version.h>. The headers are expected to be in a folder named "openjph". The cmake configuration places them there in the installation step.

However, if OpenJPH is incorporated into an application via cmake's add_subdirectory, there is no installation step, the headers are included directly out of the source tree. There is no "openjph" folder, leading the #include <openjph/ojph_version.h> to fail.

Renaming the "common" directory to "openjph" resolves the build issue, since the headers then live inside the source tree in a directory with same name as the installation. The use of the "common" directory name is entirely internal to the OpenJPH build, it has no impact on the installation. The name should be arbitrary, so there should be no downside to renaming it this way.

OpenJPH headers are included in application code via `#include
<openjph/ojph_version.h>`. The headers are expected to be in a folder
named "openjph". The cmake configuration places them there in the
installation step.

However, if OpenJPH is incorporated into an application via cmake's
`add_subdirectory`, there is no installation step, so there is no
"openjph" folder, leading the `#include <openjph/ojph_version.h>` to
fail.

Renaming the "common" directory to "openjph" resolves the build issue,
since the headers then live inside the source tree in a directory with
same name as the installation. The use of the "common" directory name
is entirely internal to the OpenJPH build, it has no impact on the
installation. The name should be arbitrary, so there should be no
downside to renaming it this way.

Signed-off-by: Cary Phillips <cary@ilm.com>
aous72 added a commit that referenced this pull request Dec 24, 2025
There are three changes here.
The first is the most important.

Change 1:  Thank you @cary-ilm
* Rename src/core/common to src/core/openjph

OpenJPH headers are included in application code via `#include
<openjph/ojph_version.h>`. The headers are expected to be in a folder
named "openjph". The cmake configuration places them there in the
installation step.

However, if OpenJPH is incorporated into an application via cmake's
`add_subdirectory`, there is no installation step, so there is no
"openjph" folder, leading the `#include <openjph/ojph_version.h>` to
fail.

Renaming the "common" directory to "openjph" resolves the build issue,
since the headers then live inside the source tree in a directory with
same name as the installation. The use of the "common" directory name
is entirely internal to the OpenJPH build, it has no impact on the
installation. The name should be arbitrary, so there should be no
downside to renaming it this way.

Signed-off-by: Cary Phillips <cary@ilm.com>

* This fixes PR compilation

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Cary Phillips <cary@ilm.com>


Change 2:
Message type constants are not descriptive enough, and they maybe inadvertently replaced by a preprocessor macro.
They are:
```
enum OJPH_MSG_LEVEL : int
  {
    ALL_MSG = 0,  // uninitialized or print all message
    INFO = 1,     // info message
    WARN = 2,     // warning message
    ERROR = 3,    // error message (the highest severity)
    NO_MSG = 4,   // no message (higher severity for message printing only)
  };
```
They were replaced with 
```
  enum OJPH_MSG_LEVEL : int
  {
    OJPH_MSG_ALL_MSG = 0,  // uninitialized or print all message
    OJPH_MSG_INFO = 1,     // info message
    OJPH_MSG_WARN = 2,     // warning message
    OJPH_MSG_ERROR = 3,    // error message (the highest severity)
    OJPH_MSG_NO_MSG = 4,   // no message (higher severity for message printing only)
  };
```
which is less like to have the identified issues.


Change 3:
Addresses the problem identified in @clshortfuse in issue #235
@aous72
Copy link
Copy Markdown
Owner

aous72 commented Dec 24, 2025

Hi Cary,

Sorry for the late reply; I got bogged down with other tasks.
Happy to adopt this change; I have integrated this PR into PR #237.

Please reopen it, if you have other issues.

Kind regards,
Aous

@aous72 aous72 closed this Dec 24, 2025
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.

2 participants