From 01fe593774530d8d1bfdde959a0700abd725c04b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Fri, 20 Sep 2024 15:03:59 +0200 Subject: [PATCH] fix: add no build if image exists + fix Git 2.46.x `git init` problem :anchor: (#176) * fix: add no build if image exists :anchor: * ci: add branches :anchor: * fix: fix Git bug 2.46.x where `reference-transaction` runs on `git init` :anchor: - `git init` runs `reference-transaction` where Githooks fails on different commands as the Git repo seems not correctly initialized which seems like a Git bug (?). --- .circleci/config.yml | 11 ++++-- docs/cli/git_hooks_images_update.md | 1 + flake.lock | 18 +++++----- flake.nix | 55 +++++++++++++++-------------- githooks/apps/runner/runner.go | 14 ++++++-- githooks/cmd/images/images.go | 13 ++++--- githooks/git/gitcommon.go | 29 +++++++++++---- githooks/go.mod | 2 +- githooks/hooks/gitconfig.go | 2 -- githooks/hooks/images.go | 22 ++++++++++-- githooks/hooks/images_test.go | 2 +- githooks/hooks/shared.go | 4 ++- tests/steps/step-006.sh | 5 ++- tests/test-alpine-nolfs.sh | 2 +- tests/test-alpine-user.sh | 10 +++++- tests/test-alpine.sh | 2 +- tests/test-centralized.sh | 2 +- tests/test-lint.sh | 2 +- tests/test-unittests-podman.sh | 24 ++++++++----- tests/test-unittests.sh | 2 +- tests/test-whitespace.sh | 2 +- tests/test-windows.sh | 4 +-- 22 files changed, 150 insertions(+), 78 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 837f32b4..bd7d42e5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -12,7 +12,7 @@ jobs: default: "test-alpine" type: string machine: - image: ubuntu-2004:202010-01 + image: ubuntu-2204:current steps: - checkout - run: bash tests/<>.sh @@ -30,12 +30,15 @@ jobs: executor: size: medium name: win/server-2022 - version: 2023.11.1 + version: current steps: - checkout - run: no_output_timeout: 30m - command: "& 'C:/Program Files/Git/bin/bash.exe' -c 'tests/<>.sh --seq <>'" + command: >- + & 'C:/Program Files/Git/bin/bash.exe' + -c 'tests/<>.sh + --seq <>' workflows: version: 2 @@ -59,7 +62,9 @@ workflows: branches: only: &task-branches - /feature\/.*/ + - /feat\/.*/ - /bugfix\/.*/ + - /fix\/.*/ - linux: matrix: parameters: diff --git a/docs/cli/git_hooks_images_update.md b/docs/cli/git_hooks_images_update.md index 32ce818e..e9277042 100644 --- a/docs/cli/git_hooks_images_update.md +++ b/docs/cli/git_hooks_images_update.md @@ -18,6 +18,7 @@ git hooks images update Useful to build images in shared repositories `githooks/.images.yaml` directory. Namespace is read from the current repository. + -b, --always-build Always build images, even if they already exist. -h, --help help for update ``` diff --git a/flake.lock b/flake.lock index 736c73d5..6e347518 100644 --- a/flake.lock +++ b/flake.lock @@ -5,11 +5,11 @@ "systems": "systems" }, "locked": { - "lastModified": 1701680307, - "narHash": "sha256-kAuep2h5ajznlPMD9rnQyffWG8EM/C73lejGofXvdM8=", + "lastModified": 1726560853, + "narHash": "sha256-X6rJYSESBVr3hBoH0WbKE5KvhPU5bloyZ2L4K60/fPQ=", "owner": "numtide", "repo": "flake-utils", - "rev": "4022d587cbbfd70fe950c1e2083a02621806a725", + "rev": "c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a", "type": "github" }, "original": { @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1702312524, - "narHash": "sha256-gkZJRDBUCpTPBvQk25G0B7vfbpEYM5s5OZqghkjZsnE=", + "lastModified": 1726463316, + "narHash": "sha256-gI9kkaH0ZjakJOKrdjaI/VbaMEo9qBbSUl93DnU7f4c=", "owner": "nixos", "repo": "nixpkgs", - "rev": "a9bf124c46ef298113270b1f84a164865987a91c", + "rev": "99dc8785f6a0adac95f5e2ab05cc2e1bf666d172", "type": "github" }, "original": { @@ -36,11 +36,11 @@ }, "nixpkgsStable": { "locked": { - "lastModified": 1702233072, - "narHash": "sha256-H5G2wgbim2Ku6G6w+NSaQaauv6B6DlPhY9fMvArKqRo=", + "lastModified": 1720535198, + "narHash": "sha256-zwVvxrdIzralnSbcpghA92tWu2DV2lwv89xZc8MTrbg=", "owner": "nixos", "repo": "nixpkgs", - "rev": "781e2a9797ecf0f146e81425c822dca69fe4a348", + "rev": "205fd4226592cc83fd4c0885a3e4c9c400efabb5", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index af39704a..a8bdee4c 100644 --- a/flake.nix +++ b/flake.nix @@ -27,41 +27,44 @@ flake-utils.url = "github:numtide/flake-utils"; }; - outputs = { - self, - nixpkgs, - nixpkgsStable, - flake-utils, - ... - } @ inputs: + outputs = + { + self, + nixpkgs, + nixpkgsStable, + flake-utils, + ... + }@inputs: flake-utils.lib.eachDefaultSystem - # Creates an attribute map `{ devShells..default = ...}` - # by calling this function: - ( - system: let - overlays = []; + # Creates an attribute map `{ devShells..default = ...}` + # by calling this function: + ( + system: + let + overlays = [ ]; - # Import nixpkgs and load it into pkgs. - pkgs = import nixpkgs { - inherit system overlays; - }; + # Import nixpkgs and load it into pkgs. + pkgs = import nixpkgs { + inherit system overlays; + }; - # Things needed only at compile-time. - nativeBuildInputs = with pkgs; [ - go_1_21 - ]; + # Things needed only at compile-time. + nativeBuildInputs = with pkgs; [ + go_1_22 + ]; - # Things needed at runtime. - buildInputs = with pkgs; []; - in - with pkgs; { + # Things needed at runtime. + buildInputs = with pkgs; [ ]; + in + with pkgs; + { devShells.default = mkShell { # To make CGO and the debugger delve work. # https://nixos.wiki/wiki/Go#Using_cgo_on_NixOS - hardeningDisable = ["fortify"]; + hardeningDisable = [ "fortify" ]; inherit buildInputs nativeBuildInputs; }; } - ); + ); } diff --git a/githooks/apps/runner/runner.go b/githooks/apps/runner/runner.go index da583a08..cc1e5e9c 100644 --- a/githooks/apps/runner/runner.go +++ b/githooks/apps/runner/runner.go @@ -57,7 +57,13 @@ func mainRun() (exitCode int) { cwd = filepath.ToSlash(cwd) settings, uiSettings := setupSettings(cwd) - assertRegistered(settings.GitX, settings.InstallDir) + + if settings.HookName != "reference-transaction" { + // Git 2.46.x apparently runs `reference-transaction` on `git init` + // where different commands fail like `git config ...` since + // the repo is not yet properly initialized (?). + assertRegistered(settings.GitX, settings.InstallDir) + } checksums, err := hooks.GetChecksumStorage(settings.GitDirWorktree) log.AssertNoErrorF(err, "Errors while loading checksum store.") @@ -140,6 +146,9 @@ func setupSettings(repoPath string) (HookSettings, UISettings) { // General execution context, in currenct working dir. execx := cm.ExecContext{Env: os.Environ()} + log.DebugF("Arguments: '%q'", os.Args) + log.DebugF("Env: '%q'", os.Environ()) + // Current git context, in current working dir. gitx := git.NewCtxAt(repoPath) log.AssertNoErrorF(gitx.InitConfigCache(nil), @@ -583,7 +592,8 @@ func updateLocalHookImages(settings *HookSettings) { settings.RepositoryDir, settings.RepositoryHooksDir, "", - settings.ContainerMgr) + settings.ContainerMgr, + false) log.AssertNoErrorF(e, "Could not updating container images from '%s'.", settings.HookDir) } diff --git a/githooks/cmd/images/images.go b/githooks/cmd/images/images.go index 9d8c908d..b9483235 100644 --- a/githooks/cmd/images/images.go +++ b/githooks/cmd/images/images.go @@ -10,14 +10,14 @@ import ( "github.com/spf13/cobra" ) -func runImagesUpdate(ctx *ccm.CmdContext, imagesFile string) { +func runImagesUpdate(ctx *ccm.CmdContext, imagesFile string, alwaysBuild bool) { repoDir, _, _ := ccm.AssertRepoRoot(ctx) containerMgr, err := hooks.NewContainerManager(ctx.GitX, false, nil) ctx.Log.AssertNoErrorPanicF(err, "Could not create container manager.") hooksDir := hooks.GetGithooksDir(repoDir) - err = hooks.UpdateImages(ctx.Log, hooksDir, repoDir, hooksDir, imagesFile, containerMgr) + err = hooks.UpdateImages(ctx.Log, hooksDir, repoDir, hooksDir, imagesFile, containerMgr, alwaysBuild) ctx.Log.AssertNoErrorF(err, "Could not build images in '%s'.", imagesFile) if strs.IsNotEmpty(imagesFile) { @@ -55,7 +55,8 @@ func runImagesUpdate(ctx *ccm.CmdContext, imagesFile string) { allRepos[rI].RepositoryDir, hooksDir, "", - containerMgr) + containerMgr, + alwaysBuild) ctx.Log.AssertNoErrorF(err, "Could not build images in '%s'.", allRepos[rI].OriginalURL) } } @@ -69,6 +70,7 @@ func NewCmd(ctx *ccm.CmdContext) *cobra.Command { Long: "Manages container images used by Githooks repositories in the current repository."} imagesFile := "" + alwaysBuild := false imagesUpdateCmd := &cobra.Command{ Use: "update", Short: `Build/pull container images.`, @@ -76,7 +78,7 @@ func NewCmd(ctx *ccm.CmdContext) *cobra.Command { "repository and shared repositories which are needed for Githooks.", PreRun: ccm.PanicIfNotExactArgs(ctx.Log, 0), Run: func(c *cobra.Command, args []string) { - runImagesUpdate(ctx, imagesFile) + runImagesUpdate(ctx, imagesFile, alwaysBuild) }} imagesUpdateCmd.Flags().StringVar(&imagesFile, @@ -86,6 +88,9 @@ func NewCmd(ctx *ccm.CmdContext) *cobra.Command { "'githooks/.images.yaml' directory.\n"+ "Namespace is read from the current repository.") + imagesUpdateCmd.Flags().BoolVarP(&alwaysBuild, + "always-build", "b", false, "Always build images, even if they already exist.") + imagesCmd.AddCommand(ccm.SetCommandDefaults(ctx.Log, imagesUpdateCmd)) imagesCmd.PersistentPreRun = func(_ *cobra.Command, _ []string) { diff --git a/githooks/git/gitcommon.go b/githooks/git/gitcommon.go index a5d9c5b8..3d93fb34 100644 --- a/githooks/git/gitcommon.go +++ b/githooks/git/gitcommon.go @@ -84,13 +84,22 @@ func (c *Context) GetMainWorktree() (string, error) { // GetGitDirCommon returns the common Git directory. // For normal repos this points to the `.git` directory. // For worktrees this points to the main worktrees git dir. -// The env. variable GIT_COMMON_DIR has especiall +// The env. variable GIT_COMMON_DIR has especially // be introduced for multiple worktrees, see: // https://github.com/git/git/commit/c7b3a3d2fe2688a30ddb8d516ed000eeda13c24e func (c *Context) GetGitDirCommon() (gitDir string, err error) { - gitDir, err = c.Get("rev-parse", "--git-common-dir") - if err != nil { - return + // Git 2.46.x apparently runs `reference-transaction` on `git init` + // where `rev-parse` fails despite the documentation saying it reports `$GIT_COMMON_DIR` if set + // (it is set!) -> Bug was reported. + gitDir = os.Getenv("GIT_COMMON_DIR") + if strs.IsEmpty(gitDir) { + gitDir = os.Getenv("GIT_DIR") + if strs.IsEmpty(gitDir) { + gitDir, err = c.Get("rev-parse", "--git-common-dir") + if err != nil { + return + } + } } if !filepath.IsAbs(gitDir) { @@ -111,9 +120,15 @@ func (c *Context) GetGitDirCommon() (gitDir string, err error) { // For normal repos this points to the `.git` directory. // For worktrees this points to the actual worktrees git dir `.git/worktrees/<....>/`. func (c *Context) GetGitDirWorktree() (gitDir string, err error) { - gitDir, err = c.Get("rev-parse", "--absolute-git-dir") - if err != nil { - return + // Git 2.46.x apparently runs `reference-transaction` on `git init` + // where `rev-parse` fails despite the documentation saying it reports `$GIT_DIR` if set + // (it is set!) -> Bug was reported. + gitDir = os.Getenv("GIT_DIR") + if strs.IsEmpty(gitDir) { + gitDir, err = c.Get("rev-parse", "--absolute-git-dir") + if err != nil { + return + } } gitDir = filepath.ToSlash(gitDir) diff --git a/githooks/go.mod b/githooks/go.mod index 78922639..3448aa11 100644 --- a/githooks/go.mod +++ b/githooks/go.mod @@ -1,6 +1,6 @@ module github.com/gabyx/githooks/githooks -go 1.21 +go 1.22 require ( code.gitea.io/sdk/gitea v0.15.0 diff --git a/githooks/hooks/gitconfig.go b/githooks/hooks/gitconfig.go index d62441b1..dddb6070 100644 --- a/githooks/hooks/gitconfig.go +++ b/githooks/hooks/gitconfig.go @@ -29,8 +29,6 @@ const ( GitCKNumThreads = "githooks.numThreads" GitCKAliasHooks = "alias.hooks" - - GitCKBuildImagesOnSharedUpdate = "githooks.buildImagesOnSharedUpdate" ) // Git config keys for local config. diff --git a/githooks/hooks/images.go b/githooks/hooks/images.go index ffd2dcbe..303bb182 100644 --- a/githooks/hooks/images.go +++ b/githooks/hooks/images.go @@ -128,7 +128,9 @@ func buildImage( stage string, imageRef string, file string, - repositoryDir string) (err error) { + repositoryDir string, + alwaysBuild bool, +) (err error) { // Do a build of the image because no `pull` but `build` specified. if filepath.IsAbs(context) { @@ -143,6 +145,17 @@ func buildImage( dockerfile, file) } + if !alwaysBuild { + exists, e := mgr.ImageExists(imageRef) + log.AssertNoError(e, "Could not check if images exists.") + + if exists { + log.InfoF("Image '%v' already exists.", imageRef) + + return nil + } + } + out, err := mgr.ImageBuild( log, path.Join(repositoryDir, dockerfile), @@ -185,7 +198,9 @@ func UpdateImages( repositoryDir string, hooksDir string, configFile string, - containerMgr container.IManager) (err error) { + containerMgr container.IManager, + alwaysBuild bool, +) (err error) { if strs.IsEmpty(configFile) { configFile = GetRepoImagesFile(hooksDir) @@ -264,7 +279,8 @@ func UpdateImages( img.Build.Stage, imageRef, configFile, - repositoryDir) + repositoryDir, + alwaysBuild) if e != nil { err = cm.CombineErrors(err, e) diff --git a/githooks/hooks/images_test.go b/githooks/hooks/images_test.go index 83bb3fb1..6332fb1f 100644 --- a/githooks/hooks/images_test.go +++ b/githooks/hooks/images_test.go @@ -123,7 +123,7 @@ RUN apk add bash mgr, err := container.NewManager("docker") assert.Nil(t, err) assert.NotNil(t, mgr) - err = UpdateImages(log, "test-repo", repo, path.Join(repo, ".githooks"), "", mgr) + err = UpdateImages(log, "test-repo", repo, path.Join(repo, ".githooks"), "", mgr, true) assert.Nil(t, err, "Update images failed: %s", err) mgr, err = container.NewManager("") diff --git a/githooks/hooks/shared.go b/githooks/hooks/shared.go index 09cd1262..e1c41dbc 100644 --- a/githooks/hooks/shared.go +++ b/githooks/hooks/shared.go @@ -471,7 +471,9 @@ func UpdateSharedHooks( hook.OriginalURL, hook.RepositoryDir, GetSharedGithooksDir(hook.RepositoryDir), - "", containerMgr) + "", + containerMgr, + false) log.AssertNoErrorF(e, "Updating container images of '%s' failed.", hook.OriginalURL) } } diff --git a/tests/steps/step-006.sh b/tests/steps/step-006.sh index 8fbfba5f..1d2ddc16 100755 --- a/tests/steps/step-006.sh +++ b/tests/steps/step-006.sh @@ -61,5 +61,8 @@ mkdir -p "$GH_TEST_TMP/test6" && check_local_install_run_wrappers # Reinstall and check again. -"$GH_INSTALL_BIN_DIR/githooks-cli" install +"$GH_INSTALL_BIN_DIR/githooks-cli" install || { + echo "! Reinstall failed." + exit 1 +} check_local_install_run_wrappers diff --git a/tests/test-alpine-nolfs.sh b/tests/test-alpine-nolfs.sh index 63350b54..64989271 100755 --- a/tests/test-alpine-nolfs.sh +++ b/tests/test-alpine-nolfs.sh @@ -11,7 +11,7 @@ TEST_DIR="$ROOT_DIR/tests" cd "$ROOT_DIR" cat <