[PATCH] build: Drop execute permissions on virsh bash completion command

Jim Fehlig posted 1 patch 4 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200131143849.13227-1-jfehlig@suse.com
tools/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] build: Drop execute permissions on virsh bash completion command
Posted by Jim Fehlig 4 years, 2 months ago
rpmlint complains about "script-without-shebang" due to the execute
permissions on /usr/share/bash-completion/completions/vsh. Use
INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
execute permissions.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
on my installation, only vsh has execute permissions set.

 tools/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 426a6ba661..d9d1a2f43f 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status
 if WITH_BASH_COMPLETION
 install-bash-completion:
 	$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
-	$(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+	$(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
 		"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
 	( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
 		rm -f virsh virt-admin && \
-- 
2.16.4


Re: [PATCH] build: Drop execute permissions on virsh bash completion command
Posted by Michal Privoznik 4 years, 2 months ago
On 1/31/20 3:38 PM, Jim Fehlig wrote:
> rpmlint complains about "script-without-shebang" due to the execute
> permissions on /usr/share/bash-completion/completions/vsh. Use
> INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
> execute permissions.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
> on my installation, only vsh has execute permissions set.
> 
>   tools/Makefile.am | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 426a6ba661..d9d1a2f43f 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status
>   if WITH_BASH_COMPLETION
>   install-bash-completion:
>   	$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
> -	$(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
> +	$(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
>   		"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>   	( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
>   		rm -f virsh virt-admin && \
> 

Frankly, I don't know why I wanted it to be executable. Perhaps I 
thought that if it wasn't bash would not load it, but apparently, that's 
not true. BTW, I have another executable file there, but the majority isn't.

# find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/perf

# find /usr/share/bash-completion/completions/ -type f \! -executable | 
wc -l
576


Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [PATCH] build: Drop execute permissions on virsh bash completion command
Posted by Jim Fehlig 4 years, 2 months ago
On 1/31/20 8:39 AM, Michal Privoznik wrote:
> On 1/31/20 3:38 PM, Jim Fehlig wrote:
>> rpmlint complains about "script-without-shebang" due to the execute
>> permissions on /usr/share/bash-completion/completions/vsh. Use
>> INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
>> execute permissions.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>
>> BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
>> on my installation, only vsh has execute permissions set.
>>
>>   tools/Makefile.am | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>> index 426a6ba661..d9d1a2f43f 100644
>> --- a/tools/Makefile.am
>> +++ b/tools/Makefile.am
>> @@ -357,7 +357,7 @@ libvirt-guests.service: libvirt-guests.service.in 
>> $(top_builddir)/config.status
>>   if WITH_BASH_COMPLETION
>>   install-bash-completion:
>>       $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>> -    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>> +    $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
>>           "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>>       ( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
>>           rm -f virsh virt-admin && \
>>
> 
> Frankly, I don't know why I wanted it to be executable. Perhaps I thought that 
> if it wasn't bash would not load it, but apparently, that's not true. BTW, I 
> have another executable file there, but the majority isn't.

I should have been as precise :-). Turns out there are a lot of links in that 
directory.

> # find /usr/share/bash-completion/completions/ -type f -executable
> /usr/share/bash-completion/completions/vsh
> /usr/share/bash-completion/completions/perf

/usr/share/bash-completion/completions/vsh
/usr/share/bash-completion/completions/colormgr

> # find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
> 576

615

> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Thanks!

Regards,
Jim


Re: [PATCH] build: Drop execute permissions on virsh bash completion command
Posted by Ján Tomko 4 years, 2 months ago
On Fri, Jan 31, 2020 at 09:01:41AM -0700, Jim Fehlig wrote:
>On 1/31/20 8:39 AM, Michal Privoznik wrote:
>>On 1/31/20 3:38 PM, Jim Fehlig wrote:
>>>rpmlint complains about "script-without-shebang" due to the execute
>>>permissions on /usr/share/bash-completion/completions/vsh. Use
>>>INSTALL_DATA instead of INSTALL_SCRIPT to avoid the unnecessary
>>>execute permissions.
>>>
>>>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>>---
>>>
>>>BTW, of the nearly 800 files in /usr/share/bash-completion/completions/
>>>on my installation, only vsh has execute permissions set.
>>>
>>>  tools/Makefile.am | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/tools/Makefile.am b/tools/Makefile.am
>>>index 426a6ba661..d9d1a2f43f 100644
>>>--- a/tools/Makefile.am
>>>+++ b/tools/Makefile.am
>>>@@ -357,7 +357,7 @@ libvirt-guests.service: 
>>>libvirt-guests.service.in $(top_builddir)/config.status
>>>  if WITH_BASH_COMPLETION
>>>  install-bash-completion:
>>>      $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>>>-    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>>>+    $(INSTALL_DATA) $(srcdir)/bash-completion/vsh \
>>>          "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>>>      ( cd $(DESTDIR)$(BASH_COMPLETIONS_DIR) && \
>>>          rm -f virsh virt-admin && \
>>>
>>
>>Frankly, I don't know why I wanted it to be executable. Perhaps I 
>>thought that if it wasn't bash would not load it, but apparently, 
>>that's not true. BTW, I have another executable file there, but the 
>>majority isn't.
>
>I should have been as precise :-). Turns out there are a lot of links 
>in that directory.
>
>># find /usr/share/bash-completion/completions/ -type f -executable
>>/usr/share/bash-completion/completions/vsh
>>/usr/share/bash-completion/completions/perf
>
>/usr/share/bash-completion/completions/vsh
>/usr/share/bash-completion/completions/colormgr
>
>># find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
>>576
>
>615
>

$ find /usr/share/bash-completion/completions/ -type f -executable
/usr/share/bash-completion/completions/fwupdate
$ find /usr/share/bash-completion/completions/ -type f \! -executable | wc -l
629

If you need more data points.

Jano

>>Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>
>Thanks!
>
>Regards,
>Jim
>
>