Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indent script messes up permissions. #16427

Closed
bangerth opened this issue Jan 7, 2024 · 4 comments · Fixed by #16429
Closed

Indent script messes up permissions. #16427

bangerth opened this issue Jan 7, 2024 · 4 comments · Fixed by #16429

Comments

@bangerth
Copy link
Member

bangerth commented Jan 7, 2024

#16419 results in mayhem on my system because it changes the permissions of the source directory to 644 -- which is bad for a directory, because you can't access that directory any more and everything that's running inside it seriously freaks out.

@drwells FYI

@peterrum
Copy link
Member

peterrum commented Jan 7, 2024

@bangerth Could you check #16428? Thanks!

@bangerth
Copy link
Member Author

bangerth commented Jan 7, 2024

The issue, at its core, rests here:

process_changed()
{
  LAST_MERGE_COMMIT="$(git log --format="%H" --merges --max-count=1 master)"
  COMMON_ANCESTOR_WITH_MASTER="$(git merge-base "${LAST_MERGE_COMMIT}" HEAD)"

  case "${OSTYPE}" in
    darwin*)
      XARGS="xargs -E"
      ;;
    *)
      XARGS="xargs --no-run-if-empty -d"
      ;;
  esac

  ( git ls-files --others --exclude-standard -- ${1};
    git diff --name-only $COMMON_ANCESTOR_WITH_MASTER -- ${1} )|      # **1
      sort -u |
      xargs -n 1 ls -d 2>/dev/null |
      grep -E "^${2}$" |
      ${XARGS} '\n' -P 10 -I {} bash -c "${3} {}"
}

If no file is changed, but you call the indent script anyway, the command at # **1 returns an empty list, which sort leaves as an empty list, but then xargs returns just . (the current directory) because echo | xargs -n 1 ls -d 2>/dev/null returns .. We then execute the chmod on this directory.

@bangerth
Copy link
Member Author

bangerth commented Jan 7, 2024

We really shouldn't be passing "*" as second argument of process_changed.

@drwells
Copy link
Member

drwells commented Jan 7, 2024

Whoops. I definitely missed that line in the original PR. My mistake!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants