linuxmint / xapp

Cross-desktop libraries and common resources
GNU Lesser General Public License v3.0
126 stars 44 forks source link

Drop bash requirement for scripts #179

Closed Chocobo1 closed 4 months ago

Chocobo1 commented 4 months ago

These scripts does not contain anything that required bash specifically.

leigh123linux commented 4 months ago

I'm -1 to merging this change.

Chocobo1 commented 4 months ago

I'm -1 to merging this change.

Care to elaborate why?

leigh123linux commented 4 months ago

I see it as a pointless change for no gain .

Chocobo1 commented 4 months ago

I see it as a pointless change for no gain.

Are you implying the following?

Surely those aren't the intended restriction. Having a script that is compatible with the POSIX shell and can be run by every shell in the wild do look like something worth pursuing to me.

Chocobo1 commented 4 months ago

BTW there are a few scripts lying in my /etc/X11/xinit/xinitrc.d, they are shipped by other projects:

All of them use #!/bin/sh except one. Also notice the one with the strange name...

leigh123linux commented 4 months ago

I'm implying this is a pointless change.

[leigh@mpd-pc xinitrc.d]$ grep bin/bash -r
50-xinput.sh:#!/usr/bin/bash
10-qt6-check-opengl2.sh:#!/usr/bin/bash
10-qt5-check-opengl2.sh:#!/usr/bin/bash
80xapp-gtk3-module.sh:#!/usr/bin/bash
[leigh@mpd-pc xinitrc.d]$ grep bin/sh -r
50-systemd-user.sh:#!/usr/bin/sh
98vboxadd-xclient.sh:#!/usr/bin/sh
00-start-message-bus.sh:#!/usr/bin/sh
localuser.sh:#!/usr/bin/sh
[leigh@mpd-pc xinitrc.d]$ 
Chocobo1 commented 4 months ago

I'm implying this is a pointless change.

[leigh@mpd-pc xinitrc.d]$ grep bin/bash -r
50-xinput.sh:#!/usr/bin/bash
10-qt6-check-opengl2.sh:#!/usr/bin/bash
10-qt5-check-opengl2.sh:#!/usr/bin/bash
80xapp-gtk3-module.sh:#!/usr/bin/bash
[leigh@mpd-pc xinitrc.d]$ grep bin/sh -r
50-systemd-user.sh:#!/usr/bin/sh
98vboxadd-xclient.sh:#!/usr/bin/sh
00-start-message-bus.sh:#!/usr/bin/sh
localuser.sh:#!/usr/bin/sh
[leigh@mpd-pc xinitrc.d]$ 

Just because other projects ships with bash doesn't mean bash is the only answer. Maybe they really require bashism and that is ok. Interestingly 10-qt5-check-opengl2.sh changed from #!/bin/sh to #!/bin/bash without any explanation in this commit: https://src.fedoraproject.org/rpms/qt5-qtbase/c/ba28649c77880437471fad1ca4f89fdbd11e5dac?branch=rawhide and 10-qt6-check-opengl2.sh just copied it. 50-xinput.sh doesn't seem to exists in archlinux, at least I couldn't find it.

Sorry, but you still didn't address any of my concerns from https://github.com/linuxmint/xapp/pull/179#issuecomment-2066436364. It would be great if you can try make an effort to make the discussion constructive instead of (rage?) closing without providing proper rationale.

clefebvre commented 4 months ago

sh is just a symlink to whatever shell might be, on my system it happens to be dash, on somebody else's it could be bash or something else.

bash is just bash for everybody.

Different shells accept/support different features and different syntaxes. From a project point of view it makes sense to be specific about the interpreter we target and to know the language/features/syntax we can use, whether we actually make use of them or not.

Chocobo1 commented 4 months ago

From a project point of view it makes sense to be specific about the interpreter we target and to know the language/features/syntax we can use, whether we actually make use of them or not.

If the script is not distributed to users then sure, any shell could be used. But if the script is installed on user's system then IMO there is interest to maximize compatibility (by using POSIX shell). Because otherwise you'll create a dependency on some specific shell.

Anyway, if that is the project decision, I respect that. Thanks for the explanation!