On Thu, Jul 07, 2022 at 08:09:36PM +0200, David Disseldorp wrote:
> On Wed, 6 Jul 2022 23:54:52 +0200, David Disseldorp wrote:
>
> > Thanks for the follow-up feedback, Darrick...
> >
> > On Wed, 6 Jul 2022 12:00:07 -0700, Darrick J. Wong wrote:
> ...
> > > >
> > > > +# retain files which would be overwritten in subsequent reruns of the same test
> > > > +_stash_fail_loop_files() {
> > > > + local test_seq="$1"
> > > > + local suffix="$2"
> > > > +
> > > > + for i in "${REPORT_DIR}/${test_seq}.full" \
> > > > + "${REPORT_DIR}/${test_seq}.dmesg" \
> > > > + "${REPORT_DIR}/${test_seq}.out.bad"; do
> > > > + [ -f "$i" ] && cp "$i" "${i}${suffix}"
> > >
> > > I wonder, is there any particular reason to copy the output file and let
> > > it get overwritten instead of simply mv'ing it?
> >
> > The copy is left over from an earlier version I had where xunit report
> > generation was done after the copy. Looking closer:
> > - .full is removed in _begin_fstest()
> > - _check_dmesg() overwrites .dmesg and retains on failure or KEEP_DMESG
> > - out.bad is removed in the main check loop prior to seq invocation
> > - .notrun, .core and .hints are also removed in the check loop at
> > various places before seq (.hints again in _begin_fstest())
> >
> > One concern I have in changing this to a move is that external scripts
> > may check for presence / parse these files after check invocation. I'd
> > considered moving and then copying / symlinking back the .rerun0 files
> > on rerun-on-failure loop completion but that's also pretty ugly. IMO
> > leaving this as a copy, with the non-suffix file state left to reflect
> > the results of the last rerun-on-failure loop, would make the most
> > sense for now.
>
> As a follow up here, I plan on squashing in the following change to
> cover the extra notrun, core and hints files, and also avoid stale
> .rerun# state:
OK, will wait your v4 of this patch, hope to catch the release of this week.
Thanks,
Zorro
>
> --- a/check
> +++ b/check
> @@ -560,13 +560,14 @@ _expunge_test()
>
> # retain files which would be overwritten in subsequent reruns of the same test
> _stash_fail_loop_files() {
> - local test_seq="$1"
> - local suffix="$2"
> + local seq_prefix="${REPORT_DIR}/${1}"
> + local cp_suffix="$2"
>
> - for i in "${REPORT_DIR}/${test_seq}.full" \
> - "${REPORT_DIR}/${test_seq}.dmesg" \
> - "${REPORT_DIR}/${test_seq}.out.bad"; do
> - [ -f "$i" ] && cp "$i" "${i}${suffix}"
> + for i in ".full" ".dmesg" ".out.bad" ".notrun" ".core" ".hints"; do
> + rm -f "${seq_prefix}${i}${cp_suffix}"
> + if [ -f "${seq_prefix}${i}" ]; then
> + cp "${seq_prefix}${i}" "${seq_prefix}${i}${cp_suffix}"
> + fi
> done
> }
>
> Cheers, David
>
