twpayne / chezmoi

Manage your dotfiles across multiple diverse machines, securely.
https://www.chezmoi.io/
MIT License
13.27k stars 493 forks source link

Do not execute scripts without commands #2878

Closed goto1134 closed 1 year ago

goto1134 commented 1 year ago

Is your feature request related to a problem? Please describe.

I use the scripts to check my OS settings and update them if they somehow change. So the script is composed the following way:

#!/bin/sh

{{- if the condition is met }}
<some commands here>
{{- end }}

Every time I run the chezmoi status command, I get the script ready to be executed. But most of the time, it contains only the shebang line #!/bin/sh.

Describe the solution you'd like

If a script does not have any commands, chezmoi should not run it and should not show it in the status command output.

twpayne commented 1 year ago

chezmoi already does not execute empty scripts.

Move your condition to before the shebang line (note the changes to the template markers too):

{{ if the condition is met -}}
#!/bin/sh

<some commands here>
{{ end -}}

There's an example in the user manual.

goto1134 commented 1 year ago

The problem is that multiple conditions are in the same file with different actions. The real code is as follows:

{{- if eq .chezmoi.os "darwin" }}
#!/bin/sh
    {{/* https://git.herrbischoff.com/awesome-macos-command-line/about/#keyboard-media-keys */}}
    {{- if contains "com.apple.rcd" (output "launchctl" "list") -}}
launchctl unload -w /System/Library/LaunchAgents/com.apple.rcd.plist
    {{- end }}
    {{- $finderWasUpdated := false }}
    {{- range $app, $keyValues := (dict
    "com.apple.finder" (dict
        "AppleShowAllFiles" "1"
        "ShowExternalHardDrivesOnDesktop" "1"
        "ShowExternalHardDrivesOnDesktop" "1"
        "ShowHardDrivesOnDesktop" "1"
        "ShowRemovableMediaOnDesktop" "1"
        "ShowMountedServersOnDesktop" "1"
        "SidebarTagsSctionDisclosedState" "0"
        "QuitMenuItem" "1"
        "ShowPathbar" "1"
        "ShowStatusBar" "0"
        "NewWindowTarget" "PfLo"
        "NewWindowTargetPath" (printf "file://%s" .chezmoi.homeDir)
    )
        "kCFPreferencesAnyApplication" (dict
        "AppleShowAllExtensions" "1"
        "AppleInterfaceStyleSwitchesAutomatically" "1"
        "AppleMetricUnits" "1"
        "AppleTemperatureUnit" "Celsius"
        "AppleMeasurementUnits" "Centimeters"
        "AppleWindowTabbingMode" "always"
    )
    ) }}
        {{- $config := output "defaults" "read" $app }}
        {{- range $key, $value := $keyValues }}
            {{- if  not (and (contains $key $config) (contains $value (output "defaults" "read" $app $key))) }}
defaults write {{ $app }} {{ $key }} {{ $value }}
                {{- if eq $app "com.apple.finder" }}
                    {{- $finderWasUpdated = true }}
                {{- end }}
            {{- end }}
        {{- end }}
    {{- end }}
    {{- if $finderWasUpdated }}
killall Finder -q
    {{- end}}
{{- end }}

Is there an easy way to remove the shebang line from the file if no other line was added?

bradenhilton commented 1 year ago

If the script should not run, its contents should be completely empty. This is what placing the shebang inside the condition achieves.

goto1134 commented 1 year ago

Ok, I managed to add the shebang only if the script is not empty. I expected it to be more convenient than it was, but it still works.

I have the following strange behavior: if I run chezmoi status with the non-empty script, it shows me the script. If I call chezmoi diff <scriptName>, the command immediately returns without output. If I do the same sequence of steps with a non-empty script, I will see the diff.

What I expect is that

  1. If the script is empty, it does not show up in chezmoi status.
  2. If there is anything in chezmoi status, I can see the diff.
halostatue commented 1 year ago

IMO, you’re making the Chezmoi templating engine run too hard. Every time you call output, you’re shelling back out to execute default from chezmoi, which is at least one level off in terms of reasoning. Better to make a relatively easy-to-read shell script that always executes. As a bonus, because of the recently-added feature of $CHEZMOI_* variables, this no longer needs to be a templated script:

#! /usr/bin/env bash

# This is compatible with Bash 3+.

# https://git.herrbischoff.com/awesome-macos-command-line/about/#keyboard-media-keys

if launchctl list | grep -q '^com\.apple\.rcd$'; then
  launchctl unload -w /System/Library/LaunchAgents/com.apple.rcd.plist
fi

declare finder_was_updated key value
finder_was_updated=false

update_defaults() {
  local -i updated=0
  local app key value
  app="$1"
  shift

  if (($# % 2 != 0)); then
    echo >&2 "Default list for app ${app} is not a list of paired keys and values ($# items)"
    exit 1
  fi

  while (($#)); do
    key=$1
    value=$2

    shift 2

    if [[ "$(defaults read "${app}" "${key}" 2>/dev/null)" != "${value}" ]]; then
      defaults write "${app}" "${key}" "${value}"
      updated=1
    fi
  done

  return ${updated}
}

update_defaults com.apple.finder \
  AppleShowAllFiles 1 \
  ShowExternalHardDrivesOnDesktop 1 \
  ShowExternalHardDrivesOnDesktop 1 \
  ShowHardDrivesOnDesktop 1 \
  ShowRemovableMediaOnDesktop 1 \
  ShowMountedServersOnDesktop 1 \
  SidebarTagsSctionDisclosedState 0 \
  QuitMenuItem 1 \
  ShowPathbar 1 \
  ShowStatusBar 0 \
  NewWindowTarget PfLo \
  NewWindowTargetPath "file://${CHEZMOI_HOME_DIR}"

if (($?)); then
  killall Finder -q
fi

update_defaults kCFPreferencesAnyApplication \
  AppleShowAllExtensions 1 \
  AppleInterfaceStyleSwitchesAutomatically 1 \
  AppleMetricUnits 1 \
  AppleTemperatureUnit Celsius \
  AppleMeasurementUnits Centimeters \
  AppleWindowTabbingMode always
halostatue commented 1 year ago

I’ve been trying to reduce the number of template scripts I have to run (the ones that remain templates are so because I am using the digest of an external file to manage them).

twpayne commented 1 year ago

Hopefully this is now resolved. Please re-open if needed.