From b194ce84c3818d7314ae7b0159a52fba48834ee8 Mon Sep 17 00:00:00 2001 From: Roxy Light Date: Sat, 2 Nov 2024 18:33:12 -0700 Subject: [PATCH] Fix *Config.ListRemotes on Git 2.46+ Caught by Windows CI. Updated CI matrix to run newer versions on Linux. --- .github/workflows/build.yml | 6 ++++-- CHANGELOG.md | 1 + config.go | 25 ++++++++++++++++++----- config_test.go | 6 ++++++ flake.lock | 40 +++++++++++++++++++++++++++++++++---- flake.nix | 20 ++++++++++++++++++- git.go | 34 +++++++++++++++++++++++++++++++ status.go | 15 ++------------ 8 files changed, 122 insertions(+), 25 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04d73ea..2a89378 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,9 @@ jobs: strategy: matrix: git: - - "2.27.0" # latest + - "2.46.1" # latest + - "2.45.2" + - "2.27.0" - "2.25.1" # Ubuntu LTS 20.04 focal - "2.20.1" # Debian buster - "2.17.1" # Ubuntu LTS 18.04 bionic @@ -66,7 +68,7 @@ jobs: -race \ ./... env: - GIT_VERSION: "2.27.0" + GIT_VERSION: "2.46.1" windows: name: Windows runs-on: windows-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 1873244..df55c17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#34](https://github.com/gg-scm/gg-git/pull/34)) - `object.Tree` now orders directories correctly. (Thanks to [@yangchi](https://github.com/yangchi) for discovering this issue.) +- `*Config.ListRemotes` matches behavior when running with Git 2.46+. ## [0.11.0][] - 2023-08-01 diff --git a/config.go b/config.go index f49e8af..b2412cf 100644 --- a/config.go +++ b/config.go @@ -24,11 +24,14 @@ import ( // Config is a collection of configuration settings. type Config struct { - data []byte + data []byte + gitVersion string } // ReadConfig reads all the configuration settings from Git. func (g *Git) ReadConfig(ctx context.Context) (*Config, error) { + version, _ := g.getVersion(ctx) + stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) err := g.runner.RunGit(ctx, &Invocation{ @@ -44,6 +47,7 @@ func (g *Git) ReadConfig(ctx context.Context) (*Config, error) { if err != nil { return nil, fmt.Errorf("read git config: %w", err) } + cfg.gitVersion = version return cfg, nil } @@ -157,6 +161,16 @@ func (cfg *Config) ListRemotes() map[string]*Remote { fetchURLsSet := make(map[string]bool) pushURLsSet := make(map[string]bool) remotePrefix := []byte("remote.") + + gitMajor, gitMinor, knownVersion := parseVersion(cfg.gitVersion) + + // Prior to Git 2.46 (specifically commit b68118d2e85eef7aa993ef8e944e53b5be665160 AFAICT), + // Git would use the first found "url" and "pushurl" settings + // rather than the last. + // Furthermore, Git would leave the fetch URL blank if the first setting was empty. + // Later versions use the remote's name as a fetch URL. + improvedHandling := !knownVersion || gitMajor > 2 || (gitMajor == 2 && gitMinor >= 46) + for off := 0; off < len(cfg.data); { k, v, end := splitConfigEntry(cfg.data[off:]) if end == -1 { @@ -182,16 +196,14 @@ func (cfg *Config) ListRemotes() map[string]*Remote { } // Update appropriate setting. - // Oddly, Git seems to use the first found setting instead of - // the last. This is verified by the test. switch string(k[i+1:]) { case "url": - if !fetchURLsSet[name] { + if improvedHandling || !fetchURLsSet[name] { remote.FetchURL = string(v) fetchURLsSet[name] = true } case "pushurl": - if !pushURLsSet[name] { + if improvedHandling || !pushURLsSet[name] { remote.PushURL = string(v) pushURLsSet[name] = true } @@ -200,6 +212,9 @@ func (cfg *Config) ListRemotes() map[string]*Remote { } } for _, remote := range remotes { + if improvedHandling { + remote.FetchURL = remote.Name + } if !pushURLsSet[remote.Name] { remote.PushURL = remote.FetchURL } diff --git a/config_test.go b/config_test.go index 0bc1d8c..cbd78cc 100644 --- a/config_test.go +++ b/config_test.go @@ -234,6 +234,12 @@ func TestListRemotes(t *testing.T) { "[remote \"myfork\"]\n" + "url = https://example.com/foo-fork.git\n", }, + { + name: "ClearPushURL", + config: "[remote \"origin\"]\n" + + "url = https://example.com/foo.git\n" + + "pushurl =\n", + }, } gitPath, err := findGit() if err != nil { diff --git a/flake.lock b/flake.lock index a6d8af9..55905e5 100644 --- a/flake.lock +++ b/flake.lock @@ -47,11 +47,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1685938391, - "narHash": "sha256-96Jw6TbWDLSopt5jqCW8w1Fc1cjQyZlhfBnJ3OZGpME=", + "lastModified": 1730272153, + "narHash": "sha256-B5WRZYsRlJgwVHIV6DvidFN7VX7Fg9uuwkRW9Ha8z+w=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "31cd1b4afbaf0b1e81272ee9c31d1ab606503aed", + "rev": "2d2a9ddbe3f2c00747398f3dc9b05f7f2ebb0f53", "type": "github" }, "original": { @@ -123,6 +123,36 @@ "type": "indirect" } }, + "nixpkgs-git_2_45_2": { + "locked": { + "lastModified": 1718368315, + "narHash": "sha256-QNrk+j9yUCH2HSbn1X9G0vqPn1VcSQM8CeiOPdJzIbY=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "53054089b25f3a55c8ca7af466223b94e80941b6", + "type": "github" + }, + "original": { + "id": "nixpkgs", + "rev": "53054089b25f3a55c8ca7af466223b94e80941b6", + "type": "indirect" + } + }, + "nixpkgs-git_2_46_1": { + "locked": { + "lastModified": 1726733501, + "narHash": "sha256-2vIFxheDtXnmwa1nAAPH2ZGH8NM6+QfrZgNMEyYrG5A=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "892b7e93c849a214efb4a689ed1aa310b0bfa95e", + "type": "github" + }, + "original": { + "id": "nixpkgs", + "rev": "892b7e93c849a214efb4a689ed1aa310b0bfa95e", + "type": "indirect" + } + }, "root": { "inputs": { "flake-compat": "flake-compat", @@ -132,7 +162,9 @@ "nixpkgs-git_2_17_1": "nixpkgs-git_2_17_1", "nixpkgs-git_2_21_0": "nixpkgs-git_2_21_0", "nixpkgs-git_2_25_1": "nixpkgs-git_2_25_1", - "nixpkgs-git_2_27_0": "nixpkgs-git_2_27_0" + "nixpkgs-git_2_27_0": "nixpkgs-git_2_27_0", + "nixpkgs-git_2_45_2": "nixpkgs-git_2_45_2", + "nixpkgs-git_2_46_1": "nixpkgs-git_2_46_1" } }, "systems": { diff --git a/flake.nix b/flake.nix index 71bb610..15fcec9 100644 --- a/flake.nix +++ b/flake.nix @@ -19,6 +19,12 @@ url = "nixpkgs/98c44f565746165a556953cda769d23d732466f4"; flake = false; }; + nixpkgs-git_2_45_2 = { + url = "nixpkgs/53054089b25f3a55c8ca7af466223b94e80941b6"; + }; + nixpkgs-git_2_46_1 = { + url = "nixpkgs/892b7e93c849a214efb4a689ed1aa310b0bfa95e"; + }; git_2_20_1 = { url = "https://www.kernel.org/pub/software/scm/git/git-2.20.1.tar.xz"; @@ -38,7 +44,7 @@ let pkgs = import nixpkgs { inherit system; }; in { - packages.go = pkgs.go_1_19; + packages.go = pkgs.go_1_23; packages.git = pkgs.git; @@ -81,6 +87,16 @@ outputs = [ "out" ]; }); + packages.git_2_45_2 = (self.lib.buildGit { + inherit pkgs; + packagePath = "${inputs.nixpkgs-git_2_45_2}/pkgs/applications/version-management/git"; + }); + + packages.git_2_46_1 = (self.lib.buildGit { + inherit pkgs; + packagePath = "${inputs.nixpkgs-git_2_46_1}/pkgs/applications/version-management/git"; + }); + devShells.default = pkgs.mkShell { packages = [ self.packages.${system}.go @@ -92,6 +108,8 @@ lib.buildGit = { pkgs, packagePath, args ? {} }: let defaultArgs = { + inherit (pkgs.darwin.apple_sdk.frameworks) CoreServices Security; + guiSupport = false; sendEmailSupport = false; svnSupport = false; diff --git a/git.go b/git.go index ae7dafb..4e83410 100644 --- a/git.go +++ b/git.go @@ -25,6 +25,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "sync" @@ -126,6 +127,39 @@ func (g *Git) getVersion(ctx context.Context) (string, error) { return v, nil } +// parseVersion parses a Git version string. +// Please use this as an absolute last resort: +// the user may have a custom version of Git +// or the format may change over time. +// When working around buggy behavior, +// the best practice is to write your code in the non-buggy way, +// then have an explicit list of versions +// that use the workaround. +func parseVersion(version string) (major, minor int, ok bool) { + const prefix = "git version " + if !strings.HasPrefix(version, prefix) { + return 0, 0, false + } + version = version[len(prefix):] + // Only consider the first line of version output. + version, _, _ = strings.Cut(version, "\n") + + majorString, rest, ok := strings.Cut(version, ".") + if !ok { + return 0, 0, false + } + majorUint, err := strconv.ParseUint(majorString, 10, 31) + if err != nil { + return 0, 0, false + } + minorString, _, _ := strings.Cut(rest, ".") + minorUint, err := strconv.ParseUint(minorString, 10, 31) + if err != nil { + return int(majorUint), 0, false + } + return int(majorUint), int(minorUint), true +} + // Exe returns the absolute path to the Git executable. // This method will panic if g's Runner is not of type *Local. // diff --git a/status.go b/status.go index 7a3fd53..ebd5d7e 100644 --- a/status.go +++ b/status.go @@ -78,19 +78,8 @@ func (g *Git) Status(ctx context.Context, opts StatusOptions) ([]StatusEntry, er // not the new added file. See https://github.com/gg-scm/gg/issues/60 // for a full explanation. func affectedByStatusRenameBug(version string) bool { - prefixes := []string{ - "git version 2.11", - "git version 2.12", - "git version 2.13", - "git version 2.14", - "git version 2.15", - } - for _, p := range prefixes { - if strings.HasPrefix(version, p) && (len(version) == len(p) || version[len(p)] == '.') { - return true - } - } - return false + major, minor, ok := parseVersion(version) + return ok && major == 2 && 11 <= minor && minor <= 15 } // A StatusEntry describes the state of a single file in the working copy.