Run custom hooks in entrypoint for non empty directories

[d0niek]

Hi all!

You allow to run custom script via hooks here.

First you're looking for any files in hook directory:

if [ -z "$(ls -A "${hook_folder_path}")" ]; then

Then you iterate over *.sh files:

for script_file_path in "${hook_folder_path}/"*.sh; do

The problem is when hook directory contains some file(s) which don't have *.sh extension. if condition pass but for loop will give an one empty file and this will create an issue:

nextcloud_1  | => Searching for scripts (*.sh) to run, located in the folder: /docker-entrypoint-hooks.d/pre-upgrade
nextcloud_1  | ==> Running the script (cwd: /var/www/html): "/docker-entrypoint-hooks.d/pre-upgrade/*.sh"
nextcloud_1  | sh: 1: /docker-entrypoint-hooks.d/pre-upgrade/*.sh: not found
nextcloud_1  | ==> Failed at executing "/docker-entrypoint-hooks.d/pre-upgrade/*.sh". Exit code: 127

Here is my little hack to omit this. Create empty.sh file.

I think you should change ls -A ${hook_folder_path} with find ${hook_folder_path} -name "*.sh"

[joshtrichards]

Looks like a good catch, though I don't have time to look too closely this morning. Feel up for submitting a PR that adjusts the if clause target? I don't think find is even necessary: should be able to append the *.sh extension check to the ls even.

Cc: @dvaerum

[dvaerum]

So when I wrote this #1964, I originally had based it on using find

https://github.com/dvaerum/nextcloud-docker/blob/edea64b530ca145fb065be2e9178e7ce8a1edbca/docker-entrypoint.sh#L23-L49

I had some back and forth with @J0WI and I think in all of that I forgot that using find completely prevents the problem that @d0niek is running in too, and if we don't want to include sub-folders we can just use the argument -maxdepth.

I don't really believe that I have found a better way than find to solve this problem. Everything just ends op solving it in a more hacky way

[d0niek]

@dvaerum why we don't want to include sub-folders?

[dvaerum]

Seem it was @J0WI preference #1964 (comment), and I didn't have a preference either way.

However, here I should have realised my mistake about switching away from find - #1964 (comment)

but instead of continuing down the path of not using find - #1964 (comment)

[dvaerum]

I have tried to redeem my mistake with #2095 😉

[d0niek]

It looks really good! Still I don't understood why -maxdepth is nesesery but maybe it's for other discussion.

[dvaerum]

I guess that one could make an environment variable, which defaults to 1 but is otherwise possible to overwrite, some like "${SEARCH_MAX_DEPTH:-1}". Like, if it is a feature you need, but otherwise you need to take to J0WI. I did not need the feature, so did not push for it.

[J0WI]

I don't think that it's worth to add an extra variable for maxdepth. It's good to have some kind of limitation to avoid any excessive lookups if you mount a large directory (by accident) and I think 1 is a clearer policy than any other random number. It's still possible to extend this functionality with a mounted script but not vice versa.

[J0WI]

Fixed in #2095