[Insight-developers] openjpg include dir not exported

Luis Ibanez luis.ibanez at kitware.com
Sat Aug 28 15:58:20 EDT 2010


Hi Marcus,

When we identify a bug during the review, or simply
something that can be improved in the code,... is it
possible to make the modification in our local git
repository, and push it back to Gerrit, and have it
associated to the code that we were reviewing ?

For example, for the comments that I put in
http://review.source.kitware.com/#change,5

It probably would have made more sense to patch
the patch, and have that modified patch be related
to the original change in Gerrit.


On the other hand,

Looking at the "Life of a Patch" for Android:
http://source.android.com/source/life-of-a-patch.html

It seems that they always send modifications back
to the original authors...


What would you recommend as a best practice ?


       Luis


-------------------------------------------------------------------------
On Sat, Aug 28, 2010 at 2:23 PM, Marcus D. Hanwell <
marcus.hanwell at kitware.com> wrote:

> On Sat, Aug 28, 2010 at 2:18 PM, Luis Ibanez <luis.ibanez at kitware.com>wrote:
>
>> Hi Brad,
>>
>> That's a good point.
>>
>> I'm not sure if Gerrit should be aware of conflicts between patches.
>> After all, they are supposed to be in different branches, isn't it ?
>>
>
> Gerrit does not assess conflicts between review requests, this should be
> handled by Git when merging the patch in.
>
>>
>> ---
>>
>> I'm now testing your patches (well.. figuring out how to do it..)
>> will merge them with the openjpegpimpl branch before pushing
>> them.
>>
>
> You can fetch the final commit in the topic branch, using the copy/paste
> instructions, then git checkout -b tree, give it a topic name. You can then
> merge that topic branch into yours (move to your topic branch, and then git
> merge other-topic). At that point you can resolve the conflict (if there is
> one), and once you are done that topic can be staged and merged.
>
> Hope that makes sense.
>
> Marcus
>
>>
>> -----------------------------------
>>
>> On Sat, Aug 28, 2010 at 2:02 PM, Bradley Lowekamp <blowekamp at mail.nih.gov
>> > wrote:
>>
>>> Luis,
>>>
>>> That is a great idea to use a private implementation. Should help with
>>> compile time too (every little bit helps)!
>>>
>>> Unfortunately, your patch is going to conflict with mine. This is
>>> interesting in Gerrit because I would expect something to come up to showing
>>> that there is a conflict between these two topic. But I don't see anything.
>>>
>>> Luis you started to review my topic. Do you think it's good to go? So I
>>> can hurry up to stage and merge my topic, before yours :P ( I already had
>>> the burden of rebasing and merging after the uncrustification )
>>>
>>> Brad
>>>
>>> On Aug 28, 2010, at 12:17 PM, Luis Ibanez wrote:
>>>
>>> BTW,
>>>
>>> We probably could use PIMPL in the itkJPEG2000ImageIO
>>> class, so that the headers and data structures of openjpeg
>>> are not exposed to ITK developers...
>>>
>>> I'll give it a try...
>>>
>>>
>>>      Luis
>>>
>>>
>>>
>>> --------------------------------------------------------------------------
>>> On Sat, Aug 28, 2010 at 12:15 PM, Luis Ibanez <luis.ibanez at kitware.com>wrote:
>>>
>>>>
>>>> Hi Gaetan,
>>>>
>>>>
>>>> Thanks for the clarification.
>>>>
>>>>
>>>> Can you please try this patch:
>>>>
>>>> diff --git a/itkIncludeDirectories.cmake b/itkIncludeDirectories.cmake
>>>> index 36b2ab5..38584fb 100644
>>>> --- a/itkIncludeDirectories.cmake
>>>> +++ b/itkIncludeDirectories.cmake
>>>> @@ -37,6 +37,7 @@ SET(ITK_INCLUDE_DIRS_BUILD_TREE
>>>> ${ITK_INCLUDE_DIRS_BUILD_TREE}
>>>>    ${ITK_SOURCE_DIR}/Utilities/nifti/niftilib
>>>>    ${ITK_SOURCE_DIR}/Utilities/nifti/znzlib
>>>>    ${ITK_SOURCE_DIR}/Utilities/itkExtHdrs
>>>> +  ${ITK_SOURCE_DIR}/Utilities/openjpeg
>>>>    ${ITK_BINARY_DIR}/Utilities
>>>>    ${ITK_SOURCE_DIR}/Utilities
>>>>  )
>>>> @@ -149,6 +150,7 @@ SET(ITK_INCLUDE_RELATIVE_DIRS
>>>> ${ITK_INCLUDE_RELATIVE_DIRS}
>>>>    Utilities/nifti/niftilib
>>>>    Utilities/nifti/znzlib
>>>>    Utilities/itkExtHdrs
>>>> +  Utilities/openjpeg
>>>>    Utilities
>>>>  )
>>>>
>>>>
>>>> and if it works,
>>>> please go ahead and push the change.
>>>>
>>>>
>>>>      Thanks
>>>>
>>>>
>>>>              Luis
>>>>
>>>>
>>>> -----------------------------------------------------------
>>>> 2010/8/28 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>
>>>>
>>>>
>>>>> Le 28 août 10 à 00:53, Luis Ibanez a écrit :
>>>>>
>>>>>
>>>>>> Hi Gaetan,
>>>>>>
>>>>>
>>>>> Hi Luis,
>>>>>
>>>>>
>>>>>
>>>>>> Are you doing this with an installed version of ITK ?
>>>>>>
>>>>>
>>>>> No, I'm using it in the build tree.
>>>>>
>>>>>
>>>>>
>>>>>> I just verified that I can use JPEG2000ImageIO
>>>>>> from a small example outside of the ITK build tree.
>>>>>>
>>>>>> (Note that you must have configured ITK with
>>>>>>  ITK_USE_REVIEW ON).
>>>>>>
>>>>>
>>>>> Yes, that's on.
>>>>>
>>>>>
>>>>>
>>>>>> Please let me know how I can reproduce the problem
>>>>>> that you are observing.
>>>>>>
>>>>>
>>>>> I'm trying to add JPEG2000ImageIO to wrapitk, but openjpeg.h can't be
>>>>> found.
>>>>> I have to add the path to this file in ITK_INCLUDE_DIRS in
>>>>> ITKConfig.cmake
>>>>>
>>>>> Without this manual fix, I get the following error message:
>>>>>
>>>>> [ 80%] Generating wrap_ITKIOBase.xml
>>>>> In file included from
>>>>> /home/glehmann/src/tests/wrapitk/build/Libraries/IO/wrap_ITKIOBase.cxx:77:
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:35:24:
>>>>> error: openjpeg.h: No such file or directory
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:36:19:
>>>>> error: j2k.h: No such file or directory
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:37:19:
>>>>> error: jp2.h: No such file or directory
>>>>> In file included from
>>>>> /home/glehmann/src/tests/wrapitk/build/Libraries/IO/wrap_ITKIOBase.cxx:77:
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:111:
>>>>> error: 'opj_dparameters_t' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:130:
>>>>> error: ISO C++ forbids declaration of 'opj_codec_t' with no type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:130:
>>>>> error: expected ';' before '*' token
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:132:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:133:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:135:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:136:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:138:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:139:
>>>>> error: 'OPJ_UINT32' does not name a type
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h: In
>>>>> member function 'void itk::JPEG2000ImageIO::SetTileSize(int, int)':
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:102:
>>>>> error: 'm_TileWidth' was not declared in this scope
>>>>> /home/glehmann/src/tests/ITK/Code/Review/itkJPEG2000ImageIO.h:103:
>>>>> error: 'm_TileHeight' was not declared in this scope
>>>>> make[3]: *** [Libraries/IO/wrap_ITKIOBase.xml] Erreur 1
>>>>> make[2]: *** [Libraries/IO/CMakeFiles/IOIdx.dir/all] Erreur 2
>>>>> make[1]: *** [Libraries/IO/CMakeFiles/IOPython.dir/rule] Erreur 2
>>>>> make: *** [IOPython] Erreur 2
>>>>>
>>>>> Gaëtan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>     Thanks
>>>>>>
>>>>>>
>>>>>>         Luis
>>>>>>
>>>>>>
>>>>>> -------------------
>>>>>> 2010/8/27 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I found a small problem with openjpg: the include dir is not exported,
>>>>>> and thus openjpeg.h can't be found while using JPEG2000ImageIO outside of
>>>>>> the ITK build tree.
>>>>>> I tried to fix that but I'm not sure how to do it right.
>>>>>>
>>>>>> It would be nice if someone used to how the utilities are managed in
>>>>>> ITK can fix that!
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Gaëtan
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Gaëtan Lehmann
>>>>>> Biologie du Développement et de la Reproduction
>>>>>> INRA de Jouy-en-Josas (France)
>>>>>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>>>>>> http://voxel.jouy.inra.fr  http://www.itk.org
>>>>>> http://www.mandriva.org  http://www.bepo.fr
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Powered by www.kitware.com
>>>>>>
>>>>>> Visit other Kitware open-source projects at
>>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>>
>>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>>> http://kitware.com/products/protraining.html
>>>>>>
>>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>>
>>>>>> Follow this link to subscribe/unsubscribe:
>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>
>>>>>>
>>>>>>
>>>>> --
>>>>> Gaëtan Lehmann
>>>>> Biologie du Développement et de la Reproduction
>>>>> INRA de Jouy-en-Josas (France)
>>>>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>>>>> http://voxel.jouy.inra.fr  http://www.itk.org
>>>>> http://www.mandriva.org  http://www.bepo.fr
>>>>>
>>>>>
>>>>
>>> <ATT00001..txt>
>>>
>>>
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.html
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
>>>
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.html
>>
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100828/e0fcf64b/attachment.htm>


More information about the Insight-developers mailing list