gnome-3-34 extension - build-environment needs to merge

[Heather Ellsworth]

The gnome-3-34 extension currently sets a given build-environment for all parts. However, if there is a user-defined build-environment in the yaml that uses the extension, the extension build-environment overwrites the user-defined one.

1. These two build-environments should be merged
2. Where a user has defined an environment variable that is also defined in the extension, the user-defined property should overwrite the extension one.

[Heather Ellsworth (hellsworth)]

The gnome-3-34 extension has not yet been merged. The open PR is: https://github.com/snapcore/snapcraft/pull/2794

[Heather Ellsworth (hellsworth)]

On the surface, it appeared that we were just unable to merge the build-environment section that is defined in a user's snapcraft.yaml with the build-environment defined in the gnome-3-34 extension.

Here is the snippet in _utils.py responsible for merging two lists.

def _merge_lists(list1: List[str], list2: List[str]) -> List[str]:
    """Merge two lists while maintaining order and removing duplicates."""
    seen = set() # type: Set[str]
    merged = list() # type: List[str]

    for item in list1 + list2:
        if item not in seen:
            seen.add(item)
            merged.append(item)

    return merged

list1 is the build-environment content provided in the user-defined snapcraft.yaml.
list2 is the build-environment content provided in the gnome_3_34.py extension

list1 is a list of OrderedDicts while list2 is a list of dicts. The problem is with the "if item not in seen" line because the set datastructure in python requires objects that the __hash__() method can be implemented on and OrderedDict (a subclass of dict) does not implement hash() so then we get the following error:

Sorry, an error occurred in Snapcraft:
unhashable type: 'collections.OrderedDict'

Then I asked “Well how are the root_snippets merged?”, looking for an example of how it should be done. Looking into the root_snippet section, it became clear that this actually affects the extension's root snippet additions too! So for example, if a user defines some layout in their snapcraft.yaml (passed around in _utils.py as an OrderedDict) and if there is a layout defined in the extension (passed around in _utils.py as a dict), then _merge_lists is never called and the extension_property is returned and is the only layout used. So this "build-environment issue" a larger issue, not just limited to merging the part definitions.

Looking at the _apply_extension_property definition, we see that two cases are handled: when both the user-defined and extension-defined elements are lists and when both are dicts. There is no handling of cases with OrderedDict.

In the root_snippet section, the existing_property is itself an OrderedDict. Since the extension_property is a dict, _merge_lists is never called and only the extension_property is returned.

In the part_snippet section, the existing_property is a list of OrderedDicts. Since the extension_property is a list too, _merge_lists is called but fails on iterating over the items.

To account for both cases of OrderedDicts, we can make the following changes to _apply_extension_property:

1. Convert any existing_property that is a list of OrderedDicts to a list of dicts
2. Convert any existing_property that is an OrderedDict to a dict

    if isinstance(existing_property, collections.OrderedDict):
        existing_property = dict(existing_property)
    if isinstance(existing_property, list) and isinstance(extension_property, list):
        temp_list = existing_property
        for item in existing_property:
            if isinstance(item, collections.OrderedDict):
                existing_property[item] = dict(item)
            else:
                break
        return _merge_lists(existing_property, extension_property)
    e...

[Heather Ellsworth (hellsworth)]

I'm still having an issue properly iterating over the list of OrderedDicts that represents the build-environment vars. Probably a separate loop will need to be added to deal with this case.

[Heather Ellsworth (hellsworth)]

This issue should be addressed with the _utils.py changes found in the merge request:
https://github.com/snapcore/snapcraft/pull/2794/files

[Heather Ellsworth (hellsworth)]

Waiting to close this until the MR in the previous comment is merged.

[Sergio Schvezov (sergiusens)]

We went down a different path for this as implemented in part by you (reporter) :-)