Why do you think so? Changing directory can only be done, if it exists, otherwise cd will error out and rm command not run. After the command the temporary directory is empty and cd can go up one dir, to use rmdir on an empty directory. Which of the steps are fragile and why?

My reply is rejecting (most of) your suggestions, with reasons off course. I am glad you bring them up, so we can talk about it.

I would avoid set -e option, as I do not want he entire script to exit on error. So instead I can use the exit command when I really want to on specific errors. I rather would like to handle errors myself directly, maybe even not exiting, but displaying error code with $? in example.

TEMPDIR=$(mktemp -d || echo "/invalid")

If anything, it would make more sense to just exit the script with || exit. In fact that is what I’m doing in the script after the trap command by cd "${TEMPDIR}/tests" || exit, so the script never continues without a successful mktemp directory.

TEMPDIR=${TEMPDIR:-"/invalid"}

I always forget that Bash has default values for variables! mktemp actually makes sure it never returns an empty value. I’m not worried about what it returns, but that my script could change the value of $TEMPDIR by accident (in example to something empty). So assigning a default value after mktemp will never have a chance to get the default value at all.

rm -rf ${TEMPDIR:-"/invalid"}

This on the other hand I like a lot. Now I will not stop doing my other additional checks, but for good habit this can’t be wrong. Maybe instead a custom directory name with an unlikely name, what about pointing it to /dev/null? I actually like this idea and might incorporate it.

Funny enough I’m writing (and wrote before) a Python script that is all about files and renaming. And this Bash script is about creating a test environment. Maybe its actually a good idea to write this in Python too.

I know, good habit, check variable and so on. But mistakes happen, so what’s wrong with hardening the case? There are decades of studies and good practice for coding, yet the best programmers still do mistakes. And having a script that could potentially delete files on your system (even all subdirectories) is dangerous and should be handled with respect. I really don’t understand the opposition here.

In terms of defensive programming, anything extra is added complexity that will only make an error more likely imo.

I disagree here. Adding checks will help in catching those errors. Being not defensive about it will make it only more likely to make errors.

no amount of trap logic is going to make a future programming error impossible.

No amount of any programming will make it impossible to error out. That does not mean we shouldn’t try to make it as secure as we can think. Renaming the variable is putting the risk to another name, not really solving the issue.

That’s an interesting way of doing it. I do not try to avoid rm completely, just want to make sure the construct doesn’t have any obvious issues. Your suggestion is a bit too much for the few test files I wanted to create and delete.

Isn’t cd with && essentially doing that? Chain only runs, if TEMPDIR exists as a directory.

Edit: Please ignore this reply here, and read the discussion following the answers. I absolutely misunderstood the above reply.

mktemp --suffix=$$ does not really solve the issue I have. I don’t want to use ${TEMPDIR}, but have something hardcoded when using the variable, as in rm -rf "${TEMPDIR}" vs rm -rf "${TEMPDIR}/tests". Because I’m not worried about what mktemp gives me back, but about when the variable is used at later time. The content of the variable could be altered after its initial creation.

In your trap assert that the content of var TEMPDIR ends with your same current PID or fail before you clean anything up.

I quiet don’t understand this point here. The trap command will run in any case the script exits, be a crash or normal exit. If the variable or directory is invalid, then the cleanup will not be executed. But that is by design, because I do not want to cleanup something that is not working correctly. I rather leave it to be cleaned up automatically with next reboot.

You could also assert TEMPDIR is prefixed with $TMP or /tmp for even more robustness.

It has already a fixed suffix part with “/tests” in use. I’m not worried about the TEMPDIR content if mktemp created it correctly. I’m more worried about the variable being altered and invalid at later point in the script. I would rather leave mktemp create the directory where it thinks is the best place (mostly it is /tmp, but that is not guaranteed). So prefixing the variable content itself doesn’t really solve the trust issues I have here, as it is not the creation time that I’m worried about.

it’s susceptible to a race condition where another process adds to TEMPDIR some other way during the script, and potentially even recreates tests after you delete it and before you remove the parent directory.

You mean a subprocess from this script? In that case, the variable $TEMPDIR is never changed from the perspective of the script. Because its not exposed to subprocesses and the name is totally random by mktemp. So I don’t see how a subprocess should be able to do that.

Usually with Bash, the most readable solution is the best. I’d recommend a simple test for $TEMPDIR existing before a simple rm -rf “$TEMPDIR”.

This is what I want to avoid. Because what if I make mistakes in my own script and reassign $TEMPDIR by accident in a loop, instead reading from it. So at the time of execution of rm -rf, there is a chance that $TEMPDIR could potentially point to a different directory in example.

I usually avoid it too. But there are cases when its needed, like creating a clean test environment for test cases for scripts manipulating files and directories.

./$TEMPDIR

That doesn’t solve my fear at all. “./” is still just relative. Because if in example “${TEMPDIR” happens to be empty, for whatever reason like wrong variable assignment or a typo, then “./${TEMPDIR” might resolve to “./” or any random directory that it has been assigned to. I would rather have a hardcoded name that is not just relative.

It’s a script for testing another script or program. I want to set up a clean environment of files, that are created and deleted each time before running the test cases.

I think you should start the last line with [ -d “$TEMPDIR” ] && cd …

So make sure directory exists, before cd into it? cd can only enter a directory that exists anyway, and the following && ensures following command runs only if directory exists and cd changed into it.

Do you even need to use -f with rm?

It depends if I create subdirectories in an uncontrolled manner. That was the plan, but I might change the plan. So just create temporary files only, then I would not need any recursive deletion.

I actually have trash-cli installed and exclusively use it for deleting all trash directories available on my system. Because I experience some inconsistencies how applications handle trash directories, involving mounted external drives.

Moving files instead deleting them, in case they are important files is a good advice. However in case of temporary created and deleted files for testing software, I think this goes a bit too far. But it could prevent data loss, in case something goes wrong and I delete the wrong directory (hopefully the trash directory does not get deleted too, due to recursive deletion). Overall, this is a good advice to have in mind, I just think it goes a bit too far in this use case.