search

ossimlabs / ossim

Core OSSIM (Open Source Software Image Map) package including C++ code for OSSIM library, command-line applications, tests, and build system
MIT License
300 stars 142 forks source link

cmake script fixes #119

Closed rkanavath closed 7 years ago

rkanavath commented 7 years ago
gpotts commented 7 years ago

Looking at the modifications now. May take a few. Not sure about all platforms yet for the jpeg12 versus libjpeg12 prefix for headers. Will see if I can get Burken to look at the jpeg12 modifications

rkanavath commented 7 years ago

changes to Nitf source should be reverted. I will replace the file with one from dev. you can take care of cmake changes.

gpotts commented 7 years ago

Hello Rashad. Thank you for the modifications. I also see that your dev might be out of sync?? I see a change that says "Merge remote-tracking branch 'upstream/dev' into dev" that seems your local dev is out of sync with the remote? I am checking the CMAKE modifications now. Testing first on my MAC build then will check it in and then test on the CentOS7 build.

Will let you know when to do an update and verify.

Thank you again!

rkanavath commented 7 years ago

pr updated now

gpotts commented 7 years ago

Hello Rashad. Could you verify. I manually looked at your changes and pushed them to DEV. Builds on both MAC and my CentOS box. I did not execute the merge on the pull request but instead go through the pull request and merge items into my local dev.

Take care

rkanavath commented 7 years ago

I will check the current 'dev' from ossimlabs/ossim this evening and let you know Thanks for your review and merge

rkanavath commented 7 years ago

did a quick look in to OssimUtilities.cmake file. one change is missed in your merge. see: https://github.com/ossimlabs/ossim/pull/119/files#diff-c2cafb9d1964137cd1a44e4752de4d75R183

and ossimlabs/dev https://github.com/ossimlabs/ossim/blob/dev/cmake/CMakeModules/OssimUtilities.cmake#L180

the line is ADD_DEPENDENCIES(). This make sure all applications depend (or wait) on ossim lib. all apps will be re-built if your libossim changes.


+      ENDIF(APPLICATION_COMMAND_LINE)
-
+ 
+    ADD_DEPENDENCIES(${TARGET_TARGETNAME} ${OSSIM_LIB_NAME})

I will do a better check later

gpotts commented 7 years ago

Hello Rashad. Added the missing line

omarossim commented 7 years ago

Hello Rashad:

Looking at the builds on CentOS/unix The headers from the sub projects such as ossim-oms, … etc are not being installed now. Not sure why.

Take care

Garrett

On Sep 4, 2017, at 11:07 AM, Rashad Kanavath notifications@github.com wrote:

Closed #119 https://github.com/ossimlabs/ossim/pull/119.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ossimlabs/ossim/pull/119#event-1233689210, or mute the thread https://github.com/notifications/unsubscribe-auth/AGpvymxLuGlCceXI2D2-Y5UA_ZXOCMqZks5sfBIWgaJpZM4PLpky.

rkanavath commented 7 years ago

Can you send me build log. I can have a look and see if this was my changes.

rkanavath commented 7 years ago

there is another commit in ossim-oms.

gpotts commented 7 years ago

Hello Rashad:

I am going to have to revert the cmake changes for now. I am getting link errors with ossim-oms and the ossim headers get installed but no other headers are being installed. Not sure why that is.

gpotts commented 7 years ago

Can you verify on a unix system that you can build and install the include headers? Let's do just a pull request for the cmake files

gpotts commented 7 years ago

Added the GEOS QUIET back into dev and also added "CMake: update cmake module path" to dev.

rkanavath commented 7 years ago

i confirm that i can install libossim and then build ossim-oms

rkanavath commented 7 years ago

after installing ossim with all changes in PR, you have to apply the other PR in ossim-oms. If you still have issue and thinks this changes is breaking other stuff, you can revert them and let me know which changes are reverted.

you can also clone my rkanavath/ossim and rkanvath/ossim-oms to test

gpotts commented 7 years ago

Hello Rashad:

Just merged both ossim and oms and testing the build on the build server.

rkanavath commented 7 years ago

Hope it works now.

gpotts commented 7 years ago

Hello Rashad:

I think all tests have passed and the build is good :). Thank you and take care

On Sep 5, 2017, at 10:25 AM, Rashad Kanavath notifications@github.com wrote:

Hope it works now.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ossimlabs/ossim/pull/119#issuecomment-327191865, or mute the thread https://github.com/notifications/unsubscribe-auth/ACL9v_Y9YdRyDhqKIXjfukwwOgel4Fmaks5sfVnFgaJpZM4PLpky.