specifying a non-empty output dir erases the entire contents of that dir without warning

[luckman212]

I just had an oops moment when I misunderstood that the --output parameter requires a non-existing or empty folder.

I ran the following command:

poetry run icons-asset-generator \
--path ~/Downloads/affinity/svg \
--image-name-remove c sq \
--output ~/Desktop \
diagrams.net

I got an error message that the dir wasn't empty, and then the program aborted. To my surprise, my entire ~/Desktop directory had been nuked. Luckily I had a backup, but this could easily be catastrophic.

I think this is the problematic code:

icons-asset-generator/icons_asset_generator/util/io.py

Lines 1 to 7 in dd0ed3e

	import os
	import shutil
	def create_output_dir(dir_name) -> None:
	shutil.rmtree(dir_name, ignore_errors=True)
	os.mkdir(dir_name)

Maybe a check for empty dir could be used instead (this is just an example)

import os
import shutil

from icons_asset_generator.util.logger import get_logger
logger = get_logger(__name__)

def is_directory_empty(path):
    if not os.path.isdir(path):
        raise ValueError("Invalid directory path")
    with os.scandir(path) as entries:
        for entry in entries:
            if not entry.name.startswith('.'):
                return False
    return True

def create_output_dir(dir_name) -> None:
    try:
        os.makedirs(dir_name, exist_ok=True)
    except Exception as e:
        logger.error(repr(e))
        exit(1) 
    try:
        assert(is_directory_empty(dir_name))
    except AssertionError:
        logger.error(f'directory {dir_name} is not empty!')
        exit(1)

[m-radzikowski]

To my surprise, my entire ~/Desktop directory had been nuked. Luckily I had a backup, but this could easily be catastrophic.

Oh, that's bad! Sorry for that!

Yes, as you noted, the script is trying to remove and re-create the output dir. This is useful if you are fiddling with the parameters and want to have a fresh output every time, but in retrospective, I agree that's dangerous. And you can do rm -rf ./library && poetry run icons-asset-generator ... to clean the output dir consciously.

I think requiring a nonexisting or empty output location is reasonable. If you can put this into a PR, I'll be happy to change this.

[luckman212]

Sure, let me put that together.