src/apply.bash: Update pacman database before installing packages #204

AleksanderBobinski · 2024-09-11T21:53:35Z

On a fresh install it might happen that after priority files including the pacman config are installed, the pacman database is not up to date.

: Applying configuration...
:: Installing priority files...
::: Installing /etc/pacman.conf...
:::: Done.
::: Done.
:: Configuring packages...
::: Unpinning 4 unknown packages.
warning: database file for 'community' does not exist (use '-Sy' to download)
warning: database file for 'multilib' does not exist (use '-Sy' to download)

This can later result in issues with finding the correct package during the "Apply packages" step.

error: target not found: lib32-vulkan-intel
error: target not found: lib32-vulkan-mesa-layers
error: target not found: steam
error: target not found: wine

To avoid this issue do as pacman suggests and update the database before "Apply packages"

Please let me know if I missed something.

CyberShadow · 2024-09-13T12:06:06Z

Hi, unfortunately I don't think this is desired in all circumstances. I think it might also be potentially dangerous, as a database sync must always be followed by a full system upgrade - otherwise, installing a package would result in a partial upgrade.

Could we detect that the databases are missing? In which case, we could do a pacman -Syu in this case, guarded by a Confirm.

AleksanderBobinski · 2024-09-27T20:35:02Z

Good catch. I didn't think about partial upgrades. Allow me to throw some code at you and please let me know if this is the right direction.

CyberShadow · 2024-09-28T06:23:56Z

src/apply.bash

@@ -212,6 +212,16 @@ function AconfApply() {

 	LogLeave # Installing priority files

+	if ! sudo "$PACMAN" --database --check --check


I am not sure that the check (--database --check --check) and the cure (--sync --refresh --upgrades) are checking for / curing the same thing. For example, on my system, this check complains about some missing dependencies, which doesn't seem to be related to the matter at hand, and I don't think would require --refresh to fix.

Perhaps a single --check would do the job? But, if it would be good if we had some way that we could be certain that the check will be positive only in the situations where the fix will do what is needed.

CyberShadow · 2024-09-28T06:25:48Z

test/t/mocks/pacman

-			done
+			if $opt_db_sync_check
+			then
+				exit_code=1


I'm not sure this is very helpful. The database files missing is an extraordinary circumstance, which I think should have one test dedicated to it, rather than a situation encountered in every test.

In this case, an integration test (which deletes the database files) would probably be the most helpful.

Any tips on how to create such an integration test? I've been staring at the tests for a while but it's not exactly clear to me how to achieve the removal of a pacman db in the test container.

Perhaps take t-2_apply-1_packages-1_new.sh as a base and add TestIntegrationOnly at the top + command sudo rm -rf /var/lib/pacman/sync at the end of the TestPhase_Setup section?

It does seem to work, thanks!

CyberShadow · 2024-09-28T06:30:18Z

src/apply.bash

@@ -212,6 +212,16 @@ function AconfApply() {

 	LogLeave # Installing priority files

+	if ! sudo "$PACMAN" --database --check --check


This check takes a couple seconds for me, even with one --check. It does seem to be more involved than what we need, which is to check that the database files are present. If there's no better way to check for that situation, then probably it should be in a LogEnter / LogLeave block.

I've poked around and I could grep for something like "database file for 'community' does not exist" since it's printed even when just invoking pacman with no args. What do you think?

[testme@64d38ef3aa04 aconfmgr]$ pacman warning: database file for 'community' does not exist (use '-Sy' to download) warning: database file for 'multilib' does not exist (use '-Sy' to download) error: no operation specified (use -h for help) [testme@64d38ef3aa04 aconfmgr]$ echo $? 1 [testme@64d38ef3aa04 aconfmgr]$ sudo pacman --database warning: database file for 'community' does not exist (use '-Sy' to download) warning: database file for 'multilib' does not exist (use '-Sy' to download) [testme@64d38ef3aa04 aconfmgr]$ echo $? 0

If there is no cleaner solution then I guess that would work. Just have to make sure to run it with LC_ALL=C.

CyberShadow · 2024-09-28T06:32:28Z

src/apply.bash

@@ -212,6 +212,16 @@ function AconfApply() {

 	LogLeave # Installing priority files

+	if ! sudo "$PACMAN" --database --check --check
+	then
+		function Details() { Log "The pacman database is out of sync with the configuration, update?" ; }


Suggested change

function Details() { Log "The pacman database is out of sync with the configuration, update?" ; }

function Details() { Log "The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n" ; }

CyberShadow · 2024-09-28T06:33:10Z

src/apply.bash

+		function Details() { Log "The pacman database is out of sync with the configuration, update?" ; }
+		Confirm Details
+
+		LogEnter 'Updating pacman database...\n'


Suggested change

LogEnter 'Updating pacman database...\n'

LogEnter 'Performing a full refresh and system upgrade...\n'

CyberShadow · 2024-09-28T06:36:51Z

src/apply.bash

+		Confirm Details
+
+		LogEnter 'Updating pacman database...\n'
+		sudo "$PACMAN" --sync --refresh --upgrades


I think this should be --sysupgrade not --upgrades (see CI failure).

Indeed - done

CyberShadow

LGTM otherwise!

CyberShadow · 2024-10-22T19:01:09Z

src/apply.bash

+		function Details() { Log "The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n" ; }
+		Confirm Details


I think this is going to be a little less confusing, as the user doesn't know what they're agreeing to unless they ask for details:

Suggested change

function Details() { Log "The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n" ; }

Confirm Details

Log 'The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n'

Confirm ''

Done. I've added "resolves #33" into the commit message too.

On a fresh install it might happen that after priority files, including the pacman config, are installed the pacman database is not up to date. : Applying configuration... :: Installing priority files... ::: Installing /etc/pacman.conf... :::: Done. ::: Done. :: Configuring packages... ::: Unpinning 4 unknown packages. warning: database file for 'community' does not exist (use '-Sy' to download) warning: database file for 'multilib' does not exist (use '-Sy' to download) This can later result in issues with finding the correct package during the "Apply packages" step. error: target not found: lib32-vulkan-intel error: target not found: lib32-vulkan-mesa-layers error: target not found: steam error: target not found: wine To avoid this issue ask the user if he wishes to do as pacman suggests and update the database before "Apply packages". To avoid an inconsistent state upgrade packages for which database entries were updated, in other words perform a system upgrade. Resolves CyberShadow#33

AleksanderBobinski · 2024-10-23T05:31:12Z

Cool! Thanks for your patience!

AleksanderBobinski force-pushed the bugfix/stale_database branch from e0c4a51 to b01b05c Compare September 27, 2024 20:34

CyberShadow reviewed Sep 28, 2024

View reviewed changes

AleksanderBobinski force-pushed the bugfix/stale_database branch from b01b05c to 0ac1229 Compare October 10, 2024 21:02

AleksanderBobinski force-pushed the bugfix/stale_database branch from 0ac1229 to 2287353 Compare October 21, 2024 17:29

CyberShadow approved these changes Oct 22, 2024

View reviewed changes

AleksanderBobinski force-pushed the bugfix/stale_database branch from 2287353 to 5cb934c Compare October 22, 2024 21:39

CyberShadow merged commit 788860a into CyberShadow:master Oct 23, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src/apply.bash: Update pacman database before installing packages #204

src/apply.bash: Update pacman database before installing packages #204

AleksanderBobinski commented Sep 11, 2024

CyberShadow commented Sep 13, 2024

AleksanderBobinski commented Sep 27, 2024

CyberShadow Sep 28, 2024

CyberShadow Sep 28, 2024

AleksanderBobinski Oct 10, 2024

CyberShadow Oct 11, 2024

AleksanderBobinski Oct 22, 2024

CyberShadow Sep 28, 2024

AleksanderBobinski Sep 30, 2024

CyberShadow Sep 30, 2024

CyberShadow Sep 28, 2024 •

edited

Loading

AleksanderBobinski Oct 10, 2024

CyberShadow Sep 28, 2024

AleksanderBobinski Oct 10, 2024

CyberShadow Sep 28, 2024

AleksanderBobinski Oct 10, 2024

CyberShadow left a comment

CyberShadow Oct 22, 2024

AleksanderBobinski Oct 22, 2024

AleksanderBobinski commented Oct 23, 2024

		@@ -212,6 +212,16 @@ function AconfApply() {

		LogLeave # Installing priority files

		if ! sudo "$PACMAN" --database --check --check

	function Details() { Log "The pacman database is out of sync with the configuration, update?" ; }
	function Details() { Log "The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n" ; }

	LogEnter 'Updating pacman database...\n'
	LogEnter 'Performing a full refresh and system upgrade...\n'

		function Details() { Log "The pacman local package database is internally inconsistent. Perform a full refresh and system upgrade now?\n" ; }
		Confirm Details

src/apply.bash: Update pacman database before installing packages #204

src/apply.bash: Update pacman database before installing packages #204

Conversation

AleksanderBobinski commented Sep 11, 2024

CyberShadow commented Sep 13, 2024

AleksanderBobinski commented Sep 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyberShadow Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyberShadow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AleksanderBobinski commented Oct 23, 2024

CyberShadow Sep 28, 2024 •

edited

Loading