devtools: download scripts from linux if not found

Message ID 20240315141441.259594-2-rjarry@redhat.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series devtools: download scripts from linux if not found |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Robin Jarry March 15, 2024, 2:14 p.m. UTC
Both checkpatches.sh and get-maintainer.sh require scripts that come
from the linux sources. And they require DPDK_*_PATH variables to be set
to point at these scripts locally. For new contributors this can be
tedious since they will have to clone the whole linux sources just to
have simple perl scripts.

Update checkpatches.sh and get-maintainer.sh to have them download the
upstream scripts if they are not found locally. Store the downloaded
scripts in the .git/ folder so that they are found for future runs.

If either DPDK_CHECKPATCH_PATH or DPDK_GETMAINTAINER_PATH are set, they
will be used in priority.

Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
 devtools/checkpatches.sh            |  6 ++++
 devtools/get-maintainer.sh          |  9 ++++++
 devtools/load-devel-config          | 45 +++++++++++++++++++++++++++++
 doc/guides/contributing/patches.rst |  1 +
 4 files changed, 61 insertions(+)
  

Comments

Stephen Hemminger Oct. 4, 2024, 4:31 p.m. UTC | #1
On Fri, 15 Mar 2024 15:14:42 +0100
Robin Jarry <rjarry@redhat.com> wrote:

> Both checkpatches.sh and get-maintainer.sh require scripts that come
> from the linux sources. And they require DPDK_*_PATH variables to be set
> to point at these scripts locally. For new contributors this can be
> tedious since they will have to clone the whole linux sources just to
> have simple perl scripts.
> 
> Update checkpatches.sh and get-maintainer.sh to have them download the
> upstream scripts if they are not found locally. Store the downloaded
> scripts in the .git/ folder so that they are found for future runs.
> 
> If either DPDK_CHECKPATCH_PATH or DPDK_GETMAINTAINER_PATH are set, they
> will be used in priority.
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>

This makes live easier for new contributors.
Not sure if is the best way to autodownload, or where to put the scripts
but something is better than the current way.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
  
Thomas Monjalon Nov. 26, 2024, 2:03 p.m. UTC | #2
04/10/2024 18:31, Stephen Hemminger:
> On Fri, 15 Mar 2024 15:14:42 +0100
> Robin Jarry <rjarry@redhat.com> wrote:
> 
> > Both checkpatches.sh and get-maintainer.sh require scripts that come
> > from the linux sources. And they require DPDK_*_PATH variables to be set
> > to point at these scripts locally. For new contributors this can be
> > tedious since they will have to clone the whole linux sources just to
> > have simple perl scripts.
> > 
> > Update checkpatches.sh and get-maintainer.sh to have them download the
> > upstream scripts if they are not found locally. Store the downloaded
> > scripts in the .git/ folder so that they are found for future runs.
> > 
> > If either DPDK_CHECKPATCH_PATH or DPDK_GETMAINTAINER_PATH are set, they
> > will be used in priority.
> > 
> > Signed-off-by: Robin Jarry <rjarry@redhat.com>
> 
> This makes live easier for new contributors.
> Not sure if is the best way to autodownload, or where to put the scripts
> but something is better than the current way.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

I would prefer we host our own fork of checkpatch.pl.
There are too many irrelevant checks that we should drop.
  
Robin Jarry Nov. 26, 2024, 3:16 p.m. UTC | #3
Thomas Monjalon, Nov 26, 2024 at 15:03:
> I would prefer we host our own fork of checkpatch.pl.
> There are too many irrelevant checks that we should drop.

How about get_maintainer.pl? Should we host our copy as well?
  
Thomas Monjalon Nov. 26, 2024, 3:36 p.m. UTC | #4
26/11/2024 16:16, Robin Jarry:
> Thomas Monjalon, Nov 26, 2024 at 15:03:
> > I would prefer we host our own fork of checkpatch.pl.
> > There are too many irrelevant checks that we should drop.
> 
> How about get_maintainer.pl? Should we host our copy as well?

It may makes sense.

The central question is to know how to get those GPL2 scripts accepted in DPDK.
  
Robin Jarry Nov. 26, 2024, 3:56 p.m. UTC | #5
Thomas Monjalon, Nov 26, 2024 at 16:36:
> The central question is to know how to get those GPL2 scripts accepted 
> in DPDK.

These are scripts, not linked to any DPDK related binary. So I think it 
is OK in terms of license contamination. We MUST preserve the original 
license headers in these imported scripts though.

I could find this link that discusses the matter:
https://wiki.tcl-lang.org/page/GPL+Scripts

I am not a legal expert though :)
  

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 8f245ebdabc4..bfacd77f398a 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -43,6 +43,8 @@  print_usage () {
 	Run Linux kernel checkpatch.pl with DPDK options.
 	The environment variable DPDK_CHECKPATCH_PATH can be set, if not we will
 	try to find the script in the sources of the currently running kernel.
+	If the script cannot be found, you will be prompted for downloading it
+	from the upstream linux sources.
 
 	The patches to check can be from stdin, files specified on the command line,
 	latest git commits limited with -n option, or commits in the git range
@@ -376,10 +378,14 @@  while getopts hn:qr:v ARG ; do
 done
 shift $(($OPTIND - 1))
 
+url="https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.pl"
+
 if [ ! -f "$DPDK_CHECKPATCH_PATH" ] || [ ! -x "$DPDK_CHECKPATCH_PATH" ] ; then
 	default_path="/lib/modules/$(uname -r)/source/scripts/checkpatch.pl"
 	if [ -f "$default_path" ] && [ -x "$default_path" ] ; then
 		DPDK_CHECKPATCH_PATH="$default_path"
+	elif download_script DPDK_CHECKPATCH_PATH "$url"; then
+		true
 	else
 		print_usage >&2
 		echo
diff --git a/devtools/get-maintainer.sh b/devtools/get-maintainer.sh
index bba4d3f68db8..010aef221226 100755
--- a/devtools/get-maintainer.sh
+++ b/devtools/get-maintainer.sh
@@ -18,10 +18,19 @@  print_usage () {
 	the get_maintainer.pl script located in Linux kernel sources. Example:
 	DPDK_GETMAINTAINER_PATH=~/linux/scripts/get_maintainer.pl
 
+	If the script cannot be found, you will be prompted for downloading it
+	from the upstream linux sources.
+
 	Also refer to devtools/load-devel-config to store your configuration.
 	END_OF_HELP
 }
 
+if [ ! -f "$DPDK_GETMAINTAINER_PATH" ] ||
+   [ ! -x "$DPDK_GETMAINTAINER_PATH" ] ; then
+	download_script DPDK_GETMAINTAINER_PATH \
+		"https://raw.githubusercontent.com/torvalds/linux/master/scripts/get_maintainer.pl"
+fi
+
 # Requires DPDK_GETMAINTAINER_PATH devel config option set
 if [ ! -f "$DPDK_GETMAINTAINER_PATH" ] ||
    [ ! -x "$DPDK_GETMAINTAINER_PATH" ] ; then
diff --git a/devtools/load-devel-config b/devtools/load-devel-config
index 69a0ac623a6e..46baf73fc0b3 100644
--- a/devtools/load-devel-config
+++ b/devtools/load-devel-config
@@ -1,4 +1,5 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
+# vim: ft=sh
 
 # This file is intended to be sourced into shell
 
@@ -14,3 +15,47 @@  test ! -r $(dirname $(readlink -f $0))/../.develconfig ||
         . $(dirname $(readlink -f $0))/../.develconfig
 
 # The config files must export variables in the shell style
+
+# This function is called to download missing scripts by subcommands
+#
+# Example:
+#   download_script DPDK_CHECKPATCH_PATH \
+#       https://raw.githubusercontent.com/torvalds/linux/master/scripts/checkpatch.pl
+download_script() { # <var_name> <url>
+	var_name=$1
+	url=$2
+
+	script_name=$(basename "$url")
+	git_dir=$(git rev-parse --git-dir) || return 1
+	dl_path="$git_dir/$script_name"
+
+	if [ -f "$dl_path" ] && [ -x "$dl_path" ]; then
+		eval "$var_name='$dl_path'"
+		return 0
+	fi
+
+	if ! [ -t 0 ]; then
+		# not in an interactive terminal, abort
+		return 1
+	fi
+	prompt="Download $url and set $var_name=$dl_path? [y/n] "
+
+	while printf '%s' "$prompt" && read y && ! [ "$y" = y ]; do
+		if [ "$y" = n ]; then
+			return 1
+		fi
+	done
+
+	if which curl >/dev/null 2>&1; then
+		curl -Lo "$dl_path" "$url" || return 1
+	elif which wget >/dev/null 2>&1; then
+		wget -O "$dl_path" "$url" || return 1
+	else
+		echo "error: neither curl nor wget are available." >&2
+		return 1
+	fi
+	chmod 755 "$dl_path" || return 1
+
+	eval "$var_name='$dl_path'"
+	return 0
+}
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index e286d9e6d59e..d24327bebb6c 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -462,6 +462,7 @@  This uses the Linux kernel development tool ``checkpatch.pl`` which  can be obta
 updating the Linux kernel sources.
 
 The path to the original Linux script must be set in the environment variable ``DPDK_CHECKPATCH_PATH``.
+If this variable is not set, ``checkpatches.sh`` will prompt before downloading it locally.
 
 Spell checking of commonly misspelled words is enabled
 by default if installed in ``/usr/share/codespell/dictionary.txt``.