diff mbox series

selftests: introduce gen_tar Makefile target

Message ID 20200428123841.2953099-1-vkabatov@redhat.com (mailing list archive)
State Changes Requested
Headers show
Series selftests: introduce gen_tar Makefile target | expand

Commit Message

Veronika Kabatova April 28, 2020, 12:38 p.m. UTC
The gen_kselftest_tar.sh always packages *all* selftests and doesn't
pass along any variables to `make install` to influence what should be
built. This can result in an early error on the command line ("Unknown
tarball format TARGETS=XXX"), or unexpected test failures as the
tarball contains tests people wanted to skip on purpose.

Since the makefile already contains all the logic, we can add a target
for packaging. Keep the default .gz target the script uses, and actually
extend the supported formats by using tar's autodetection.

To not break current workflows, keep the gen_kselftest_tar.sh script as
it is, with an added suggestion to use the makefile target instead.

Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
 Documentation/dev-tools/kselftest.rst        | 23 ++++++++++++++++++++
 tools/testing/selftests/Makefile             |  9 +++++++-
 tools/testing/selftests/gen_kselftest_tar.sh |  5 +++++
 3 files changed, 36 insertions(+), 1 deletion(-)

Comments

Shuah May 1, 2020, 2:49 p.m. UTC | #1
Hi Veronica,

On 4/28/20 6:38 AM, Veronika Kabatova wrote:
> The gen_kselftest_tar.sh always packages *all* selftests and doesn't
> pass along any variables to `make install` to influence what should be
> built. This can result in an early error on the command line ("Unknown
> tarball format TARGETS=XXX"), or unexpected test failures as the
> tarball contains tests people wanted to skip on purpose.
> 
> Since the makefile already contains all the logic, we can add a target
> for packaging. Keep the default .gz target the script uses, and actually
> extend the supported formats by using tar's autodetection.
> 

Thanks for working on this. gen_kselftest_tar.sh  a while back before a
lot of the install features went in and Makefile supports it fully. It
makes perfect sense to use Makefile drive this.

> To not break current workflows, keep the gen_kselftest_tar.sh script as
> it is, with an added suggestion to use the makefile target instead.
> 

Not sure how many people use this. It is a good idea keeping it around
for now.

> Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>   Documentation/dev-tools/kselftest.rst        | 23 ++++++++++++++++++++
>   tools/testing/selftests/Makefile             |  9 +++++++-
>   tools/testing/selftests/gen_kselftest_tar.sh |  5 +++++
>   3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> index 61ae13c44f91..3fc559bcb597 100644
> --- a/Documentation/dev-tools/kselftest.rst
> +++ b/Documentation/dev-tools/kselftest.rst
> @@ -151,6 +151,29 @@ note some tests will require root privileges::
>      $ cd kselftest
>      $ ./run_kselftest.sh
>   
> +Packaging selftests
> +===================
> +
> +In some cases packaging is desired, such as when tests need to run on a
> +different system. To package selftests, run::
> +
> +   $ make -C tools/testing/selftests gen_tar
> +

Does this work in the case of relocatable build.cross-build cases?

> +This generates a tarball in the `INSTALL_PATH/kselftest-packages` directory. By
> +default, `.gz` format is used. The tar format can be overriden by specifying
> +a `FORMAT` make variable. Any value recognized by `tar's auto-compress`_ option
> +is supported, such as::
> +
> +    $ make -C tools/testing/selftests gen_tar FORMAT=.xz
> +
> +`make gen_tar` invokes `make install` so you can use it to package a subset of
> +tests by using variables specified in `Running a subset of selftests`_
> +section::
> +
> +    $ make -C tools/testing/selftests gen_tar TARGETS="bpf" FORMAT=.xz

Does this work in the case of relocatable build.cross-build cases?

Please try these cases as well and let me know.

I would like to get this in for 5.8-rc1.

thanks,
-- Shuah
Veronika Kabatova May 4, 2020, 11:50 a.m. UTC | #2
----- Original Message -----
> From: "shuah" <shuah@kernel.org>
> To: "Veronika Kabatova" <veronicca114@gmail.com>
> Cc: sbrivio@redhat.com, linux-kselftest@vger.kernel.org, "Veronika Kabatova" <vkabatov@redhat.com>, "shuah"
> <shuah@kernel.org>
> Sent: Friday, May 1, 2020 4:49:34 PM
> Subject: Re: [PATCH] selftests: introduce gen_tar Makefile target
> 
> Hi Veronica,
> 

Hi,

> On 4/28/20 6:38 AM, Veronika Kabatova wrote:
> > The gen_kselftest_tar.sh always packages *all* selftests and doesn't
> > pass along any variables to `make install` to influence what should be
> > built. This can result in an early error on the command line ("Unknown
> > tarball format TARGETS=XXX"), or unexpected test failures as the
> > tarball contains tests people wanted to skip on purpose.
> > 
> > Since the makefile already contains all the logic, we can add a target
> > for packaging. Keep the default .gz target the script uses, and actually
> > extend the supported formats by using tar's autodetection.
> > 
> 
> Thanks for working on this. gen_kselftest_tar.sh  a while back before a
> lot of the install features went in and Makefile supports it fully. It
> makes perfect sense to use Makefile drive this.
> 
> > To not break current workflows, keep the gen_kselftest_tar.sh script as
> > it is, with an added suggestion to use the makefile target instead.
> > 
> 
> Not sure how many people use this. It is a good idea keeping it around
> for now.
> 
> > Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
> > Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >   Documentation/dev-tools/kselftest.rst        | 23 ++++++++++++++++++++
> >   tools/testing/selftests/Makefile             |  9 +++++++-
> >   tools/testing/selftests/gen_kselftest_tar.sh |  5 +++++
> >   3 files changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/dev-tools/kselftest.rst
> > b/Documentation/dev-tools/kselftest.rst
> > index 61ae13c44f91..3fc559bcb597 100644
> > --- a/Documentation/dev-tools/kselftest.rst
> > +++ b/Documentation/dev-tools/kselftest.rst
> > @@ -151,6 +151,29 @@ note some tests will require root privileges::
> >      $ cd kselftest
> >      $ ./run_kselftest.sh
> >   
> > +Packaging selftests
> > +===================
> > +
> > +In some cases packaging is desired, such as when tests need to run on a
> > +different system. To package selftests, run::
> > +
> > +   $ make -C tools/testing/selftests gen_tar
> > +
> 
> Does this work in the case of relocatable build.cross-build cases?
> 
> > +This generates a tarball in the `INSTALL_PATH/kselftest-packages`
> > directory. By
> > +default, `.gz` format is used. The tar format can be overriden by
> > specifying
> > +a `FORMAT` make variable. Any value recognized by `tar's auto-compress`_
> > option
> > +is supported, such as::
> > +
> > +    $ make -C tools/testing/selftests gen_tar FORMAT=.xz
> > +
> > +`make gen_tar` invokes `make install` so you can use it to package a
> > subset of
> > +tests by using variables specified in `Running a subset of selftests`_
> > +section::
> > +
> > +    $ make -C tools/testing/selftests gen_tar TARGETS="bpf" FORMAT=.xz
> 
> Does this work in the case of relocatable build.cross-build cases?
> 

The command only adds a "tar" call on top of "make install" and doesn't
reach outside of INSTALL_PATH. If the cases you mention are supported by
the regular "make install" then they should work with "gen_tar" as well.



Veronika

> Please try these cases as well and let me know.
> 
> I would like to get this in for 5.8-rc1.
> 
> thanks,
> -- Shuah
> 
>
Shuah May 6, 2020, 2:56 p.m. UTC | #3
On 5/4/20 5:50 AM, Veronika Kabatova wrote:
> 
> 
> ----- Original Message -----
>> From: "shuah" <shuah@kernel.org>
>> To: "Veronika Kabatova" <veronicca114@gmail.com>
>> Cc: sbrivio@redhat.com, linux-kselftest@vger.kernel.org, "Veronika Kabatova" <vkabatov@redhat.com>, "shuah"
>> <shuah@kernel.org>
>> Sent: Friday, May 1, 2020 4:49:34 PM
>> Subject: Re: [PATCH] selftests: introduce gen_tar Makefile target
>>
>> Hi Veronica,
>>
> 
> Hi,
> 
>> On 4/28/20 6:38 AM, Veronika Kabatova wrote:
>>> The gen_kselftest_tar.sh always packages *all* selftests and doesn't
>>> pass along any variables to `make install` to influence what should be
>>> built. This can result in an early error on the command line ("Unknown
>>> tarball format TARGETS=XXX"), or unexpected test failures as the
>>> tarball contains tests people wanted to skip on purpose.
>>>
>>> Since the makefile already contains all the logic, we can add a target
>>> for packaging. Keep the default .gz target the script uses, and actually
>>> extend the supported formats by using tar's autodetection.
>>>
>>
>> Thanks for working on this. gen_kselftest_tar.sh  a while back before a
>> lot of the install features went in and Makefile supports it fully. It
>> makes perfect sense to use Makefile drive this.
>>
>>> To not break current workflows, keep the gen_kselftest_tar.sh script as
>>> it is, with an added suggestion to use the makefile target instead.
>>>
>>
>> Not sure how many people use this. It is a good idea keeping it around
>> for now.
>>
>>> Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
>>> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
>>> ---
>>>    Documentation/dev-tools/kselftest.rst        | 23 ++++++++++++++++++++
>>>    tools/testing/selftests/Makefile             |  9 +++++++-
>>>    tools/testing/selftests/gen_kselftest_tar.sh |  5 +++++
>>>    3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/dev-tools/kselftest.rst
>>> b/Documentation/dev-tools/kselftest.rst
>>> index 61ae13c44f91..3fc559bcb597 100644
>>> --- a/Documentation/dev-tools/kselftest.rst
>>> +++ b/Documentation/dev-tools/kselftest.rst
>>> @@ -151,6 +151,29 @@ note some tests will require root privileges::
>>>       $ cd kselftest
>>>       $ ./run_kselftest.sh
>>>    
>>> +Packaging selftests
>>> +===================
>>> +
>>> +In some cases packaging is desired, such as when tests need to run on a
>>> +different system. To package selftests, run::
>>> +
>>> +   $ make -C tools/testing/selftests gen_tar
>>> +
>>
>> Does this work in the case of relocatable build.cross-build cases?
>>
>>> +This generates a tarball in the `INSTALL_PATH/kselftest-packages`
>>> directory. By
>>> +default, `.gz` format is used. The tar format can be overriden by
>>> specifying
>>> +a `FORMAT` make variable. Any value recognized by `tar's auto-compress`_
>>> option
>>> +is supported, such as::
>>> +
>>> +    $ make -C tools/testing/selftests gen_tar FORMAT=.xz
>>> +
>>> +`make gen_tar` invokes `make install` so you can use it to package a
>>> subset of
>>> +tests by using variables specified in `Running a subset of selftests`_
>>> +section::
>>> +
>>> +    $ make -C tools/testing/selftests gen_tar TARGETS="bpf" FORMAT=.xz
>>
>> Does this work in the case of relocatable build.cross-build cases?
>>
> 
> The command only adds a "tar" call on top of "make install" and doesn't
> reach outside of INSTALL_PATH. If the cases you mention are supported by
> the regular "make install" then they should work with "gen_tar" as well.
> 

Sounds good. I will queue this up for 5.8-rc1

thanks,
-- Shuah
Shuah May 19, 2020, 6:39 p.m. UTC | #4
Hi Veronika,

On 5/6/20 8:56 AM, shuah wrote:
> On 5/4/20 5:50 AM, Veronika Kabatova wrote:
>>
>>
>> ----- Original Message -----
>>> From: "shuah" <shuah@kernel.org>
>>> To: "Veronika Kabatova" <veronicca114@gmail.com>
>>> Cc: sbrivio@redhat.com, linux-kselftest@vger.kernel.org, "Veronika 
>>> Kabatova" <vkabatov@redhat.com>, "shuah"
>>> <shuah@kernel.org>
>>> Sent: Friday, May 1, 2020 4:49:34 PM
>>> Subject: Re: [PATCH] selftests: introduce gen_tar Makefile target
>>>
>>> Hi Veronica,
>>>
>>
>> Hi,
>>
>>> On 4/28/20 6:38 AM, Veronika Kabatova wrote:
>>>> The gen_kselftest_tar.sh always packages *all* selftests and doesn't
>>>> pass along any variables to `make install` to influence what should be
>>>> built. This can result in an early error on the command line ("Unknown
>>>> tarball format TARGETS=XXX"), or unexpected test failures as the
>>>> tarball contains tests people wanted to skip on purpose.
>>>>
>>>> Since the makefile already contains all the logic, we can add a target
>>>> for packaging. Keep the default .gz target the script uses, and 
>>>> actually
>>>> extend the supported formats by using tar's autodetection.
>>>>
>>>
>>> Thanks for working on this. gen_kselftest_tar.sh  a while back before a
>>> lot of the install features went in and Makefile supports it fully. It
>>> makes perfect sense to use Makefile drive this.
>>>
>>>> To not break current workflows, keep the gen_kselftest_tar.sh script as
>>>> it is, with an added suggestion to use the makefile target instead.
>>>>
>>>
>>> Not sure how many people use this. It is a good idea keeping it around
>>> for now.
>>>
>>>> Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>

There is a mispatch between your from email address and signed-off line.
Please make sure they match.

Also there is a spelling error in the document.


WARNING: 'overriden' may be misspelled - perhaps 'overridden'?
#125: FILE: Documentation/dev-tools/kselftest.rst:163:
+default, `.gz` format is used. The tar format can be overriden by 
specifying

WARNING: Missing Signed-off-by: line by nominal patch author 'Veronika 
Kabatova <veronicca114@gmail.com>'

Please fix them and sen me v2.

thanks,
-- Shuah
Veronika Kabatova May 19, 2020, 7:50 p.m. UTC | #5
----- Original Message -----
> From: "shuah" <shuah@kernel.org>
> To: "Veronika Kabatova" <vkabatov@redhat.com>
> Cc: "Veronika Kabatova" <veronicca114@gmail.com>, sbrivio@redhat.com, linux-kselftest@vger.kernel.org, "shuah"
> <shuah@kernel.org>
> Sent: Tuesday, May 19, 2020 8:39:23 PM
> Subject: Re: [PATCH] selftests: introduce gen_tar Makefile target
> 
> Hi Veronika,
> 
> On 5/6/20 8:56 AM, shuah wrote:
> > On 5/4/20 5:50 AM, Veronika Kabatova wrote:
> >>
> >>
> >> ----- Original Message -----
> >>> From: "shuah" <shuah@kernel.org>
> >>> To: "Veronika Kabatova" <veronicca114@gmail.com>
> >>> Cc: sbrivio@redhat.com, linux-kselftest@vger.kernel.org, "Veronika
> >>> Kabatova" <vkabatov@redhat.com>, "shuah"
> >>> <shuah@kernel.org>
> >>> Sent: Friday, May 1, 2020 4:49:34 PM
> >>> Subject: Re: [PATCH] selftests: introduce gen_tar Makefile target
> >>>
> >>> Hi Veronica,
> >>>
> >>
> >> Hi,
> >>
> >>> On 4/28/20 6:38 AM, Veronika Kabatova wrote:
> >>>> The gen_kselftest_tar.sh always packages *all* selftests and doesn't
> >>>> pass along any variables to `make install` to influence what should be
> >>>> built. This can result in an early error on the command line ("Unknown
> >>>> tarball format TARGETS=XXX"), or unexpected test failures as the
> >>>> tarball contains tests people wanted to skip on purpose.
> >>>>
> >>>> Since the makefile already contains all the logic, we can add a target
> >>>> for packaging. Keep the default .gz target the script uses, and
> >>>> actually
> >>>> extend the supported formats by using tar's autodetection.
> >>>>
> >>>
> >>> Thanks for working on this. gen_kselftest_tar.sh  a while back before a
> >>> lot of the install features went in and Makefile supports it fully. It
> >>> makes perfect sense to use Makefile drive this.
> >>>
> >>>> To not break current workflows, keep the gen_kselftest_tar.sh script as
> >>>> it is, with an added suggestion to use the makefile target instead.
> >>>>
> >>>
> >>> Not sure how many people use this. It is a good idea keeping it around
> >>> for now.
> >>>
> >>>> Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
> 
> There is a mispatch between your from email address and signed-off line.
> Please make sure they match.
> 
> Also there is a spelling error in the document.
> 
> 
> WARNING: 'overriden' may be misspelled - perhaps 'overridden'?
> #125: FILE: Documentation/dev-tools/kselftest.rst:163:
> +default, `.gz` format is used. The tar format can be overriden by
> specifying
> 

Thanks for pointing it out.

> WARNING: Missing Signed-off-by: line by nominal patch author 'Veronika
> Kabatova <veronicca114@gmail.com>'
> 

Yes, that's because the work email decided to drop some public addresses
and the original patch didn't make it to you or the list. We got it fixed
so I'll send a v2 from there in a minute, sorry for the trouble.


Veronika

> Please fix them and sen me v2.
> 
> thanks,
> -- Shuah
> 
>
diff mbox series

Patch

diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 61ae13c44f91..3fc559bcb597 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -151,6 +151,29 @@  note some tests will require root privileges::
    $ cd kselftest
    $ ./run_kselftest.sh
 
+Packaging selftests
+===================
+
+In some cases packaging is desired, such as when tests need to run on a
+different system. To package selftests, run::
+
+   $ make -C tools/testing/selftests gen_tar
+
+This generates a tarball in the `INSTALL_PATH/kselftest-packages` directory. By
+default, `.gz` format is used. The tar format can be overriden by specifying
+a `FORMAT` make variable. Any value recognized by `tar's auto-compress`_ option
+is supported, such as::
+
+    $ make -C tools/testing/selftests gen_tar FORMAT=.xz
+
+`make gen_tar` invokes `make install` so you can use it to package a subset of
+tests by using variables specified in `Running a subset of selftests`_
+section::
+
+    $ make -C tools/testing/selftests gen_tar TARGETS="bpf" FORMAT=.xz
+
+.. _tar's auto-compress: https://www.gnu.org/software/tar/manual/html_node/gzip.html#auto_002dcompress
+
 Contributing new tests
 ======================
 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2ff68702fd41..1195bd85af38 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -249,10 +249,17 @@  else
 	$(error Error: set INSTALL_PATH to use install)
 endif
 
+FORMAT ?= .gz
+TAR_PATH = $(abspath ${INSTALL_PATH}/kselftest-packages/kselftest.tar${FORMAT})
+gen_tar: install
+	@mkdir -p ${INSTALL_PATH}/kselftest-packages/
+	@tar caf ${TAR_PATH} --exclude=kselftest-packages -C ${INSTALL_PATH} .
+	@echo "Created ${TAR_PATH}"
+
 clean:
 	@for TARGET in $(TARGETS); do \
 		BUILD_TARGET=$$BUILD/$$TARGET;	\
 		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean;\
 	done;
 
-.PHONY: khdr all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean
+.PHONY: khdr all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean gen_tar
diff --git a/tools/testing/selftests/gen_kselftest_tar.sh b/tools/testing/selftests/gen_kselftest_tar.sh
index 8b2b6088540d..4a974bc03385 100755
--- a/tools/testing/selftests/gen_kselftest_tar.sh
+++ b/tools/testing/selftests/gen_kselftest_tar.sh
@@ -49,6 +49,11 @@  main()
 	# directory
 	./kselftest_install.sh "$install_dir"
 	(cd "$install_work"; tar $copts "$dest"/kselftest${ext} $install_name)
+
+	# Don't put the message at the actual end as people may be parsing the
+	# "archive created" line in their scripts.
+	echo -e "\nConsider using 'make gen_tar' instead of this script\n"
+
 	echo "Kselftest archive kselftest${ext} created!"
 
 	# clean up top-level install work directory