[PATCH i-g-t v1 2/2] CONTRIBUTING: Add guide about igt libraries

[Kamil Konieczny]

Hi Zbigniew,
On 2025-03-31 at 12:14:12 +0200, Zbigniew Kempczyński wrote:
> On Thu, Mar 27, 2025 at 04:31:30PM +0100, Kamil Konieczny wrote:
> > Add some general guide about adding new library function and
> > a few guides for their usage outside of tests.
> > 
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

+cc Andrzej

> > ---
> >  CONTRIBUTING.md | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> > index b571b5e68..d85266a6c 100644
> > --- a/CONTRIBUTING.md
> > +++ b/CONTRIBUTING.md
> > @@ -38,6 +38,33 @@ The Code
> >  
> >  [igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
> >  
> > +IGT libraries
> > +-------------
> > +- Tests and benchmarks are the main usage of IGT libraries, so they
> > +  could use test specific macros/functions, for example igt_assert,
> > +  igt_require, igt_skip, igt_info or igt_debug.
> > +
> > +- New library function could be written when it will have at least two
> > +  different users, for example if it could be used by two or more tests.
> 
> I think there's some exception and single user could be allowed. When
> it is very likely function will be used in the future I would accept
> single user of such library function.
> 

Right, I will add:

In some cases single user can be accepted, when it is very
likely it will be used in future.

> > +
> > +- In a new library function():
> > +  if it has macros (for example igt_assert/igt_require/igt_skip) then
> > +    write also __function() counterpart without them;
> > +  limit also usage of prints igt_info/igt_debug, this should be a part
> > +    of test code.
> 
> What if function has some input requirements? As an example check
> blt_block_copy() and emit_blt_block_copy(). How to handle such cases
> without igt_assert?
> 

Hmm, I will change this to "then consider to write also..."
so it is not strictly required.

Regards,
Kamil

> > +
> > +- Libraries and igt_runner
> > +  Runner should not use lib functions. It is crucial for CI runs so using
> > +  libraries puts a risk of bringing changes meant for tests which in turn
> > +  could break runner.
> > +  Note: You will find places where igt_runner uses lib functions - this will
> > +  be on ToDo list to be fixed.
> 
> Agree, runner should be separated from igt library code.
> 
> --
> Zbigniew
> 
> > +
> > +- Libraries and tools/
> > +  Tools should try to not use lib functions. Any abnormal condition should
> > +  be simply reported by printf or fprintf to stdout/stderr and then tool
> > +  should exit gracefully. Do not use igt_abort nor igt_assert, igt_print,
> > +  igt_debug nor other testing/printing macros from igt lib/
> >  
> >  Sending Patches
> >  ---------------
> > -- 
> > 2.49.0
> >