-
Notifications
You must be signed in to change notification settings - Fork 175
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
driver: remove mktemp
tool dependency in ShellDriver
#1481
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Felix Zwettler <[email protected]>
53273a6
to
d17417b
Compare
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. |
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 didn't know that file permissions are a concern here, let me add a chmod 600 which should replicate the Regarding not failing when a file with the same name already exists - if really wanted a check can be added before calling |
raise ExecutionError('Could not make temporary file on target') | ||
tmpfile = tmpfile[0] | ||
tmpfile = f'/tmp/tmp-{uuid4()}' | ||
self._run_check(f'touch {tmpfile}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about only doing this as a workaround when mktemp fails? Then we can keep using the normal approach for most cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work for me - as a fallback. What are your thoughts about it @jluebbe ?
Description
This removes the use of the
mktemp
tool on targets when transferring files through theShellDriver
.It is replaced by
touch /tmp/tmp-<random-uuid>
which should be always available sincetouch
is part of the GNU core utilities.Checklist