From 06fcab4ce709e22aa360422e19bb2d9415294a99 Mon Sep 17 00:00:00 2001 From: Maxim Slipenko Date: Sun, 8 Jun 2025 17:57:18 +0000 Subject: [PATCH] fix: prevent for building dependencies twice (#99) closes #94 Reviewed-on: https://gitea.plemya-x.ru/Plemya-x/ALR/pulls/99 Co-authored-by: Maxim Slipenko Co-committed-by: Maxim Slipenko --- .gitea/workflows/e2e-tests.yaml | 3 - build.go | 8 +- e2e-tests/issue_94_twice_build_test.go | 49 +++++++++ install.go | 2 +- internal/translations/default.pot | 16 +-- internal/translations/po/ru/default.po | 24 ++--- pkg/build/build.go | 138 +++++++++++++++---------- pkg/build/safe_script_executor.go | 16 +-- pkg/build/script_executor.go | 24 ++--- pkg/build/utils.go | 14 +-- 10 files changed, 182 insertions(+), 112 deletions(-) create mode 100644 e2e-tests/issue_94_twice_build_test.go diff --git a/.gitea/workflows/e2e-tests.yaml b/.gitea/workflows/e2e-tests.yaml index e3fd27d..da72564 100644 --- 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 diff --git a/build.go b/build.go index 3a8efb9..8efeb9c 100644 --- 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) } diff --git a/e2e-tests/issue_94_twice_build_test.go b/e2e-tests/issue_94_twice_build_test.go new file mode 100644 index 0000000..ad15380 --- /dev/null +++ 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 . + +//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")) + }, + ) +} diff --git a/install.go b/install.go index 4ab493d..245784d 100644 --- 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{ diff --git a/internal/translations/default.pot b/internal/translations/default.pot index 67071f0..4d8214a 100644 --- 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 "" diff --git a/internal/translations/po/ru/default.po b/internal/translations/po/ru/default.po index 55606d0..0372580 100644 --- 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()" diff --git a/pkg/build/build.go b/pkg/build/build.go index 3721ff8..d559750 100644 --- 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 } diff --git a/pkg/build/safe_script_executor.go b/pkg/build/safe_script_executor.go index daf3442..974aa69 100644 --- 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 } diff --git a/pkg/build/script_executor.go b/pkg/build/script_executor.go index ba402f1..a1b95f0 100644 --- 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( diff --git a/pkg/build/utils.go b/pkg/build/utils.go index 9296447..e307b25 100644 --- 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) } }