[Insight-developers] GDCM empty cxx files

Bill Lorensen bill.lorensen at gmail.com
Sun Mar 27 13:30:45 EDT 2011 

There have been action on your reviews:
http://review.source.kitware.com/#change,1173

but not this one:
http://review.source.kitware.com/#change,1213

Also, try to follow gerrit process defined here:
http://www.itk.org/Wiki/ITK/Git/Develop

Your gerrit topics should be submitted with a topic master. It is always
best to work on a branch:

git checkout -b myBranch
do your thing
git commit -a
git gerrit-push


On Sun, Mar 27, 2011 at 1:04 PM, Mark Roden <mmroden at gmail.com> wrote:

> Hi Hans,
>
> If it's acceptable to wallpaper over the warnings, we can go ahead and
> pragma them out for now.
>
> As for the lack of response, my understanding is that I have to go
> through the gerrit process to submit fixes.  I went through said
> process, and my submission received no attention.  This happened
> twice.  So, if my understanding of the process is wrong and I can
> expedite these things (should I be sending poking emails to people to
> check my commits?), then please correct me.
>
> Mark
>
> On Sun, Mar 27, 2011 at 7:36 AM, Johnson, Hans J <hans-johnson at uiowa.edu>
> wrote:
> > Mark,
> >
> > If the larger GDCM issues are being addressed outside of the ITK
> > development process, then perhaps you could just silence the current ITK
> > warnings and failing tests, and explicitly state in the commit message
> > that this is a short term "problem avoidance in ITK" because the "problem
> > solution is being addressed out-of-band and will contributed back to ITK
> > proper when it is complete".   I may not be correct here, but the biggest
> > problem with the GDCM compiler warnings and testing problems is that it
> is
> > adding a lot of noise to the efforts of squashing other problems.
> >
> > Regarding you comments of ITK developers not responding to your request
> to
> > fist test.  I want to remind you the YOU are THEY,  that is to say that
> > you are empowered (and likely the most qualified) to fix these problems.
> >
> > Hans
> >
> > On 3/26/11 4:57 PM, "Mark Roden" <mmroden at gmail.com> wrote:
> >
> >>Hi Alex,
> >>
> >>The reality is, the tests just aren't there in ITK proper.  Fixing the
> >>warnings in ITK as they stand now will cause tests in GDCM proper to
> >>fail, which means that many files can't be read.  Bill's most recent
> >>submission, for instance, will cause two GDCM tests to fail, but won't
> >>cause any ITK tests to fail, since the test coverage just isn't there.
> >> Couple that with the lack of response I got for my submissions in
> >>ITK, and it's just faster for me to work in GDCM proper, fix the
> >>warnings there with as much test coverage as I can get (and getting
> >>those tests solved is no mean feat, especially since they're exposing
> >>some interesting corner cases), and then port to ITK knowing that the
> >>tests are working.
> >>
> >>The breaking test in ITK, the one that's not been working for many
> >>months now, is just not a correct test.  The test itself was made a
> >>test using a broken, invalid DICOM test, and then breaks because GDCM
> >>refuses to read the broken image, which is the proper behavior as
> >>specified in the DICOM specification.  The test still hasn't been
> >>fixed after both Mathieu and I have repeatedly pointed out that it's a
> >>broken test, and attempts to fix it have been ignored.
> >>
> >>So, I believe that working in GDCM proper is the right thing to do to
> >>fix these warnings in some kind of timely manner.  The ITK development
> >>process has just been too slow, and I want to ensure that
> >>functionality isn't broken in ITK that works in GDCM proper due to
> >>attempts to fix warnings quickly.  I would also love it if the GDCM
> >>tests and testing library could be included in ITK's test suite, but I
> >>don't know how that would work.
> >>
> >>Mark
> >>
> >>On Sat, Mar 26, 2011 at 12:41 AM, Alexandre GOUAILLARD
> >><agouaillard at gmail.com> wrote:
> >>> mark,
> >>>
> >>> as indicated many times,
> >>> our duty goes to ITK and not to gdcm proper.
> >>> The priority is to fix the warning on the itk dashboard first.
> >>> Brad king mentionned you could modify in ITK and backport to gdcm
> later.
> >>> I explained to you how you could also have a separate git rep. to be
> >>> the buffer between gdcm and itk if need be.
> >>>
> >>> now, our duty goes to ITK. The priority is to make the ITK dashboard
> >>>green
> >>>
> >>> alex.
> >>>
> >>>
> >>> On Sat, Mar 26, 2011 at 6:14 AM, Bill Lorensen
> >>><bill.lorensen at gmail.com> wrote:
> >>>> Whatever is the best approach to fix the gdcm warnings. They are the
> >>>>major
> >>>> source of warnings are really getting annoying. They have been present
> >>>>since
> >>>> last summer.  I will continue to fix gdcm warnings until I see them
> >>>>dropping
> >>>> through another means.
> >>>>
> >>>> On Fri, Mar 25, 2011 at 6:10 PM, Mark Roden <mmroden at gmail.com>
> wrote:
> >>>>>
> >>>>> Ah, thanks for that.
> >>>>>
> >>>>> Since it took so long for my last gerrit patch to get a review, I've
> >>>>> switched to fixing tests and warnings in the gdcm main branch.  My
> >>>>> hope was to be able to drop that chunk of code into ITK, with the
> >>>>> warnings fixed there.  Many of them have already been fixed in ITK,
> >>>>> but moving over fixes from ITK to gdcm main produced some failing
> gdcm
> >>>>> tests.  These tests don't exist in ITK (hence removing the GDCM build
> >>>>> option from the ITK cmake options), and the ITK gdcm tests appear to
> >>>>> be building and running just fine (with the exception of IO test 5,
> >>>>> and that looks to be the same broken test it's always been, and
> should
> >>>>> be removed from the test suite).
> >>>>>
> >>>>> Is this approach a valid one?  I mean, it's taken around 2 weeks for
> >>>>> someone to review an ITK gerrit patch from me, but in the meantime,
> >>>>> I've been able to make much faster progress in the gdcm main branch
> >>>>> with this approach.  And, I think, a safer one, since there is more
> >>>>> complete test coverage.
> >>>>>
> >>>>> Mark
> >>>>>
> >>>>> On Fri, Mar 25, 2011 at 3:05 PM, Bill Lorensen
> >>>>><bill.lorensen at gmail.com>
> >>>>> wrote:
> >>>>> > I have the fix in my gerrit patch.
> >>>>> >
> >>>>> > On Fri, Mar 25, 2011 at 3:33 PM, Mark Roden <mmroden at gmail.com>
> >>>>>wrote:
> >>>>> >>
> >>>>> >> I thought I submitted a patch for this as well, but it probably
> >>>>>didn't
> >>>>> >> make it over the ITK modularization boundary.
> >>>>> >>
> >>>>> >> The last patch I submitted was done after the ITK changeover, but
> I
> >>>>> >> guess the changeover was ongoing when I submitted the patch, and
> so
> >>>>> >> now I have to redo it.  That's the
> >>>>> >> getting-rid-of-superfluous-cmake-options patch.
> >>>>> >>
> >>>>> >> On Fri, Mar 25, 2011 at 12:21 PM, Ryan, William J.
> >>>>> >> <ryan.william at mayo.edu> wrote:
> >>>>> >> > I think Mark Roden had a patch submitted that may fix this, but
> >>>>>I'm
> >>>>> >> > not
> >>>>> >> > 100%
> >>>>> >> > sure about that.  Let's see what he has to say.
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >   Bill Ryan
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >
> >>>>> >> > From: insight-developers-bounces at itk.org
> >>>>> >> > [mailto:insight-developers-bounces at itk.org] On Behalf Of Bill
> >>>>> >> > Lorensen
> >>>>> >> > Sent: Friday, March 25, 2011 1:23 PM
> >>>>> >> > To: Mathieu Malaterre
> >>>>> >> > Cc: Insight Developers
> >>>>> >> > Subject: Re: [Insight-developers] GDCM empty cxx files
> >>>>> >> >
> >>>>> >> >
> >>>>> >> >
> >>>>> >> > Mathieu,
> >>>>> >> >
> >>>>> >> > We still support VS7.1. I would like to submit a patch that
> >>>>>removes
> >>>>> >> > these
> >>>>> >> > files from the lib and see if it works.
> >>>>> >> >
> >>>>> >> > Unfortunately, the VS generator does not work with the
> >>>>>modularized
> >>>>> >> > ITK.
> >>>>> >> > We
> >>>>> >> > may not be able to thoroughly test it until the patched cmake is
> >>>>> >> > released.
> >>>>> >> >
> >>>>> >> > Thanks,
> >>>>> >> >
> >>>>> >> > Bill
> >>>>> >> >
> >>>>> >> > On Fri, Mar 25, 2011 at 2:13 PM, Mathieu Malaterre
> >>>>> >> > <mathieu.malaterre at gmail.com> wrote:
> >>>>> >> >
> >>>>> >> > From the top of my head, this was V7.1 with /MT as compilation
> >>>>>flag
> >>>>> >> > (instead of the default /MD used by Cmake). Can't remember if
> >>>>>this
> >>>>> >> > was
> >>>>> >> > using shared or static lib.
> >>>>> >> >
> >>>>> >> > Does anyone still have this compiler ?
> >>>>> >> >
> >>>>> >> > HTH
> >>>>> >> >
> >>>>> >> > On Fri, Mar 25, 2011 at 7:06 PM, Bill Lorensen
> >>>>> >> > <bill.lorensen at gmail.com>
> >>>>> >> > wrote:
> >>>>> >> >> Mathieu,
> >>>>> >> >>
> >>>>> >> >> Several files including gdcmObject.cxx have no code in them.
> >>>>>Your
> >>>>> >> >> comment
> >>>>> >> >> in
> >>>>> >> >> gdcmObject.cxx says:
> >>>>> >> >>   // Don't ask why, but this is EXTREMELY important on Win32
> >>>>> >> >>   // Apparently the compiler is doing something special the
> >>>>>first
> >>>>> >> >> time
> >>>>> >> >> it
> >>>>> >> >> compiles
> >>>>> >> >>   // this instanciation unit
> >>>>> >> >>   // If this fake file is not present I get an unresolved
> >>>>>symbol for
> >>>>> >> >> each
> >>>>> >> >> function
> >>>>> >> >>   // of the gdcm::Object class
> >>>>> >> >>
> >>>>> >> >> When linking these "empty" files, VS10 reports several
> warnings:
> >>>>> >> >>  warning LNK4221: This object file does not define any
> >>>>>previously
> >>>>> >> >> undefined
> >>>>> >> >> public symbols, so it will not be used by any link operation
> >>>>>that
> >>>>> >> >> consumes
> >>>>> >> >> this library
> >>>>> >> >>
> >>>>> >> >> I have removed this file from the lib as well as
> >>>>> >> >> gdcmProgressEvent.cxx, gdcmString.cxx, gdcmException.cxx
> >>>>> >> >> gdcmDeflateStream.cxx and gdcmByteSwap.cxx
> >>>>> >> >>
> >>>>> >> >> When I build the modified lib, I do not get the warnoings, nor
> >>>>>do I
> >>>>> >> >> get
> >>>>> >> >> any
> >>>>> >> >> errors.
> >>>>> >> >>
> >>>>> >> >> What compiler was giving the errors you mention in the
> comments?
> >>>>> >> >>
> >>>>> >> >> Bill
> >>>>> >> >>
> >>>>> >> >
> >>>>> >> >
> >>>>> >> > --
> >>>>> >> > Mathieu
> >>>>> >> >
> >>>>> >> >
> >>>>> >
> >>>>> >
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> 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
> >
> >
> >
> > ________________________________
> > Notice: This UI Health Care e-mail (including attachments) is covered by
> the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is
> confidential and may be legally privileged.  If you are not the intended
> recipient, you are hereby notified that any retention, dissemination,
> distribution, or copying of this communication is strictly prohibited.
>  Please reply to the sender that you have received the message in error,
> then delete it.  Thank you.
> > ________________________________
> >
> _______________________________________________
> 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/20110327/901317a6/attachment.htm>

More information about the Insight-developers mailing list