Re: [PATCH i-g-t 0/3] Convert sh scripts to C variants.

Daniel Vetter
2016-10-24T08:49:25Z

On Mon, Oct 24, 2016 at 10:46:14AM +0200, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 10:38:17AM +0300, Joonas Lahtinen wrote:
> > On pe, 2016-10-21 at 00:00 +0300, Jani Nikula wrote:
> > > > On Thu, 20 Oct 2016, Marius Vlad <marius.c.vlad@intel.com> wrote:
> > > >
> > > > This series adds some library support to help converting sh
> > > > scripts to C version. Converted drv_module_reload_basic and
> > > > kms_sysfs_edid_timing.
> > >
> > > >
> > > >  18 files changed, 600 insertions(+), 180 deletions(-)
> > >
> > > Someone please justify this, plus pulling in two new dependencies. I can
> > > think of a thing or two, but it needs to be in the commit messages. And
> > > I'm not convinced by the justification I came up with.
>
> - Be able to reuse the subtest/logging/whatever else igt stuff for more
> consistency, without having to reinvent the wheel in the bash world a
> 2nd time. Often this is also tricky (we have unit tests for igt
> libraries for a reason). I rejected C++ as a new language for similar
> reasons, I think getting rid of bash is useful.
>
> - dmesg logging. Imo the piglit dmesg capturing serious sucks, it'd be
> great to move it into igt. Reasons for that: a) in C it's much faster,
> b) integrated with igt logging (consistent timestamps, ordering,
> crashbox log, ...) c) we could put the filtering of dmesg next to the
> tests, atm it's some rough filter in piglit's igt.py. c) has been one of
> Chris' wishlist things, e.g. make underrun tests hunt for underruns
> explicitly.
>
> - It's a nice ramp-up task for igt, that's why I bumped it up a bit. Would
> be fairly low-prio otherwise.
>
> > Hmm, not sure how Daniel instructed things. Original idea was to just
> > execv the same commands as the scripts do. To get rid of the
> > interpreter differences and allow running in a minimal environment.
>
> Yeah, I was thinking of a pretty minimal conversion to C with just lots of
> calls to system. Maybe long-term we could extract some shared code, but
> meh. Creating good libraries is a lot more work than it generally looks
> like, and code reuse for code reuse's sake is in my experience often not
> worth the hassle.

On that note, we'd probably need an igt helper to run a shell command with
full igt integration. The important bits there would be to read
stderr/stdout and feed them into igt_warn and igt_debug levels. Of course
in parallel to running the command to make sure the timestamps are
correct, and will line up with the dmesg timestampes. And long-term also
interleave with dmesg output (once we've moded that into igt proper).

That function also needs some flags to optionally ignore stderr or stdout,
which essentially would be reimplenting > /dev/null and &> /dev/null.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation