On Sat, Jul 07, 2012 at 06:41:39PM +0200, Reinhard Tartler wrote: > On Sat, Jul 7, 2012 at 6:04 PM, Diego Biurrun <di...@biurrun.de> wrote: > > On Sat, Jul 07, 2012 at 08:54:12AM -0700, Ronald S. Bultje wrote: > >> On Sat, Jul 7, 2012 at 5:26 AM, Diego Biurrun <di...@biurrun.de> wrote: > >> > On Fri, Jul 06, 2012 at 09:59:50PM -0700, Ronald S. Bultje wrote: > >> >> On Fri, Jul 6, 2012 at 6:53 PM, Diego Biurrun <di...@biurrun.de> wrote: > >> >> > On Fri, Jul 06, 2012 at 10:33:19AM -0700, Ronald S. Bultje wrote: > >> >> >> On Fri, Jul 6, 2012 at 10:27 AM, Luca Barbato <lu_z...@gentoo.org> > >> >> >> wrote: > >> >> >> > On 07/06/2012 07:13 PM, Ronald S. Bultje wrote: > >> >> >> >> On Wed, Jul 4, 2012 at 2:09 PM, Måns Rullgård <m...@mansr.com> > >> >> >> >> wrote: > >> >> >> >>> "Ronald S. Bultje" <rsbul...@gmail.com> writes: > >> >> >> >>>> On Tue, Jul 3, 2012 at 8:14 PM, Ronald S. Bultje > >> >> >> >>>> <rsbul...@gmail.com> wrote: > >> >> >> >>>>> From: "Ronald S. Bultje" <rsbul...@gmail.com> > >> >> >> >>>>> > >> >> >> >>>>> This allows compiling and running these tests on systems > >> >> >> >>>>> lacking a built- > >> >> >> >>>>> in version of getopt(), such as MSVC. > >> >> >> >>>>> --- > >> >> >> >>>>> configure | 2 ++ > >> >> >> >>>>> libavcodec/dct-test.c | 7 +++++ > >> >> >> >>>>> libavcodec/fft-test.c | 6 ++++ > >> >> >> >>>>> libavcodec/getopt.c | 84 > >> >> >> >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++ > >> >> >> >>>>> 4 files changed, 99 insertions(+) > >> >> >> >>>>> create mode 100644 libavcodec/getopt.c > >> >> >> >>>> > >> >> >> >>>> Ping. > >> >> >> >>> > >> >> >> >>> No matter what, a replacement getopt.c does *not* belong in > >> >> >> >>> libavcodec/ > >> >> >> >> > >> >> >> >> So where does it go? Also, ping re: rest of the patch. > >> >> >> > > >> >> >> > Ops my email got lost... > >> >> >> > > >> >> >> > libavutil probably, is it the only place in which getopt is used? > >> >> >> > >> >> >> git says: > >> >> >> tools/graph2dot.c > >> >> >> libavcodec/motion-test.c > >> >> >> libavcodec/fft-test.c > >> >> >> libavcodec/dct-test.c > >> >> > > >> >> > IMO this is not worth the trouble. Test for getopt in configure and > >> >> > compile those programs conditionally. > >> >> > >> >> They're part of fate. > >> > > >> > So? Just run the fate tests conditionally as well. > >> > > >> >> I don't understand the trouble part. I already did all the effort. > >> >> What more trouble could there possibly be? Is deciding where to put > >> >> getopt.c too much trouble? > >> > > >> > The trouble is having ever more replacements for basic system functions > >> > in libav. That creates a maintenance burden going into the future, > >> > which is in no way worth the gain of running two tests under MSVC. > >> > >> It is already written, so there is no burden. > > I concur with Luca, libavutil seems like the proper place for the > getopt replacement. > > > > > Let me be more explicit then: This patch is rejected. > > > >> Libavutil/ sounds ok to me for placement, any other comments (Mans?) > >> or can I commit it as such? I can also add a new directory called > >> "libbrokenos/" and place it there if people prefer that. > > > > No. > > Why? What's the problem with the getopt replacement? Having FATE > running on a msvc compiler sounds like a worthwile goal for me. > And replacing the test programs that currently use getopt with > something that does not seems just ridiculous to me. So what do you > suggest to achieve the goal instead?
There is no harm in not running fft-test and dct-test as part of fate on msvc targets IMO. There are plenty of other tests that exercise that code. Furthermore we already have native option parsing code that we can leverage. Those tests are simply not essential parts of fate, unlike the code they test (in isolation). Just witness how late those tests were added to the fate framework. Diego _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
