761586 - mozcrash.check_for_crashes does the wrong thing when MINIDUMP_SAVE_PATH is set to a nonexistent directory

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[(not currently active) Ted Mielczarek]

If MINIDUMP_SAVE_PATH is set to a nonexistent dir, then the shutil.move in checkForCrashes will just move the dump file to a file with that name. We're hitting this on the slaves right now, they're just getting stuck in a file named "minidumps", with the newest one overwriting the previous one.

We'll have to stick in an os.path.isdir check there, mkdir it if it doesn't exist, and also unlink it if it exists as a file already, to clear up the existing mess.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: handle non-existent dump_save_path

Since our test harnesses are using mozcrash now, we'll just fix it there.

Comment 2

[Ed Morley [:emorley]]

Comment on attachment 748028 [details] [diff] [review]
handle non-existent dump_save_path

Review of attachment 748028 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozcrash/mozcrash/mozcrash.py
@@ +125,4 @@
>              if dump_save_path is None:
>                  dump_save_path = os.environ.get('MINIDUMP_SAVE_PATH', None)
>              if dump_save_path:
> +                # This code did not previouslycreate the directory,

Missing space between "previously" and "create".

@@ +131,5 @@
> +                    os.unlink(dump_save_path)
> +                if not os.path.isdir(dump_save_path):
> +                    try:
> +                        os.makedirs(dump_save_path)
> +                    except OSError, e:

This results in pyflakes complaining about 'e' since not used, can we drop it please.

Comment 3

[(not currently active) Ted Mielczarek]

Pushed with those nits fixed:
https://github.com/mozilla/mozbase/commit/65d40d28e111e3bfe2d2c623e3bd11302c47bfd2