[feature] Potential changes for lockfile bundle structure

[solvingj]

When actually working with the structure, I've identified some potential UX improvements. I'll just be dropping notes here and potentially updating as I get more experience with them.

First, I suggest the following structural change, but I admit, that it's mostly a matter of preference or cosmetics.

The first thing that jumped out is that it's awkward that there's a property named package_id and it contains another object rather than a string of the package_id. It seems we might have multiple package_id listed here, so in this case, a list of objects seems to be to be a more appropriate data structure than a map. The current json structure is the right structure when your common case is selecting an object based on a known key. But, our case is iterating over objects and creating a stage for each package_id. So, it's just slightly more intuitive and self-descriptive to allow users to iterate over each object and calling it.package_id to get the package_id string.

Current lockfile structure.

    "lock_bundle": {
        "libb/1.0@ci/stable#d5ea655b818266c5d206f8ac40d7ceeb": {
            "package_id": {
                "156178a1b5fe4c6bc11c8fcd8a39a4f328ba3808": {
                    "lockfiles": {
                        "locks/dev/libb/1.0/./app1/1.0/release-gcc7-app1/conan.lock": [
                            "3"
                        ]
                    },
                    "prev": "ab004536b7bbeafff110adf14d1a75cc",
                    "modified": null
                }
            },
        "requires": [
            "liba/1.0@ci/stable#e5c376e53be7064cafdc796d63a3a77c"
        ]
    },

Suggested...

    "lock_bundle": [{
        "reference": "libb/1.0@ci/stable#d5ea655b818266c5d206f8ac40d7ceeb",
        "packages": [{
            "package_id": "156178a1b5fe4c6bc11c8fcd8a39a4f328ba3808",
            "lockfiles": [{
                "path": "locks/dev/libb/1.0/./app1/1.0/release-gcc7-app1/conan.lock",
                "id": "3",
            }],
            "prev": "ab004536b7bbeafff110adf14d1a75cc",
            "modified": null
            "requires": [
                "liba/1.0@ci/stable#e5c376e53be7064cafdc796d63a3a77c"
            ]
        }]
    }],

The only thin I'm not sure of is if the implementation depends on the map being ordered, and whether the list might behave differently. So, in that case, you might just have to add an "id" field to each object in side lock_bundle to indicate order.

If this is rejected due to simply "disagree", I totally understand.

• I've read the CONTRIBUTING guide.

[memsharded]

Hi @solvingj

There is a reason for this. The bundle contains unique references, making them a list could potentially introduce duplicates. But the real issue is the access pattern from the build-order, note the for-loop in:

for level in build_order:  # iterate the build_order
    for ref in level:  # All refs in this level could be built in parallel
        # Now get the package_ids and lockfile information
        package_ids = bundle[ref]["package_id"]

Accessing the bundle[ref] simplifies the access. Otherwise the user code should first iterate the list and construct a dictionary that would accept the [ref] as key.

I agree the inner one will be much nicer with a packages field as you suggest, I will change it and submit it for 1.34.1. Do you think it still make sense to change the reference one?

[solvingj]

It think so yes. Can you so the new structure you're proposing then?

[memsharded]

I would suggest this (accept the inner level suggestion, keep the "references" level one:

 "lock_bundle": {
        "libb/1.0@ci/stable#d5ea655b818266c5d206f8ac40d7ceeb": {
            "packages": [
                   {
                    "package_id": "156178a1b5fe4c6bc11c8fcd8a39a4f328ba3808",
                    "lockfiles": {
                        "locks/dev/libb/1.0/./app1/1.0/release-gcc7-app1/conan.lock": [
                            "3"
                        ]
                     },
                    "prev": "ab004536b7bbeafff110adf14d1a75cc",
                    "modified": null
                   }
             ],
           "requires": [ "liba/1.0@ci/stable#e5c376e53be7064cafdc796d63a3a77c" ]
    },

[solvingj]

Ok, looks good. Does requires go inside the package object though?

[memsharded]

Does requires go inside the package object though?

No, requires is a sibling of packages, it belongs to the reference (we are ordering build_order by references, not packages).

[solvingj]

I was thinking that requires depended on package_id, and packages with different package_id might have different requires

[memsharded]

I was thinking that requires depended on package_id, and packages with different package_id might have different requires

That is true for regular lockfiles, but one of the main purpose of this bundle is to orchestrate build per reference, so all "package_ids" of the same reference will be built in parallel, but in the same CI task. Thus, requires capture the dependencies at the reference level, not the package level. Does it make sense?

[memsharded]

Implemented in #8579