Skip to content

C++: add conversions.h to CMakeLists.txt#3671

Merged
joshmoore merged 1 commit intoome:developfrom
joshmoore:510-cpp-fix
Mar 31, 2015
Merged

C++: add conversions.h to CMakeLists.txt#3671
joshmoore merged 1 commit intoome:developfrom
joshmoore:510-cpp-fix

Conversation

@joshmoore
Copy link
Copy Markdown
Member

Without conversions.h in lib/include/omero, compiling
a client is all but impossible.

cc: @rleigh-dundee @emilroz
--no-rebase

Without conversions.h in lib/include/omero, compiling
a client is all but impossible.
@ghost
Copy link
Copy Markdown

ghost commented Mar 30, 2015

The change looks fine and good to merge if everything is green.

Were any other headers added since the cmake conversion which might also be missing? There weren't last time I checked, but I haven't looked in detail over the last few months at what might have been added in the interim.

On the CI side, we might want to develop some OmeroCpp clients independent of openmicroscopy.git to be built against the installed library/headers. Or have an alternative CMakeLists.txt for the examples so they can be built independently against external headers/libs. Either would allow validation of the install which we don't currently have.

@joshmoore
Copy link
Copy Markdown
Member Author

Were any other headers added since the cmake conversion which might also be missing?

Was just checking this. This seems to say, "no":

$ git diff --name-status v5.1.0-m1..HEAD . | grep -E ^A | grep -E h\$ | xargs -IX echo X | cut -b33- | xargs -IY ls /opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac\ OS\ X-10.10.2-x86_64/include/Y
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/IceNoWarnPop.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/IceNoWarnPush.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/conversions.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/ElectricPotentialI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/FrequencyI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/LengthI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/PowerI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/PressureI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/TemperatureI.h
/opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include/omero/model/TimeI.h
ls: /opt/ome8/target/OMERO.cpp-5.1.0-m5-ice35-SNAPSHOT-Mac OS X-10.10.2-x86_64/include//safegtest.h: No such file or directory

@joshmoore
Copy link
Copy Markdown
Member Author

Or have an alternative CMakeLists.txt for the examples

Agreed. In fact, I thought that was still happening.

@ghost
Copy link
Copy Markdown

ghost commented Mar 30, 2015

@joshmoore The examples are a standard part of the cmake build, so they build in-tree every time you do a build. The reason was so that they wouldn't bitrot (they hadn't been built for well over a year or so with scons as far as I could tell and needed fixing up). We can easily add a second alternatively-named file here to set up the include/library paths and then source the regular CMakeLists.txt. Could probably be added to the same build job, even.

@ghost
Copy link
Copy Markdown

ghost commented Mar 31, 2015

I've done a test build and install and I can confirm the header is now installed correctly. Please merge.

joshmoore added a commit that referenced this pull request Mar 31, 2015
C++: add conversions.h to CMakeLists.txt
@joshmoore joshmoore merged commit 9fcabd3 into ome:develop Mar 31, 2015
@joshmoore joshmoore deleted the 510-cpp-fix branch March 31, 2015 09:19
@sbesson sbesson added this to the 5.1.0 milestone Apr 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants