Code bash, est-il "bien écrit"?

Tags: #<Tag:0x00007f4e6051bd40>

Bonjour,

J’enregistre un flux radio par internet à intervalle régulier : le 1er mardi de chaque mois et le 1er vendredi à des horaires différents. La durée d’enregistrement diffère aussi selon le jour.
Pour cela j’utilise systemd-cron qui exécute un script.
Ce mois ci, j’ai testé la programmaion par systemd-cron et le script. Tout a fonctionné comme voulu.

Cependant je pense que mon script peut être mieux écrit ou de manière plus simple.

En particulier 2 points :

  1. l’action à exécuter selon que l’on soit un mardi ou vendredi
    Dans mon script je lui dit si on est un mardi exécute la commande A, si on est une vendredi exécute la commande B. Si on est un lundi, mercredi, jeudi, samedi ou dimanche arrête le script. Ce qui n’est pas trop compliqué dans ce cas puisqu’il n’y a que 7 jours dans une semaine.
    Mais si je dois utiliser le même principe avec un échantillon de 100 variables et ne vouloir déclenché une commande qu’avec 2 ou 3 variables, cela devient très fastidieux.
    Existe une façon d’écrire : si on est mardi exécute la commande A, si on est vendredi exécute la commande B et pour tout autre valeur arrête le script ?
  1. Je pense que la partie pour renommer les fichiers peut être simplifiée pour arriver au même résultat.

ci-dessous le script :
#!/bin/bash

day=$(date +%A)

if [[ "$day" == "mardi" ]]; then
streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l 8000
fi

if [[ "$day" == "vendredi" ]]; then
streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l 32000
fi

if [[ "$day" == "dimanche" && "lundi" && "mercredi" && "jeudi" && "samedi" ]]; then
exit 0
fi

# renommer les fichiers enregistrés
cd '/home/tulum/Musique/BAGeL Radio/' # On se place dans le répertoire

for file in *.mp3; do
dcreation=$(stat -c %w "$file") # On récupère la date de création du fichier
dcreation="${dcreation:0:16}" # On garde que la date et l'heure sans les secondes, les 16 premiers caractères
dcreation="${dcreation// /_}" # remplacement des espaces par des underscores
dcreation="${dcreation//:/-}" # Remplacement des : par -
nom="${dcreation}_bagelradio.mp3" # Nouveau nom
mv "$file" "$nom" # Renommage des fichiers
done
rm -R incomplete

Ci-dessous le timer de systemd-cron :
[Unit]
Description=Enregistrement de BAGeL Radio

[Timer]
OnCalendar=Fri *-*-01..07 14:53
OnCalendar=Tue *-*-01..07 20:55
Persistent=false

[Install]
WantedBy=default.target

Merci

1 J'aime

Bonjour,

Le bloc de conditions semble avoir un bug et je pense qu’il devrait utiliser des || (ou logique) au lieu des && pour quitter lorsque le jour n’est ni un mardi ni un vendredi.

Ceci dit, les conditions peuvent être réécrites comme cela :

if [[ "$day" == "mardi" ]]; then
    streamripper ...
elif [[ "$day" == "vendredi" ]]; then
    streamripper ...
else
    exit 0
fi

Ou, mieux encore :

case "$day" in
    mardi)
        streamripper ...
        ;;
    vendredi)
        streamripper ...
        ;;
    *)
        exit 0
        ;;
esac

Sinon, est-ce qu’il ne serait pas préférable d’utiliser %u ou %w pour récupérer le numéro du jour de la semaine plutôt que son nom ?


AnonymousCoward

En plus compact, à tester:

#!/bin/bash

case $(date +%A) in 
    mardi) L=8000  ;;
 vendredi) L=32000 ;;
        *) exit 0  ;;
esac

cd '/home/tulum/Musique/BAGeL Radio/'
streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l $L

for file in *.mp3 ; do
    dcreation=$(stat -c %w $file | sed 's/:[^:]*$//;s/ /_/')
    mv "$file" ${dcreation}_bagelradio.mp3
done
exit

On peut faire plus court en utilisant rename (à installer avec apt install -y rename’)

#!/bin/bash

case $(date +%A) in 
    mardi) L=8000  ;;
 vendredi) L=32000 ;;
        *) exit 0  ;;
esac

streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l $L -a %d_bagelradio.mp3 -A -d /home/tulum/Musique/BAGeL\ Radio/

rename -e 's/([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)(_.*)/\1-\2-\3_\4-\5\7/' /home/tulum/Musique/BAGeL\ Radio/*.cfue
rename -e 's/([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)_([0-9]*)(_.*)/\1-\2-\3_\4-\5\7/' /home/tulum/Musique/BAGeL·\ Radio/*.mp3

exit

le flux donne un fichier 2026_03_07_13_46_59_bagelradio.mp3
qui est renommé en 2026-03-07_13-46_bagelradio.mp3
Avec un seul fichier à chaque enregistrement.
Le fichier cue c’est le détail de l’enregistrement mp3

Merci pour vos propositions, je vais testé tout ça le mois prochain.

C’est une erreur fréquente s’expliquant aisément par le fait que le ET et le OU de notre langue française s’oppose fréquemment à celui de la logique booléenne: les tables de vérité des fonctions logiques aident à lever le doute, même pour les plus expérimentés.

Quelques réflexions éventuellement utiles pour les 3 semaines à venir.
Avant de se focaliser sur le détail des options secondaires de ‹ streamripper ›, il faut commencer par le début: la syntaxe du code bash, conformément au titre. streamripper peut éventuellement faire l’objet d’un autre sujet si réel besoin et intérêt.
Pour commencer:

[[ "$day" == "dimanche" && "lundi" && "mercredi" && "jeudi" && "samedi" ]]

Cette syntaxe est incohérente, incomprise, que ce soit avec && ou ||.
Le test sera toujours négatif, donc inutile.
Cette syntaxe est correcte en Bash:

[[ $day =~ dimanche\|lundi\|mercredi\|jeudi\|samedi ]]  # expression regex POSIX

7 ou 100 variables ne change rien, et se résume à seulement 3 conditions:
→ mardi ou vendredi ou ‹ autre chose ›.
La syntaxe correcte avec ‹ if › est donc:

  if [ "$day" = mardi ]    ; then <action A>
elif [ "$day" = vendredi ] ; then <action B>
else exit 0
  fi

J’emploie volontairement [ ] et non pas [[ ]] juste pour réflexion, mais rentrer dans la subtilité non triviale des différences sortirait du cadre de ce sujet.

Plus généralement, un script utilisé non interactivement (par cron par exemple) nécessite certaines précautions et doit être le moins sensible possible aux aléas d’évolution système (mise à jour/installations/désinstallations).
‹ mv › est fourni par ‹ coreutils › (paquet required/essentiel) et est donc la commande à utiliser pour un renommage basique de fichier.

Enfin, une autre proposition de script, sans ‹ if › ni ‹ case ›, pour investigation plus avancée:
1) ne renomme que les fichiers mp3 non déjà traités dans le répertoire
2) ajout ’date <nom fichier>.mp3' à la liste mp3.list
3) option -M 10 : sécurité pour test / arrêt de streamripper au delà de 10 MB.

#!/bin/bash
L=$(date +%A | sed 's/mardi/8/;s/vendredi/32/' )
[[ $L =~ [0-9] ]] || exit 0

D='/home/tulum/Musique/BAGeL'
streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l ${L}000 -M 10 -d $D

while read dcreation Y ; do fname=${Y##*@} ; dir=${Y%@*}
  mv "$dir/$fname" "$dir/${dcreation}_bagelradio.mp3"
  printf "$dcreation\t$fname\n" >> "$dir/mp3.list"
done < <(find "$D" -name '[[:alpha:]]*.mp3' -printf '%AF_%AH:%AM %h@%f\n')

exit

Bonne réflexion et bons tests.

Problème n°1 :

C’est moche. Pourquoi ne pas directement faire comme dans la boucle de ton propre script ?

L=$(sed 's/mardi/8/;s/vendredi/32/' < <$(date +%A))

ou encore

L=$( date +%A | sed 's/mardi/8/;s/vendredi/32/' )

Problème n°2 :

Ce if déguisé est totalement buggé.

Problème n°3 :

Tu n’aurais pas pu déclarer tes deux variables sur des lignes séparées ? Ce n’est pas cohérent avec le reste.

Problème n°4 :

Il ne faut pas utiliser de substitution de variables dans la chaine de formatage de printf. Et encore moins en tout début de cette chaine !
Mais plutôt :

printf '%s\t%s\n' "$dcreation" "$fname" >> "$dir/mp3.list"

Et une suggestion :

find "$D" -name '[[:alpha:]]*.mp3' -printf '%AF_%AH:%AM %h@%f\n' |
while read dcreation Y ; do
  ...
done


AnonymousCoward

C’est intéressant, personne n’utilise les capacités de nommage de fichier se streamripper. Après un simple replace pour que ce soit nommé& avec le nom demandé. Ça fait deux lignes, aucune boucle.

Simplement parce-que streamripper ne permet pas de directement formater le nom du fichier tel que spécifié par @Tulum: YYYY-MM-DD_hh:mm_bagelradio.mp3

@anonymous_coward (peureux anonyme)
Tes commentaires n’apportant rien de constructif qui puisse aider @Tulum, je pourrai répondre au choix:
1- la critique et le dénigrement limite ridicule (jusqu’au ‹ ; ›) est facile, l’art plus difficile (n’est-ce pas);
2- c’est à la fin du marché qu’on compte les bouses;
3- l’important est de participer, même si des fois, on ferait mieux de s’abstenir;
4- ou … ne pas répondre (perte de temps, troll detected).

Je vais quand même t’éclairer sur un point, puisque les redirections de Bash semblent t’intriguer.
Initialement, la variable day=$(date +%A) était utilisée.
J’ai une aversion en Bash pour les syntaxes du type 'echo $day | <action>', peut-être un peu déformé par la recherche d’optimisation de nombreux scripts que j’ai écrits pour des tâches lourdes, obsédé à minimiser le nombre d’instructions nécessaires.
Ayant considéré que la variable ‹ day › était superflue, et bien que la redirection ‹ <<< › soit parfaitement correcte, j’ai remplacé le ‹ <<< › de mon script précédent par un pipe, pour ne pas heurter les âmes sensibles. Aucune différence fonctionnelle.
Par contre, où tu fais une erreur, non pas dans ce cas précis qui est simple, c’est que <action> | while..done n’est pas équivalent à while...done < <( <action> ). Bien que non POSIX, les redirections n’ont pas été créées pour rien dans Bash/zsh, et c’est par la pratique que j’ai été confronté à cette différence, que je ne développe pas ici (hors sujet).
La théorie c’est bien, la pratique en plus c’est mieux (voir ta remarque de débutant sur printf):
« Il ne faut pas utiliser de substitution de variables dans la chaine de formatage de printf »

Il n’y a ici justement aucun « formatage » au sens de printf ! printf est flexible et très puissant, plus que la théorie primaire ne laisse penser.
Il faut pratiquer, pratiquer… et non pas se contenter de lectures superficielles et généralistes.

A tes « pourquoi ci et pourquoi ça et patati et patata », si j’ai diversifié les possibilités d’un script à l’autre, c’est bien justement pour montrer qu’il n’y a pas de solution unique et ‹ investiguer › le man de Bash pour apprendre ce qui n’est pas forcément trivial.
En 3 semaines, temps estimé par @Tulum pour réfléchir à sa solution idéale, après avoir testé un script qui lui demandera 10 minutes, on a le temps de découvrir des choses. @Tulum a toutes les billes pour adapter son script initial déjà bien réléchi et clairement décomposé.

En résumé, débouler dans un sujet déjà traité pour exprimer ce que "tu aimes ou pas" ou critiquer uniquement la forme et non le fond n’apporte rien.

Rectifié avec un simple rename sans avoir à faire la moindre boucle.

Et que je sache le résultat ci-dessus non plus.

Je ne reviens pas sur les remarques déjà faites sur ‹ rename ›, en notant que ‹ rename › utilise une syntaxe regex PERL. Si ‹ streamripper › avait permis un formatage direct du nom de fichier conformément à la demande initiale, l’usage unique de ‹ streamripper › aurait été la solution idéale. L’usage de ‹ rename › complique et alourdit plus qu’autre chose.

Concernant l’usage de boucles ‹ for/while...done ›, il s’agit de commandes ‹ builtin › intégrées au shell, une boucle étant nécessaire pour conserver les noms originaux des fichiers pour en créer une liste, ce qui n’était certes pas demandé, mais qui avait une probabilité forte de l’être par la suite.

Pour ne pas diverger de la demande initiale et recentrer, voici une proposition légère, la plus consensuelle possible, tout en étant POSIX (/usr/bin/sh -> /usr/bin/dash):

#!/usr/bin/sh
day=$(date +%A)

  if [ $day = mardi ]    ; then L=8000
elif [ $day = vendredi ] ; then L=32000
else exit 0
  fi

cd /home/tulum/Musique/
streamripper http://ais-sa3.cdnstream1.com/2606_128.mp3 -l $L -M 10

cd './BAGeL Radio/'
for F in [[:alpha:]]*.mp3 ; do
    mv "$F" $(stat -c %w "$F" | sed 's/:[^:]*$//;s/ /_/')_bagelradio.mp3
done
exit

• option -M 10: pour test / arrêt de streamripper au delà de 10 MB;
• L’usage de [[:alpha:]]*.mp3 (=globbing et non regex) a pour objectif d’exclure de la boucle les fichiers déjà renommés.