External storage not supported

[AlvaroAguilera]

I don't know whether this is an issue in the workflow engine itself or in this particular workflow but I'm unable to run scripts on files stored on a SMB/CIFS network storage connected using the external storage option of NC (31.0.3).

None of the file paths passed to the script is available when the cron job is run.

[blizzz]

Actually, '%f' should be used with external storages.

[AlvaroAguilera]

The path I get with %f doesn't exist when an external storage is involved. I guess the drive isn't mounted at the time of execution. Everything works file with local drives though.

[AlvaroAguilera]

Problem still present in 31.0.4.

[blizzz]

%f is resolved at script execution time:

https://github.com/nextcloud/workflow_script/blob/21552a2427c2ee1458d355b29bedaf04f1cb68f1/lib/BackgroundJobs/Launcher.php#L31-L35

Do you see any related warning in your log?

[AlvaroAguilera]

I don't get inside the IF clause:

$command = (string)$argument['command'];
error_log(print_r($argument, true));

Array ( [command] => /data/bin/multiextract.sh '/data/tmp/oc_tmp_YIRYtiqxKDbhOdD4bKOGD8oo8b89VWdd.zip' '8266862' 'admin' )

The %f is replaced somewhere before run().

[blizzz]

Hm… as a workaround, try to comment out this line in Operation.php:

https://github.com/nextcloud/workflow_script/blob/21552a2427c2ee1458d355b29bedaf04f1cb68f1/lib/Operation.php#L189

[AlvaroAguilera]

That doesn't change anything. The %f is still replaced before the run somewhere else.

[AlvaroAguilera]

It appears I'm not even entering Operation::buildCommand()

[blizzz]

The other place is in the background job as shown in https://github.com/nextcloud/workflow_script/issues/385#issuecomment-2821871876

There are no other places where it would be replaced.

[AlvaroAguilera]

I commented both of the lines, yet I still get a path on Launcher::run:

echo $wrapper:

sh -c '/data/bin/multiextract.sh '\''/data/tmp/oc_tmp_kDLk9DHuVQiBJWNzR3OeUO845Iydwm92.zip'\'' '\''8266862'\'' '\''admin'\''' >/dev/null &

[AlvaroAguilera]

Some context:

[blizzz]

Yes, $wrapper has this replaced, as it should be.

But since it runs in the background, it could be that the tmp file is cleaned up by Nextcloud along the way, when cron.php finished executing 🤔

[AlvaroAguilera]

This only happens with external storage. Local files work fine. Are you familiar with how this works? Are SMB drives mounted on-demand? Perhaps the drive gets unmounted before the run()?

[blizzz]

The toTmpFile() would copy it to the local machine

[AlvaroAguilera]

I did some more testing today.

Operation::buildCommand() is working fine. It creates a temporary file and replaces the %f with the path. The file still exists at the end of buildCommand() which is run as soon as I set the trigger tag.

Launcher::run() on the other hand, is triggered by the cron-job. At this point the temporary file doesn't exist anymore.

[blizzz]

I did some more testing today.

Operation::buildCommand() is working fine. It creates a temporary file and replaces the %f with the path. The file still exists at the end of buildCommand() which is run as soon as I set the trigger tag.

Launcher::run() on the other hand, is triggered by the cron-job. At this point the temporary file doesn't exist anymore.

Good insights!

https://github.com/nextcloud/workflow_script/issues/385#issuecomment-2823464411 should then work however, as it would suppress replacing %f in buildCommand and leave it for the background job.

[AlvaroAguilera]

I fixed the issue by making the following changes:

• deactivating the %f substitution in Operation::buildCommand()
• removing the escapeshellarg() from the str_replace() in Launcher::run() because it was messing things up.

A couple of things that are problematic. Copying the files every time is not good when working with big files. It can also lead to storage exhaustion if the server admin didn't size /tmp properly. I think now two copies of the file are being created, once when tagging (using the settings of Apache's php.ini) and a second by the cron job (using the cli-php.ini).

Something I don't like when working with %f is that you lose the original directory you are working on. I would like to rescan it as a last step but I don't have the original path, so I'll need to also pass %n to my script. To make it more complicated, scanning external storage is different than scanning local one. It would be nice to also have tags for this.

[blizzz]

I fixed the issue by making the following changes:

* deactivating the %f substitution in Operation::buildCommand()

Then we need better logic on when to decide to have an early replacement (for files that are available on the system) or when to leave them for the background job. Actually best might be to leave it up for the background job in any case.

* removing the escapeshellarg() from the str_replace() in Launcher::run() because it was messing things up.

🤔 was it double escaped?

A couple of things that are problematic. Copying the files every time is not good when working with big files. It can also lead to storage exhaustion if the server admin didn't size /tmp properly. I think now two copies of the file are being created, once when tagging (using the settings of Apache's php.ini) and a second by the cron job (using the cli-php.ini).

If you are talking about automatted tagging, then it does not download anything, it works directly on the event when the cache is involved. The file itself is passed on to the destination storage.

Some housekeeping about stray temp files might be a different thing though, and could be a worthy addition.

Something I don't like when working with %f is that you lose the original directory you are working on. I would like to rescan it as a last step but I don't have the original path, so I'll need to also pass %n to my script. To make it more complicated, scanning external storage is different than scanning local one. It would be nice to also have tags for this.

Exactly, you use %f when you want to do something with that file and if you need the location in Nextcloud, use %n, or both to cover both cases as you do.

Or you use %n solely (or directly the id %i) and utilize occ' (if possible) and its file:` commands.

[AlvaroAguilera]

Yes, it was being double escaped.

The thing is that in the context of the script you are running, you don't know whether you are working on local or external storage. They both have two different occ commands for rescanning: "occ files:scan" and "occ files_external:scan".

The output of the script is also stored in the temporary directory of %f. I need to figure out how to move the files created there back to the original folder (local or external).

[blizzz]

occ info:file can help with it, alas it does not have a nicely consumable output apart of plain text

[blizzz]

But apart of scan, I think most other commands work with any file, but did not double check.