LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Shuah Khan <shuahkh@osg.samsung.com>
Cc: linux-kernel@vger.kernel.org, davej@codemonkey.org.uk,
	mmarek@suse.cz, linux-api@vger.kernel.org
Subject: Re: [PATCH v4 4/9] selftests: Add install target
Date: Thu, 12 Mar 2015 14:11:53 +1100	[thread overview]
Message-ID: <1426129913.25936.2.camel@ellerman.id.au> (raw)
In-Reply-To: <5500675A.1000700@osg.samsung.com>

On Wed, 2015-03-11 at 10:03 -0600, Shuah Khan wrote:
> On 03/11/2015 07:18 AM, Shuah Khan wrote:
> > 
> > I don't see my comments addressed. If you want me to take
> > this work, please address the following comments:
> > 
> > - Name install directory kselftest. It should work with the
> >   the use-case.
> > 
> >   make INSTALL_PATH=/tmp make install
> >   The install directory should be /tmp/kselftest
> > 
> > - Flatten the directory with all tests under /tmp/kselftest
> > 
> 
> Yes. There is the issue of duplicate executable names and data
> file to be concerned about. It is a name-space problem. It is
> not unique to this case, example /usr/bin. This problem is better
> solved without adding the complexity of hierarchical directory
> structure.
> 
> 1. Individual test Makefiles ensure unique names for their
>    executables and data files. This can be done as a process
>    when new tests are accepted.
> 2. Install process makes sure they are unique - this patch is
>    solving the problem by creating a directory hierarchy. A simpler
>    way to do it is by adding pre-fix to when there is name space
>    conflict.
> 
> The problem with directory hierarchy is for one thing:
> 
> 1. It is complex and way too many directories get created during
>    install and this problem will mushroom as more and more tests
>    get added.

One directory is created for each test directory under selftests. That's not
"way too many" directories.

Why does it make sense to have directories for the selftest sources, but not
the installed tests? Why don't we just put all the tests in one directory under
selftests? A: Because that would be stupid.

$ find tools/testing/selftests/ -maxdepth 1 -type d
tools/testing/selftests/
tools/testing/selftests/size
tools/testing/selftests/memory-hotplug
tools/testing/selftests/ipc
tools/testing/selftests/ptrace
tools/testing/selftests/timers
tools/testing/selftests/efivarfs
tools/testing/selftests/net
tools/testing/selftests/kcmp
tools/testing/selftests/mqueue
tools/testing/selftests/ftrace
tools/testing/selftests/mount
tools/testing/selftests/vm
tools/testing/selftests/firmware
tools/testing/selftests/cpu-hotplug
tools/testing/selftests/user
tools/testing/selftests/sysctl
tools/testing/selftests/powerpc
tools/testing/selftests/exec
tools/testing/selftests/memfd
tools/testing/selftests/breakpoints
tools/testing/selftests/rcutorture

Oh no directories, they're so complex!

> 2. User won't be able to define a simple path variable to invoke
>    individual tests. This is a user experience problem.

Who cares? No one does that. Many of the tests aren't just simple executables
that can be run via $PATH, they require test files in the current directory.

cheers



  reply	other threads:[~2015-03-12  3:11 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-11  4:05 [PATCH v4 1/9] kbuild: Don't pass -rR to selftest makefiles Michael Ellerman
2015-03-11  4:05 ` [PATCH v4 2/9] kbuild: Don't pass LDFLAGS to selftest Makefile Michael Ellerman
2015-03-11 16:08   ` Shuah Khan
2015-03-13 18:06   ` Shuah Khan
2015-03-11  4:05 ` [PATCH v4 3/9] selftests: Introduce minimal shared logic for running tests Michael Ellerman
2015-03-13 17:19   ` Shuah Khan
2015-03-13 20:28     ` Shuah Khan
2015-03-11  4:06 ` [PATCH v4 4/9] selftests: Add install target Michael Ellerman
2015-03-11 13:18   ` Shuah Khan
2015-03-11 16:03     ` Shuah Khan
2015-03-12  3:11       ` Michael Ellerman [this message]
2015-03-12  3:15     ` Michael Ellerman
2015-03-13 16:33       ` Dave Jones
2015-03-13 17:08         ` Shuah Khan
2015-03-13 17:20   ` Shuah Khan
2015-03-13 21:32     ` Shuah Khan
2015-03-13 23:08       ` Shuah Khan
2015-03-16  3:04       ` Michael Ellerman
2015-03-16  3:12         ` Michael Ellerman
2015-03-18 14:30         ` Shuah Khan
2015-03-11  4:06 ` [PATCH v4 5/9] selftests: Add install support for the powerpc tests Michael Ellerman
2015-03-11  4:06 ` [PATCH v4 6/9] selftests: Set CC using CROSS_COMPILE once in lib.mk Michael Ellerman
2015-03-11  4:06 ` [PATCH v4 7/9] selftests/timers: Use implicit rules Michael Ellerman
2015-03-11  4:06 ` [PATCH v4 8/9] selftests/mqueue: " Michael Ellerman
2015-03-11  4:06 ` [PATCH v4 9/9] selftests/mount: " Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1426129913.25936.2.camel@ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=davej@codemonkey.org.uk \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=shuahkh@osg.samsung.com \
    --subject='Re: [PATCH v4 4/9] selftests: Add install target' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).