2018-02-21 21:56:13

by Daniel Díaz

[permalink] [raw]
Subject: [PATCH 1/2] selftests: gpio: restructure Makefile

From: Fathi Boudra <[email protected]>

Signed-off-by: Fathi Boudra <[email protected]>
---
tools/testing/selftests/gpio/Makefile | 36 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 1bbb475..6890f61 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,31 +1,29 @@
# SPDX-License-Identifier: GPL-2.0

+CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
+LDLIBS += -lmount -I/usr/include/libmount
+
TEST_PROGS := gpio-mockup.sh
-TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
-BINARIES := gpio-mockup-chardev
-EXTRA_PROGS := ../gpiogpio-event-mon ../gpiogpio-hammer ../gpiolsgpio
-EXTRA_DIRS := ../gpioinclude/
-EXTRA_OBJS := ../gpiogpio-event-mon-in.o ../gpiogpio-event-mon.o
-EXTRA_OBJS += ../gpiogpio-hammer-in.o ../gpiogpio-utils.o ../gpiolsgpio-in.o
-EXTRA_OBJS += ../gpiolsgpio.o
+TEST_FILES := gpio-mockup-sysfs.sh
+TEST_PROGS_EXTENDED := gpio-mockup-chardev

-include ../lib.mk
+GPIODIR := ../../../gpio
+GPIOOBJ := gpio-utils.o
+GPIOINC := gpio.h

-all: $(BINARIES)
+all: $(GPIOINC) $(TEST_PROGS_EXTENDED)

override define CLEAN
- $(RM) $(BINARIES) $(EXTRA_PROGS) $(EXTRA_OBJS)
- $(RM) -r $(EXTRA_DIRS)
+ $(RM) $(TEST_PROGS_EXTENDED)
+ $(MAKE) -C $(GPIODIR) clean
endef

-CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
-LDLIBS += -lmount -I/usr/include/libmount
-
-$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
+include ../lib.mk

-../../../gpio/gpio-utils.o:
- make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)

-../../../../usr/include/linux/gpio.h:
- make -C ../../../.. headers_install INSTALL_HDR_PATH=$(shell pwd)/../../../../usr/
+$(GPIODIR)/$(GPIOOBJ):
+ $(MAKE) -C $(GPIODIR)

+$(GPIOINC):
+ $(MAKE) -C ../../../.. headers_install
--
2.7.4



2018-02-21 21:55:44

by Daniel Díaz

[permalink] [raw]
Subject: [PATCH 2/2] selftests/gpio: Fix OUTPUT directory in Makefile

When simply running `make' from the selftests top dir, this
error shows up:

cc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid gpio-mockup-chardev.c ../../../gpio/gpio-utils.o -lmount -o gpio-mockup-chardev
cc: error: ../../../gpio/gpio-utils.o: No such file or directory
<builtin>: recipe for target 'gpio-mockup-chardev' failed
make[1]: *** [gpio-mockup-chardev] Error 1

because the output directory is set to "selftests/gpio" and
all binaries built from ../../../gpio/ end up there. In fact,
they appear as, exempli gratia:
* gpiogpio-event-mon
* gpiogpio-hammer
* gpioinclude/
* gpiolsgpio
which is wrong, as it's missing directory separator somewhere.

This patch sets straight the output directory when building
../../../gpio/ so that binaries don't cross paths.

Signed-off-by: Daniel Díaz <[email protected]>
---
tools/testing/selftests/gpio/Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 6890f61..46a131a 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -7,7 +7,7 @@ TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
TEST_PROGS_EXTENDED := gpio-mockup-chardev

-GPIODIR := ../../../gpio
+GPIODIR := $(realpath ../../../gpio)
GPIOOBJ := gpio-utils.o
GPIOINC := gpio.h

@@ -15,7 +15,7 @@ all: $(GPIOINC) $(TEST_PROGS_EXTENDED)

override define CLEAN
$(RM) $(TEST_PROGS_EXTENDED)
- $(MAKE) -C $(GPIODIR) clean
+ $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean
endef

include ../lib.mk
@@ -23,7 +23,7 @@ include ../lib.mk
$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)

$(GPIODIR)/$(GPIOOBJ):
- $(MAKE) -C $(GPIODIR)
+ $(MAKE) OUTPUT=$(GPIODIR)/ -C $(GPIODIR)

$(GPIOINC):
$(MAKE) -C ../../../.. headers_install
--
2.7.4


2018-02-21 22:07:53

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] selftests: gpio: restructure Makefile

On Wed, Feb 21, 2018 at 11:52 PM, Daniel Díaz <[email protected]> wrote:
> From: Fathi Boudra <[email protected]>

What are you doing here?
Why?
What is the benefit of it?

> Signed-off-by: Fathi Boudra <[email protected]>

--
With Best Regards,
Andy Shevchenko

2018-02-21 22:10:51

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/gpio: Fix OUTPUT directory in Makefile

On Wed, Feb 21, 2018 at 11:52 PM, Daniel Díaz <[email protected]> wrote:
> When simply running `make' from the selftests top dir, this
> error shows up:
>
> cc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid gpio-mockup-chardev.c ../../../gpio/gpio-utils.o -lmount -o gpio-mockup-chardev
> cc: error: ../../../gpio/gpio-utils.o: No such file or directory
> <builtin>: recipe for target 'gpio-mockup-chardev' failed
> make[1]: *** [gpio-mockup-chardev] Error 1
>
> because the output directory is set to "selftests/gpio" and
> all binaries built from ../../../gpio/ end up there. In fact,
> they appear as, exempli gratia:
> * gpiogpio-event-mon
> * gpiogpio-hammer
> * gpioinclude/
> * gpiolsgpio
> which is wrong, as it's missing directory separator somewhere.
>
> This patch sets straight the output directory when building
> ../../../gpio/ so that binaries don't cross paths.

This patch doesn't sound right like previous one.
Does selftest infrastructure have it's own build system like tools?
Does it use tools' one?
What's wrong with the current approach?
Why do you need tools in selftests?

--
With Best Regards,
Andy Shevchenko

2018-02-21 22:46:45

by Daniel Díaz

[permalink] [raw]
Subject: Re: [PATCH 1/2] selftests: gpio: restructure Makefile

Hello!


On 02/21/2018 04:06 PM, Andy Shevchenko wrote:
> On Wed, Feb 21, 2018 at 11:52 PM, Daniel Díaz <[email protected]> wrote:
>> From: Fathi Boudra <[email protected]>
>
> What are you doing here?
> Why?
> What is the benefit of it?

Thanks for looking into this. I will resend with description.

The purpose of the patch is to clean up the Makefile by restructuring
explicit paths in targets for variables, substituting a variable
(BINARIES) for one of the build system (TEST_PROGS_EXTENDED), and proper
cleaning up of the EXTRA objects.

The resulting Makefile is much more readable and manageable.

Greetings!

Daniel Díaz
[email protected]

2018-02-21 23:06:10

by Daniel Díaz

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/gpio: Fix OUTPUT directory in Makefile

Hello!


On 02/21/2018 04:09 PM, Andy Shevchenko wrote:
> On Wed, Feb 21, 2018 at 11:52 PM, Daniel Díaz <[email protected]> wrote:
>> When simply running `make' from the selftests top dir, this
>> error shows up:
>>
>> cc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid gpio-mockup-chardev.c ../../../gpio/gpio-utils.o -lmount -o gpio-mockup-chardev
>> cc: error: ../../../gpio/gpio-utils.o: No such file or directory
>> <builtin>: recipe for target 'gpio-mockup-chardev' failed
>> make[1]: *** [gpio-mockup-chardev] Error 1
>>
>> because the output directory is set to "selftests/gpio" and
>> all binaries built from ../../../gpio/ end up there. In fact,
>> they appear as, exempli gratia:
>> * gpiogpio-event-mon
>> * gpiogpio-hammer
>> * gpioinclude/
>> * gpiolsgpio
>> which is wrong, as it's missing directory separator somewhere.
>>
>> This patch sets straight the output directory when building
>> ../../../gpio/ so that binaries don't cross paths.
>
> This patch doesn't sound right like previous one.
> Does selftest infrastructure have it's own build system like tools?

Yes. See linux/Documentation/dev-tools/kselftest.rst § "Contributing new
tests (details)" or linux/tools/testing/selftests/lib.mk itself, which
starts saying:
# This mimics the top-level Makefile. We do it explicitly here so that this
# Makefile can operate with or without the kbuild infrastructure.


> Does it use tools' one?

No, as far as I can tell. If anything, it uses it only to build inside
tools/gpio/ whatever is needed.


> What's wrong with the current approach?

When building from the tools/testing/selftests/ directory, binaries end
up in the current directory (and with an improper name), which is
undesirable. The gpio/ selftest fails to build when make'ing from there.

This is with current master:
/linux/tools/testing/selftests$ make
make[1]: Entering directory '/linux/tools/testing/selftests/gpio'
[...]
gcc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
gpio-mockup-chardev.c ../../../gpio/gpio-utils.o
../../../../usr/include/linux/gpio.h -lmount -I/usr/include/libmount -o
gpio-mockup-chardev
gcc: error: ../../../gpio/gpio-utils.o: No such file or directory
<builtin>: recipe for target 'gpio-mockup-chardev' failed
make[1]: *** [gpio-mockup-chardev] Error 1
make[1]: Leaving directory '/linux/tools/testing/selftests/gpio'
Makefile:32: recipe for target 'all' failed
make: *** [all] Error 2


> Why do you need tools in selftests?

I'll leave that to the test author to answer, as this is only a fix in
the Makefile, but the only part of tools needed here is tools/gpio/*,
for make'ing the object files this test will be built against.

Thanks and greetings!

Daniel Díaz
[email protected]


2018-02-21 23:12:34

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 2/2] selftests/gpio: Fix OUTPUT directory in Makefile

On 02/21/2018 04:04 PM, Daniel Díaz wrote:
> Hello!
>
>
> On 02/21/2018 04:09 PM, Andy Shevchenko wrote:
>> On Wed, Feb 21, 2018 at 11:52 PM, Daniel Díaz <[email protected]> wrote:
>>> When simply running `make' from the selftests top dir, this
>>> error shows up:
>>>
>>> cc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid gpio-mockup-chardev.c ../../../gpio/gpio-utils.o -lmount -o gpio-mockup-chardev
>>> cc: error: ../../../gpio/gpio-utils.o: No such file or directory
>>> <builtin>: recipe for target 'gpio-mockup-chardev' failed
>>> make[1]: *** [gpio-mockup-chardev] Error 1
>>>
>>> because the output directory is set to "selftests/gpio" and
>>> all binaries built from ../../../gpio/ end up there. In fact,
>>> they appear as, exempli gratia:
>>> * gpiogpio-event-mon
>>> * gpiogpio-hammer
>>> * gpioinclude/
>>> * gpiolsgpio
>>> which is wrong, as it's missing directory separator somewhere.
>>>
>>> This patch sets straight the output directory when building
>>> ../../../gpio/ so that binaries don't cross paths.
>>
>> This patch doesn't sound right like previous one.
>> Does selftest infrastructure have it's own build system like tools?
>
> Yes. See linux/Documentation/dev-tools/kselftest.rst § "Contributing new
> tests (details)" or linux/tools/testing/selftests/lib.mk itself, which
> starts saying:
> # This mimics the top-level Makefile. We do it explicitly here so that this
> # Makefile can operate with or without the kbuild infrastructure.
>
>
>> Does it use tools' one?
>
> No, as far as I can tell. If anything, it uses it only to build inside
> tools/gpio/ whatever is needed.
>
>
>> What's wrong with the current approach?
>
> When building from the tools/testing/selftests/ directory, binaries end
> up in the current directory (and with an improper name), which is
> undesirable. The gpio/ selftest fails to build when make'ing from there.
>
> This is with current master:
> /linux/tools/testing/selftests$ make
> make[1]: Entering directory '/linux/tools/testing/selftests/gpio'
> [...]
> gcc -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
> gpio-mockup-chardev.c ../../../gpio/gpio-utils.o
> ../../../../usr/include/linux/gpio.h -lmount -I/usr/include/libmount -o
> gpio-mockup-chardev
> gcc: error: ../../../gpio/gpio-utils.o: No such file or directory
> <builtin>: recipe for target 'gpio-mockup-chardev' failed
> make[1]: *** [gpio-mockup-chardev] Error 1
> make[1]: Leaving directory '/linux/tools/testing/selftests/gpio'
> Makefile:32: recipe for target 'all' failed
> make: *** [all] Error 2
>
>
>> Why do you need tools in selftests?
>
> I'll leave that to the test author to answer, as this is only a fix in
> the Makefile, but the only part of tools needed here is tools/gpio/*,
> for make'ing the object files this test will be built against.
>

This test depends on tools/libs built in tools/gpio. The original
commit has details on the benefits of adding this test. Adding author
to the thread for further comments.

22f6592b23ef8a0c09283bcb13087340721e1154
commit 22f6592b23ef8a0c09283bcb13087340721e1154
Author: Bamvor Jian Zhang <[email protected]>

thanks,
-- Shuah

2018-02-21 23:24:20

by Daniel Díaz

[permalink] [raw]
Subject: [PATCH v2] selftests: gpio: restructure Makefile

From: Fathi Boudra <[email protected]>

This patch cleans up the Makefile by restructuring a couple of
things, namely:
1) change explicit paths in targets for variables
2) substitute a variable (BINARIES) for another, part of the
selftests build system (TEST_PROGS_EXTENDED)
3) proper cleaning up of the EXTRA objects

The resulting Makefile is much more readable and manageable.

Signed-off-by: Fathi Boudra <[email protected]>
Signed-off-by: Daniel Díaz <[email protected]>
---
v2: Add commit description

tools/testing/selftests/gpio/Makefile | 36 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 1bbb475..6890f61 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,31 +1,29 @@
# SPDX-License-Identifier: GPL-2.0

+CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
+LDLIBS += -lmount -I/usr/include/libmount
+
TEST_PROGS := gpio-mockup.sh
-TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
-BINARIES := gpio-mockup-chardev
-EXTRA_PROGS := ../gpiogpio-event-mon ../gpiogpio-hammer ../gpiolsgpio
-EXTRA_DIRS := ../gpioinclude/
-EXTRA_OBJS := ../gpiogpio-event-mon-in.o ../gpiogpio-event-mon.o
-EXTRA_OBJS += ../gpiogpio-hammer-in.o ../gpiogpio-utils.o ../gpiolsgpio-in.o
-EXTRA_OBJS += ../gpiolsgpio.o
+TEST_FILES := gpio-mockup-sysfs.sh
+TEST_PROGS_EXTENDED := gpio-mockup-chardev

-include ../lib.mk
+GPIODIR := ../../../gpio
+GPIOOBJ := gpio-utils.o
+GPIOINC := gpio.h

-all: $(BINARIES)
+all: $(GPIOINC) $(TEST_PROGS_EXTENDED)

override define CLEAN
- $(RM) $(BINARIES) $(EXTRA_PROGS) $(EXTRA_OBJS)
- $(RM) -r $(EXTRA_DIRS)
+ $(RM) $(TEST_PROGS_EXTENDED)
+ $(MAKE) -C $(GPIODIR) clean
endef

-CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
-LDLIBS += -lmount -I/usr/include/libmount
-
-$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
+include ../lib.mk

-../../../gpio/gpio-utils.o:
- make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)

-../../../../usr/include/linux/gpio.h:
- make -C ../../../.. headers_install INSTALL_HDR_PATH=$(shell pwd)/../../../../usr/
+$(GPIODIR)/$(GPIOOBJ):
+ $(MAKE) -C $(GPIODIR)

+$(GPIOINC):
+ $(MAKE) -C ../../../.. headers_install
--
2.7.4