Re: [Quilt-dev] Term::ReadKey in test/run

[John Vandenberg]

On 9/6/05, Andreas Gruenbacher <address@hidden> wrote:
> On Tuesday 06 September 2005 11:54, John Vandenberg wrote:
> > On 9/6/05, Jean Delvare <address@hidden> wrote:
> > > Hi John,
> > >
> > > [John Vandenberg]
> > >
> > > > Could the following patch be applied to allow test/run to work without
> > > > Term::ReadKey?  It isnt installed by default on several unix/linux
> > > > favours I have run into, and I haven't found a way to compile it into
I> > > > MSYS perl.
> > >
> > > I didn't realize that Term::ReadKey was solely used to get the number of
> > > columns the terminal has. Sounds a bit overkill to me, especially since
> > > this module isn't part of the standard distribution.
> 
> Maybe there is another Perl module that's generally available. $ENV{COLUMNS}
> isn't set on many systems, at least as fas as I can see. Bash here defines it
> as a variable, but doesn't export it; tcsh doesn't define it at all.
> 
> Instead of the BEGIN hack, if we don't find a better solution, let's hardcode
> this to 80, or use a command-line option.
> 
> > Sounds like you are running into a lot of problems I've already
> > addressed; albeit without your elegance :)
> >
> > You may want to pore through this series of patches.
> >
> > http://zeroj.hda0.net/quilt-patches/series-all
> >
> > I tried to use "quilt mail" to submit this smaller series on Saturday
> > night, but it appears to have never made it.  I tested sending it to
> > another email address, which worked.  I was waiting for bounces before
> > investigating further.
> >
> > http://zeroj.hda0.net/quilt-patches/series
> >
> > Specifically for RH 6, these may be causing you grief ...
> >
> > config-nls.diff
> 
> The patch uses ``head -1'', which is not POSIX compiant. Instead, please use
> ``head -n 1''.
> 
> > config-misc.diff
> 
> Adds two tests like ``if test -x "@foo@"; then "@foo@"; else ...; fi''. How
> about setting @foo@ to a suitable replacement value instead of testing here?
> In the case of TAC, why not do this in configure.ac:
> 
>    TAC="perl -e 'print reverse <>'"
>    AC_SUBST(TAC)

ok

> For column:
> 
>    COLUMN="cat"
>    AC_SUBST(COLUMN)
> 
> This requires to not put @foo@ in quotes...

"@COLUMN@ -t" will do the wrong thing. I understand your point though;
I dont mind looking for other options for that one.
n- 
> Is the getopt replacement your own, or did you find it somewhere else? In the
> latter case, maybe we should add a reference for picking up future fixes,
> etc.

It is my own code.  Included is a license to explicitly place it into
the public domain so it can be used elsewhere; I dont mind what
license is used when it is checked in.

> > test-example1.diff
> 
> Patch has a change to test/run:
> 
> +      $line =~ s/(sed .*)\\t/\1\t/g;  # convert \t to tab
> 
> Can we please fix this in the test cases instead, by either using $'\t' or
> literal tab characters?

I like $'\t', however that depends on bash.  If you have time, could
you take a look at this patch:

http://zeroj.hda0.net/quilt-patches/bash-2.04.diff

> test/example1.test:
> 
> -       $ sed -e "4aFetch me that flower; the herb I shew'd thee once:" 
> Oberon.txt >
> Oberon.new
> +       $ awk '{print $0;if(FNR==4)print "'"Fetch me that flower; the herb I 
> shew'd
> thee once:"'"}' Oberon.txt > Oberon.new
> 
> Do these work?
>    sed -e '1,3d'
>    sed -ne '1,3p'
> 
> If so; I'd prefer this:
> 
>    sed -ne '1,3p' Oberon.txt > Oberon.new
>    echo "Fetch me that flower; the herb I shew'd thee once:" >> Oberon.new
>    sed -e '1,3d' Oberon.txt >> Oberon.new

Nice.  It should be fine.  I'll test tomorrow.

> > Also, see the first item in my TODO:
> >
> > - move.test in pop-overwrite-deleted.diff exhibits erratic behaviour in pop
> >   Affects: MSYS(GnuWin32), SuSE 7.2
> >
> >   Often the overwrite operation will proceed without error (Serious
> > dataloss)
> >
> >   Fedora Core 3 2.6.10-1.766 always halts pop attempting to overwrite.
> 
> Do you refer to pop-verbose-fail.diff here? I guess yes. I like the approach,
> and I don't readily see what's broken on some systems. A ``quilt
> --trace ...'' trace would probably show what's wrong.

The problem is that on a few platforms, it intermittantly doesn't
halt.  On msys it doesnt halt around 10% of the time; on SuSE 7.2 it
was a smaller percent.  I couldnt reproduce the problem with --trace,
so I put it on my todo.

> Why do you not also report when a file that is expected to exist does not
> exist? Irrespective of why pop failed, the two ways to proceed are either
> refresh, or pop with -f, so I don't see why you are changing that in the
> patch.

To recap, the scenario that I think could act differently is when a
file is deleted in a patch, but turns up unexpectedly afterwards,
typically because it's now a generated file.

Popping the patches will halt if the generated file is different from
the original file.  In this scenario, the pop halts on the patch above
the file delete, and using refresh is useless, because the generated
file isnt managed by quilt until the patch beneath.

> I don't think we should speculate why a file is there or missing when
> it shouldn't be, so I disagree with that part.

pop -f works fine, so I would be satisfied to have the user informed
that this is a different type of failure, and pop -f is the only
option.  However, because there is only one option, and -a should
automaticly pop the patches, I personally would prefer to end up at
the requested patch, provided I know that deleted files will re-appear
and any local changes not managed by quilt could be destroyed. 
Ultimately, im not concerned about this. pop -f works satifactorily
for me.

Thanks for the comments.
--
John