Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(installer): Fixes the same pkgbase being built multiple times #2534

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

Ferdi265
Copy link
Contributor

When building a PKGBUILD pkgbase with multiple pkgnames, installAURPackages() invokes buildPkg() multiple times for the same pkgbase. This causes prepare() to be run multiple times for the same pkgbase, since detection of already built packages happens after prepare().

Additionally, detection of already built packages can fail if the split debug packages are enabled and the package does not contain any binaries, causing no -debug package to be created by makepkg even though it is listed by makepkg --packagelist.

This PR fixes this by keeping track of the pkgdests built by buildPkg() and avoiding rebuilds of the same pkgbase in the same yay invocation.

Fixes #2340.

@Ferdi265 Ferdi265 requested a review from Jguer as a code owner November 18, 2024 16:52
@Ferdi265
Copy link
Contributor Author

Ferdi265 commented Nov 18, 2024

Just noticed this still has test failures locally, will fix those.

It appears I broke something related to installing built packages, marking this as a draft until that is fixed.

Broken tests as of d2837e1:
--- FAIL: TestIntegrationLocalInstall (0.00s)
--- FAIL: TestIntegrationLocalInstallNeeded (0.00s)
--- FAIL: TestIntegrationLocalInstallGenerateSRCINFO (0.00s)
--- FAIL: TestIntegrationLocalInstallWithDepsProvides (0.00s)
--- FAIL: TestIntegrationLocalInstallTwoSrcInfosWithDeps (0.00s)
--- FAIL: TestSyncUpgradeAURPinnedSplitPackage (0.01s)
--- FAIL: TestInstaller_KeepSrc (0.00s)
    --- FAIL: TestInstaller_KeepSrc/--keepsrc (0.00s)
--- FAIL: TestInstaller_InstallNeeded (0.01s)
    --- FAIL: TestInstaller_InstallNeeded/not_installed_and_not_built (0.00s)
    --- FAIL: TestInstaller_InstallNeeded/not_installed_and_built (0.00s)
--- FAIL: TestInstaller_InstallSplitPackage (0.01s)
    --- FAIL: TestInstaller_InstallSplitPackage/jellyfin (0.01s)
--- FAIL: TestInstaller_InstallRebuild (0.01s)
    --- FAIL: TestInstaller_InstallRebuild/--norebuild(default)_when_built_and_not_installed (0.00s)
    --- FAIL: TestInstaller_InstallRebuild/--rebuild_when_built_and_not_installed (0.00s)
    --- FAIL: TestInstaller_InstallRebuild/--rebuild_when_built_and_installed (0.00s)
    --- FAIL: TestInstaller_InstallRebuild/--rebuild_when_built_and_installed_previously_as_dep (0.00s)
--- FAIL: TestInstaller_CompileFailed (0.01s)
    --- FAIL: TestInstaller_CompileFailed/one_layer_--_fail_install (0.00s)
--- FAIL: TestInstaller_InstallMixedSourcesAndLayers (0.01s)
    --- FAIL: TestInstaller_InstallMixedSourcesAndLayers/same_layer_--_different_sources (0.00s)
    --- FAIL: TestInstaller_InstallMixedSourcesAndLayers/different_layer_--_different_sources (0.00s)
    --- FAIL: TestInstaller_InstallMixedSourcesAndLayers/same_layer_--_aur (0.00s)
    --- FAIL: TestInstaller_InstallMixedSourcesAndLayers/different_layer_--_aur (0.00s)

@Ferdi265 Ferdi265 marked this pull request as draft November 18, 2024 17:33
When building a PKGBUILD pkgbase with multiple pkgnames,
installAURPackages() invokes buildPkg() multiple times for the same
pkgbase. This causes prepare() to be run multiple times for the same
pkgbase, since detection of already built packages happens after
prepare().

Additionally, detection of already built packages can fail if the split
debug packages are enabled and the package does not contain any
binaries, causing no -debug package to be created by makepkg even though
it is listed by makepkg --packagelist.

This commit fixes this by keeping track of the pkgdests built by
buildPkg() and avoiding rebuilds of the same pkgbase in the same yay
invocation.

Fixes Jguer#2340.

Signed-off-by: Ferdinand Bachmann <[email protected]>
@Ferdi265 Ferdi265 force-pushed the fix-pkgbase-built-multiple-times branch from d2837e1 to 5064027 Compare November 18, 2024 18:30
@Ferdi265
Copy link
Contributor Author

Ferdi265 commented Nov 18, 2024

Fixed accidently shadowing the pkgdests variable in d9e17f9. This caused installing built packages to be broken, which is now fixed.

Broken tests as of d9e17f9:
--- FAIL: TestIntegrationLocalInstall (0.00s)
--- FAIL: TestIntegrationLocalInstallNeeded (0.00s)
--- FAIL: TestIntegrationLocalInstallGenerateSRCINFO (0.00s)
--- FAIL: TestSyncUpgradeAURPinnedSplitPackage (0.01s)
--- FAIL: TestInstaller_InstallSplitPackage (0.00s)
    --- FAIL: TestInstaller_InstallSplitPackage/jellyfin (0.00s)

@Ferdi265
Copy link
Contributor Author

Ferdi265 commented Nov 18, 2024

The test changes in 5064027 only update the list of expected commands for building split packages, since some redundant invocations of makepkg have been removed by the changes in this PR.

Broken tests as of 5064027: all green 👍

@Ferdi265 Ferdi265 marked this pull request as ready for review November 18, 2024 18:42
Previously, the buildPkg invocation for a pkgbase only considered
whether the current pkgname is part of installer.origTargets. This made
the decision whether to rebuild the package order-dependent.

This commit fixes this by keeping track of which pkgbases are part of
installer.origTargets and rebuilding the pkgbase if any of its pkgnames
is part of origTargets.
The previous two commits changed how split packages (packages with the same
pkgbase) are built, ensuring that those packages aren't built multiple
times.

This commit updates the lists of commands that the tests expect to be
run so that `makepkg` isn't run multiple times per pkgbase.
@Ferdi265 Ferdi265 force-pushed the fix-pkgbase-built-multiple-times branch from 5064027 to 242cc18 Compare November 18, 2024 19:19
@Ferdi265
Copy link
Contributor Author

Fixed another issue in a26822f: Previously, whether a pkgbase was built was considered part of installer.origTargets was dependent on the order of the packages. This wasn't as big of an issue before this PR since at least one of the buildPkg() invocations passed the correct value of isTarget to buildPkg(). d9e17f9 changed this to only a single invocation of buildPkg() per pkgbase, so now the isTarget argument has to be correct for the first (and only) invocation. Fixed this by tracking which pkgbases are part of installer.origTargets in handleLayer() and passing that to installAURPackages().

Copy link
Owner

@Jguer Jguer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good @Ferdi265 , let me play with it a bit locally to see if I can find a weird package that we currently support, otherwise all good to merge

@Jguer
Copy link
Owner

Jguer commented Nov 19, 2024

👍 let's get some feedback from yay-git

@Jguer Jguer merged commit ec837c8 into Jguer:next Nov 19, 2024
2 checks passed
@Ferdi265 Ferdi265 deleted the fix-pkgbase-built-multiple-times branch November 19, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

avoid multiple executions of makepkg for the same PKGBUILD
2 participants