Cannot substitute Int only Str in recipe

[jgstew]

Describe the problem

If you try to substitute anything other than a string using %substitution% within a recipe, autopkg throws an error, even for things like Int or Boolean that are easily represented as strings.

As far as I know this has ALWAYS been the case with autopkg, but it is quite annoying.

Consider this recipe example: https://github.com/jgstew/jgstew-recipes/blob/main/Test-Recipes/IntSubtitution.test.recipe.yaml

---
Description: Test Int Subtitution - failing test
Identifier: com.github.jgstew.test.IntSubtitution
Input:
  NAME: IntSubtitutionTest
  Number: 123
MinimumVersion: "2.3"
Process:
  - Processor: EndOfCheckPhase
    Arguments:
      test_string: "%Number%"

This does NOT happen if Number: "123" is used instead of Number: 123

Preferences contents

N/A

AutoPkg output

**load_recipe time: 0.007173467020038515
Processing C:\Users\james\Documents\_Code\jgstew-recipes\Test-Recipes\IntSubtitution.test.recipe.yaml...
WARNING: C:\Users\james\Documents\_Code\jgstew-recipes\Test-Recipes\IntSubtitution.test.recipe.yaml is missing trust info and FAIL_RECIPES_WITHOUT_TRUST_INFO is not set. Proceeding...
EndOfCheckPhase
Traceback (most recent call last):
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkg", line 2725, in <module>
    sys.exit(main(sys.argv))
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkg", line 2721, in main
    return subcommands[verb]["function"](argv)
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkg", line 2218, in run_recipes
    autopackager.process(recipe)
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkglib\__init__.py", line 866, in process
    processor.inject(step.get("Arguments", {}))
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkglib\__init__.py", line 643, in inject
    update_data(self.env, key, value)
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkglib\__init__.py", line 488, in update_data
    a_dict[key] = do_variable_substitution(value)
  File "C:\Users\james\Documents\_Code\autopkg\Code\autopkglib\__init__.py", line 470, in do_variable_substitution
    item = RE_KEYREF.sub(getdata, item)
TypeError: sequence item 0: expected str instance, int found

Expected behavior
I expect autopkg to turn the int value into a string and substitute it into the variable.

Version (please complete the following information):

• OS version: Win, Mac, Linux
• AutoPkg Version: All (3.0.0)

[jgstew]

The problem appears to be in here:

autopkg/Code/autopkglib/__init__.py

Lines 466 to 470 in e7bdfe1

	def do_variable_substitution(item):
	"""Do variable substitution for item"""
	if isinstance(item, str):
	try:
	item = RE_KEYREF.sub(getdata, item)

[jgstew]

I don't actually know if this is a good idea or what the implications might be, but this does seem to fix this specific issue:

def update_data(a_dict, key, value):
    """Update a_dict keys with value. Existing data can be referenced
    by wrapping the key in %percent% signs."""

    def getdata(match):
        """Returns data from a match object"""
        return a_dict[match.group("key")]

    def getdata_str(match):
        """Returns string data from a match object"""
        return str(a_dict[match.group("key")])

    def do_variable_substitution(item):
        """Do variable substitution for item"""
        if isinstance(item, str):
            try:
                item = RE_KEYREF.sub(getdata, item)
            except KeyError as err:
                log_err(f"Use of undefined key in variable substitution: {err}")
            except TypeError as err:
                if "sequence item 0: expected str instance, int found" in str(err):
                    log(f"WARNING: subtituting int for string: {err}")
                    item = RE_KEYREF.sub(getdata_str, item)
                else:
                    raise err
        elif isinstance(item, (list, NSArray)):
            for index in range(len(item)):
                item[index] = do_variable_substitution(item[index])
        elif isinstance(item, (dict, NSDictionary)):
            # Modify a copy of the original
            if isinstance(item, dict):
                item_copy = item.copy()
            else:
                # Need to specify the copy is mutable for NSDictionary
                item_copy = item.mutableCopy()
            for key, value in list(item.items()):
                item_copy[key] = do_variable_substitution(value)
            return item_copy
        return item

    a_dict[key] = do_variable_substitution(value)

[jgstew]

I just submitted a PR that addresses this, though I am just now realizing this does NOT address the boolean case, which I just tested, due to the way I have it written.

In the boolean case, you get:

TypeError: sequence item 0: expected str instance, bool found

[jgstew]

I just realized I could also fix this issue by adding a processor:

• https://github.com/jgstew/jgstew-recipes/blob/main/SharedProcessors/VariableToString.py
• https://github.com/jgstew/jgstew-recipes/blob/main/Test-Recipes/VariableToString.test.recipe.yaml

I still think it would be nice if autopkg handled this transparently though, but this lets you handle it explicitly.

[MLBZ521]

Just skiming through here and happened to come across this issue and it's related PR. It looks very similar to the issue that I described and submitted a PR for in #911. Does this sound accurate @jgstew ?

I'm also not sure if my PR has the correct "assumption" in mind when it comes to the expectations of substitution variables.

[jgstew]

@MLBZ521 I do think your PR is related, but not exactly the same, though I'm uncertain.

From looking over your PR, it seems like it should be the processor that handles the case better.

This PR is more about handling the case where variable substitution is being used with non strings and should apply to both YAML and non YAML I think?

I couldn't quite tell if your issue related to variable substitution specifically or not?

[MLBZ521]

I think it's both.

The Processor could be written to handle the types better....but if AutoPkg (proper) was updated to handle types besides str, then Processors would have to be written to better handle more than str types.

What my PR is proposing is:

• Allow the recipe to be written with whatever type (e.g. value can be an int or boolean, etc.)
• When the recipes is "loaded" those values are converted to str

This allows the Processors to still expect strs as they have traditionally done.

However, my PR may be to "heavy" on converting everything to a str...

[MLBZ521]

But yes -- my PR is specifically for YAML only.

[gregneagle]

Maybe I'm misunderstanding but we definitely don't want autopkg to behave differently if a recipe happens to be formatted in yaml versus one formatted as a plist.

I think I agree that autopkg should just convert all values to strings before doing string substitution, even when doing so might have "interesting" consequences. (Like converting an array of strings into a string). Those consequences are really on the recipe writer to anticipate.

Alternately, we could convert only "scalar" types and not lists/arrays or dictionaries.

[MLBZ521]

I think we're on the same page Greg. It's which direction would be the preferred way to go:

• Convert all values to strings
  • This is what Switch to yaml.BaseLoader to prevent automatic type conversions #911 does
• Convert only scalar types to strings
  • I can either adjust Switch to yaml.BaseLoader to prevent automatic type conversions #911 or open a separate PR to provide this functionality
  • I am leaning to this myself, just to make it "easier" to support these non-scalar types being values

[jgstew]

So the way I was proposing handling this is to attempt to convert variables into strings ONLY when doing substitution, but otherwise their type would be maintained.

I think this is important because I do have processors that set variables as non strings and operate on them as non strings intentionally (dictionaries, arrays)

I basically figured out some options for when the variable substitution function throws an error, then at that point attempt to convert the items to a string and do it again.

This addresses the issue regardless of YAML or Plist recipe.

[MLBZ521]

Yeah, I think long term adjusting the do_variable_substitution logic would provide much more flexibility to recipe writers, regardless of the recipe language format.

In the short term, I've proposed an new solution for YAML recipes in #929. This solution is the latter option in my previous reply on 12/7/2023.