labgrid icon indicating copy to clipboard operation
labgrid copied to clipboard

driver: remove `mktemp` tool dependency in `ShellDriver`

Open flxzt opened this issue 1 year ago • 2 comments

Description

This removes the use of the mktemp tool on targets when transferring files through the ShellDriver. It is replaced by touch /tmp/tmp-<random-uuid> which should be always available since touch is part of the GNU core utilities.

Checklist

  • [x] PR has been tested

flxzt avatar Aug 20 '24 13:08 flxzt

This is not equivalent to the old behavior, as touch may create files readable by other users. Also, touch doesn't fail if the file already exists.

At least, this change needs a good reasoning why this changes is an improvement and doesn't break existing usecases.

jluebbe avatar Aug 20 '24 15:08 jluebbe

I can only speak for my use-case but it is the following: I came across a target that has only the most basic tools available, not much more than coreutils and does not have the ability to easily add additional tools when testing the production image. I had hoped this dependency removal is something which benefits other users with similar targets as well. I can't see how it breaks existing use-cases since the created tempfile really is only used for a short while until it's contents are dd'ed to the actual destination.

I didn't know that file permissions are a concern here, let me add a chmod 600 which should replicate the mktemp's default behaviour (unless you have a different suggestion).

Regarding not failing when a file with the same name already exists - if really wanted a check can be added before calling touch, but the probability of collision of a UUID4 which the to-be-created file contains in it's path according to wikipedia : number of random version-4 UUIDs which need to be generated in order to have a 50% probability of at least one collision is 2.71 quintillion source.

flxzt avatar Aug 20 '24 16:08 flxzt