rpm-software-management / dnf5

Next-generation RPM package management system
Other
238 stars 79 forks source link

Replaying serialized transaction with multiple actions per nevra should cause an error. #1573

Open kontura opened 3 months ago

kontura commented 3 months ago

I think that replaying a transaction such as:

{
    "rpms": [
        {
            "action": "Reason Change",
            "nevra": "top-b-1.0-1.x86_64",
            "reason": "Dependency",
            "repo_id": "transaction-sr"
        },
        {
            "action": "Reinstall",
            "nevra": "top-b-1.0-1.x86_64",
            "reason": "Group",
            "group_id": "test-group",
            "repo_id": "transaction-sr"
        },
        {
            "action": "Replaced",
            "nevra": "top-b-1.0-1.x86_64",
            "reason": "Group",
            "group_id": "test-group",
            "repo_id": "@System"
        }
    ],
    "version": "1.0"
}

should be an error.

It is not clear which reason should be used, it depends on which action was "first".

Currently it is not possible to generate such transaction by dnf5, it has to be created manually.

ppisar commented 3 months ago

Is the transaction JSON format documented? I cannot see the format in dnf5-replay(5), neither in section 7.

Why does not the example mean: First change the reason, then reinstall the package, and then obsolete it with test-group group (what does the Replaced because of Group actually mean?)?

I ask because I don't know the format. Maybe if we had a documentation, it would be clear the each /rpms/nevra value must be unique.

kontura commented 3 months ago

Is the transaction JSON format documented? I cannot see the format in dnf5-replay(5), neither in section 7.

The documentation for the new 1.0 version for dnf5 is missing, it still has to be created. There are only docs for the previous 0.0 version.

Why does not the example mean: First change the reason, then reinstall the package, and then obsolete it with test-group group ?

Because libdnf5 sorts the package actions in a specific order in a transaction. It is independent of the order in the JSON. In this example it should be:

  1. reason change
  2. replaced (which is essentially a remove for replay functionality)
  3. reinstall

So the package would end up with a reason Group (if test-group group was installed) from the Reinstall action.

I guess we could document the ordering but still in this example the reason change action is pointless, it has no effect. I think it would be good to inform the user.

I am not completely sure but I think its not possible to have multiple effective actions per NEVRA in a single transaction.

(what does the Replaced because of Group actually mean?)?

I would describe it as: the package was replaced by some other package in an operation triggered by a "test-group" group. It is kind of confusing for the Reinstall but the Replaced actions accompany actions that replace one package for another (Upgade, Downgrade, Reinstall and even Install if it obsoletes something).

I don't think a group operation could trigger a Reinstall (like I said the example was created manually) but something like this could happen by $ dnf5 group upgrade test-group:

{
  "groups": [
    {
      "id": "test-group",
      "action": "Upgrade",
      "reason": "User",
      "repo_id": "transaction-sr"
    }
  ],
  "rpms": [
    {
      "nevra": "top-b-2.0-1.x86_64",
      "action": "Upgrade",
      "reason": "Group",
      "group_id": "test-group",
      "repo_id": "transaction-sr"
    },
    {
      "nevra": "top-b-1.0-1.x86_64",
      "action": "Replaced",
      "reason": "Group",
      "group_id": "test-group",
      "repo_id": "@System"
    }
  ],
  "version": "1.0"
}
ppisar commented 3 months ago

Thanks for the explanation.

Because libdnf5 sorts the package actions in a specific order in a transaction. It is independent of the order in the JSON.

I think this is the main cause. If the order of the actions inside a transaction is undefined, then it should be clearly documented and DNF should reject a transaction with multiple actions on the same NEVRA. Otherwise, as you noticed, the result would be quite random. That's also why a mere warning is not enough, it needs to be an error. Otherwise, people would complain that their transactions do not reply identically.

It's important to understand the transaction reply as a yet another interface to DNF (besides a command arguments and D-Bus). People will notice this new interface and will be clever in crafting reply JSON documents to achieve things that are impossible with the other interfaces (e.g. swap multiple pairs of packages, or change a reason and reinstall a package).

(Originally I wanted to propose chaging the "rpms" JSON array into a JSON object (=associative array):

  "rpms": {
     "top-b-2.0-1.x86_64": {
        "action": "Upgrade",
        "reason": "Group",
        "group_id": "test-group",
        "repo_id": "transaction-sr"
      },
     "top-b-1.0-1.x86_64": {
        "action": "Replaced",
        "reason": "Group",
        "group_id": "test-group",
        "repo_id": "@System"
      }
  },

But that would still impose to DNF checking for the duplicities and most importantly JSON does not require the keys to be unique.)