autopkg / dataJAR-recipes

Elegant and powerful Apple services for business and education.
https://www.datajar.co.uk
Apache License 2.0
116 stars 85 forks source link

AdobeAdminConsolePackagesImporter.py Support for multiple Override Dirs #371

Open davidbpirie opened 1 week ago

davidbpirie commented 1 week ago

I have multiple directories configured in RECIPE_OVERRIDE_DIRS, which results in an error when using AdobeAdminConsolePackagesImporter.py. I believe the issue starts at line 476:

    # If "RECIPE_OVERRIDE_DIRS" is defined within "autopkg_preferences"
    if 'RECIPE_OVERRIDE_DIRS' in autopkg_preferences:
        # Set "override_dirs" to "RECIPE_OVERRIDE_DIRS"
        override_dirs = os.path.expanduser(autopkg_preferences['RECIPE_OVERRIDE_DIRS']).split()
    # If not defined
    else:
        # Set to the default path
        override_dirs = [os.path.expanduser('~/Library/AutoPkg/RecipeOverrides')]

    # If override_dirs is a string
    if isinstance(override_dirs, str):
        # Convert to list, expanding if needed
        override_dirs = [os.path.expanduser(override_dirs)]

I am not sure what the purpose of the .split() is for, but given RECIPE_OVERRIDE_DIRS can be either a string or array of strings (see here) I changed this section to the below in my fork and it corrected my issues:

    # If "RECIPE_OVERRIDE_DIRS" is defined within "autopkg_preferences"
    if 'RECIPE_OVERRIDE_DIRS' in autopkg_preferences:
        # Set "override_dirs" to "RECIPE_OVERRIDE_DIRS"
        # If override_dirs is a list
        if isinstance(autopkg_preferences['RECIPE_OVERRIDE_DIRS'], list):
            override_dirs = [os.path.expanduser(x) for x in autopkg_preferences['RECIPE_OVERRIDE_DIRS']]
        else:
            override_dirs = [os.path.expanduser(autopkg_preferences['RECIPE_OVERRIDE_DIRS'])]
    # If not defined
    else:
        # Set to the default path
        override_dirs = [os.path.expanduser('~/Library/AutoPkg/RecipeOverrides')]

I was going to do a PR, but I also had a fix for yaml overrides that you have since patched, so I would have first get my branch into sync with yours, and thought it would be easier just to paste the code here. I also wanted to check I wasn't missing something with the use of the .split(). However let me know if you would prefer this as a PR.

macmule commented 1 week ago

PR’s are always preferred.

We don’t tend to have multiple override dirs.. so guess in fudged it.

Please rev the version in your commit and add example output with the additional override dirs.. we can test with a single one.. and if all good, we’ll merge.