0

I have a bash script that make symlinks of all files in Folder A to Folder B. It works all fines when it is all files, but it also has recursion where it calls the function to do the same for folders within Folder A.

The issue is that once it enters a folder in Folder A, the for-loop ends there as if it has reached the end of the array to loop.

Any ideas why is that? I already learned about need for the local variables and did try to change to them as best I could but for some reason I cannot keep the loop iterator?

function add_core_files() {
  local FOLDER="${1}"
  local LINK_TARGET="${2}"

  printf "Copying symlinks from %s -> %s\n" "${FOLDER}" "${LINK_TARGET}"

  cd "${FOLDER}" || ""

  local FILES;
  FILES=$(ls) # doing this to make the array local

  printf "Found files (%s)\n" "${FILES}"

  local FILE;

  for FILE in ${FILES};
  do
    if [ "${FILE}" == ".." ] || [ "${FILE}" == "." ] || [ "${FILE}" == ".." ]; then
      continue;
    elif [ -L "${FILE}" ]; then
      ## symlink handling
      # get the target dir of the symlink
      local SYM_TARGET;
      SYM_TARGET=$(cd "${FILE}"; pwd -P);
      printf "Making symlink of symlink from %s -> %s\n" "${FOLDER}/${FILE}" "${LINK_TARGET}/${FILE}"
      ln -s -f "${SYM_TARGET}" "${LINK_TARGET}/${FILE}"
    elif [ -f "${FILE}" ]; then
      printf "Making symlinks from %s -> %s\n" "${FOLDER}/${FILE}" "${LINK_TARGET}/${FILE}"
      ln -s -f "${FOLDER}/${FILE}" "${LINK_TARGET}/${FILE}"
    elif [ -d "${FILE}" ]; then
      local TEMP_FOLDER="${FOLDER}/${FILE}";
      local TEMP_TARGET="${LINK_TARGET}/${FILE}";
      add_core_files "${TEMP_FOLDER}" "${TEMP_TARGET}"
      printf "Back, FOLDER: %s LINK_TARGET: %s\n files: (%s)\n" "${FOLDER}" "${LINK_TARGET}" "${FILES}"
      # <<<< ERROR HERE. Does not continue the for-loop
    fi
  done
Randommm
  • 410
  • 1
  • 9
  • BTW, your `FILES` isn't an array at all. Use `local -a files; files=( * )` to make a proper _array_ of files. User-defined variables shouldn't be all-caps -- POSIX specifies all-caps names for variables defined by or meaningful to the shell and other POSIX tools – Charles Duffy Jun 07 '23 at 11:16
  • If you had a proper array (and fixed your naming convention), you'd be writing `for file in "${files[@]}"; do ...` – Charles Duffy Jun 07 '23 at 11:17
  • Also, note that you're merging the 1980s ksh syntax `function add_core_files {` and the modern POSIX syntax `add_core_files() {` is a manner that's incompatible with both legacy ksh _and_ POSIX. – Charles Duffy Jun 07 '23 at 11:19
  • (and what is the `|| ""` supposed to do? Create a `"": command not found` error? -- I'd expect you'd want `|| return` to abort function execution early should the `cd` fail) – Charles Duffy Jun 07 '23 at 11:20
  • @CharlesDuffy that's a "fix" for "when cd fails" according to my IDE suggestion – Randommm Jun 07 '23 at 11:21
  • It's a bad one. You should have `|| return` to abort the function or `|| exit` to abort the script. Your IDE is right to tell you to handle the error, but you shouldn't handle it by just creating another error and then ignoring that one too. – Charles Duffy Jun 07 '23 at 11:21
  • See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html re: variable naming conventions; note in reading it that setting a regular shell variable overwrites any like-named environment variable, so the naming conventions specified for environment variables necessarily apply to regular shell variables as well. – Charles Duffy Jun 07 '23 at 11:22
  • BTW, what happens if you put something like `echo "Finished loop over ${FILES[*]@Q}, last executed was ${FILE@Q}" >&2` after the `done`, before the end of the function? (Assuming bash 5.x for the syntax). – Charles Duffy Jun 07 '23 at 11:24
  • The suggestion to switch from `ls` to a glob fed into an array, btw, is particularly relevant if you have any filenames with spaces; see [Why not parse the output of `ls`?](https://mywiki.wooledge.org/ParsingLs) – Charles Duffy Jun 07 '23 at 11:25
  • The static checker your IDE is almost certainly using, by the way, is shellcheck; its page describing how to fix the warning you were given is https://www.shellcheck.net/wiki/SC2164, where `|| return` and `|| exit` are demonstrated, and `|| ""` is very much not. – Charles Duffy Jun 07 '23 at 11:27
  • Not specific to this script but as a matter of good habit, I'd also suggest declaring _all_ your locals up-top; so, beginning of the function, one line `local file folder link_target temp_folder temp_target`, next line `local -a files` -- that way you aren't tempted to do things like `local var=$(somecommand)`, which shadows the exit status of `somecommand` with the exit status of `local` so `$?` isn't set correctly. You can do things like `if somevar=$(somecommand); then` to branch on `somecommand`, but you _can't_ do that with `if local somevar=$(somecommand); then`. – Charles Duffy Jun 07 '23 at 11:29
  • (but seriously, before I do any further thinking about failure modes around the specific question being asked, I want to see the output of the "Finished loop" `echo` command suggested above). – Charles Duffy Jun 07 '23 at 11:32
  • 1
    `Finished loop over $'file_1\nfile_2\nfolder_1\n....\nfile_last', last executed was 'file_last'` seems like it does go thru the list, but somehow none of the files after returning are taken as files/folders/symlinks... – Randommm Jun 07 '23 at 11:40
  • Running with `set -x` (`bash -x yourscript`) is a good place to go next. If you want the trace log to include line numbers, set something like `PS4=':$LINENO+'` – Charles Duffy Jun 07 '23 at 11:42
  • 1
    Okay, found the error. I cd into the directory, but then ofc I cant find the files there anymore. Added cd "${FOLDER}" after the recursion to go back to the right place – Randommm Jun 07 '23 at 11:43
  • Also consider doing the recursion in a subshell if the parent doesn't need variables the child sets; that way the `cd` is scoped to a subshell that only survives for as long as the function takes to run. – Charles Duffy Jun 07 '23 at 11:45
  • See [symlink-copying a directory hierarchy](https://stackoverflow.com/q/1240636/4154375) and [How to copy a folder structure and make symbolic links to files?](https://unix.stackexchange.com/q/196537/264812). – pjh Jun 07 '23 at 17:46

1 Answers1

0

Alright, figured out the error after Charles helped with debugging. cd "${FOLDER}" || "" changes the current folder so upon return the remaining list won't be valid as it refers to files found in the original folder. Added the same command after the recursion call to continue with the file list

...
elif [ -d "${FILE}" ]; then
      local TEMP_FOLDER="${FOLDER}/${FILE}";
      local TEMP_TARGET="${LINK_TARGET}/${FILE}";
      add_core_files "${TEMP_FOLDER}" "${TEMP_TARGET}"
      printf "Back, FOLDER: %s LINK_TARGET: %s\n files: (%s)\n" "${FOLDER}" "${LINK_TARGET}" "${FILES}"
      cd "${FOLDER}" || exit
    fi
Randommm
  • 410
  • 1
  • 9