[dpdk-dev,RFC] scripts: make load-devel-config not to appear as executable

Message ID 1470146092-12115-1-git-send-email-christian.ehrhardt@canonical.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Christian Ehrhardt Aug. 2, 2016, 1:54 p.m. UTC
  Quoting the first line of the script: "#! /bin/echo must be loaded with ."
Given that we should drop the .sh file ending as well as the executable
flag - both are not needed to source the file.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 MAINTAINERS                  |  2 +-
 scripts/checkpatches.sh      |  2 +-
 scripts/load-devel-config    | 14 ++++++++++++++
 scripts/load-devel-config.sh | 14 --------------
 scripts/test-build.sh        |  4 ++--
 5 files changed, 18 insertions(+), 18 deletions(-)
 create mode 100644 scripts/load-devel-config
 delete mode 100755 scripts/load-devel-config.sh
  

Comments

Thomas Monjalon Aug. 2, 2016, 3:30 p.m. UTC | #1
2016-08-02 15:54, Christian Ehrhardt:
> Quoting the first line of the script: "#! /bin/echo must be loaded with ."
> Given that we should drop the .sh file ending as well as the executable
> flag - both are not needed to source the file.

Hmmm, it is still a file containing some shell commands, right?
So why removing the .sh extension?
  
Christian Ehrhardt Aug. 3, 2016, 8:11 a.m. UTC | #2
sorry, I accidentally dropped dev list in one of my replies, readding.

On Tue, Aug 2, 2016 at 11:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> > > > Given that we should drop the .sh file ending as well as the
> executable
> > > > flag - both are not needed to source the file.
> > >
> > > Hmmm, it is still a file containing some shell commands, right?
> > > So why removing the .sh extension?
> > >
> >
> > I wanted to discuss on #dpdk today, but everyone seemed busy today.
> > So I expected the discussion on file extension to come up on the patch
> > submission - which is fine and just as it should be.
> >
> > My reasoning was primarily to discourage people to think to call it.
>
> I think it is the contrary: the executable files for users have no
> extension.


I totally understand that for commands in the path, but that doesn't count
here.
Could we have anybodies opinion as a tie breaker so I can submit a v2
without RFC then?

P.S. I understand there was no objection on changing the file mode - which
might be quite unobvious in the diff?
  
Bruce Richardson Aug. 3, 2016, 9:26 a.m. UTC | #3
On Wed, Aug 03, 2016 at 10:11:32AM +0200, Christian Ehrhardt wrote:
> sorry, I accidentally dropped dev list in one of my replies, readding.
> 
> On Tue, Aug 2, 2016 at 11:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
> 
> > > > > Given that we should drop the .sh file ending as well as the
> > executable
> > > > > flag - both are not needed to source the file.
> > > >
> > > > Hmmm, it is still a file containing some shell commands, right?
> > > > So why removing the .sh extension?
> > > >
> > >
> > > I wanted to discuss on #dpdk today, but everyone seemed busy today.
> > > So I expected the discussion on file extension to come up on the patch
> > > submission - which is fine and just as it should be.
> > >
> > > My reasoning was primarily to discourage people to think to call it.
> >
> > I think it is the contrary: the executable files for users have no
> > extension.
> 
> 
> I totally understand that for commands in the path, but that doesn't count
> here.
> Could we have anybodies opinion as a tie breaker so I can submit a v2
> without RFC then?
> 
> P.S. I understand there was no objection on changing the file mode - which
> might be quite unobvious in the diff?
> 
Definitely no objection on the file mode change.

For the dropping of the .sh extension, I don't think it matters much. However,
given that .sh files are generally scripts to be executed, I think dropping the
extension will reduce confusion.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
Christian Ehrhardt Aug. 3, 2016, 9:42 a.m. UTC | #4
On Wed, Aug 3, 2016 at 11:26 AM, Bruce Richardson <
bruce.richardson@intel.com> wrote:

> Definitely no objection on the file mode change.
>
> For the dropping of the .sh extension, I don't think it matters much.
> However,
> given that .sh files are generally scripts to be executed, I think
> dropping the
> extension will reduce confusion.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>

Thank you Bruce

@Thomas - given we are 2:1 now, the patch would stay as-is.
Do you want me to resubmit as-is but without RFC tag or would you accept it
as already submitted?
  
Thomas Monjalon Aug. 3, 2016, 1:19 p.m. UTC | #5
2016-08-03 11:42, Christian Ehrhardt:
> On Wed, Aug 3, 2016 at 11:26 AM, Bruce Richardson <
> bruce.richardson@intel.com> wrote:
> 
> > Definitely no objection on the file mode change.
> >
> > For the dropping of the .sh extension, I don't think it matters much.
> > However,
> > given that .sh files are generally scripts to be executed, I think
> > dropping the
> > extension will reduce confusion.
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> 
> Thank you Bruce
> 
> @Thomas - given we are 2:1 now, the patch would stay as-is.

:D

> Do you want me to resubmit as-is but without RFC tag or would you accept it
> as already submitted?

I will apply it as is.
  
Christian Ehrhardt Aug. 3, 2016, 2:54 p.m. UTC | #6
On Wed, Aug 3, 2016 at 3:19 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> > Do you want me to resubmit as-is but without RFC tag or would you accept
> it
> > as already submitted?
>
> I will apply it as is.


As just discussed on #dpdk now that it is no more executable I'd even drop
the #!/bin/echo.
That is to avoid the
https://lintian.debian.org/tags/unusual-interpreter.html warning that still
is emitted.

Will submit a v2 soon.
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 6536c6b..f8b99ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27,7 +27,7 @@  F: MAINTAINERS
 F: scripts/check-maintainers.sh
 F: scripts/check-git-log.sh
 F: scripts/checkpatches.sh
-F: scripts/load-devel-config.sh
+F: scripts/load-devel-config
 F: scripts/test-build.sh
 
 Stable Branches
diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh
index 7111558..b596b4e 100755
--- a/scripts/checkpatches.sh
+++ b/scripts/checkpatches.sh
@@ -33,7 +33,7 @@ 
 # Load config options:
 # - DPDK_CHECKPATCH_PATH
 # - DPDK_CHECKPATCH_LINE_LENGTH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config
 
 length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
 
diff --git a/scripts/load-devel-config b/scripts/load-devel-config
new file mode 100644
index 0000000..489f007
--- /dev/null
+++ b/scripts/load-devel-config
@@ -0,0 +1,14 @@ 
+#! /bin/echo must be loaded with .
+
+# Load DPDK devel config and allow override
+# from system file
+test ! -r /etc/dpdk/devel.config ||
+        . /etc/dpdk/devel.config
+# from user file
+test ! -r ~/.config/dpdk/devel.config ||
+        . ~/.config/dpdk/devel.config
+# from local file
+test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
+        . $(dirname $(readlink -m $0))/../.develconfig
+
+# The config files must export variables in the shell style
diff --git a/scripts/load-devel-config.sh b/scripts/load-devel-config.sh
deleted file mode 100755
index 489f007..0000000
--- a/scripts/load-devel-config.sh
+++ /dev/null
@@ -1,14 +0,0 @@ 
-#! /bin/echo must be loaded with .
-
-# Load DPDK devel config and allow override
-# from system file
-test ! -r /etc/dpdk/devel.config ||
-        . /etc/dpdk/devel.config
-# from user file
-test ! -r ~/.config/dpdk/devel.config ||
-        . ~/.config/dpdk/devel.config
-# from local file
-test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
-        . $(dirname $(readlink -m $0))/../.develconfig
-
-# The config files must export variables in the shell style
diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index d2cafc1..e8971fe 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -48,7 +48,7 @@  default_path=$PATH
 # - DPDK_NOTIFY (notify-send)
 # - LIBSSO_SNOW3G_PATH
 # - LIBSSO_KASUMI_PATH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config
 
 print_usage () {
 	echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]"
@@ -211,7 +211,7 @@  for conf in $configs ; do
 	# reload config with DPDK_TARGET set
 	DPDK_TARGET=$target
 	reset_env
-	. $(dirname $(readlink -e $0))/load-devel-config.sh
+	. $(dirname $(readlink -e $0))/load-devel-config
 
 	options=$(echo $conf | sed 's,[^~+]*,,')
 	dir=$conf