Re: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling

[Marcos Paulo de Souza <mpdesouza@suse.com>]

On Fri, 23 Feb 2024 11:40:00 +0800 Yujie Liu <yujie.liu@intel.com> wrote:

> On Wed, Feb 21, 2024 at 09:29:47AM -0300, Marcos Paulo de Souza wrote:
> > On Mon, 19 Feb 2024 15:16:51 +0800 kernel test robot <lkp@intel.com> wrote:
> > 
> > > Hi Marcos,
> > > 
> > > kernel test robot noticed the following build warnings:
> > > 
> > > [auto build test WARNING on 345e8abe4c355bc24bab3f4a5634122e55be8665]
> > > 
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/selftests-lib-mk-Do-not-process-TEST_GEN_MODS_DIR/20240216-021601
> > > base:   345e8abe4c355bc24bab3f4a5634122e55be8665
> > > patch link:    https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc%40suse.com
> > > patch subject: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
> > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402191502.dALlSRz0-lkp@intel.com/reproduce)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202402191502.dALlSRz0-lkp@intel.com/
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> Makefile:11: warning: overriding recipe for target 'all'
> > > >> ../lib.mk:62: warning: ignoring old recipe for target 'all'
> > 
> > I couldn't reproduce this one locally. Shuah, have you seen this issue in your
> > setup? I followed the steps to reproduce this issue (it's the same sequence
> > already reported in earlier patches...)
> 
> Hi Marcos,
> 
> This seems to be a warning when compiling futex selftest.
> 
> linux/tools/testing/selftests/futex$ make
> Makefile:11: warning: overriding recipe for target 'all'
> ../lib.mk:62: warning: ignoring old recipe for target 'all'
> 
> There is no such warning if this patch is not applied. Looks like it is
> due to the "all" target is defined in lib.mk but overridden in futex
> Makefile? Could you please help take a look?

I believe that I understood what's going on:

Before this patch, the 'all' from lib.mk target didn't have a "recipe" only
dependencies. The TARGET_GEN_MODS_DIR variable was used while resolving the
dependencies from it.

The proposed patch simplified the process by removing the gen_mods_dir target,
and checked TARGET_GEN_MODS_DIR variable as a recipe.

Per my local tests, we can have two targets with the same name on two different
Makefiles (one that includes in the other in case), as long as only one of them
have a recipe (commands to execute). The dependencies of the target
on the included file would will be checked and executed either way.

But, if both targets have commands to execute, make will say the target was
overridden. In both cases, only the target from the file that includes the other
will execute. I believe this matches the current expectation of the futex
selftests, as they jump on into "functional" directory and execute the tests
there. That makefile also includes lib.mk...

It seems that planned to include more directories since they introduced the
selftests, but never did:


  commit 2aa8470f02a9b9e6a410d1264fe6c8fa6c402eff
  Author: Darren Hart <dvhart@linux.intel.com>
  Date:   Tue May 12 21:07:52 2015 -0700

      selftests: Add futex functional tests

      The futextest testsuite [1] provides functional, stress, and
      performance tests for the various futex op codes. Those tests will be of
      more use to futex developers if they are included with the kernel
      source.

      Copy the core infrastructure and the functional tests into selftests,
      but adapt them for inclusion in the kernel:

      - Update the Makefile to include the run_tests target, remove reference
        to the performance and stress tests from the contributed sources.
      - Replace my dead IBM email address with my current Intel email address.
      - Remove the warrantee and write-to paragraphs from the license blurbs.
      - Remove the NAME section as the filename is easily determined. ;-)
      - Make the whitespace usage consistent in a couple of places.
      - Cleanup various CodingStyle violations.

      A future effort will explore moving the performance and stress tests
      into the kernel.

      1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git

Either way, if my change adds a new warning, I think that we can drop that patch
and move on. OTOH, I believe that futex selftests could be simplified in order
to remove this awkward setup to run their tests.

> 
> Thanks,
> Yujie