Add `history undo` command

[kontura]

It adds add_revert_transaction(...) API to Goal. This is used by undo and later should also be used for rollback but that requires merging of history transactions.

[j-mracek]

May I ask you why users wants to use history undo command?

What do you think could be improved in comparison to DNF4?

[pmatilai]

May I ask you why users wants to use history undo command?

This almost looks like a trick-question to me :sweat_smile:

For me personally the most common use is to uninstall a thing after trying it out, ie typically "undo last". Undoing an upgrade to test whether upgrade broke/fixed stuff would be super useful, but of course that depends on more than just dnf features (the repos dont typically carry all possible versions)

[j-mracek]

May be I would be good to be verbose here. In DNF4, history commands are important but not useful much because the implementation is very strict. Undo is very important and can be very powerful.

Use case - revert the last transaction, but why

1. The transaction does not provide expected functionality
2. The transaction did not finish successfully and user wants to undo it The second option might require reinstall of packages that we want to have after undo, but they are already installed.

Both operation of the same transaction have a different starting point - some packages might be not on the system.

The command does not provide only undo of the last transaction but there is no logic that react to any changes. We can think about following cases:

1. install a-1-1.noarch
2. upgrade a-1-1.noarch to a-2-1.noarch
# dnf5 history undo 1

This may end up with following results

• Fail due to package is installed in a different version (requires good message)
• Pass with nothing to do, message - No packages to remove for argument: zsh-html-5.9-5.fc38.noarch (current behavior). It is may be a good behavior for installonly packages
• Remove a-2-1.noarch because package a wasn't installed prior the transaction (only for packages that are not installonly)

Another use case

1. install a-1-1.noarch
2. remove a-1-1.noarch
# dnf5 history undo 1

This may end up with following results

• Pass - nothing to do
• Fail - due to the version is not present on the system

Another example:

1. install a-1-1.noarch that requires b, install dependency b-1-1.noarch
2.  install c-1-1.noarch that requires b
# dnf5 history undo 1

This may end up with following results

• Remove only a
• remove a, b, c
• Fail due to transaction differs from expectations (removal of dependent package).

Additional cases with installonly packages.

Please don't take it as this implementation is wrong. I would like to brainstorm expectation for various use cases. Also I am sure that I've not covered all possibilities.

[kontura]

Use case - revert the last transaction, but why

1. The transaction does not provide expected functionality

2. The transaction did not finish successfully and user wants to undo it
   The second option might require reinstall of packages that we want to have after undo, but they are already installed.

Both operation of the same transaction have a different starting point - some packages might be not on the system.

I have also used it for this in the past but now that I think about it rollback command seems as a better fit for this use case because it doesn't have to deal with the questions @j-mracek asked below, it always reverts n last transactions.

The command does not provide only undo of the last transaction but there is no logic that react to any changes...

These are basically the same problems that the replay command has to deal with when replaying a transaction that doesn't apply cleanly. (The undo command is replay but the source of the transaction isn't a stored file but the history.)

The replay command in dnf4 is strict by default but there are options:

--ignore-installed    For the replay command, don't check for installed packages matching those in transaction
--ignore-extras       For the replay command, don't check for extra packages pulled into the transaction
--skip-unavailable    For the replay command, skip packages that are not available or have missing dependencies

We already have --skip-unavailable. What about to preserve this approach and use it also for the undo command?

[j-mracek]

I don't think that the option mentioned above answer the question related to expected behavior. They only make no changes if something is unexpected.

Lets return to the first example

1. install a-1-1.noarch
2. upgrade a-1-1.noarch to a-2-1.noarch
# dnf5 history undo 1

The current behavior reports nothing to do with success and any of those options would help. I don't think that success is a correct result of the transaction, because it did nothing.

For this situation a user might expect

• it will move package a to state prior transaction 1 - no package a installed on the system
• ensure that a-1-1.noarch is not installed but other versions are fine
• ensure that a-1-1.noarch is not installed but if something related to a is different (version), fail to allow user to make a decision.

[kontura]

Yes I meant that in addition to those options we would make the command more strict (like the dnf4 replay command).

Regarding the possibilities you listed:

• it will move package a to state prior transaction 1 - no package a installed on the system

This sounds like a rollback, personally I don't like this behavior for undo of a specific transaction.

• ensure that a-1-1.noarch is not installed but other versions are fine

As far as I understand this is the current behavior. It successfully ensured that a-1-1.noarch is not installed.

• ensure that a-1-1.noarch is not installed but if something related to a is different (version), fail to allow user to make a decision.

This would be the new default behavior and the user could specify --ignore-installed to make it pass (essentially like this test case: https://github.com/rpm-software-management/ci-dnf-stack/blob/main/dnf-behave-tests/dnf/transaction-sr/replay.feature#L1577-L1599).

What do you think?

[j-mracek]

• ensure that a-1-1.noarch is not installed but if something related to a is different (version), fail to allow user to make a decision.

This would be the new default behavior and the user could specify --ignore-installed to make it pass (essentially like this test case: https://github.com/rpm-software-management/ci-dnf-stack/blob/main/dnf-behave-tests/dnf/transaction-sr/replay.feature#L1577-L1599).

What do you think?

I think this is a better option then the current behavior. It requires good messaging to ensure that user understand the problem and possible solutions.

[kontura]

Ok I have added several commits and I tried to focus on clear messages and logs.

@j-mracek for the cases you mentioned it looks something like this:

1. install a-1-1.noarch
2. upgrade a-1-1.noarch to a-2-1.noarch
# dnf5 history undo 1
Updating and loading repositories:
Repositories loaded.
Failed to resolve the transaction:
Cannot perform action 'Remove' for Package 'a-1-1.noarch' becasue it is not installed.
You can try to add to command line:
  --ignore-installed to allow mismatches between installed and stored transaction packages.
# dnf5 history undo 1 --ignore-installed
Updating and loading repositories:
Repositories loaded.
Cannot perform action 'Remove' for Package 'a-1-1.noarch' becasue it is not installed.

Nothing to do.

(With --ignore-installed the operation succeeds but the warning remains.)

1. install a-1-1.noarch
2. remove a-1-1.noarch
# dnf5 history undo 1
Updating and loading repositories:
Repositories loaded.
Failed to resolve the transaction:
Cannot perform action 'Remove' for Package 'a-1-1.noarch' becasue it is not installed.
You can try to add to command line:
  --ignore-installed to allow mismatches between installed and stored transaction packages.
# dnf5 history undo 1 --ignore-installed
Updating and loading repositories:
Repositories loaded.
Cannot perform action 'Remove' for Package 'a-1-1.noarch' becasue it is not installed.

Nothing to do.

(Same as the previous example.)

1. install a-1-1.noarch that requires b, install dependency b-1-1.noarch
2. install c-1-1.noarch that requires b
# dnf5 history undo 1
Updating and loading repositories:
Repositories loaded.
Failed to resolve the transaction:
Extra package 'c-1-1.noarch' (with action 'Remove') which is not present in the stored transaction was pulled into the transaction.

You can try to add to command line:
  --ignore-extras to allow extra packages in the transaction
# dnf5 history undo 1 --ignore-extras
Removes a, b and c.

(This is a change in comparison to dnf4, the undo command has allow_erasing set which allows to remove c but it requires the --ignore-extras flag because c is not part of the original transaction.)

[j-mracek]

I have tested several scenario and it looks good. Only one thing is not perfect and it is related to messaging when a different version is installed and undo is supposed to remove. The behavior is correct.

1. install a-1-1.noarch
2. upgrade a-1-1.noarch to a-2-1.noarch
# dnf5 history undo 1
Updating and loading repositories:
Repositories loaded.
Failed to resolve the transaction:
Cannot perform action 'Remove' for Package 'a-1-1.noarch' becasue it is not installed.
You can try to add to command line:
  --ignore-installed to allow mismatches between installed and stored transaction packages.

In this case I would prefer to inform that 'a-1-1.noarch' is installed in a different version. And which one. The reason is that --ignore-installed will simply skip the action and undo will do nothing - no reason to perform undo at all.

[kontura]

I have added the additional reporting of other installed versions, to do that I had to rework it a bit. I have added several new replay goal actions but I think overall the code is now clearer. Also the --ignore-installed hint message was enhanced.

[kontura]

Tests: https://github.com/rpm-software-management/ci-dnf-stack/pull/1497

[j-mracek]

I have discovered that PR does not contain an update of documentation. Or did I overlooked it somehow?

[kontura]

I have discovered that PR does not contain an update of documentation. Or did I overlooked it somehow?

Yes the docs update is missing, can we leave that for a separate PR please? This one is quite big enough.

[j-mracek]

LGTM, documentation will be delivered in the next PR.