fix: prevent for building dependencies twice (#99)

closes #94 Reviewed-on: Plemya-x/ALR#99 Co-authored-by: Maxim Slipenko <no-reply@maxim.slipenko.com> Co-committed-by: Maxim Slipenko <no-reply@maxim.slipenko.com>
2025-06-08 17:57:18 +00:00
parent 7741c7368b
commit 06fcab4ce7
10 changed files with 182 additions and 112 deletions
--- a/.gitea/workflows/e2e-tests.yaml
+++ b/.gitea/workflows/e2e-tests.yaml
@@ -28,7 +28,6 @@ jobs:

    container:
      image: altlinux.space/maks1ms/actions-container-runner:latest
-      privileged: true

    steps:
      - name: Checkout
@@ -52,8 +51,6 @@ jobs:

      - name: Run E2E tests
        env:
-          DOCKER_HOST: unix:///tmp/podman.sock
          IGNORE_ROOT_CHECK: 1
        run: |
-          podman system service -t 0 unix:///tmp/podman.sock &
          make e2e-test
--- a/build.go
+++ b/build.go
@@ -97,7 +97,7 @@ func BuildCmd() *cli.Command {
 			var script string
 			var packages []string

-			var res *build.BuildResult
+			var res []*build.BuiltDep

 			var scriptArgs *build.BuildPackageFromScriptArgs
 			var dbArgs *build.BuildPackageFromDbArgs
@@ -222,9 +222,9 @@ func BuildCmd() *cli.Command {
 				return cliutils.FormatCliExit(gotext.Get("Error building package"), err)
 			}

-			for _, pkgPath := range res.PackagePaths {
-				name := filepath.Base(pkgPath)
-				err = osutils.Move(pkgPath, filepath.Join(wd, name))
+			for _, pkg := range res {
+				name := filepath.Base(pkg.Path)
+				err = osutils.Move(pkg.Path, filepath.Join(wd, name))
 				if err != nil {
 					return cliutils.FormatCliExit(gotext.Get("Error moving the package"), err)
 				}
--- a/e2e-tests/issue_94_twice_build_test.go
+++ b/e2e-tests/issue_94_twice_build_test.go
@@ -0,0 +1,49 @@
+// ALR - Any Linux Repository
+// Copyright (C) 2025 The ALR Authors
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+//go:build e2e
+
+package e2etests_test
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/efficientgo/e2e"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestE2EIssue94TwiceBuild(t *testing.T) {
+	dockerMultipleRun(
+		t,
+		"issue-94-twice-build",
+		COMMON_SYSTEMS,
+		func(t *testing.T, r e2e.Runnable) {
+			defaultPrepare(t, r)
+
+			var stderr bytes.Buffer
+			err := r.Exec(
+				e2e.NewCommand("sudo", "alr", "in", "test-94-app"),
+				e2e.WithExecOptionStderr(&stderr),
+			)
+			assert.NoError(t, err, "command failed")
+
+			output := stderr.String()
+			assert.Equal(t, 1, strings.Count(output, "Building package name=test-94-dep"))
+		},
+	)
+}
--- a/install.go
+++ b/install.go
@@ -98,7 +98,7 @@ func InstallCmd() *cli.Command {
 				return err
 			}

-			err = builder.InstallPkgs(
+			_, err = builder.InstallPkgs(
 				ctx,
 				&build.BuildArgs{
 					Opts: &types.BuildOpts{
--- a/internal/translations/default.pot
+++ b/internal/translations/default.pot
@@ -377,19 +377,19 @@ msgstr ""
 msgid "Error while running app"
 msgstr ""

-#: pkg/build/build.go:395
+#: pkg/build/build.go:417
 msgid "Building package"
 msgstr ""

-#: pkg/build/build.go:424
+#: pkg/build/build.go:446
 msgid "The checksums array must be the same length as sources"
 msgstr ""

-#: pkg/build/build.go:455
+#: pkg/build/build.go:488
 msgid "Downloading sources"
 msgstr ""

-#: pkg/build/build.go:549
+#: pkg/build/build.go:580
 msgid "Installing dependencies"
 msgstr ""

@@ -423,19 +423,19 @@ msgstr ""
 msgid "AutoReq is not implemented for this package format, so it's skipped"
 msgstr ""

-#: pkg/build/script_executor.go:241
+#: pkg/build/script_executor.go:236
 msgid "Building package metadata"
 msgstr ""

-#: pkg/build/script_executor.go:372
+#: pkg/build/script_executor.go:366
 msgid "Executing prepare()"
 msgstr ""

-#: pkg/build/script_executor.go:381
+#: pkg/build/script_executor.go:375
 msgid "Executing build()"
 msgstr ""

-#: pkg/build/script_executor.go:410 pkg/build/script_executor.go:430
+#: pkg/build/script_executor.go:404 pkg/build/script_executor.go:424
 msgid "Executing %s()"
 msgstr ""

--- a/internal/translations/po/ru/default.po
+++ b/internal/translations/po/ru/default.po
@@ -12,8 +12,8 @@ msgstr ""
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
-"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && "
-"n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
+"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n"
+"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
 "X-Generator: Gtranslator 48.0\n"

 #: build.go:42
@@ -290,8 +290,8 @@ msgid ""
 "This command is deprecated and would be removed in the future, use \"%s\" "
 "instead!"
 msgstr ""
-"Эта команда устарела и будет удалена в будущем, используйте вместо нее "
-"\"%s\"!"
+"Эта команда устарела и будет удалена в будущем, используйте вместо нее \"%s"
+"\"!"

 #: internal/db/db.go:137
 msgid "Database version mismatch; resetting"
@@ -387,19 +387,19 @@ msgstr "Показать справку"
 msgid "Error while running app"
 msgstr "Ошибка при запуске приложения"

-#: pkg/build/build.go:395
+#: pkg/build/build.go:417
 msgid "Building package"
 msgstr "Сборка пакета"

-#: pkg/build/build.go:424
+#: pkg/build/build.go:446
 msgid "The checksums array must be the same length as sources"
 msgstr "Массив контрольных сумм должен быть той же длины, что и источники"

-#: pkg/build/build.go:455
+#: pkg/build/build.go:488
 msgid "Downloading sources"
 msgstr "Скачивание источников"

-#: pkg/build/build.go:549
+#: pkg/build/build.go:580
 msgid "Installing dependencies"
 msgstr "Установка зависимостей"

@@ -437,19 +437,19 @@ msgid "AutoReq is not implemented for this package format, so it's skipped"
 msgstr ""
 "AutoReq не реализовано для этого формата пакета, поэтому будет пропущено"

-#: pkg/build/script_executor.go:241
+#: pkg/build/script_executor.go:236
 msgid "Building package metadata"
 msgstr "Сборка метаданных пакета"

-#: pkg/build/script_executor.go:372
+#: pkg/build/script_executor.go:366
 msgid "Executing prepare()"
 msgstr "Выполнение prepare()"

-#: pkg/build/script_executor.go:381
+#: pkg/build/script_executor.go:375
 msgid "Executing build()"
 msgstr "Выполнение build()"

-#: pkg/build/script_executor.go:410 pkg/build/script_executor.go:430
+#: pkg/build/script_executor.go:404 pkg/build/script_executor.go:424
 msgid "Executing %s()"
 msgstr "Выполнение %s()"

--- a/pkg/build/build.go
+++ b/pkg/build/build.go
@@ -174,9 +174,29 @@ func (s *ScriptFile) GobDecode(data []byte) error {
 	return nil
 }

-type BuildResult struct {
-	PackagePaths []string
-	PackageNames []string
+type BuiltDep struct {
+	Name string
+	Path string
+}
+
+func Map[T, R any](items []T, f func(T) R) []R {
+	res := make([]R, len(items))
+	for i, item := range items {
+		res[i] = f(item)
+	}
+	return res
+}
+
+func GetBuiltPaths(deps []*BuiltDep) []string {
+	return Map(deps, func(dep *BuiltDep) string {
+		return dep.Path
+	})
+}
+
+func GetBuiltName(deps []*BuiltDep) []string {
+	return Map(deps, func(dep *BuiltDep) string {
+		return dep.Name
+	})
 }

 type PackageFinder interface {
@@ -212,9 +232,9 @@ type ScriptExecutor interface {
 		sf *ScriptFile,
 		varsOfPackages []*types.BuildVars,
 		repoDeps []string,
-		builtNames []string,
+		builtDeps []*BuiltDep,
 		basePkg string,
-	) (*SecondPassResult, error)
+	) ([]*BuiltDep, error)
 }

 type CacheExecutor interface {
@@ -320,7 +340,7 @@ type BuildPackageFromScriptArgs struct {
 func (b *Builder) BuildPackageFromDb(
 	ctx context.Context,
 	args *BuildPackageFromDbArgs,
-) (*BuildResult, error) {
+) ([]*BuiltDep, error) {
 	scriptInfo := b.scriptResolver.ResolveScript(ctx, args.Package)

 	return b.BuildPackage(ctx, &BuildInput{
@@ -336,7 +356,7 @@ func (b *Builder) BuildPackageFromDb(
 func (b *Builder) BuildPackageFromScript(
 	ctx context.Context,
 	args *BuildPackageFromScriptArgs,
-) (*BuildResult, error) {
+) ([]*BuiltDep, error) {
 	return b.BuildPackage(ctx, &BuildInput{
 		script:     args.Script,
 		repository: "default",
@@ -350,7 +370,7 @@ func (b *Builder) BuildPackageFromScript(
 func (b *Builder) BuildPackage(
 	ctx context.Context,
 	input *BuildInput,
-) (*BuildResult, error) {
+) ([]*BuiltDep, error) {
 	scriptPath := input.script

 	slog.Debug("ReadScript")
@@ -365,7 +385,7 @@ func (b *Builder) BuildPackage(
 		return nil, err
 	}

-	builtPaths := make([]string, 0)
+	var builtDeps []*BuiltDep

 	if !input.opts.Clean {
 		var remainingVars []*types.BuildVars
@@ -375,14 +395,16 @@ func (b *Builder) BuildPackage(
 				return nil, err
 			}
 			if ok {
-				builtPaths = append(builtPaths, builtPkgPath)
+				builtDeps = append(builtDeps, &BuiltDep{
+					Path: builtPkgPath,
+				})
 			} else {
 				remainingVars = append(remainingVars, vars)
 			}
 		}

 		if len(remainingVars) == 0 {
-			return &BuildResult{builtPaths, nil}, nil
+			return builtDeps, nil
 		}
 	}

@@ -427,19 +449,32 @@ func (b *Builder) BuildPackage(
 	sources, checksums = removeDuplicatesSources(sources, checksums)

 	slog.Debug("installBuildDeps")
-	err = b.installBuildDeps(ctx, input, buildDepends)
+	alrBuildDeps, err := b.installBuildDeps(ctx, input, buildDepends)
 	if err != nil {
 		return nil, err
 	}

 	slog.Debug("installOptDeps")
-	err = b.installOptDeps(ctx, input, optDepends)
+	_, err = b.installOptDeps(ctx, input, optDepends)
 	if err != nil {
 		return nil, err
 	}

+	depNames := make(map[string]struct{})
+	for _, dep := range alrBuildDeps {
+		depNames[dep.Name] = struct{}{}
+	}
+
+	// We filter so as not to re-build what has already been built at the `installBuildDeps` stage.
+	var filteredDepends []string
+	for _, d := range depends {
+		if _, found := depNames[d]; !found {
+			filteredDepends = append(filteredDepends, d)
+		}
+	}
+
 	slog.Debug("BuildALRDeps")
-	builtPaths, builtNames, repoDeps, err := b.BuildALRDeps(ctx, input, depends)
+	newBuiltDeps, repoDeps, err := b.BuildALRDeps(ctx, input, filteredDepends)
 	if err != nil {
 		return nil, err
 	}
@@ -450,8 +485,6 @@ func (b *Builder) BuildPackage(
 		return nil, err
 	}

-	// builtPaths = append(builtPaths, newBuildPaths...)
-
 	slog.Info(gotext.Get("Downloading sources"))
 	slog.Debug("DownloadSources")
 	err = b.sourceExecutor.DownloadSources(
@@ -467,6 +500,8 @@ func (b *Builder) BuildPackage(
 		return nil, err
 	}

+	builtDeps = removeDuplicates(append(builtDeps, newBuiltDeps...))
+
 	slog.Debug("ExecuteSecondPass")
 	res, err := b.scriptExecutor.ExecuteSecondPass(
 		ctx,
@@ -474,20 +509,16 @@ func (b *Builder) BuildPackage(
 		sf,
 		varsOfPackages,
 		repoDeps,
-		builtNames,
+		builtDeps,
 		basePkg,
 	)
 	if err != nil {
 		return nil, err
 	}

-	pkgPaths := removeDuplicates(append(builtPaths, res.BuiltPaths...))
-	pkgNames := removeDuplicates(append(builtNames, res.BuiltNames...))
+	builtDeps = removeDuplicates(append(builtDeps, res...))

-	return &BuildResult{
-		PackagePaths: pkgPaths,
-		PackageNames: pkgNames,
-	}, nil
+	return builtDeps, nil
 }

 type InstallPkgsArgs struct {
@@ -523,7 +554,7 @@ func (b *Builder) InstallALRPackages(
 		}

 		err = b.installerExecutor.InstallLocal(
-			res.PackagePaths,
+			GetBuiltPaths(res),
 			&manager.Opts{
 				NoConfirm: !input.BuildOpts().Interactive,
 			},
@@ -544,13 +575,13 @@ func (b *Builder) BuildALRDeps(
 		PkgFormatProvider
 	},
 	depends []string,
-) (builtPaths, builtNames, repoDeps []string, err error) {
+) (buildDeps []*BuiltDep, repoDeps []string, err error) {
 	if len(depends) > 0 {
 		slog.Info(gotext.Get("Installing dependencies"))

 		found, notFound, err := b.repos.FindPkgs(ctx, depends) // Поиск зависимостей
 		if err != nil {
-			return nil, nil, nil, err
+			return nil, nil, err
 		}
 		repoDeps = notFound

@@ -597,20 +628,17 @@ func (b *Builder) BuildALRDeps(
 				},
 			)
 			if err != nil {
-				return nil, nil, nil, err
+				return nil, nil, err
 			}

-			builtPaths = append(builtPaths, res.PackagePaths...)
-			builtNames = append(builtNames, res.PackageNames...)
+			buildDeps = append(buildDeps, res...)
 		}
 	}

-	// Удаляем возможные дубликаты, которые могут быть введены, если
-	// несколько зависимостей зависят от одних и тех же пакетов.
 	repoDeps = removeDuplicates(repoDeps)
-	builtPaths = removeDuplicates(builtPaths)
-	builtNames = removeDuplicates(builtNames)
-	return builtPaths, builtNames, repoDeps, nil
+	buildDeps = removeDuplicates(buildDeps)
+
+	return buildDeps, repoDeps, nil
 }

 func (i *Builder) installBuildDeps(
@@ -621,19 +649,20 @@ func (i *Builder) installBuildDeps(
 		PkgFormatProvider
 	},
 	pkgs []string,
-) error {
+) ([]*BuiltDep, error) {
+	var builtDeps []*BuiltDep
 	if len(pkgs) > 0 {
 		deps, err := i.installerExecutor.RemoveAlreadyInstalled(pkgs)
 		if err != nil {
-			return err
+			return nil, err
 		}

-		err = i.InstallPkgs(ctx, input, deps) // Устанавливаем выбранные пакеты
+		builtDeps, err = i.InstallPkgs(ctx, input, deps) // Устанавливаем выбранные пакеты
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
-	return nil
+	return builtDeps, nil
 }

 func (i *Builder) installOptDeps(
@@ -644,10 +673,11 @@ func (i *Builder) installOptDeps(
 		PkgFormatProvider
 	},
 	pkgs []string,
-) error {
+) ([]*BuiltDep, error) {
+	var builtDeps []*BuiltDep
 	optDeps, err := i.installerExecutor.RemoveAlreadyInstalled(pkgs)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	if len(optDeps) > 0 {
 		optDeps, err := cliutils.ChooseOptDepends(
@@ -657,19 +687,19 @@ func (i *Builder) installOptDeps(
 			input.BuildOpts().Interactive,
 		) // Пользователя просят выбрать опциональные зависимости
 		if err != nil {
-			return err
+			return nil, err
 		}

 		if len(optDeps) == 0 {
-			return nil
+			return builtDeps, nil
 		}

-		err = i.InstallPkgs(ctx, input, optDeps) // Устанавливаем выбранные пакеты
+		builtDeps, err = i.InstallPkgs(ctx, input, optDeps) // Устанавливаем выбранные пакеты
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
-	return nil
+	return builtDeps, nil
 }

 func (i *Builder) InstallPkgs(
@@ -680,18 +710,18 @@ func (i *Builder) InstallPkgs(
 		PkgFormatProvider
 	},
 	pkgs []string,
-) error {
-	builtPaths, _, repoDeps, err := i.BuildALRDeps(ctx, input, pkgs)
+) ([]*BuiltDep, error) {
+	builtDeps, repoDeps, err := i.BuildALRDeps(ctx, input, pkgs)
 	if err != nil {
-		return err
+		return nil, err
 	}

-	if len(builtPaths) > 0 {
-		err = i.installerExecutor.InstallLocal(builtPaths, &manager.Opts{
+	if len(builtDeps) > 0 {
+		err = i.installerExecutor.InstallLocal(GetBuiltPaths(builtDeps), &manager.Opts{
 			NoConfirm: !input.BuildOpts().Interactive,
 		})
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}

@@ -700,9 +730,9 @@ func (i *Builder) InstallPkgs(
 			NoConfirm: !input.BuildOpts().Interactive,
 		})
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}

-	return nil
+	return builtDeps, nil
 }
--- a/pkg/build/safe_script_executor.go
+++ b/pkg/build/safe_script_executor.go
@@ -151,7 +151,7 @@ type ExecuteSecondPassArgs struct {
 	Sf             *ScriptFile
 	VarsOfPackages []*types.BuildVars
 	RepoDeps       []string
-	BuiltNames     []string
+	BuiltDeps      []*BuiltDep
 	BasePkg        string
 }

@@ -161,16 +161,16 @@ func (s *ScriptExecutorRPC) ExecuteSecondPass(
 	sf *ScriptFile,
 	varsOfPackages []*types.BuildVars,
 	repoDeps []string,
-	builtNames []string,
+	builtDeps []*BuiltDep,
 	basePkg string,
-) (*SecondPassResult, error) {
-	var resp *SecondPassResult
+) ([]*BuiltDep, error) {
+	var resp []*BuiltDep
 	err := s.client.Call("Plugin.ExecuteSecondPass", &ExecuteSecondPassArgs{
 		Input:          input,
 		Sf:             sf,
 		VarsOfPackages: varsOfPackages,
 		RepoDeps:       repoDeps,
-		BuiltNames:     builtNames,
+		BuiltDeps:      builtDeps,
 		BasePkg:        basePkg,
 	}, &resp)
 	if err != nil {
@@ -179,20 +179,20 @@ func (s *ScriptExecutorRPC) ExecuteSecondPass(
 	return resp, nil
 }

-func (s *ScriptExecutorRPCServer) ExecuteSecondPass(args *ExecuteSecondPassArgs, resp *SecondPassResult) error {
+func (s *ScriptExecutorRPCServer) ExecuteSecondPass(args *ExecuteSecondPassArgs, resp *[]*BuiltDep) error {
 	res, err := s.Impl.ExecuteSecondPass(
 		context.Background(),
 		args.Input,
 		args.Sf,
 		args.VarsOfPackages,
 		args.RepoDeps,
-		args.BuiltNames,
+		args.BuiltDeps,
 		args.BasePkg,
 	)
 	if err != nil {
 		return err
 	}
-	*resp = *res
+	*resp = res
 	return err
 }

--- a/pkg/build/script_executor.go
+++ b/pkg/build/script_executor.go
@@ -152,11 +152,6 @@ func (e *LocalScriptExecutor) ExecuteFirstPass(ctx context.Context, input *Build
 	return pkgs.BasePkgName, varsOfPackages, nil
 }

-type SecondPassResult struct {
-	BuiltPaths []string
-	BuiltNames []string
-}
-
 func (e *LocalScriptExecutor) PrepareDirs(
 	ctx context.Context,
 	input *BuildInput,
@@ -185,9 +180,9 @@ func (e *LocalScriptExecutor) ExecuteSecondPass(
 	sf *ScriptFile,
 	varsOfPackages []*types.BuildVars,
 	repoDeps []string,
-	builtNames []string,
+	builtDeps []*BuiltDep,
 	basePkg string,
-) (*SecondPassResult, error) {
+) ([]*BuiltDep, error) {
 	dirs, err := getDirs(e.cfg, sf.Path, basePkg)
 	if err != nil {
 		return nil, err
@@ -213,7 +208,7 @@ func (e *LocalScriptExecutor) ExecuteSecondPass(

 	dec := decoder.New(input.info, runner)

-	var builtPaths []string
+	// var builtPaths []string

 	err = e.ExecuteFunctions(ctx, dirs, dec)
 	if err != nil {
@@ -247,7 +242,7 @@ func (e *LocalScriptExecutor) ExecuteSecondPass(
 			dirs,
 			append(
 				repoDeps,
-				builtNames...,
+				GetBuiltName(builtDeps)...,
 			),
 			funcOut.Contents,
 		)
@@ -273,14 +268,13 @@ func (e *LocalScriptExecutor) ExecuteSecondPass(
 			return nil, err
 		}

-		builtPaths = append(builtPaths, pkgPath)
-		builtNames = append(builtNames, vars.Name)
+		builtDeps = append(builtDeps, &BuiltDep{
+			Name: vars.Name,
+			Path: pkgPath,
+		})
 	}

-	return &SecondPassResult{
-		BuiltPaths: builtPaths,
-		BuiltNames: builtNames,
-	}, nil
+	return builtDeps, nil
 }

 func buildPkgMetadata(
--- a/pkg/build/utils.go
+++ b/pkg/build/utils.go
@@ -288,14 +288,14 @@ func packageNames(pkgs []db.Package) []string {
 */

 // Функция removeDuplicates убирает любые дубликаты из предоставленного среза.
-func removeDuplicates(slice []string) []string {
-	seen := map[string]struct{}{}
-	result := []string{}
+func removeDuplicates[T comparable](slice []T) []T {
+	seen := map[T]struct{}{}
+	result := []T{}

-	for _, s := range slice {
-		if _, ok := seen[s]; !ok {
-			seen[s] = struct{}{}
-			result = append(result, s)
+	for _, item := range slice {
+		if _, ok := seen[item]; !ok {
+			seen[item] = struct{}{}
+			result = append(result, item)
 		}
 	}