From 6870d46ece5eec9c7dc421933b18bec223ab5dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:40:38 +0200 Subject: [PATCH 01/22] Ignore core.fsmonitor, and say gitlib trusts the repository it opens The configuration of a repository may name a command in core.fsmonitor, which git runs to read the working tree, and even to diff a commit: getUntrackedFiles(), the diffs of the working copy and Commit::getDiff() ran it. CliDriver sets core.fsmonitor back to false, on every command. The doc said safe.directory applies: it doesn't, as CliDriver runs git with --git-dir, which git trusts, whoever owns the repository. Its hooks and the filters of its .gitattributes still run, like with git: the doc says to only open a repository whose .git directory is trusted. --- CHANGELOG.md | 4 +++ UPGRADE-2.0.md | 2 ++ doc/repository.md | 25 +++++++++++++------ .../Git/Driver/Cli/CliRepositoryDriver.php | 4 +++ tests/Gitonomy/Git/Tests/WorkingCopyTest.php | 21 ++++++++++++++++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d9bfe6..43afcfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,10 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - Ignore the replacements of objects (`git replace`, `refs/replace/`) with `CliDriver`, like `LibGit2Driver`: whoever could push such a reference to a repository changed the message of a commit, or the content of a blob, gitlib read for a hash. +- Ignore `core.fsmonitor` with `CliDriver`: git ran the command the configuration of a + repository names there to read the working tree, and even to diff a commit. Like git with + `--git-dir`, gitlib trusts the repositories it opens: `safe.directory` doesn't apply, see + ["Git configuration"](doc/repository.md#git-configuration). - Fix `WorkingCopy::checkout()` of a path, like `checkout('README.md')`, discarding the changes of the file: a path is no revision, and throws a `ReferenceNotFoundException`. - Fix moving or deleting a symbolic reference, which changed the branch it points to instead: diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index d308148..62654f9 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -473,6 +473,8 @@ Git configuration * `CliDriver` ignores the replacements of objects (`git replace`, `refs/replace/`), like `LibGit2Driver`: `run()` and `runProcess()` too. Run git yourself to read them. +* `CliDriver` ignores `core.fsmonitor`, whose command git runs to read the + working tree: `run()` and `runProcess()` too. Dates ----- diff --git a/doc/repository.md b/doc/repository.md index 1eb3277..a3fa59f 100644 --- a/doc/repository.md +++ b/doc/repository.md @@ -492,6 +492,8 @@ creates, `CliDriver` sets it back to git's defaults: `LibGit2Driver`, a branch `WorkingCopy::checkoutNewBranch()` creates from a remote branch tracks it, without rebasing; - `branch.sort`, `log.follow` and `log.showSignature`; +- `core.fsmonitor`, whose command git runs to read the working tree, and + even to diff a commit; - for diffs, `diff.algorithm`, `diff.context`, `diff.external`, `diff.indentHeuristic`, `diff.interHunkContext`, `diff.orderFile`, `diff.submodule`, `diff.suppressBlankEmpty`, the prefixes @@ -504,15 +506,24 @@ reference to a repository would change what your application shows for a hash. The rest of the configuration applies, like with git: the identity, the -credentials and the rewrites of URLs, `safe.directory`, line endings, hooks, -`merge.log`, `blame.ignoreRevsFile`, or `core.abbrev`, the length of short -hashes, with `LibGit2Driver` too. +credentials and the rewrites of URLs, line endings, hooks, `merge.log`, +`blame.ignoreRevsFile`, or `core.abbrev`, the length of short hashes, with +`LibGit2Driver` too. + +`safe.directory` doesn't apply: `CliDriver` runs git with `--git-dir`, which +git trusts, whoever owns the repository. Like git, gitlib then runs the +commands the configuration of the repository names: its hooks, and the +filters of its `.gitattributes`, when it checks files out, merges, or reads +the working tree. Only open a repository whose `.git` directory you trust: +not one another user of the machine owns, nor one extracted from an archive +your users send. `LibGit2Driver` runs neither hooks nor external filters. `run()` and `runProcess()` get `branch.autoSetupMerge`, -`branch.autoSetupRebase`, `branch.sort`, `log.follow`, `log.showSignature`, -`diff.indentHeuristic` and `diff.suppressBlankEmpty` set back too, and ignore -the replacements, but not the rest, which gitlib sets with the options of the -commands it runs: pass `--no-color` or `--no-ext-diff` yourself. +`branch.autoSetupRebase`, `branch.sort`, `core.fsmonitor`, `log.follow`, +`log.showSignature`, `diff.indentHeuristic` and `diff.suppressBlankEmpty` set +back too, and ignore the replacements, but not the rest, which gitlib sets with +the options of the commands it runs: pass `--no-color` or `--no-ext-diff` +yourself. Logger ------ diff --git a/src/Gitonomy/Git/Driver/Cli/CliRepositoryDriver.php b/src/Gitonomy/Git/Driver/Cli/CliRepositoryDriver.php index 025699e..7d060db 100644 --- a/src/Gitonomy/Git/Driver/Cli/CliRepositoryDriver.php +++ b/src/Gitonomy/Git/Driver/Cli/CliRepositoryDriver.php @@ -47,12 +47,16 @@ * * branch.autoSetupMerge and branch.autoSetupRebase: a branch created from * a remote branch tracks it, without rebasing, like with LibGit2Driver. + * core.fsmonitor: git runs the command it names, which the configuration + * of a repository may set, to read the working tree, and even to diff a + * commit. * diff.indentHeuristic is the default since git 2.14, and LibGit2Driver's. */ private const array CONFIG = [ '-c', 'branch.autoSetupMerge=true', '-c', 'branch.autoSetupRebase=never', '-c', 'branch.sort=refname', + '-c', 'core.fsmonitor=false', '-c', 'diff.indentHeuristic=true', '-c', 'diff.suppressBlankEmpty=false', '-c', 'log.follow=false', diff --git a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php index 87ba273..200af57 100644 --- a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php +++ b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php @@ -158,6 +158,27 @@ public function testAFileAddedWithIntentToAddThenDeletedIsDeleted(): void $this->assertSame([], $wc->getDiff()->getFiles()); } + /** + * The configuration of a repository may name a command in core.fsmonitor, + * which git runs to read the working tree, and even to diff a commit. + */ + public function testTheCommandOfCoreFsmonitorDoesNotRun(): void + { + $repository = self::createFoobarRepository(false); + $marker = self::createTempDir().'/ran'; + self::git($repository, 'config', ['core.fsmonitor', 'touch '.$marker.'; false']); + file_put_contents($repository->getWorkingDir().'/README.md', 'local change'); + $wc = $repository->getWorkingCopy(); + + $wc->getUntrackedFiles(); + $wc->getDiffPending(); + $wc->getDiffStaged(); + $wc->getDiff(); + $repository->getHeadCommit()->getDiff(); + + $this->assertFileDoesNotExist($marker); + } + public function testDiffLeavesOutAFileTheWorkingTreeChangesBack(): void { $repository = self::createFoobarRepository(false); From 5cc0e41a12397b1d20f3bac17f2f90efa3c79384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:41:43 +0200 Subject: [PATCH 02/22] Look for a bundle in a local path only LibGit2Driver checks whether the URL of a clone is a bundle with is_file(), which hands a URL to the stream wrapper of its scheme: for "ftp://user:password@host/...", PHP connected to the host and sent the credentials, before libgit2 saw the URL. With "ssh://" or "git://", PHP warned that no wrapper exists, which error handlers turn into an exception whose trace held the URL. Only a local path, "file://" included, is looked at, and the URL is a #[\SensitiveParameter]. --- src/Gitonomy/Git/Driver/Ffi/LibGit2Driver.php | 13 +++- .../Gitonomy/Git/Tests/LibGit2DriverTest.php | 71 ++++++++++++++++--- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/src/Gitonomy/Git/Driver/Ffi/LibGit2Driver.php b/src/Gitonomy/Git/Driver/Ffi/LibGit2Driver.php index dbb3e54..3d227cc 100644 --- a/src/Gitonomy/Git/Driver/Ffi/LibGit2Driver.php +++ b/src/Gitonomy/Git/Driver/Ffi/LibGit2Driver.php @@ -191,10 +191,19 @@ public function isValidRepositoryAndBranch(#[\SensitiveParameter] string $url, s * * @throws UnsupportedOperationException $url is a bundle */ - private static function assertNotBundle(string $url): void + private static function assertNotBundle(#[\SensitiveParameter] string $url): void { + // A local path only, a Windows one included: is_file() would hand any + // other URL to the stream wrapper of its scheme, like the one of + // "ftp://", which connects with the credentials of the URL. A + // "host:path" address isn't local either. + $path = str_starts_with($url, 'file://') ? substr($url, \strlen('file://')) : $url; + if (1 === preg_match('#^[a-zA-Z][a-zA-Z0-9+.-]*:#', $path) && 1 !== preg_match('#^[a-zA-Z]:[\\\/]#', $path)) { + return; + } + // The signatures of the formats of bundles. - if (is_file($url) && is_readable($url) && \in_array(file_get_contents($url, length: 16), ["# v2 git bundle\n", "# v3 git bundle\n"], true)) { + if (is_file($path) && is_readable($path) && \in_array(file_get_contents($path, length: 16), ["# v2 git bundle\n", "# v3 git bundle\n"], true)) { throw new UnsupportedOperationException(\sprintf('"%s" is a bundle, which LibGit2Driver doesn\'t support; use CliDriver.', $url)); } } diff --git a/tests/Gitonomy/Git/Tests/LibGit2DriverTest.php b/tests/Gitonomy/Git/Tests/LibGit2DriverTest.php index e48c1ab..a69fecf 100644 --- a/tests/Gitonomy/Git/Tests/LibGit2DriverTest.php +++ b/tests/Gitonomy/Git/Tests/LibGit2DriverTest.php @@ -13,6 +13,7 @@ namespace Gitonomy\Git\Tests; use Gitonomy\Git\Driver\Ffi\LibGit2Driver; +use Gitonomy\Git\Exception\DriverException; use Gitonomy\Git\Exception\UnsupportedOperationException; use Gitonomy\Git\Repository; use PHPUnit\Framework\Attributes\DataProvider; @@ -117,28 +118,80 @@ public function testReftableRepositoriesAreUnsupported(): void } /** - * @param \Closure(LibGit2Driver, string): void $clone + * @param \Closure(LibGit2Driver, string, string): void $clone */ - #[DataProvider('provideClones')] - public function testBundlesAreUnsupported(\Closure $clone): void + #[DataProvider('provideBundleClones')] + public function testBundlesAreUnsupported(\Closure $clone, string $url): void { if (!LibGit2Driver::isSupported()) { $this->markTestSkipped('libgit2 1.9 is not installed.'); } $this->expectException(UnsupportedOperationException::class); - $this->expectExceptionMessage(\sprintf('"%s" is a bundle, which LibGit2Driver doesn\'t support; use CliDriver.', self::REPOSITORY_URL)); + $this->expectExceptionMessage(\sprintf('"%s" is a bundle, which LibGit2Driver doesn\'t support; use CliDriver.', $url)); + + $clone(new LibGit2Driver(), self::createTempDir().'/clone', $url); + } + + /** + * @return iterable + */ + public static function provideBundleClones(): iterable + { + foreach (self::provideClones() as $name => [$clone]) { + yield $name => [$clone, self::REPOSITORY_URL]; + yield $name.', file:// URL' => [$clone, 'file://'.self::REPOSITORY_URL]; + } + } + + /** + * is_file() hands a URL to the stream wrapper of its scheme: the one of + * "ftp://" connects to the host, with the credentials of the URL. + * + * @param \Closure(LibGit2Driver, string, string): void $clone + */ + #[DataProvider('provideClones')] + public function testOnlyALocalPathIsLookedAtForABundle(\Closure $clone): void + { + if (!LibGit2Driver::isSupported()) { + $this->markTestSkipped('libgit2 1.9 is not installed.'); + } + + $wrapper = new class { + /** + * @var list + */ + public static array $urls = []; + + public mixed $context; + + public function url_stat(string $path, int $flags): false + { + self::$urls[] = $path; + + return false; + } + }; + stream_wrapper_register('gitlib-test', $wrapper::class); + + try { + $clone(new LibGit2Driver(), self::createTempDir().'/clone', 'gitlib-test://user:secret@example.com/repository.git'); + $this->fail('libgit2 knows no "gitlib-test" transport.'); + } catch (DriverException) { + } finally { + stream_wrapper_unregister('gitlib-test'); + } - $clone(new LibGit2Driver(), self::createTempDir().'/clone'); + $this->assertSame([], $wrapper::$urls); } /** - * @return iterable + * @return iterable */ public static function provideClones(): iterable { - yield 'clone' => [static fn (LibGit2Driver $driver, string $path) => $driver->cloneFrom(self::REPOSITORY_URL, $path, null)]; - yield 'bare clone' => [static fn (LibGit2Driver $driver, string $path) => $driver->cloneBareFrom(self::REPOSITORY_URL, $path, null)]; - yield 'mirror' => [static fn (LibGit2Driver $driver, string $path) => $driver->mirrorFrom(self::REPOSITORY_URL, $path)]; + yield 'clone' => [static fn (LibGit2Driver $driver, string $path, string $url) => $driver->cloneFrom($url, $path, null)]; + yield 'bare clone' => [static fn (LibGit2Driver $driver, string $path, string $url) => $driver->cloneBareFrom($url, $path, null)]; + yield 'mirror' => [static fn (LibGit2Driver $driver, string $path, string $url) => $driver->mirrorFrom($url, $path)]; } } From a047731187098217d0ca3de685cc523c5438864d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:42:28 +0200 Subject: [PATCH 03/22] Refuse fd:: URLs An "fd::7" URL makes git talk through file descriptors it inherits from PHP: an open file, the socket of a database... isValidRepository() of such a URL, which an application may get from its users, made git wait on a socket of the application until it timed out, an hour by default. Both drivers refuse it with an InvalidArgumentException, like an "ext::" URL. --- CHANGELOG.md | 3 ++- UPGRADE-2.0.md | 3 ++- doc/admin.md | 7 +++++-- doc/repository.md | 6 +++--- src/Gitonomy/Git/Driver/DriverInterface.php | 5 +++-- src/Gitonomy/Git/Util/RemoteUrl.php | 13 ++++++++++--- tests/Gitonomy/Git/Tests/RemoteUrlTest.php | 13 +++++++++++++ 7 files changed, 38 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43afcfa..8b1682c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,7 +128,8 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). a file, and `Repository::isValidRepository('--upload-pack=...')` run a command. A path may start with a dash: `init()` and the clones end the options of git with `--`. - Refuse an `ext::` URL, like `ext::sh -c ...`, whose command git runs, by default before git - 2.12. + 2.12, and an `fd::` URL, which makes git talk through a file descriptor it inherits from PHP, + like the socket of a database, and wait on it. - Refuse an ssh URL whose host or user starts with a dash, even encoded (`%2D`), like `ssh://-oProxyCommand=.../repository`: git before 2.14.1 passes it to ssh, which reads it as an option and runs the command (CVE-2017-1000117). A `git://` URL whose host starts with a dash diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 62654f9..10ac016 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -443,7 +443,8 @@ Errors `isValidRepository('--upload-pack=...')` running a command. A path may start with a dash. * An `ext::` URL throws an `InvalidArgumentException`, with both drivers: - git runs its command, by default before git 2.12. + git runs its command, by default before git 2.12. So does an `fd::` URL, + which makes git talk through a file descriptor it inherits from PHP. * An ssh URL whose host or user starts with a dash, even encoded (`%2D`), like `ssh://-oProxyCommand=.../repository` or `ssh://-oProxyCommand=...@host/path`, or a `git://` URL whose host does, diff --git a/doc/admin.md b/doc/admin.md index 6e42f23..10e6cfe 100644 --- a/doc/admin.md +++ b/doc/admin.md @@ -101,13 +101,16 @@ error output, when `CliDriver` runs git. With `CliDriver`, every method, `isValidRepository()` included, throws a `DriverException` when git can't start, like a missing binary. See [Errors](repository.md#errors). -A URL that makes git run a command throws an `InvalidArgumentException`, -with both drivers: +A URL that makes git run a command, or use a file of PHP, throws an +`InvalidArgumentException`, with both drivers: - a URL starting with a dash, which git would read as an option, like `--upload-pack=...`; a path may start with one; - an `ext::` URL, like `ext::sh -c ...`, whose command git runs, unless its `protocol.allow` setting forbids it, as it does by default since git 2.12; +- an `fd::` URL, like `fd::7`, which makes git talk through a file + descriptor it inherits from PHP, like the socket of a database: git waits + on it until it times out; - an ssh URL whose host or user starts with a dash, even encoded (`%2D`), like `ssh://-oProxyCommand=.../repository` or `ssh://-oProxyCommand=...@host/repository`: before 2.14.1, git passes it diff --git a/doc/repository.md b/doc/repository.md index a3fa59f..46508ce 100644 --- a/doc/repository.md +++ b/doc/repository.md @@ -237,9 +237,9 @@ or empty data instead. Every exception of gitlib implements byte, which git doesn't accept, a revision, a reference, a hash or a URL starting with a dash, which git would read as an option (`getRevision('--output=/path')`), an `ext::` URL, whose command git - would run, a URL whose ssh host or user starts with a dash, which ssh - would read as an option, or a reference to delete outside `refs/`, like - `HEAD`. + would run, an `fd::` URL, a URL whose ssh host or user starts with a + dash, which ssh would read as an option, or a reference to delete + outside `refs/`, like `HEAD`. - `RuntimeException`: any other failure, like a hook that exists already (`Hooks::set()`), a file of the git directory that can't be written, or an output of git that gitlib can't read. `DriverException` and diff --git a/src/Gitonomy/Git/Driver/DriverInterface.php b/src/Gitonomy/Git/Driver/DriverInterface.php index 9323c76..88ca48c 100644 --- a/src/Gitonomy/Git/Driver/DriverInterface.php +++ b/src/Gitonomy/Git/Driver/DriverInterface.php @@ -33,8 +33,9 @@ * An application may get the URL of a repository from its users: every method * taking one refuses, with an {@see InvalidArgumentException}, a URL git would * misread, before running git. A URL starting with a dash is an option for - * git, an "ext::" URL runs its command, and an ssh host or user starting with - * a dash is an option for ssh. + * git, an "ext::" URL runs its command, an "fd::" URL talks through a file + * descriptor of PHP, and an ssh host or user starting with a dash is an + * option for ssh. * * @author Grégoire Pineau */ diff --git a/src/Gitonomy/Git/Util/RemoteUrl.php b/src/Gitonomy/Git/Util/RemoteUrl.php index 2923b3a..34a4bd1 100644 --- a/src/Gitonomy/Git/Util/RemoteUrl.php +++ b/src/Gitonomy/Git/Util/RemoteUrl.php @@ -15,8 +15,8 @@ use Gitonomy\Git\Exception\InvalidArgumentException; /** - * Refuses the URLs of a repository that make git run a command, as an - * application may get them from its users. + * Refuses the URLs of a repository that make git run a command, or use a + * file descriptor of PHP, as an application may get them from its users. * * @author Grégoire Pineau * @@ -25,7 +25,7 @@ final class RemoteUrl { /** - * @throws InvalidArgumentException $url starts with a dash, uses the "ext::" transport, or has a host or a user starting with a dash, encoded or not + * @throws InvalidArgumentException $url starts with a dash, uses the "ext::" or "fd::" transport, or has a host or a user starting with a dash, encoded or not */ public static function assertSafe(#[\SensitiveParameter] string $url): void { @@ -37,6 +37,13 @@ public static function assertSafe(#[\SensitiveParameter] string $url): void throw new InvalidArgumentException(\sprintf('"%s" uses the "ext::" transport, which runs a command.', Credentials::redact($url))); } + // "fd::7" talks through the file descriptors git inherits from PHP: + // an open file, the socket of a database... git waits on it until it + // times out, an hour by default. + if (str_starts_with($url, 'fd::')) { + throw new InvalidArgumentException(\sprintf('"%s" uses the "fd::" transport, which talks through a file descriptor of PHP.', Credentials::redact($url))); + } + // Before 2.14.1 (CVE-2017-1000117), git passes the "user@host" of an ssh // URL to ssh, without its brackets: ssh reads "ssh://-oProxyCommand=..." // as an option, and runs the command. The command of core.gitProxy gets diff --git a/tests/Gitonomy/Git/Tests/RemoteUrlTest.php b/tests/Gitonomy/Git/Tests/RemoteUrlTest.php index cdb56a2..5df1f4a 100644 --- a/tests/Gitonomy/Git/Tests/RemoteUrlTest.php +++ b/tests/Gitonomy/Git/Tests/RemoteUrlTest.php @@ -69,6 +69,19 @@ public function testAnExtUrlIsRefused(\Closure $call): void $this->assertFileDoesNotExist($file); } + /** + * git talks through the file descriptors of an "fd::" URL, which it + * inherits from PHP, like the socket of a database. + */ + #[DataProvider('provideCalls')] + public function testAnFdUrlIsRefused(\Closure $call): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('"fd::0" uses the "fd::" transport, which talks through a file descriptor of PHP.'); + + $call('fd::0', self::createTempDir().'/clone'); + } + /** * @return iterable */ From 0624727a250de9b23cdd21095465afd54395fa9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:42:40 +0200 Subject: [PATCH 04/22] Say in the security policy that 1.x is no longer maintained The policy promised 12 months of support to the previous major release, but 1.x has no branch, and gets no fix for the security issues 2.0 fixes. It now says only 2.x gets security fixes, and keeps the promise for the next major releases. --- .github/SECURITY.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/SECURITY.md b/.github/SECURITY.md index 0c3e8a2..e8e4f0c 100644 --- a/.github/SECURITY.md +++ b/.github/SECURITY.md @@ -2,9 +2,15 @@ ## Supported Versions -After each new major release, the previous release will be supported for no -less than 12 months, unless explictly stated otherwise. This may mean that -there are multiple supported versions at any given time. +Only 2.x gets security fixes: 1.x is no longer maintained. 2.0 fixes security +issues 1.x has, which won't be fixed there, like revisions and URLs starting +with a dash, which git reads as options, or the credentials of URLs in the +messages of exceptions. To upgrade, see +[UPGRADE-2.0.md](https://github.com/gitonomy/gitlib/blob/main/UPGRADE-2.0.md). + +After each new major release from 3.0 on, the previous release will be +supported for no less than 12 months, unless explicitly stated otherwise. +This may mean that there are multiple supported versions at any given time. ## Reporting a Vulnerability From 25d448e6b4747679b5dc9243935093ecaf543359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:43:15 +0200 Subject: [PATCH 05/22] Remove the arguments of git merge of WorkingCopy::merge(), never released WorkingCopy::merge() came with #248, after 1.6.0: no release has its array of arguments of git merge ([], ["--no-ff"] or ["--ff-only"]), which 2.0 deprecated in favor of FastForward. 2.0 would have shipped a deprecation of its first day, and a FastForward|array argument for all of 2.x. merge() takes a FastForward only. The deprecation, its section of UPGRADE-2.0.md and its line in the CHANGELOG go. --- CHANGELOG.md | 3 +-- UPGRADE-2.0.md | 17 ----------------- doc/workingcopy.md | 3 --- src/Gitonomy/Git/WorkingCopy.php | 18 +----------------- tests/Gitonomy/Git/Tests/WorkingCopyTest.php | 14 -------------- 5 files changed, 2 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b1682c..e694425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,8 +114,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - A range of lines as a string in `Repository::getBlame()`, `Blame::getGroupedLines()` and `Blame\Line::getLine()`. - `CommitReference`, `Tree::getCommitReferenceEntries()`, reading tree entries as arrays. -- The branch to create of `WorkingCopy::checkout()`, the arguments of `git merge` of - `WorkingCopy::merge()`. +- The branch to create of `WorkingCopy::checkout()`. - `Commit::getShortMessage()` and `getFixedShortHash()`, `Repository::shell()`, `PushReference::getReference()` and `getRevision()`. - `Repository::getSize()`, in kilobytes: use `getSizeInBytes()`. diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 10ac016..6f328de 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -322,23 +322,6 @@ Working copy $workingCopy->checkoutNewBranch('feature', 'origin/feature'); ``` -* `WorkingCopy::merge()` takes a `FastForward` instead of arguments of - `git merge`, deprecated. - - Before: - - ```php - $workingCopy->merge('feature', ['--no-ff']); - ``` - - After: - - ```php - use Gitonomy\Git\FastForward; - - $workingCopy->merge('feature', FastForward::Never); - ``` - Commits, trees and blobs ------------------------ diff --git a/doc/workingcopy.md b/doc/workingcopy.md index 52431f1..2e02ae6 100644 --- a/doc/workingcopy.md +++ b/doc/workingcopy.md @@ -117,9 +117,6 @@ $repo->run('merge', ['--abort']); > throws without changing anything, instead of leaving conflict markers in > the working copy. -Passing arguments of `git merge` as an array (`[]`, `['--no-ff']` or -`['--ff-only']`) is deprecated since 2.0: pass a `FastForward`. - Modifications since a revision ------------------------------ diff --git a/src/Gitonomy/Git/WorkingCopy.php b/src/Gitonomy/Git/WorkingCopy.php index a7374be..c2cc298 100644 --- a/src/Gitonomy/Git/WorkingCopy.php +++ b/src/Gitonomy/Git/WorkingCopy.php @@ -14,7 +14,6 @@ use Gitonomy\Git\Diff\Diff; use Gitonomy\Git\Exception\DriverException; -use Gitonomy\Git\Exception\InvalidArgumentException; use Gitonomy\Git\Exception\LogicException; use Gitonomy\Git\Exception\ReferenceNotFoundException; use Gitonomy\Git\Reference\Branch; @@ -130,26 +129,11 @@ public function checkoutNewBranch(string $branch, Revision|string $startPoint = /** * Merges a revision into the current branch. * - * An array of arguments of "git merge" ([], ["--no-ff"] or ["--ff-only"]) is deprecated. - * - * @param FastForward|string[] $fastForward - * * @throws ReferenceNotFoundException $revision names no commit * @throws DriverException The merge fails: a conflict, staged changes, an unrelated history... */ - public function merge(Revision|string $revision, FastForward|array $fastForward = FastForward::Allow): static + public function merge(Revision|string $revision, FastForward $fastForward = FastForward::Allow): static { - if (\is_array($fastForward)) { - trigger_deprecation('gitonomy/gitlib', '2.0', 'Passing arguments of "git merge" to "%s()" is deprecated, pass a "%s" instead.', __METHOD__, FastForward::class); - - $fastForward = match ($fastForward) { - [] => FastForward::Allow, - ['--no-ff'] => FastForward::Never, - ['--ff-only'] => FastForward::Only, - default => throw new InvalidArgumentException(\sprintf('Unsupported arguments "%s": pass a "%s" instead.', implode(' ', $fastForward), FastForward::class)), - }; - } - // Like checkout(). The driver gets the name, which the message of the merge commit has. ($revision instanceof Revision ? $revision : $this->repository->getRevision($revision))->getCommit(); diff --git a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php index 200af57..2d39193 100644 --- a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php +++ b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php @@ -388,20 +388,6 @@ public function testMergeOnlyFastForwards(): void $wc->merge('origin/diff-features', FastForward::Only); } - #[Group('legacy')] - #[IgnoreDeprecations] - public function testLegacyMergeArguments(): void - { - $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: Passing arguments of "git merge" to "Gitonomy\Git\WorkingCopy::merge()" is deprecated, pass a "Gitonomy\Git\FastForward" instead.'); - - $repository = self::createFoobarRepository(false); - $head = $repository->getHeadCommit(); - - $repository->getWorkingCopy()->merge($head->getHash(), ['--no-ff']); - - $this->assertSame($head->getHash(), $repository->getHeadCommit()->getHash()); - } - public function testMergeFastForwardsTheCurrentBranch(): void { $repository = self::createFoobarRepository(false); From d94de0ae155aeb3e620455b7afb77b8377861df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:45:21 +0200 Subject: [PATCH 06/22] Refuse a negative offset or limit of a log A negative offset or limit gave wrong data, different with each driver: withLimit(-1) counted -1 commits, and iterated the whole history with CliDriver, but one commit with LibGit2Driver. withOffset(-5) counted more commits than it iterated. Log, Repository::getLog() and Revision::getLog() throw an InvalidArgumentException for one. A limit of 0 gives no commit, with both drivers. --- UPGRADE-2.0.md | 1 + src/Gitonomy/Git/Log.php | 42 ++++++++++++++++++++++++---- src/Gitonomy/Git/Repository.php | 2 ++ src/Gitonomy/Git/Revision.php | 3 ++ tests/Gitonomy/Git/Tests/LogTest.php | 34 ++++++++++++++++++++++ 5 files changed, 76 insertions(+), 6 deletions(-) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 6f328de..b4638ca 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -200,6 +200,7 @@ History `withLimit()` return another `Log`. * `count($log)` returns the number of commits the log iterates, after its offset and up to its limit; `countCommits()` still counts them all. +* A negative offset or limit throws an `InvalidArgumentException`. * The revisions of a log are names, ranges (`a..b`, `a...b`), names to exclude (`^a`) or `--all`: any other option of `git log` throws an `InvalidArgumentException`, instead of being passed to git. An unknown diff --git a/src/Gitonomy/Git/Log.php b/src/Gitonomy/Git/Log.php index 038edcc..c099e0b 100644 --- a/src/Gitonomy/Git/Log.php +++ b/src/Gitonomy/Git/Log.php @@ -45,6 +45,8 @@ final class Log implements \Countable, \IteratorAggregate * @param string[]|string|null $paths paths to filter on, with git's wildcards * @param int|null $offset start list from a given position * @param int|null $limit limit number of fetched elements + * + * @throws InvalidArgumentException $offset or $limit is negative */ public function __construct( private readonly Repository $repository, @@ -65,8 +67,8 @@ public function __construct( $this->revisions = $revisions; $this->paths = $paths; - $this->offset = $offset; - $this->limit = $limit; + $this->offset = self::offset($offset); + $this->limit = self::limit($limit); } /** @@ -99,11 +101,13 @@ public function getOffset(): ?int /** * @return self The log, starting from the commit $offset + * + * @throws InvalidArgumentException $offset is negative */ public function withOffset(?int $offset): self { $log = clone $this; - $log->offset = $offset; + $log->offset = self::offset($offset); return $log; } @@ -115,7 +119,7 @@ public function setOffset(?int $offset): static { trigger_deprecation('gitonomy/gitlib', '2.0', 'The "%s()" method is deprecated, use "withOffset()" instead.', __METHOD__); - $this->offset = $offset; + $this->offset = self::offset($offset); return $this; } @@ -127,11 +131,13 @@ public function getLimit(): ?int /** * @return self The log, of $limit commits at most + * + * @throws InvalidArgumentException $limit is negative */ public function withLimit(?int $limit): self { $log = clone $this; - $log->limit = $limit; + $log->limit = self::limit($limit); return $log; } @@ -143,7 +149,7 @@ public function setLimit(?int $limit): static { trigger_deprecation('gitonomy/gitlib', '2.0', 'The "%s()" method is deprecated, use "withLimit()" instead.', __METHOD__); - $this->limit = $limit; + $this->limit = self::limit($limit); return $this; } @@ -262,4 +268,28 @@ private function commitHash(string $revision): string { return $this->repository->getRevision('' === $revision ? 'HEAD' : $revision)->getCommit()->getHash(); } + + /** + * @throws InvalidArgumentException $offset is negative + */ + private static function offset(?int $offset): ?int + { + if (null !== $offset && $offset < 0) { + throw new InvalidArgumentException(\sprintf('The offset of a log can\'t be negative, %d given.', $offset)); + } + + return $offset; + } + + /** + * @throws InvalidArgumentException $limit is negative + */ + private static function limit(?int $limit): ?int + { + if (null !== $limit && $limit < 0) { + throw new InvalidArgumentException(\sprintf('The limit of a log can\'t be negative, %d given.', $limit)); + } + + return $limit; + } } diff --git a/src/Gitonomy/Git/Repository.php b/src/Gitonomy/Git/Repository.php index 1d95ca3..ca47ed0 100644 --- a/src/Gitonomy/Git/Repository.php +++ b/src/Gitonomy/Git/Repository.php @@ -450,6 +450,8 @@ public function getBlame(Revision|string $revision, string $file, int|string|nul * paths * @param int|null $offset start from a given offset in results * @param int|null $limit limit number of total results + * + * @throws InvalidArgumentException $offset or $limit is negative */ public function getLog(RevisionList|Revision|string|array|null $revisions = null, array|string|null $paths = null, ?int $offset = null, ?int $limit = null): Log { diff --git a/src/Gitonomy/Git/Revision.php b/src/Gitonomy/Git/Revision.php index 8c8f5e9..3e13120 100644 --- a/src/Gitonomy/Git/Revision.php +++ b/src/Gitonomy/Git/Revision.php @@ -12,6 +12,7 @@ namespace Gitonomy\Git; +use Gitonomy\Git\Exception\InvalidArgumentException; use Gitonomy\Git\Exception\ReferenceNotFoundException; /** @@ -35,6 +36,8 @@ public function __construct( /** * @param string[]|string|null $paths Paths to filter on, with git's wildcards + * + * @throws InvalidArgumentException $offset or $limit is negative */ public function getLog(array|string|null $paths = null, ?int $offset = null, ?int $limit = null): Log { diff --git a/tests/Gitonomy/Git/Tests/LogTest.php b/tests/Gitonomy/Git/Tests/LogTest.php index 597001e..d4d1743 100644 --- a/tests/Gitonomy/Git/Tests/LogTest.php +++ b/tests/Gitonomy/Git/Tests/LogTest.php @@ -380,6 +380,40 @@ public function testLogOfExcludedAndSymmetricRevisions(): void $this->assertSame([$main, $base], self::hashes($repository->getLog('main'))); } + /** + * @param \Closure(Repository): Log $log + */ + #[DataProvider('provideNegativeBounds')] + public function testANegativeOffsetOrLimitIsRefused(\Closure $log, string $message): void + { + $repository = self::createFoobarRepository(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + + $log($repository); + } + + /** + * @return iterable + */ + public static function provideNegativeBounds(): iterable + { + yield 'offset' => [static fn (Repository $repository): Log => $repository->getLog(null, null, -1), 'The offset of a log can\'t be negative, -1 given.']; + yield 'limit' => [static fn (Repository $repository): Log => $repository->getLog(null, null, null, -1), 'The limit of a log can\'t be negative, -1 given.']; + yield 'offset of a revision' => [static fn (Repository $repository): Log => $repository->getRevision('master')->getLog(null, -2), 'The offset of a log can\'t be negative, -2 given.']; + yield 'withOffset()' => [static fn (Repository $repository): Log => $repository->getLog()->withOffset(-5), 'The offset of a log can\'t be negative, -5 given.']; + yield 'withLimit()' => [static fn (Repository $repository): Log => $repository->getLog()->withLimit(-1), 'The limit of a log can\'t be negative, -1 given.']; + } + + public function testALimitOfZeroGivesNoCommit(): void + { + $log = self::createFoobarRepository()->getLog(null, null, null, 0); + + $this->assertSame([], $log->getCommits()); + $this->assertCount(0, $log); + } + /** * The command line of a process has a limit, 2 MB by default on Linux: a * log of thousands of branches would exceed it. From 266ea1fe92cf52e83cef834307f74460df29685d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:46:39 +0200 Subject: [PATCH 07/22] Throw gitlib's exceptions for a missing hook and an unreadable directory Hooks threw an InvalidArgumentException for a hook that doesn't exist in get(), but a RuntimeException in remove(), where the UPGRADE announced a RuntimeException for both. remove() throws an InvalidArgumentException too, which is still the LogicException 1.x threw. getSizeInBytes() let PHP's UnexpectedValueException out for a directory it can't read, or which git deletes while it counts, like the quarantine of a push. It walks the git directory itself: a directory git deleted counts for nothing, and one it can't read throws a RuntimeException of gitlib, like a file. --- CHANGELOG.md | 5 +- UPGRADE-2.0.md | 8 ++- doc/hooks.md | 5 +- doc/repository.md | 3 +- src/Gitonomy/Git/Hooks.php | 7 ++- src/Gitonomy/Git/Repository.php | 68 +++++++++++++++------ tests/Gitonomy/Git/Tests/HooksTest.php | 2 +- tests/Gitonomy/Git/Tests/RepositoryTest.php | 23 +++++++ 8 files changed, 89 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e694425..e3b8b68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -199,8 +199,9 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - Fix `Hooks` failing silently, in a repository without hooks directory, and writing the hooks of a linked worktree where git doesn't run them. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Fix `Repository::getSize()` throwing for a link to a missing file in the git directory, like - a hook not deployed yet: it no longer follows links. A file git deletes meanwhile, like a lock - during a `gc` or a push, no longer makes it throw either. + a hook not deployed yet: it no longer follows links. A file or a directory git deletes + meanwhile, like a lock during a `gc` or the quarantine of a push, no longer makes it throw + either, and one it can't read throws a `RuntimeException` of gitlib, instead of PHP's. - Fix blame lines starting with a TAB character (`Blame\Line::getContent()`): code removing it must stop doing so. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Fix reading a commit whose `encoding` header isn't UTF-8, which threw: its message, author and diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index b4638ca..ad0fbbd 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -412,9 +412,11 @@ Errors `Tree::resolvePath()`. * `Commit::getShortHash()` throws a `ReferenceNotFoundException` for an unknown commit, instead of a `ProcessException`. -* `Hooks` throws a `RuntimeException` when a hook exists already, doesn't - exist, or can't be written, instead of a `LogicException` or a silent - failure, and creates the hooks directory. +* `Hooks` throws a `RuntimeException` when a hook exists already, or can't + be written, instead of a `LogicException` or a silent failure, and + creates the hooks directory. `remove()` of a hook that doesn't exist + throws an `InvalidArgumentException`, like `get()`, still a + `LogicException`. * The `Admin` methods throw a `ProcessException`, still a `RuntimeException`, with the first line of git's error output in its message, or a `DriverException` when git can't start. diff --git a/doc/hooks.md b/doc/hooks.md index 21ca84d..06ccd80 100644 --- a/doc/hooks.md +++ b/doc/hooks.md @@ -95,5 +95,6 @@ To remove a hook from a repository, use the function *remove*: $hooks->remove('pre-receive'); ``` -It throws a `Gitonomy\Git\Exception\RuntimeException` if the hook does not -exist, or if it can't be removed. +It throws a `Gitonomy\Git\Exception\InvalidArgumentException` if the hook +does not exist, like *get*, and a `Gitonomy\Git\Exception\RuntimeException` +if it can't be removed. diff --git a/doc/repository.md b/doc/repository.md index 46508ce..fbcf193 100644 --- a/doc/repository.md +++ b/doc/repository.md @@ -165,7 +165,8 @@ Compute size of a repository `getSizeInBytes()` returns the size of the files of the git directory, working tree excluded, in bytes. It doesn't follow links, and leaves out a -file git deletes meanwhile, like a lock during a `gc` or a push: +file or a directory git deletes meanwhile, like a lock during a `gc` or the +quarantine of a push. One it can't read throws a `RuntimeException`: ```php $size = $repository->getSizeInBytes(); diff --git a/src/Gitonomy/Git/Hooks.php b/src/Gitonomy/Git/Hooks.php index 8f5d4ab..29d145a 100644 --- a/src/Gitonomy/Git/Hooks.php +++ b/src/Gitonomy/Git/Hooks.php @@ -108,13 +108,14 @@ public function set(string $name, string $content): void * * @param string $name Name of the hook * - * @throws InvalidArgumentException The name isn't the one of a file of the hooks directory - * @throws RuntimeException The hook doesn't exist, or can't be removed + * @throws InvalidArgumentException The hook doesn't exist, or the name isn't the one of a file of the hooks directory + * @throws RuntimeException The hook can't be removed */ public function remove(string $name): void { + // Like get(). if (!$this->has($name)) { - throw new RuntimeException(\sprintf('The hook "%s" does not exist.', $name)); + throw new InvalidArgumentException(\sprintf('The hook "%s" does not exist.', $name)); } if (false === @unlink($this->getPath($name))) { diff --git a/src/Gitonomy/Git/Repository.php b/src/Gitonomy/Git/Repository.php index ca47ed0..de2217d 100644 --- a/src/Gitonomy/Git/Repository.php +++ b/src/Gitonomy/Git/Repository.php @@ -527,29 +527,12 @@ public function getLegacyDiff(RevisionList|Revision|string|array|null $revisions /** * @return int The size of the files of the git directory, working tree excluded, in bytes + * + * @throws RuntimeException A file or a directory of the git directory can't be read */ public function getSizeInBytes(): int { - $size = 0; - /** @var \SplFileInfo $file */ - foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->gitDir, \FilesystemIterator::SKIP_DOTS)) as $file) { - // A link isn't followed: it may point to a file not deployed yet, like a hook. - if ($file->isLink()) { - continue; - } - - try { - $size += $file->getSize(); - } catch (\RuntimeException $e) { - // git deleted it since it was listed, during a gc or a push: - // a lock, a temporary pack... It no longer counts. - if (file_exists($file->getPathname())) { - throw $e; - } - } - } - - return $size; + return self::directorySize($this->gitDir); } /** @@ -798,4 +781,49 @@ private function fullHash(string $hash, ObjectType $type): string return $fullHash; } + + /** + * git may delete a file or a directory while it is counted, during a gc + * or a push: a lock, a temporary pack, the quarantine of a push... It no + * longer counts. + * + * @throws RuntimeException A file or a directory of $dir can't be read + */ + private static function directorySize(string $dir): int + { + try { + $files = new \FilesystemIterator($dir, \FilesystemIterator::SKIP_DOTS); + } catch (\UnexpectedValueException $e) { + if (!file_exists($dir)) { + return 0; + } + + throw new RuntimeException(\sprintf('Unable to read the directory "%s".', $dir), 0, $e); + } + + $size = 0; + /** @var \SplFileInfo $file */ + foreach ($files as $file) { + // A link isn't followed: it may point to a file not deployed yet, like a hook. + if ($file->isLink()) { + continue; + } + + if ($file->isDir()) { + $size += self::directorySize($file->getPathname()); + + continue; + } + + try { + $size += $file->getSize(); + } catch (\RuntimeException $e) { + if (file_exists($file->getPathname())) { + throw new RuntimeException(\sprintf('Unable to read the size of "%s".', $file->getPathname()), 0, $e); + } + } + } + + return $size; + } } diff --git a/tests/Gitonomy/Git/Tests/HooksTest.php b/tests/Gitonomy/Git/Tests/HooksTest.php index cbde3a5..8c780fc 100644 --- a/tests/Gitonomy/Git/Tests/HooksTest.php +++ b/tests/Gitonomy/Git/Tests/HooksTest.php @@ -229,7 +229,7 @@ public function testRemove(bool $bare): void public function testRemoveNotExistingThrows(bool $bare): void { $repository = self::createFoobarRepository($bare); - $this->expectException(RuntimeException::class); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The hook "foo" does not exist.'); $repository->getHooks()->remove('foo'); diff --git a/tests/Gitonomy/Git/Tests/RepositoryTest.php b/tests/Gitonomy/Git/Tests/RepositoryTest.php index 2900f1c..85602cc 100644 --- a/tests/Gitonomy/Git/Tests/RepositoryTest.php +++ b/tests/Gitonomy/Git/Tests/RepositoryTest.php @@ -24,6 +24,7 @@ use Gitonomy\Git\Exception\ProcessException; use Gitonomy\Git\Exception\ProcessTimedOutException; use Gitonomy\Git\Exception\ReferenceNotFoundException; +use Gitonomy\Git\Exception\RuntimeException; use Gitonomy\Git\Exception\UnsupportedOperationException; use Gitonomy\Git\Reference\Branch; use Gitonomy\Git\Repository; @@ -714,6 +715,28 @@ public function testGetSizeInBytesFollowsNoLink(): void $this->assertSame($size + 1000, $repository->getSizeInBytes()); } + public function testGetSizeInBytesOfAnUnreadableDirectoryThrows(): void + { + $repository = self::createEmptyRepository(); + $dir = $repository->getGitDir().'/unreadable'; + mkdir($dir); + chmod($dir, 0); + + try { + if (is_readable($dir)) { + $this->markTestSkipped('The user of the tests reads any directory, like root.'); + } + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(\sprintf('Unable to read the directory "%s".', $dir)); + + $repository->getSizeInBytes(); + } finally { + // Or the directory can't be deleted. + chmod($dir, 0o700); + } + } + #[Group('legacy')] #[IgnoreDeprecations] public function testLegacyGetSizeInKilobytes(): void From e568d10ec18e32210d08adff749603c7693859d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:50:47 +0200 Subject: [PATCH 08/22] Read "@" as HEAD with LibGit2Driver git reads "@" as HEAD since 1.8.5, alone or followed by "~", "^" or ":". libgit2 only reads it alone, and Revision::getCommit() always adds "^{commit}": getRevision('@'), getLog('@~2..@'), getDiff('@~1', '@'), checkout('@') or merge('@') threw a ReferenceNotFoundException with LibGit2Driver, but not with CliDriver. LibGit2Driver reads it as HEAD before libgit2 parses the revision, and checkout('@') leaves HEAD attached, like checkout('HEAD') and git since 2.45. --- .../Driver/Ffi/LibGit2RepositoryDriver.php | 23 +++++++--- tests/Gitonomy/Git/Tests/RevisionTest.php | 43 +++++++++++++++++++ tests/Gitonomy/Git/Tests/WorkingCopyTest.php | 11 ++++- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php index 3162bef..a71df02 100644 --- a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php +++ b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php @@ -153,7 +153,7 @@ public function resolveRevision(string $revision): string LeadingDash::assertNone($revision); $object = $this->ffi->new('git_object*'); - $rc = $this->ffi->git_revparse_single(\FFI::addr($object), $this->repo, $revision); + $rc = $this->ffi->git_revparse_single(\FFI::addr($object), $this->repo, self::expandAt($revision)); if (\in_array($rc, [LibGit2::GIT_ENOTFOUND, self::GIT_EAMBIGUOUS, self::GIT_EINVALIDSPEC, self::GIT_EPEEL], true)) { throw new ReferenceNotFoundException(\sprintf('The revision "%s" does not exist.', $revision)); } @@ -630,8 +630,8 @@ public function checkout(string $revision): void // `git checkout ` attaches HEAD when names a // local branch, before a tag of the same name, or is @{-}, the nth - // branch checked out before. It leaves HEAD as it is for HEAD, and - // detaches it for anything else (a tag, a hash, a remote branch, a + // branch checked out before. It leaves HEAD as it is for HEAD, or @, + // and detaches it for anything else (a tag, a hash, a remote branch, a // full reference name...). $name = $this->expandPriorCheckout($revision); $branch = $this->referenceExists('refs/heads/'.$name) ? 'refs/heads/'.$name : null; @@ -642,7 +642,7 @@ public function checkout(string $revision): void if (null !== $branch) { LibGit2::check($this->ffi, $this->ffi->git_repository_set_head($this->repo, $branch), 'Unable to update HEAD'); - } elseif ('HEAD' !== $revision) { + } elseif ('HEAD' !== self::expandAt($revision)) { $oid = $this->hexToOid($hash); LibGit2::check($this->ffi, $this->ffi->git_repository_set_head_detached($this->repo, \FFI::addr($oid)), 'Unable to detach HEAD'); } @@ -1461,6 +1461,19 @@ private function dwimReferences(string $name): array return $references; } + /** + * Like git, reads "@" as HEAD, alone or at the start of $revision before + * "~", "^" or ":": libgit2 only reads it alone. "@{...}" is left as it is. + */ + private static function expandAt(string $revision): string + { + if ('@' === $revision || (str_starts_with($revision, '@') && \in_array($revision[1], ['~', '^', ':'], true))) { + return 'HEAD'.substr($revision, 1); + } + + return $revision; + } + /** * Like git, replaces @{-} at the start of $revision with the nth * branch checked out before, or commit when HEAD was detached. @@ -1743,7 +1756,7 @@ private function isDescendantOf(CData $commit, CData $ancestor): bool private function revparse(string $spec): CData { $out = $this->ffi->new('git_object*'); - LibGit2::check($this->ffi, $this->ffi->git_revparse_single(\FFI::addr($out), $this->repo, $spec), \sprintf('Unable to resolve "%s"', $spec)); + LibGit2::check($this->ffi, $this->ffi->git_revparse_single(\FFI::addr($out), $this->repo, self::expandAt($spec)), \sprintf('Unable to resolve "%s"', $spec)); return $out; } diff --git a/tests/Gitonomy/Git/Tests/RevisionTest.php b/tests/Gitonomy/Git/Tests/RevisionTest.php index 41377da..b0b267d 100644 --- a/tests/Gitonomy/Git/Tests/RevisionTest.php +++ b/tests/Gitonomy/Git/Tests/RevisionTest.php @@ -12,6 +12,8 @@ namespace Gitonomy\Git\Tests; +use Gitonomy\Git\Commit; +use Gitonomy\Git\Driver\Cli\CliDriver; use Gitonomy\Git\Exception\ReferenceNotFoundException; use PHPUnit\Framework\Attributes\DataProvider; @@ -38,6 +40,47 @@ public function testGetFailingReference(bool $bare): void $repository->getRevision('non-existent-commit')->getCommit(); } + /** + * "@" is HEAD for git since 1.8.5, and for libgit2 alone only. + */ + #[DataProvider('provideAtRevisions')] + public function testAtIsHead(string $revision, string $expected): void + { + if (self::getDriver() instanceof CliDriver && version_compare(self::gitVersion(), '1.8.5', '<')) { + $this->markTestSkipped('git reads "@" as HEAD since 1.8.5.'); + } + + $repository = self::createFoobarRepository(); + $head = $repository->getHeadCommit(); + + $this->assertSame('HEAD' === $expected ? $head->getHash() : $head->getParentHashes()[0], $repository->getRevision($revision)->getCommit()->getHash()); + } + + /** + * @return iterable + */ + public static function provideAtRevisions(): iterable + { + yield 'alone' => ['@', 'HEAD']; + yield 'ancestor' => ['@~1', 'parent']; + yield 'parent' => ['@^', 'parent']; + yield 'peeled' => ['@^{commit}', 'HEAD']; + } + + public function testALogOfARangeFromAt(): void + { + if (self::getDriver() instanceof CliDriver && version_compare(self::gitVersion(), '1.8.5', '<')) { + $this->markTestSkipped('git reads "@" as HEAD since 1.8.5.'); + } + + $repository = self::createFoobarRepository(); + + $hashes = static fn (string $range): array => array_map(static fn (Commit $commit): string => $commit->getHash(), $repository->getLog($range)->getCommits()); + + $this->assertNotSame([], $hashes('HEAD~2..HEAD')); + $this->assertSame($hashes('HEAD~2..HEAD'), $hashes('@~2..@')); + } + #[DataProvider('provideBare')] public function testGetLog(bool $bare): void { diff --git a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php index 2d39193..0b756c0 100644 --- a/tests/Gitonomy/Git/Tests/WorkingCopyTest.php +++ b/tests/Gitonomy/Git/Tests/WorkingCopyTest.php @@ -538,11 +538,18 @@ public function testCheckoutPrefersTheBranchToATagOfTheSameName(): void $this->assertSame([], $repository->getWorkingCopy()->getDiffStaged()->getFiles()); } - public function testCheckoutOfHeadLeavesItAttached(): void + #[TestWith(['HEAD'])] + #[TestWith(['@'])] + public function testCheckoutOfHeadLeavesItAttached(string $head): void { + // git 2.43 detaches HEAD for "@", 2.53 doesn't: git 2.45 learned that "@" is HEAD in checkout. + if ('@' === $head && self::getDriver() instanceof CliDriver && version_compare(self::gitVersion(), '2.45.0', '<')) { + $this->markTestSkipped('git checks "@" out like HEAD since 2.45.'); + } + $repository = self::createFoobarRepository(false); - $repository->getWorkingCopy()->checkout('HEAD'); + $repository->getWorkingCopy()->checkout($head); $this->assertSame('refs/heads/master', self::headBranch($repository)->getFullname()); } From 0401527062baca4d9bd3a18d9aa0fa55626e554c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:51:35 +0200 Subject: [PATCH 09/22] Walk from HEAD too in a log of every reference with LibGit2Driver A log without revisions is the one of every reference and HEAD, like "git log --all". LibGit2Driver only walked from the references: with a detached HEAD on a commit no reference has, like after the checkout of a hash, the log lacked commits, and count() was wrong, without error. --- .../Driver/Ffi/LibGit2RepositoryDriver.php | 21 +++++++++++++++++-- tests/Gitonomy/Git/Tests/LogTest.php | 18 ++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php index a71df02..c72b6d3 100644 --- a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php +++ b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php @@ -759,7 +759,7 @@ public function merge(string $revision, FastForward $fastForward): void * yields the commits that change them, and simplifies the history like * git does. * - * @param list|null $includes null for every reference + * @param list|null $includes null for every reference and HEAD * @param list $excludes * @param string[] $paths * @@ -827,11 +827,28 @@ private function revwalk(array $tips, array $bottoms, int $sorting = self::GIT_S } /** - * @return list The commits the references point to; like `git log --all`, ignores the references to other objects and the dangling ones + * @return list The commits HEAD and the references point to; like `git log --all`, ignores the references to other objects and the dangling ones */ private function referencedCommits(): array { $hashes = []; + + // A detached HEAD may name a commit no reference has. + $head = $this->ffi->new('git_reference*'); + $rc = $this->ffi->git_repository_head(\FFI::addr($head), $this->repo); + if (self::GIT_EUNBORNBRANCH !== $rc && LibGit2::GIT_ENOTFOUND !== $rc) { + LibGit2::check($this->ffi, $rc, 'Unable to resolve HEAD'); + + try { + $commit = $this->peeledCommitHex($head); + if (null !== $commit) { + $hashes[] = $commit; + } + } finally { + $this->ffi->git_reference_free($head); + } + } + foreach ($this->iterateReferences() as $ref) { $commit = $this->peeledCommitHex($ref); if (null !== $commit) { diff --git a/tests/Gitonomy/Git/Tests/LogTest.php b/tests/Gitonomy/Git/Tests/LogTest.php index d4d1743..2c7e684 100644 --- a/tests/Gitonomy/Git/Tests/LogTest.php +++ b/tests/Gitonomy/Git/Tests/LogTest.php @@ -380,6 +380,24 @@ public function testLogOfExcludedAndSymmetricRevisions(): void $this->assertSame([$main, $base], self::hashes($repository->getLog('main'))); } + /** + * Like "git log --all", which walks from HEAD too: a detached HEAD may + * name a commit no reference has, like after the checkout of a hash. + */ + public function testALogOfEveryReferenceIncludesADetachedHead(): void + { + $repository = self::createFoobarRepository(); + $master = $repository->getHeadCommit(); + $hash = self::writeObject($repository, 'commit', "tree {$master->getTreeHash()}\nparent {$master->getHash()}\nauthor A 1700000000 +0000\ncommitter A 1700000000 +0000\n\nDetached\n"); + self::git($repository, 'update-ref', ['--no-deref', 'HEAD', $hash]); + $repository = new Repository($repository->getPath(), driver: self::getDriver()); + + $log = $repository->getLog(); + + $this->assertContains($hash, self::hashes($log)); + $this->assertCount(\count($log->getCommits()), $log); + } + /** * @param \Closure(Repository): Log $log */ From de9cd57def2d2c323f31cc8db1ec7c98b6db8673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:52:41 +0200 Subject: [PATCH 10/22] Don't throw for a branch another process created Commit::getIncludingBranches() and Repository::getHead() read the branches from git, then looked them up in the references gitlib read before: a branch another process created or checked out since then made them throw a ReferenceNotFoundException, which a long-running worker meets. getIncludingBranches() also threw for every commit of a regular refs/remotes/origin/HEAD, which a mirror leaves, and which the references leave out. Both read the references again for a branch they don't know. getIncludingBranches() then leaves out a branch still unknown: the default branch of a remote, or one deleted meanwhile. --- CHANGELOG.md | 4 +++ src/Gitonomy/Git/Commit.php | 13 ++++++++- src/Gitonomy/Git/Repository.php | 8 +++++- tests/Gitonomy/Git/Tests/CommitTest.php | 31 +++++++++++++++++++++ tests/Gitonomy/Git/Tests/RepositoryTest.php | 12 ++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3b8b68..2d0b4ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,10 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). ### Bug fixes +- Fix `Commit::getIncludingBranches()` and `Repository::getHead()` throwing for a branch another + process created or checked out since gitlib read the references, and + `getIncludingBranches()` throwing for a regular `refs/remotes/origin/HEAD`, as a mirror leaves + it. - Fix the configuration of the user changing what `CliDriver` reads: `color.ui=always`, `log.showSignature`, `diff.noprefix`, `diff.external`, `diff.submodule`, textconv filters, `column.ui`... made reads throw or return wrong data, and `diff.algorithm`, `diff.context`, diff --git a/src/Gitonomy/Git/Commit.php b/src/Gitonomy/Git/Commit.php index 150f935..e57b42f 100644 --- a/src/Gitonomy/Git/Commit.php +++ b/src/Gitonomy/Git/Commit.php @@ -198,10 +198,21 @@ public function getIncludingBranches(bool $local = true, bool $remote = true): a throw new InvalidArgumentException('You should a least set one argument to true.'); } + $fullnames = $this->repository->getRepositoryDriver()->getIncludingBranches($this->revision); $references = $this->repository->getReferences(); + // A branch another process created since gitlib read the references. + if ([] !== array_filter($fullnames, static fn (string $fullname): bool => !$references->has($fullname))) { + $references->invalidate(); + } $branches = []; - foreach ($this->repository->getRepositoryDriver()->getIncludingBranches($this->revision) as $fullname) { + foreach ($fullnames as $fullname) { + // Unknown still: the default branch of a remote, which the references leave out, even + // when a mirror leaves it as a regular reference, or a branch deleted meanwhile. + if (!$references->has($fullname)) { + continue; + } + $branch = $references->get($fullname); if ($branch instanceof Branch && ($branch->isLocal() ? $local : $remote)) { $branches[] = $branch; diff --git a/src/Gitonomy/Git/Repository.php b/src/Gitonomy/Git/Repository.php index de2217d..6b9ca6b 100644 --- a/src/Gitonomy/Git/Repository.php +++ b/src/Gitonomy/Git/Repository.php @@ -261,7 +261,13 @@ public function getHead(): Branch|Commit return $this->getCommit($this->repositoryDriver->resolveRevision('HEAD')); } - $head = $this->getReferences()->get($reference); + $references = $this->getReferences(); + // A branch another process checked out since gitlib read the references. + if (!$references->has($reference)) { + $references->invalidate(); + } + + $head = $references->get($reference); // git lets HEAD point to any reference under "refs/", a tag included. if (!$head instanceof Branch) { throw new ReferenceNotFoundException(\sprintf('HEAD points to "%s", which is not a branch.', $reference)); diff --git a/tests/Gitonomy/Git/Tests/CommitTest.php b/tests/Gitonomy/Git/Tests/CommitTest.php index 48eb8db..5d414a8 100644 --- a/tests/Gitonomy/Git/Tests/CommitTest.php +++ b/tests/Gitonomy/Git/Tests/CommitTest.php @@ -18,10 +18,12 @@ use Gitonomy\Git\Exception\InvalidArgumentException; use Gitonomy\Git\Exception\ReferenceNotFoundException; use Gitonomy\Git\Exception\RuntimeException; +use Gitonomy\Git\Reference\Branch; use Gitonomy\Git\Repository; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\IgnoreDeprecations; +use Symfony\Component\Process\Process; class CommitTest extends AbstractTestCase { @@ -383,6 +385,35 @@ public function testGetIncludingBranchesNamedLikeRemoteOnes(): void $this->assertContains($repository->getReferences()->getBranch('remotes/origin/fake'), $branches); } + /** + * gitlib doesn't see the writes of another process, but doesn't fail on them. + */ + public function testGetIncludingBranchesCreatedByAnotherProcess(): void + { + $repository = self::createFoobarRepository(); + $repository->getReferences()->getAll(); + new Process([self::gitCommand(), 'branch', 'elsewhere', self::INITIAL_COMMIT], $repository->getPath(), self::gitEnvs())->mustRun(); + + $fullnames = array_map(static fn (Branch $branch): string => $branch->getFullname(), $repository->getCommit(self::INITIAL_COMMIT)->getIncludingBranches()); + + $this->assertContains('refs/heads/elsewhere', $fullnames); + } + + /** + * A mirror leaves the default branch of a remote as a regular reference, + * which the branches leave out, like a symbolic one. + */ + public function testGetIncludingBranchesLeavesOutTheDefaultBranchOfARemote(): void + { + $repository = self::createFoobarRepository(); + self::git($repository, 'update-ref', ['--no-deref', 'refs/remotes/origin/HEAD', self::INITIAL_COMMIT]); + + $fullnames = array_map(static fn (Branch $branch): string => $branch->getFullname(), $repository->getCommit(self::INITIAL_COMMIT)->getIncludingBranches()); + + $this->assertNotContains('refs/remotes/origin/HEAD', $fullnames); + $this->assertContains('refs/heads/master', $fullnames); + } + #[DataProvider('provideBare')] public function testGetLastModification(bool $bare): void { diff --git a/tests/Gitonomy/Git/Tests/RepositoryTest.php b/tests/Gitonomy/Git/Tests/RepositoryTest.php index 85602cc..1c58b0c 100644 --- a/tests/Gitonomy/Git/Tests/RepositoryTest.php +++ b/tests/Gitonomy/Git/Tests/RepositoryTest.php @@ -249,6 +249,18 @@ public function testTheHeadOfARepositoryWithoutCommitsThrows(): void $repository->getHead(); } + /** + * gitlib doesn't see the writes of another process, but doesn't fail on them. + */ + public function testTheHeadAnotherProcessChecksOut(): void + { + $repository = self::createFoobarRepository(false); + $repository->getReferences()->getAll(); + new Process([self::gitCommand(), 'checkout', '-q', '-b', 'elsewhere'], $repository->getWorkingDir(), self::gitEnvs())->mustRun(); + + $this->assertSame('refs/heads/elsewhere', self::headBranch($repository)->getFullname()); + } + public function testHeadPointingToATagIsNoBranch(): void { $repository = self::createFoobarRepository(); From 63c6a6a65a1742c0b593549229663cae48b2dd5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:53:31 +0200 Subject: [PATCH 11/22] Read the blame of a commit under two paths After a merge whose sides kept a file under two names, git blames a commit under both, and "git blame -p" repeats its "previous" and "filename" lines on each of its groups of lines: the parser only read them on the first one, and threw ('Expected "\t", got "filename b"'). LibGit2Driver read it. --- CHANGELOG.md | 2 + .../Git/Driver/Cli/Parser/BlameParser.php | 12 ++++++ tests/Gitonomy/Git/Tests/BlameTest.php | 43 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d0b4ee..d498931 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,8 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). process created or checked out since gitlib read the references, and `getIncludingBranches()` throwing for a regular `refs/remotes/origin/HEAD`, as a mirror leaves it. +- Fix the blame of a file throwing with `CliDriver` after a merge whose sides kept it under two + names: git repeats the path of a commit it blames under both, which gitlib didn't read. - Fix the configuration of the user changing what `CliDriver` reads: `color.ui=always`, `log.showSignature`, `diff.noprefix`, `diff.external`, `diff.submodule`, textconv filters, `column.ui`... made reads throw or return wrong data, and `diff.algorithm`, `diff.context`, diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php index 844805a..6d739cb 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php @@ -64,6 +64,18 @@ protected function doParse(): void $this->consumeTo("\n"); // filename $this->consumeNewLine(); $this->seenHashes[$hash] = true; + } else { + // git repeats them on each group of a commit it blames under + // several paths, after a merge whose sides renamed the file. + if ($this->expects('previous ')) { + $this->consumeTo("\n"); + $this->consumeNewLine(); + } + + if ($this->expects('filename ')) { + $this->consumeTo("\n"); + $this->consumeNewLine(); + } } $this->consume("\t"); $content = $this->consumeTo("\n"); diff --git a/tests/Gitonomy/Git/Tests/BlameTest.php b/tests/Gitonomy/Git/Tests/BlameTest.php index 1a367a3..f05fe4e 100644 --- a/tests/Gitonomy/Git/Tests/BlameTest.php +++ b/tests/Gitonomy/Git/Tests/BlameTest.php @@ -15,6 +15,8 @@ use Gitonomy\Git\Blame\Line; use Gitonomy\Git\Exception\DriverException; use Gitonomy\Git\Exception\InvalidArgumentException; +use Gitonomy\Git\Exception\ProcessException; +use Gitonomy\Git\Repository; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\IgnoreDeprecations; @@ -96,6 +98,47 @@ public function testBlameOfARangeAfterTheEndThrows(): void $repository->getBlame(self::LONGFILE_COMMIT, 'README.md', 100)->getLines(); } + /** + * After a merge whose sides kept a file under two names, git blames a + * commit under both: "git blame -p" repeats its "filename" line on each + * of its groups of lines. + */ + public function testBlameOfACommitUnderTwoPaths(): void + { + $repository = Repository::init(self::createTempDir(), self::getDriver()); + $dir = $repository->getWorkingDir(); + self::git($repository, 'config', ['user.name', 'Unit Test']); + self::git($repository, 'config', ['user.email', 'unit_test@unit-test.com']); + file_put_contents($dir.'/a', "L1\nL2\nL3\n"); + file_put_contents($dir.'/b', "L1\nL2\nL3\n"); + self::git($repository, 'add', ['a', 'b']); + self::git($repository, 'commit', ['-q', '-m', 'Both']); + $both = trim(self::git($repository, 'rev-parse', ['HEAD'])); + self::git($repository, 'checkout', ['-q', '-b', 'side']); + self::git($repository, 'rm', ['-q', 'a']); + self::git($repository, 'commit', ['-q', '-m', 'Without a']); + self::git($repository, 'checkout', ['-q', 'master']); + file_put_contents($dir.'/a', "L1\nL2\nNEW\n"); + self::git($repository, 'rm', ['-q', 'b']); + self::git($repository, 'commit', ['-q', '-a', '-m', 'Without b']); + + try { + // A conflict: a is deleted on one side, and changed on the other. + self::git($repository, 'merge', ['-q', '--no-edit', 'side']); + } catch (ProcessException) { + } + file_put_contents($dir.'/a', "L1\nL2x\nL3\n"); + self::git($repository, 'add', ['a']); + self::git($repository, 'commit', ['-q', '-m', 'Merge']); + $repository = new Repository($repository->getPath(), driver: self::getDriver()); + + $lines = array_map(static fn (Line $line): array => [$line->getCommit()->getHash(), $line->getContent()], $repository->getBlame('HEAD', 'a')->getLines()); + + $this->assertSame([$both, 'L1'], $lines[1]); + $this->assertSame('L2x', $lines[2][1]); + $this->assertSame('L3', $lines[3][1]); + } + public function testBlameOfAPathNormalizesIt(): void { $repository = self::createFoobarRepository(); From 03142763c6e614ed2bf4fd4ecc591e2249658c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:55:39 +0200 Subject: [PATCH 12/22] Read malformed signatures like libgit2 Old commits, imported from other tools, have authors and committers git wouldn't write anymore: "Name", no date, an offset of 5 digits, no name nor email. git and libgit2 read them, but CliDriver required "Name timestamp +hhmm" exactly, and one such commit made the whole log throw. ParserBase::parseSignature() reads them like libgit2, which LibGit2Driver reads them with: the email between the last "<" and ">", the name before it, and the date and offset optional, the epoch in UTC without them. Tags read their tagger with it too. --- CHANGELOG.md | 3 ++ .../Git/Driver/Cli/Parser/LogParser.php | 16 +------- src/Gitonomy/Git/Parser/ParserBase.php | 41 +++++++++++++++---- tests/Gitonomy/Git/Tests/CommitTest.php | 30 ++++++++++++++ 4 files changed, 67 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d498931..321cca0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,9 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). it. - Fix the blame of a file throwing with `CliDriver` after a merge whose sides kept it under two names: git repeats the path of a commit it blames under both, which gitlib didn't read. +- Fix a log throwing with `CliDriver` for a commit whose author or committer git wouldn't + write anymore, like `Name` or no date, as old imports have: it is read like libgit2 + reads it, the epoch in UTC without date. - Fix the configuration of the user changing what `CliDriver` reads: `color.ui=always`, `log.showSignature`, `diff.noprefix`, `diff.external`, `diff.submodule`, textconv filters, `column.ui`... made reads throw or return wrong data, and `diff.algorithm`, `diff.context`, diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php index 1fe8dec..4b91c5d 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php @@ -52,8 +52,8 @@ protected function doParse(): void } match ($name) { - 'author' => $author = self::parseNameEmailDate($value), - 'committer' => $committer = self::parseNameEmailDate($value), + 'author' => $author = self::parseSignature($value), + 'committer' => $committer = self::parseSignature($value), 'parent' => $parentHashes[] = $value, 'tree' => $treeHash = $value, // encoding, gpgsig, mergetag, and the headers of other tools. @@ -78,16 +78,4 @@ protected function doParse(): void $this->commits[] = new CommitData($hash, $treeHash, $parentHashes, ...$author, ...$committer, message: $message); } } - - /** - * @return array{string, string, \DateTimeImmutable} - */ - private static function parseNameEmailDate(string $value): array - { - if (!preg_match('/^(.*) <(.*)> (\d+) ([+-]\d{4})$/', $value, $vars)) { - throw new RuntimeException(\sprintf('Unable to parse the name, email and date of "%s".', $value)); - } - - return [$vars[1], $vars[2], self::signatureDate($vars[3], $vars[4])]; - } } diff --git a/src/Gitonomy/Git/Parser/ParserBase.php b/src/Gitonomy/Git/Parser/ParserBase.php index 5d1c6eb..5660901 100644 --- a/src/Gitonomy/Git/Parser/ParserBase.php +++ b/src/Gitonomy/Git/Parser/ParserBase.php @@ -131,21 +131,44 @@ protected function consumeNewLine(): string /** * @return array{string, string, \DateTimeImmutable} The name, email and date of an author, committer or tagger + * + * @throws RuntimeException The line has no email between "<" and ">" */ protected function consumeNameEmailDate(): array { - if (!preg_match('/(([^\n]*) <([^\n]*)> (\d+) ([+-]\d{4}))/A', $this->content, $vars, 0, $this->cursor)) { - throw new RuntimeException('Unable to parse name, email and date.'); + return self::parseSignature($this->consumeTo("\n")); + } + + /** + * Reads "Name 1700000000 +0200" like libgit2, which LibGit2Driver + * reads it with, and git: old commits, imported from other tools, have + * signatures git wouldn't write anymore, like "Name", or no date. + * + * @return array{string, string, \DateTimeImmutable} The name, email and date, the epoch in UTC without one + * + * @throws RuntimeException $signature has no email between "<" and ">" + */ + protected static function parseSignature(string $signature): array + { + $emailStart = strrpos($signature, '<'); + $emailEnd = strrpos($signature, '>'); + if (false === $emailStart || false === $emailEnd || $emailEnd < $emailStart) { + throw new RuntimeException(\sprintf('Unable to parse the name, email and date of "%s".', $signature)); } - $this->cursor += \strlen($vars[1]); + // The date, then the offset, are optional. + preg_match('/^\s*(\d+)(?:\s+([+-]\d+))?/', substr($signature, $emailEnd + 1), $date); - return [$vars[2], $vars[3], self::signatureDate($vars[4], $vars[5])]; + return [ + trim(substr($signature, 0, $emailStart)), + trim(substr($signature, $emailStart + 1, $emailEnd - $emailStart - 1)), + self::signatureDate($date[1] ?? '0', $date[2] ?? '+0000'), + ]; } /** * @param string $timestamp Seconds since the epoch, as git writes them - * @param string $offset The UTC offset of the author, committer or tagger, as git writes it: "+0200", "-0330"... + * @param string $offset The UTC offset of the author, committer or tagger, as git writes it: "+0200", "-0330"..., a sign then hours and minutes * * @return \DateTimeImmutable The date at $offset, like git log shows it * @@ -156,12 +179,12 @@ protected static function signatureDate(string $timestamp, string $offset): \Dat // Like libgit2, which LibGit2Driver reads dates with: an offset of // more than 14 hours or 59 minutes, which no timezone has, in a // corrupt object, gives UTC. - if ((int) substr($offset, 1, 2) > 14 || (int) substr($offset, 3) > 59) { - $offset = '+0000'; - } + $hoursAndMinutes = (int) substr($offset, 1); + [$hours, $minutes] = [intdiv($hoursAndMinutes, 100), $hoursAndMinutes % 100]; + $timezone = $hours > 14 || $minutes > 59 ? '+0000' : \sprintf('%s%02d%02d', $offset[0], $hours, $minutes); try { - return new \DateTimeImmutable('@'.$timestamp)->setTimezone(new \DateTimeZone($offset)); + return new \DateTimeImmutable('@'.$timestamp)->setTimezone(new \DateTimeZone($timezone)); } catch (\DateMalformedStringException $e) { throw new RuntimeException(\sprintf('The date "%s" is out of range.', $timestamp), 0, $e); } diff --git a/tests/Gitonomy/Git/Tests/CommitTest.php b/tests/Gitonomy/Git/Tests/CommitTest.php index 5d414a8..5e1800c 100644 --- a/tests/Gitonomy/Git/Tests/CommitTest.php +++ b/tests/Gitonomy/Git/Tests/CommitTest.php @@ -74,6 +74,36 @@ public function testTheDatesAreAtTheOffsetsOfTheAuthorAndOfTheCommitter(): void $this->assertSame(1700000001, $commit->getCommitterDate()->getTimestamp()); } + /** + * Old commits, imported from other tools, have signatures git wouldn't + * write anymore, which git and libgit2 read anyway: one of them made the + * whole log throw. + */ + #[DataProvider('provideMalformedSignatures')] + public function testAMalformedSignatureIsReadLikeGit(string $signature, string $name, string $email, string $date): void + { + $repository = self::createEmptyRepository(); + $tree = self::writeObject($repository, 'tree', ''); + $hash = self::writeObject($repository, 'commit', "tree {$tree}\nauthor {$signature}\ncommitter {$signature}\n\nMalformed\n"); + + $commits = $repository->getLog($hash)->getCommits(); + + $this->assertCount(1, $commits); + $this->assertSame([$name, $email, $date], [$commits[0]->getAuthorName(), $commits[0]->getAuthorEmail(), $commits[0]->getAuthorDate()->format(\DATE_ATOM)]); + $this->assertSame([$name, $email, $date], [$commits[0]->getCommitterName(), $commits[0]->getCommitterEmail(), $commits[0]->getCommitterDate()->format(\DATE_ATOM)]); + } + + /** + * @return iterable + */ + public static function provideMalformedSignatures(): iterable + { + yield 'no space before the email' => ['A 1700000000 +0200', 'A', 'a@example.com', '2023-11-15T00:13:20+02:00']; + yield 'no date' => ['A ', 'A', 'a@example.com', '1970-01-01T00:00:00+00:00']; + yield 'an offset of 5 digits' => ['A 1700000000 +05030', 'A', 'a@example.com', '2023-11-14T22:13:20+00:00']; + yield 'no name nor email' => ['<> 0 +0000', '', '', '1970-01-01T00:00:00+00:00']; + } + /** * Like git log, which shows "-0000" as "+0000". An offset of more than 14 * hours or 59 minutes, which no timezone has, gives UTC, like libgit2. From 2627fedf96f516bdab0176e451677f063a2f8469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:56:25 +0200 Subject: [PATCH 13/22] Name the kinds of push like the ones of a file PushReference::isCreate() and isDelete() named what Diff\File names isCreation() and isDeletion(), whose isDelete() 2.0 deprecates for this: one concept, two names. PushReference gets isCreation() and isDeletion(), and isCreate() and isDelete() are deprecated. The argument of its constructor holding the full name of the reference is named $fullname, like getFullname(). --- CHANGELOG.md | 5 +- UPGRADE-2.0.md | 7 ++- src/Gitonomy/Git/PushReference.php | 54 ++++++++++++++----- .../Gitonomy/Git/Tests/PushReferenceTest.php | 24 ++++++++- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 321cca0..00aadb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,7 +76,8 @@ deprecated until 3.0. `Log::withOffset()` and `withLimit()`, `Blame::getHunks()` and the line numbers of `Blame\Line`, `TreeEntry` and `Submodule`, `Diff\DiffLine` and `Diff\DiffLineType`, `File::isModeChange()`, `FileChange::getAdditions()` and `getDeletions()`, - `PushReference::getFullname()` and `getRange()`, and the SSH and X.509 signatures of tags. + `PushReference::getFullname()`, `getRange()`, `isCreation()` and `isDeletion()`, and the SSH + and X.509 signatures of tags. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Add `WorkingCopy::merge()` to support `git merge`, fast-forwarding or not, with a `FastForward` enum. ([#248](https://github.com/gitonomy/gitlib/pull/248), fixes #118) @@ -116,7 +117,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - `CommitReference`, `Tree::getCommitReferenceEntries()`, reading tree entries as arrays. - The branch to create of `WorkingCopy::checkout()`. - `Commit::getShortMessage()` and `getFixedShortHash()`, `Repository::shell()`, - `PushReference::getReference()` and `getRevision()`. + `PushReference::getReference()`, `getRevision()`, `isCreate()` and `isDelete()`. - `Repository::getSize()`, in kilobytes: use `getSizeInBytes()`. - `Exception\UnexpectedValueException`, which gitlib no longer throws. diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index ad0fbbd..a1b18ff 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -547,8 +547,11 @@ Data returned * `Reference\Tag::isSigned()` returns `false` for a lightweight tag. * `PushReference::isForce()` returns `true` for histories without a common ancestor, where the constructor threw. `PushReference` runs git when - `isForce()` needs it, not when constructed. `getReference()` and - `getRevision()` are deprecated: use `getFullname()` and `getRange()`. + `isForce()` needs it, not when constructed. `getReference()`, + `getRevision()`, `isCreate()` and `isDelete()` are deprecated: use + `getFullname()`, `getRange()`, `isCreation()` and `isDeletion()`, named + like the ones of `Diff\File`. The `$reference` argument of its + constructor is named `$fullname`. * `Blob::getContent()` only reads blobs, where `git cat-file -p` also printed trees and commits. * `WorkingCopy::getUntrackedFiles()` and `RevisionList::getAll()` return diff --git a/src/Gitonomy/Git/PushReference.php b/src/Gitonomy/Git/PushReference.php index 3cc87c9..4cf81c1 100644 --- a/src/Gitonomy/Git/PushReference.php +++ b/src/Gitonomy/Git/PushReference.php @@ -27,13 +27,13 @@ final class PushReference private ?bool $isForce = null; /** - * @param string $reference The full name of the pushed reference - * @param string $before Its commit before the push, ZERO for a creation - * @param string $after Its commit after the push, ZERO for a deletion + * @param string $fullname The full name of the pushed reference + * @param string $before Its commit before the push, ZERO for a creation + * @param string $after Its commit after the push, ZERO for a deletion */ public function __construct( private readonly Repository $repository, - private readonly string $reference, + private readonly string $fullname, private readonly string $before, private readonly string $after, ) { @@ -49,7 +49,7 @@ public function getRepository(): Repository */ public function getFullname(): string { - return $this->reference; + return $this->fullname; } /** @@ -59,7 +59,7 @@ public function getReference(): string { trigger_deprecation('gitonomy/gitlib', '2.0', 'The "%s()" method is deprecated, use "getFullname()" instead.', __METHOD__); - return $this->reference; + return $this->fullname; } public function getBefore(): string @@ -97,27 +97,53 @@ public function getRevision(): string */ public function getRange(): string { - if ($this->isDelete()) { + if ($this->isDeletion()) { throw new LogicException('A deletion pushes no commit.'); } - if ($this->isCreate()) { + if ($this->isCreation()) { return $this->getAfter(); } return $this->getBefore().'..'.$this->getAfter(); } - public function isCreate(): bool + /** + * @return bool Whether the push creates the reference, like File::isCreation() for a file + */ + public function isCreation(): bool { return $this->isZero($this->before); } - public function isDelete(): bool + /** + * @deprecated since gitonomy/gitlib 2.0, use isCreation() instead + */ + public function isCreate(): bool + { + trigger_deprecation('gitonomy/gitlib', '2.0', 'The "%s()" method is deprecated, use "isCreation()" instead.', __METHOD__); + + return $this->isCreation(); + } + + /** + * @return bool Whether the push deletes the reference, like File::isDeletion() for a file + */ + public function isDeletion(): bool { return $this->isZero($this->after); } + /** + * @deprecated since gitonomy/gitlib 2.0, use isDeletion() instead + */ + public function isDelete(): bool + { + trigger_deprecation('gitonomy/gitlib', '2.0', 'The "%s()" method is deprecated, use "isDeletion()" instead.', __METHOD__); + + return $this->isDeletion(); + } + public function isForce(): bool { return $this->isForce ??= $this->getForce(); @@ -125,17 +151,17 @@ public function isForce(): bool public function isFastForward(): bool { - return !$this->isDelete() && !$this->isCreate() && !$this->isForce(); + return !$this->isDeletion() && !$this->isCreation() && !$this->isForce(); } - private function isZero(string $reference): bool + private function isZero(string $hash): bool { - return self::ZERO === $reference; + return self::ZERO === $hash; } private function getForce(): bool { - if ($this->isDelete() || $this->isCreate()) { + if ($this->isDeletion() || $this->isCreation()) { return false; } diff --git a/tests/Gitonomy/Git/Tests/PushReferenceTest.php b/tests/Gitonomy/Git/Tests/PushReferenceTest.php index e3a0cf3..0f4c623 100644 --- a/tests/Gitonomy/Git/Tests/PushReferenceTest.php +++ b/tests/Gitonomy/Git/Tests/PushReferenceTest.php @@ -15,6 +15,8 @@ use Gitonomy\Git\Exception\DriverException; use Gitonomy\Git\PushReference; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\IgnoreDeprecations; class PushReferenceTest extends AbstractTestCase { @@ -27,8 +29,8 @@ class PushReferenceTest extends AbstractTestCase public function testIsers(string $reference, string $before, string $after, int $mask): void { $reference = new PushReference(self::createFoobarRepository(), $reference, $before, $after); - $this->assertSame(0 !== ($mask & self::CREATE), $reference->isCreate(), 'Create value is correct.'); - $this->assertSame(0 !== ($mask & self::DELETE), $reference->isDelete(), 'Delete value is correct.'); + $this->assertSame(0 !== ($mask & self::CREATE), $reference->isCreation(), 'Create value is correct.'); + $this->assertSame(0 !== ($mask & self::DELETE), $reference->isDeletion(), 'Delete value is correct.'); $this->assertSame(0 !== ($mask & self::FORCE), $reference->isForce(), 'Force value is correct.'); $this->assertSame(0 !== ($mask & self::FAST_FORWARD), $reference->isFastForward(), 'FastForward value is correct.'); } @@ -47,6 +49,24 @@ public static function provideIsers(): array ]; } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyIsCreate(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\PushReference::isCreate()" method is deprecated, use "isCreation()" instead.'); + + $this->assertTrue(new PushReference(self::createFoobarRepository(), 'foo', PushReference::ZERO, self::LONGFILE_COMMIT)->isCreate()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyIsDelete(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\PushReference::isDelete()" method is deprecated, use "isDeletion()" instead.'); + + $this->assertTrue(new PushReference(self::createFoobarRepository(), 'foo', self::LONGFILE_COMMIT, PushReference::ZERO)->isDelete()); + } + public function testUnknownBeforeIsAnErrorNotAForcePush(): void { $reference = new PushReference(self::createFoobarRepository(), 'foo', str_repeat('1', 40), self::LONGFILE_COMMIT); From 3acf83a09f0ed669b6784735e8f40c0763c54f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:56:44 +0200 Subject: [PATCH 14/22] Require the versions of symfony/process without advisories gitlib required symfony/process ^6.4 || ^7.4 || ^8.1, but the versions before 6.4.33 and 7.4.5 escape the arguments wrongly on Windows (CVE-2024-51736, CVE-2026-24739). Recent Composer versions refuse to install them, so the job of the lowest dependencies tested 6.4.33, not the 6.4.0 gitlib declared. It requires ^6.4.33 || ^7.4.5 || ^8.1. --- CHANGELOG.md | 2 ++ UPGRADE-2.0.md | 4 +++- composer.json | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00aadb6..c19bb36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,8 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). option: `getRevision('--output=/path')`, `WorkingCopy::checkout()` or `merge()` could write a file, and `Repository::isValidRepository('--upload-pack=...')` run a command. A path may start with a dash: `init()` and the clones end the options of git with `--`. +- Require `symfony/process` 6.4.33, 7.4.5 or 8.1 and later: the older versions escape the + arguments wrongly on Windows (CVE-2024-51736, CVE-2026-24739). - Refuse an `ext::` URL, like `ext::sh -c ...`, whose command git runs, by default before git 2.12, and an `fd::` URL, which makes git talk through a file descriptor it inherits from PHP, like the socket of a database, and wait on it. diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index a1b18ff..d7d4acb 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -19,7 +19,9 @@ Requirements * git 1.8.4 or later is required by `CliDriver`, the default driver: before, git simplifies the log of a range filtered by path differently, and it can't count commits (`Log::count()`) before 1.7.2. -* `symfony/process` must be `^6.4`, `^7.4` or `^8.1`. +* `symfony/process` must be `^6.4.33`, `^7.4.5` or `^8.1`: the older + versions escape the arguments wrongly on Windows (CVE-2024-51736, + CVE-2026-24739). * `symfony/string` `^7.4.6` or `^8.0.6`, `symfony/deprecation-contracts` `^2.5` or `^3`, and `psr/log` `^1.1`, `^2.0` or `^3.0` are new dependencies. diff --git a/composer.json b/composer.json index 7db091c..38d9796 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,7 @@ "ext-pcre": "*", "psr/log": "^1.1 || ^2.0 || ^3.0", "symfony/deprecation-contracts": "^2.5 || ^3", - "symfony/process": "^6.4 || ^7.4 || ^8.1", + "symfony/process": "^6.4.33 || ^7.4.5 || ^8.1", "symfony/string": "^7.4.6 || ^8.0.6" }, "require-dev": { From 5b73305b5ea489128e99e5f11e6d7a583b68228e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:57:29 +0200 Subject: [PATCH 15/22] Say the interfaces of the drivers may get methods in 2.x The doc invites to implement DriverInterface and RepositoryDriverInterface, in tests for instance, without saying whether 2.x may add methods to them: any new method would have broken every implementation. Both interfaces say gitlib may add methods in a minor version: the promise of backward compatibility covers the code calling them, not the drivers implementing them. The parsers of Driver\Cli\Parser, which only CliDriver uses, are internal. --- UPGRADE-2.0.md | 7 +++++-- doc/repository.md | 4 ++++ src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php | 2 ++ src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php | 2 ++ src/Gitonomy/Git/Driver/Cli/Parser/ReferenceParser.php | 2 ++ src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php | 2 ++ src/Gitonomy/Git/Driver/Cli/Parser/TreeParser.php | 2 ++ src/Gitonomy/Git/Driver/DriverInterface.php | 4 ++++ src/Gitonomy/Git/Driver/RepositoryDriverInterface.php | 4 ++++ 9 files changed, 27 insertions(+), 2 deletions(-) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index d7d4acb..59566d6 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -516,12 +516,15 @@ Extending gitlib `Gitonomy\Git\Driver\DriverInterface` and pass it as the driver of `Repository`, instead of extending `Repository`: see ["Using your own driver"](doc/repository.md#using-your-own-driver). + gitlib may add methods to the interfaces of the drivers in a minor + version of 2.x. * `Commit::__construct()` takes a `Gitonomy\Git\Driver\Data\CommitData` instead of an array, and `Commit::setData()` is internal. * `Gitonomy\Git\Util\StringHelper` is removed, with its configurable encoding: use `symfony/string`. Logs are always read in UTF-8. -* The parsers of the git output moved to `Gitonomy\Git\Driver\Cli\Parser` - and are `final`, except `Parser\DiffParser` and `Parser\ParserBase`. +* The parsers of the git output moved to `Gitonomy\Git\Driver\Cli\Parser`, + and are `final` and internal, used by `CliDriver` only, except + `Parser\DiffParser` and `Parser\ParserBase`. `CommitParser` is merged into `LogParser`, and their results are objects of `Gitonomy\Git\Driver\Data`: `LogParser::$commits` (instead of `$log`), `TagParser::$data`, `ReferenceParser::$references`, `TreeParser::$entries` diff --git a/doc/repository.md b/doc/repository.md index fbcf193..e9ce484 100644 --- a/doc/repository.md +++ b/doc/repository.md @@ -587,3 +587,7 @@ $repository->getReferences()->getBranch('main')->getCommitHash(); // $hash The other methods of `DriverInterface` create repositories: `init()`, `initBare()`, `cloneFrom()`, `cloneBareFrom()`, `mirrorFrom()`, `isValidRepository()` and `isValidRepositoryAndBranch()`. + +gitlib may add methods to both interfaces in a minor version of 2.x: the +promise of backward compatibility covers the code calling them, not the +drivers implementing them, like a stub that implements every method. diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php index 6d739cb..10d6c44 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/BlameParser.php @@ -17,6 +17,8 @@ /** * Parses the output of ``git blame -p``. + * + * @internal Used by CliDriver only */ final class BlameParser extends ParserBase { diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php index 4b91c5d..5011680 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/LogParser.php @@ -18,6 +18,8 @@ /** * Parses the output of ``git log --format=raw``. + * + * @internal Used by CliDriver only */ final class LogParser extends ParserBase { diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/ReferenceParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/ReferenceParser.php index 16e5a0a..7a5fe54 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/ReferenceParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/ReferenceParser.php @@ -18,6 +18,8 @@ /** * Parses `` `` lines. + * + * @internal Used by CliDriver only */ final class ReferenceParser extends ParserBase { diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php index bd14f34..269ffd6 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php @@ -19,6 +19,8 @@ /** * Parses a tag object, as ``git cat-file --batch`` gives it. + * + * @internal Used by CliDriver only */ final class TagParser extends ParserBase { diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/TreeParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/TreeParser.php index 54c769f..afc700f 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/TreeParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/TreeParser.php @@ -20,6 +20,8 @@ /** * Parses a tree object, as ``git cat-file --batch`` gives it: for each * entry, its mode, a space, its name, a NUL byte, and its hash in binary. + * + * @internal Used by CliDriver only */ final class TreeParser extends ParserBase { diff --git a/src/Gitonomy/Git/Driver/DriverInterface.php b/src/Gitonomy/Git/Driver/DriverInterface.php index 88ca48c..dea15a1 100644 --- a/src/Gitonomy/Git/Driver/DriverInterface.php +++ b/src/Gitonomy/Git/Driver/DriverInterface.php @@ -37,6 +37,10 @@ * descriptor of PHP, and an ssh host or user starting with a dash is an * option for ssh. * + * gitlib may add methods to this interface in a minor version: the promise of + * backward compatibility covers the code calling it, not the drivers + * implementing it outside gitlib, like a stub in tests. + * * @author Grégoire Pineau */ interface DriverInterface diff --git a/src/Gitonomy/Git/Driver/RepositoryDriverInterface.php b/src/Gitonomy/Git/Driver/RepositoryDriverInterface.php index 1cb1596..89e59ac 100644 --- a/src/Gitonomy/Git/Driver/RepositoryDriverInterface.php +++ b/src/Gitonomy/Git/Driver/RepositoryDriverInterface.php @@ -43,6 +43,10 @@ * {@see InvalidArgumentException}, a revision, a reference or a hash starting * with a dash, before running git. A path may start with one. * + * gitlib may add methods to this interface in a minor version: the promise of + * backward compatibility covers the code calling it, not the drivers + * implementing it outside gitlib, like a stub in tests. + * * @author Grégoire Pineau */ interface RepositoryDriverInterface From 3acf907e457a553d850d6cd09a1897abb2bd060d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:58:37 +0200 Subject: [PATCH 16/22] Complete the UPGRADE and the doc for the release - UPGRADE-2.0.md says an unknown revision throws a ReferenceNotFoundException, which a catch of RuntimeException doesn't catch, where getDiff(), getBlame() or checkout() threw a ProcessException; that getRawDiff() returns the output of "git diff --raw -z --patch", or nothing with LibGit2Driver; which parameters are renamed, for named arguments; and how ParserBase changes. Its examples import the classes they use. - doc/diff.md says what getRawDiff() returns. - The example of getHead() in doc/repository.md checks the class of HEAD, which PHPStan understands. - The differences of LibGit2Driver list the blame, which may attribute some lines to another commit, and ignores blame.ignoreRevsFile. - The doc says a password in a git:// URL, which takes no credentials, isn't hidden: git quotes such a URL without its scheme. --- UPGRADE-2.0.md | 24 +++++++++++++++++++++++- doc/diff.md | 6 ++++-- doc/repository.md | 16 +++++++++++----- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 59566d6..9dd77bf 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -57,6 +57,7 @@ Opening a repository ```php use Gitonomy\Git\Driver\Cli\CliDriver; + use Gitonomy\Git\Repository; $repository = new Repository($path, driver: new CliDriver(command: '/usr/bin/git', processTimeout: 60, logger: $logger)); ``` @@ -116,6 +117,7 @@ Creating repositories ```php use Gitonomy\Git\Driver\Cli\CliDriver; + use Gitonomy\Git\Repository; $repository = Repository::cloneFrom($url, $path, driver: new CliDriver(processTimeout: 60)); ``` @@ -172,6 +174,8 @@ References After: ```php + use Gitonomy\Git\Reference\Branch; + $branches = array_filter($commit->getIncludingBranches(), static fn (Branch $branch): bool => $branch->isLocal()); ``` @@ -392,6 +396,11 @@ Errors * `ProcessException` extends the new `Gitonomy\Git\Exception\DriverException`, itself a `RuntimeException`: catching `RuntimeException` or `GitExceptionInterface` still works. +* An unknown revision throws a `ReferenceNotFoundException`, an + `InvalidArgumentException`, where `Repository::getDiff()`, `getBlame()` + or `WorkingCopy::checkout()` threw a `ProcessException`: a + `catch (RuntimeException)` doesn't catch it anymore. Catch + `GitExceptionInterface`, which every exception of gitlib implements. * The message of a `ProcessException` is one line, with the error of git: the first line of its error output starting with `fatal:` or `error:`, after the warnings, or its first line: `The git command "clone" failed @@ -512,6 +521,13 @@ Extending gitlib The constructor of `Reference` requires the commit hash. Extending `Revision` or `Reference` isn't supported anymore, and won't be possible in 3.0: `Reference` declares `getName()`, abstract. +* With named arguments, some parameters are renamed: + `Repository::__construct()` `$options` (`$workingDir`), `getBlame()` + `$lineRange` (`$firstLine`), `getDiff()` `$revisions` (`$from`), + `ReferenceBag::createBranch()` and `createTag()` `$commitHash` + (`$revision`), `resolve()`, `resolveBranches()` and `resolveTags()` + `$hash` (`$commit`), and `PushReference::__construct()` `$reference` + (`$fullname`). * To change how gitlib runs git, for example in tests, implement `Gitonomy\Git\Driver\DriverInterface` and pass it as the driver of `Repository`, instead of extending `Repository`: see @@ -524,7 +540,9 @@ Extending gitlib encoding: use `symfony/string`. Logs are always read in UTF-8. * The parsers of the git output moved to `Gitonomy\Git\Driver\Cli\Parser`, and are `final` and internal, used by `CliDriver` only, except - `Parser\DiffParser` and `Parser\ParserBase`. + `Parser\DiffParser` and `Parser\ParserBase`. `ParserBase` loses + `consumeGPGSignature()` and `consumeMergeTag()`, and gets + `consumeNameEmailDate()`, `parseSignature()` and `signatureDate()`. `CommitParser` is merged into `LogParser`, and their results are objects of `Gitonomy\Git\Driver\Data`: `LogParser::$commits` (instead of `$log`), `TagParser::$data`, `ReferenceParser::$references`, `TreeParser::$entries` @@ -562,6 +580,10 @@ Data returned * `WorkingCopy::getUntrackedFiles()` and `RevisionList::getAll()` return lists, without gaps in their keys. * `Diff\File::toArray()` has a new `is_copy` key. +* `Diff::getRawDiff()` of a diff gitlib reads returns the output of + `git diff --raw -z --patch`, NUL bytes included, with the options gitlib + sets, instead of the text of `--raw --patch`; with `LibGit2Driver`, an + empty string. Other changes ------------- diff --git a/doc/diff.md b/doc/diff.md index 18660de..75fcd95 100644 --- a/doc/diff.md +++ b/doc/diff.md @@ -188,8 +188,10 @@ $output = $repository->run('diff', ['--raw', '--patch', '--submodule=short', '0. $diff = Diff::parse($output, $repository); ``` -`getRawDiff()` returns the output a diff was parsed from. It is empty for -the diffs of `LibGit2Driver`, which runs no git command. +`getRawDiff()` returns the output a diff was parsed from: for a diff gitlib +reads with `CliDriver`, the one of `git diff --raw -z --patch`, NUL bytes +included, with the options gitlib sets. It is empty for the diffs of +`LibGit2Driver`, which runs no git command. Storing a diff -------------- diff --git a/doc/repository.md b/doc/repository.md index e9ce484..0864ed5 100644 --- a/doc/repository.md +++ b/doc/repository.md @@ -75,18 +75,20 @@ tree). Your `HEAD` can be attached (pointing to a branch) or detached (pointing to a commit). ```php +use Gitonomy\Git\Reference\Branch; + $head = $repository->getHead(); // the Branch HEAD points to, or the Commit when it is detached $commit = $repository->getHeadCommit(); // the Commit, in both cases -if ($repository->isHeadAttached()) { +if ($head instanceof Branch) { echo 'On branch '.$head->getName().PHP_EOL; -} - -if ($repository->isHeadDetached()) { +} else { echo 'HEAD detached at '.$commit->getShortHash().PHP_EOL; } ``` +`isHeadAttached()` and `isHeadDetached()` tell it too. + In a repository without commits yet, `HEAD` points to a branch that doesn't exist yet: `getHead()` and `getHeadCommit()` throw a `ReferenceNotFoundException`, and `isHeadAttached()` returns `true`. @@ -261,6 +263,8 @@ try { The message of an exception hides the credentials of the URLs it holds (`https://***@example.com/repository.git`): `getErrorOutput()` and `getOutput()` of a `ProcessException` return the output of git as it is. +A `git://` URL takes no credentials: git quotes one without its scheme in +its errors, so a password put there isn't hidden. Choosing a driver ----------------- @@ -353,7 +357,9 @@ Differences with `CliDriver`: `commit.gpgSign`, and its message never lists the merged commits, whatever `merge.log`, nor has the output of gpg checking a signed tag. - Blame is much slower on a long history: see - ["Performance"](#performance). + ["Performance"](#performance). It may attribute some lines to another + commit than git, which detects renames and aligns identical lines + differently, and it ignores `blame.ignoreRevsFile`. - Rename detection in diffs may pair files differently. - Network operations go through the proxy of `https_proxy`, or of `http.proxy`, for HTTPS only: libgit2 1.9 uses no proxy for a plain From e39a6c636089f830fb0ab07092dac975aa8d7074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 18:59:49 +0200 Subject: [PATCH 17/22] Fix the CHANGELOG for the release The entries of the work of #254 had no link, or the one of #253, and some were wrong for 2.0: - reftable repositories open with CliDriver only; - run() doesn't keep its "exact original behavior", but still returns the standard output; - Repository::cloneTo() read like a replacement of Admin, and $reload like an argument of ReferenceBag::update() and Branch::isMergedTo(), which are deprecated themselves; - the fix of #239 for the "debug" option set to false is gone: 2.0 refuses that option. The header stays "Unreleased", until the release date is known. --- CHANGELOG.md | 52 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c19bb36..41b6a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ deprecated until 3.0. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Return the dates of commits and tags at the UTC offset of their author, committer or tagger, like `git log` shows them, instead of in UTC: the instant doesn't change, but `format()` - returns their local time. + returns their local time. ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Require PHP 8.4 or later, Symfony 7.4/8.1+: most classes are `final` (some `readonly`), and every method has native types. ([#238](https://github.com/gitonomy/gitlib/pull/238)) - Remove `Gitonomy\Git\Util\StringHelper`, replaced by `symfony/string`, and its configurable @@ -37,17 +37,20 @@ deprecated until 3.0. are internal: get these objects from `Repository`, which returns the same commit, tree or blob while the application uses it, and reads the references again after its writes. Extending `Revision` or `Reference` isn't supported anymore. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - `ProcessException` has a message of one line, with the error of git, the first line of its error output starting with `fatal:` or `error:`, after the warnings: `The git command "clone" failed (exit 128): fatal: ...`. The command line, the rest of the error output and the output are no longer in the message: `getErrorOutput()` and - `getOutput()` return the outputs of git. + `getOutput()` return the outputs of git. ([#254](https://github.com/gitonomy/gitlib/pull/254)) - When git can't start, a missing binary for instance, `CliDriver` throws a `DriverException` instead of a `ProcessException`, with a message of one line, without the command line: `The git command "clone" could not start: ...`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - `WorkingCopy::checkout()` throws a `ReferenceNotFoundException` for a name that no revision has, with both drivers: the name of a remote branch alone, like `feature` for `origin/feature`, created the local branch. Use `checkoutNewBranch()`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) ### Features @@ -63,8 +66,9 @@ deprecated until 3.0. which gets every command, clones included), or any `Driver\DriverInterface`, to replace it in tests; see ["Using your own driver"](doc/repository.md#using-your-own-driver). The driver finds the repository: linked worktrees, submodules, clones with `--separate-git-dir` and - reftable repositories open, and a directory that isn't a repository throws right away. - ([#253](https://github.com/gitonomy/gitlib/pull/253), fixes #200) + reftable repositories (with `CliDriver`) open, and a directory that isn't a repository + throws right away. ([#253](https://github.com/gitonomy/gitlib/pull/253), + [#261](https://github.com/gitonomy/gitlib/pull/261), fixes #200) - Add static methods to `Repository` creating repositories, `init()`, `initBare()`, `cloneFrom()`, `cloneBareFrom()` and `mirrorFrom()`, and checking a repository URL, `isValidRepository()` and `isValidRepositoryAndBranch()`; see @@ -77,14 +81,14 @@ deprecated until 3.0. `Blame\Line`, `TreeEntry` and `Submodule`, `Diff\DiffLine` and `Diff\DiffLineType`, `File::isModeChange()`, `FileChange::getAdditions()` and `getDeletions()`, `PushReference::getFullname()`, `getRange()`, `isCreation()` and `isDeletion()`, and the SSH - and X.509 signatures of tags. - ([#253](https://github.com/gitonomy/gitlib/pull/253)) + and X.509 signatures of tags. ([#253](https://github.com/gitonomy/gitlib/pull/253), + [#254](https://github.com/gitonomy/gitlib/pull/254)) - Add `WorkingCopy::merge()` to support `git merge`, fast-forwarding or not, with a `FastForward` enum. ([#248](https://github.com/gitonomy/gitlib/pull/248), fixes #118) - Add `Repository::runProcess()` to expose stderr from successful commands: `run()` only ever returned stdout, even on success, but some git commands (e.g. `push`) write - meaningful output to stderr even when they succeed. `run()` now delegates to it and keeps - its exact original behavior. + meaningful output to stderr even when they succeed. `run()` still returns the standard + output. ([#247](https://github.com/gitonomy/gitlib/pull/247), fixes #205) - Support git's mnemonic diff prefixes (`diff.mnemonicPrefix`) in the diff parser: enabling this option replaces the default `a/`/`b/` prefixes with context-specific ones @@ -101,13 +105,15 @@ deprecated until 3.0. All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). ([#253](https://github.com/gitonomy/gitlib/pull/253)) -- `Admin`, use the static methods of `Repository`, and `Repository::cloneTo()`. +- `Admin`, replaced by the static methods of `Repository`, and `Repository::cloneTo()`, + replaced by `cloneFrom()` and `cloneBareFrom()`. - The array of options of `Repository` and `Admin` (`working_dir`, `logger`, `command`, `environment_variables`, `inherit_environment_variables`, `process_timeout`, `debug`): pass the working directory and a `CliDriver` instead; `inherit_environment_variables` has no counterpart, pass the variables git needs instead. `Repository::setLogger()` and `getLogger()`. -- Passing `$reload` to `Repository::getReferences()`, `ReferenceBag::update()`, - `Branch::isMergedTo()`, and arguments to `Commit::getIncludingBranches()`. +- Passing `$reload` to `Repository::getReferences()`, and arguments to + `Commit::getIncludingBranches()`. `ReferenceBag::update()` and `Branch::isMergedTo()`, + replaced by `Branch::moveTo()` and `isMergedInto()`. - Calling `Repository::getDiff()` with one argument, `Log::getDiff()`, `Diff::setRepository()`, `File::setRepository()`, `addChange()`, `isChangeMode()`, `isDelete()` and `getAnchor()`, `FileChange::getCount()` and its `LINE_*` constants, reading diff lines as arrays. @@ -119,7 +125,9 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - `Commit::getShortMessage()` and `getFixedShortHash()`, `Repository::shell()`, `PushReference::getReference()`, `getRevision()`, `isCreate()` and `isDelete()`. - `Repository::getSize()`, in kilobytes: use `getSizeInBytes()`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - `Exception\UnexpectedValueException`, which gitlib no longer throws. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) ### Security @@ -127,37 +135,45 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). option: `getRevision('--output=/path')`, `WorkingCopy::checkout()` or `merge()` could write a file, and `Repository::isValidRepository('--upload-pack=...')` run a command. A path may start with a dash: `init()` and the clones end the options of git with `--`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Require `symfony/process` 6.4.33, 7.4.5 or 8.1 and later: the older versions escape the arguments wrongly on Windows (CVE-2024-51736, CVE-2026-24739). - Refuse an `ext::` URL, like `ext::sh -c ...`, whose command git runs, by default before git 2.12, and an `fd::` URL, which makes git talk through a file descriptor it inherits from PHP, like the socket of a database, and wait on it. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Refuse an ssh URL whose host or user starts with a dash, even encoded (`%2D`), like `ssh://-oProxyCommand=.../repository`: git before 2.14.1 passes it to ssh, which reads it as an option and runs the command (CVE-2017-1000117). A `git://` URL whose host starts with a dash is refused too: git passes it to the command of `core.gitProxy`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Hide the credentials of URLs (`https://user:token@host`), a password holding a `#` or a `?` included, in the messages of exceptions, in their traces (`#[\SensitiveParameter]`), and in the logs. The exceptions of Symfony Process, which hold the command line, no longer leak, not - even as the previous exception. + even as the previous exception. ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Refuse a hook name that isn't the one of a file of the hooks directory: `Hooks::set('../config', ...)` and `remove('../config')` wrote and deleted the - configuration of the repository. + configuration of the repository. ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix `Hooks` writing through a symbolic link to a missing file, out of the hooks directory: `has()` returned `false` for it, and `set()` followed it. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Ignore the replacements of objects (`git replace`, `refs/replace/`) with `CliDriver`, like `LibGit2Driver`: whoever could push such a reference to a repository changed the message of a commit, or the content of a blob, gitlib read for a hash. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Ignore `core.fsmonitor` with `CliDriver`: git ran the command the configuration of a repository names there to read the working tree, and even to diff a commit. Like git with `--git-dir`, gitlib trusts the repositories it opens: `safe.directory` doesn't apply, see ["Git configuration"](doc/repository.md#git-configuration). - Fix `WorkingCopy::checkout()` of a path, like `checkout('README.md')`, discarding the changes of the file: a path is no revision, and throws a `ReferenceNotFoundException`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix moving or deleting a symbolic reference, which changed the branch it points to instead: `ReferenceBag::delete('refs/remotes/origin/HEAD')` deleted `origin/main`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix `ReferenceBag::delete('HEAD')` deleting `HEAD`, which left no repository: a name outside `refs/` throws an `InvalidArgumentException`. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) ### Bug fixes @@ -176,10 +192,12 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). `diff.orderFile`, `log.follow` or `branch.sort` changed diffs, logs and the branches of a commit. gitlib sets them back to git's defaults; see ["Git configuration"](doc/repository.md#git-configuration). + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix `Commit::getShortHash()`, `Commit::getDiff()` and `Repository::getDiff()` failing when a file of the working tree has the name of the revision they read, which git found ambiguous. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix logs of thousands of revisions exceeding the limit of the command line: `CliDriver` passes - them to git on its standard input. + them to git on its standard input. ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix `Commit::getDiff()` returning an empty diff for a root commit, and throwing for a merge commit (`CliDriver`). ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Fix `Repository::getDiff()`, `WorkingCopy::getDiffPending()` and `WorkingCopy::getDiffStaged()` @@ -214,6 +232,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). a hook not deployed yet: it no longer follows links. A file or a directory git deletes meanwhile, like a lock during a `gc` or the quarantine of a push, no longer makes it throw either, and one it can't read throws a `RuntimeException` of gitlib, instead of PHP's. + ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Fix blame lines starting with a TAB character (`Blame\Line::getContent()`): code removing it must stop doing so. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Fix reading a commit whose `encoding` header isn't UTF-8, which threw: its message, author and @@ -236,9 +255,6 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). branch existed. - `Tag::getBodyMessage()` dropped or truncated a line for annotated tags using the standard `subject\n\nbody` convention. - - With the `debug` option set to `false`, several call sites (`Tag::isAnnotated()`, - `PushReference::isForce()`, `Blob::getContent()`, ...) produced wrong data or crashed - when git failed. - `Repository::shell()` interpolated the repository path into the shell command without escaping it; it is now passed through `escapeshellarg()`. - `Hooks::set()` created hook scripts world-writable (`0777`); they are now created @@ -254,7 +270,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - The tests run with a git configuration of their own, whatever the developer's (`tests/fixtures/home`), which sets what gitlib sets back to git's defaults, and without the variables of git of their environment: run from a hook, they wrote the repository of the hook - (`GIT_DIR`, `GIT_INDEX_FILE`...). + (`GIT_DIR`, `GIT_INDEX_FILE`...). ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Add a regression test for reading a blob's content in a tree that also contains a submodule. ([#243](https://github.com/gitonomy/gitlib/pull/243)) - Replace the network-dependent test fixture (cloned from GitHub on every run) with a local, From 2ecb7b4d0fbddf11c61acb03778fc2b51e410b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 19:03:00 +0200 Subject: [PATCH 18/22] Link the CHANGELOG entries of this pull request to it The entries of #263 had no link: they get one, after the one of #254 when they complete its work. --- CHANGELOG.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b6a93..9010d31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,7 @@ deprecated until 3.0. `File::isModeChange()`, `FileChange::getAdditions()` and `getDeletions()`, `PushReference::getFullname()`, `getRange()`, `isCreation()` and `isDeletion()`, and the SSH and X.509 signatures of tags. ([#253](https://github.com/gitonomy/gitlib/pull/253), - [#254](https://github.com/gitonomy/gitlib/pull/254)) + [#254](https://github.com/gitonomy/gitlib/pull/254), [#263](https://github.com/gitonomy/gitlib/pull/263)) - Add `WorkingCopy::merge()` to support `git merge`, fast-forwarding or not, with a `FastForward` enum. ([#248](https://github.com/gitonomy/gitlib/pull/248), fixes #118) - Add `Repository::runProcess()` to expose stderr from successful commands: `run()` only @@ -138,10 +138,11 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). ([#254](https://github.com/gitonomy/gitlib/pull/254)) - Require `symfony/process` 6.4.33, 7.4.5 or 8.1 and later: the older versions escape the arguments wrongly on Windows (CVE-2024-51736, CVE-2026-24739). + ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Refuse an `ext::` URL, like `ext::sh -c ...`, whose command git runs, by default before git 2.12, and an `fd::` URL, which makes git talk through a file descriptor it inherits from PHP, like the socket of a database, and wait on it. - ([#254](https://github.com/gitonomy/gitlib/pull/254)) + ([#254](https://github.com/gitonomy/gitlib/pull/254), [#263](https://github.com/gitonomy/gitlib/pull/263)) - Refuse an ssh URL whose host or user starts with a dash, even encoded (`%2D`), like `ssh://-oProxyCommand=.../repository`: git before 2.14.1 passes it to ssh, which reads it as an option and runs the command (CVE-2017-1000117). A `git://` URL whose host starts with a dash @@ -165,6 +166,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). repository names there to read the working tree, and even to diff a commit. Like git with `--git-dir`, gitlib trusts the repositories it opens: `safe.directory` doesn't apply, see ["Git configuration"](doc/repository.md#git-configuration). + ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix `WorkingCopy::checkout()` of a path, like `checkout('README.md')`, discarding the changes of the file: a path is no revision, and throws a `ReferenceNotFoundException`. ([#254](https://github.com/gitonomy/gitlib/pull/254)) @@ -180,12 +182,13 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). - Fix `Commit::getIncludingBranches()` and `Repository::getHead()` throwing for a branch another process created or checked out since gitlib read the references, and `getIncludingBranches()` throwing for a regular `refs/remotes/origin/HEAD`, as a mirror leaves - it. + it. ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix the blame of a file throwing with `CliDriver` after a merge whose sides kept it under two names: git repeats the path of a commit it blames under both, which gitlib didn't read. + ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix a log throwing with `CliDriver` for a commit whose author or committer git wouldn't write anymore, like `Name` or no date, as old imports have: it is read like libgit2 - reads it, the epoch in UTC without date. + reads it, the epoch in UTC without date. ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix the configuration of the user changing what `CliDriver` reads: `color.ui=always`, `log.showSignature`, `diff.noprefix`, `diff.external`, `diff.submodule`, textconv filters, `column.ui`... made reads throw or return wrong data, and `diff.algorithm`, `diff.context`, @@ -232,7 +235,7 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). a hook not deployed yet: it no longer follows links. A file or a directory git deletes meanwhile, like a lock during a `gc` or the quarantine of a push, no longer makes it throw either, and one it can't read throws a `RuntimeException` of gitlib, instead of PHP's. - ([#254](https://github.com/gitonomy/gitlib/pull/254)) + ([#254](https://github.com/gitonomy/gitlib/pull/254), [#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix blame lines starting with a TAB character (`Blame\Line::getContent()`): code removing it must stop doing so. ([#253](https://github.com/gitonomy/gitlib/pull/253)) - Fix reading a commit whose `encoding` header isn't UTF-8, which threw: its message, author and From 6e47aa2887a078e45a4bdee21e31abb59a4762f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 19:40:57 +0200 Subject: [PATCH 19/22] Run ls-remote out of the working directory of PHP, and refuse an empty URL isValidRepository() and isValidRepositoryAndBranch() ran ls-remote in the working directory of PHP, which may be a repository, like the checkout of an application: git read its configuration, its rewrites of URLs for instance, and an empty URL named it, which was then valid with CliDriver, not with LibGit2Driver. git 1.8.4 can't even run in a linked worktree, which made them return false. ls-remote runs in the temporary directory, and both drivers refuse an empty URL with an InvalidArgumentException. --- CHANGELOG.md | 4 ++ UPGRADE-2.0.md | 1 + doc/admin.md | 1 + src/Gitonomy/Git/Driver/Cli/CliDriver.php | 17 ++++++-- src/Gitonomy/Git/Util/RemoteUrl.php | 7 +++- tests/Gitonomy/Git/Tests/RepositoryTest.php | 44 +++++++++++++++++++++ 6 files changed, 70 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9010d31..51456c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,10 @@ All are removed in 3.0; see [UPGRADE-2.0.md](UPGRADE-2.0.md). process created or checked out since gitlib read the references, and `getIncludingBranches()` throwing for a regular `refs/remotes/origin/HEAD`, as a mirror leaves it. ([#263](https://github.com/gitonomy/gitlib/pull/263)) +- Fix `isValidRepository()` and `isValidRepositoryAndBranch()` reading, with `CliDriver`, the + configuration of the repository PHP runs in, like its rewrites of URLs, and returning `true` + for an empty URL there: it throws an `InvalidArgumentException`. + ([#263](https://github.com/gitonomy/gitlib/pull/263)) - Fix the blame of a file throwing with `CliDriver` after a merge whose sides kept it under two names: git repeats the path of a commit it blames under both, which gitlib didn't read. ([#263](https://github.com/gitonomy/gitlib/pull/263)) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 9dd77bf..8d922de 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -439,6 +439,7 @@ Errors `getRevision('--output=/path')` writing a file, or `isValidRepository('--upload-pack=...')` running a command. A path may start with a dash. +* An empty URL throws an `InvalidArgumentException`, with both drivers. * An `ext::` URL throws an `InvalidArgumentException`, with both drivers: git runs its command, by default before git 2.12. So does an `fd::` URL, which makes git talk through a file descriptor it inherits from PHP. diff --git a/doc/admin.md b/doc/admin.md index 10e6cfe..3159199 100644 --- a/doc/admin.md +++ b/doc/admin.md @@ -104,6 +104,7 @@ start, like a missing binary. See [Errors](repository.md#errors). A URL that makes git run a command, or use a file of PHP, throws an `InvalidArgumentException`, with both drivers: +- an empty URL, which would make git read the repository PHP runs in; - a URL starting with a dash, which git would read as an option, like `--upload-pack=...`; a path may start with one; - an `ext::` URL, like `ext::sh -c ...`, whose command git runs, unless its diff --git a/src/Gitonomy/Git/Driver/Cli/CliDriver.php b/src/Gitonomy/Git/Driver/Cli/CliDriver.php index 2f73b71..005a2b0 100644 --- a/src/Gitonomy/Git/Driver/Cli/CliDriver.php +++ b/src/Gitonomy/Git/Driver/Cli/CliDriver.php @@ -79,7 +79,7 @@ public function isValidRepository(#[\SensitiveParameter] string $url): bool // ls-remote only takes "--" since git 2.8. RemoteUrl::assertSafe($url); - return $this->runner()->run('ls-remote', [self::absoluteUrl($url)])->isSuccessful(); + return $this->runner()->run('ls-remote', [self::absoluteUrl($url)], workingDirectory: self::neutralDirectory())->isSuccessful(); } public function isValidRepositoryAndBranch(#[\SensitiveParameter] string $url, string $branch): bool @@ -87,7 +87,7 @@ public function isValidRepositoryAndBranch(#[\SensitiveParameter] string $url, s RemoteUrl::assertSafe($url); // git lists the branches whose name ends with the pattern: "release" matches "feature/release". - $process = $this->runner()->run('ls-remote', ['--heads', self::absoluteUrl($url), 'refs/heads/'.$branch]); + $process = $this->runner()->run('ls-remote', ['--heads', self::absoluteUrl($url), 'refs/heads/'.$branch], workingDirectory: self::neutralDirectory()); if (!$process->isSuccessful()) { return false; } @@ -150,10 +150,21 @@ private function mustRun(string $command, #[\SensitiveParameter] array $args): v } } + /** + * Out of the working directory of PHP, which may be a repository, like + * the checkout of an application: ls-remote, unlike clone, would read its + * configuration, its rewrites of URLs for instance. git 1.8.4 can't even + * run in a linked worktree. + */ + private static function neutralDirectory(): string + { + return sys_get_temp_dir(); + } + /** * Run in a repository, ls-remote reads a relative path as the name of a * remote first: "origin" would be the URL of the remote of PHP's working - * directory. + * directory. The path stays relative to it. */ private static function absoluteUrl(#[\SensitiveParameter] string $url): string { diff --git a/src/Gitonomy/Git/Util/RemoteUrl.php b/src/Gitonomy/Git/Util/RemoteUrl.php index 34a4bd1..dda2434 100644 --- a/src/Gitonomy/Git/Util/RemoteUrl.php +++ b/src/Gitonomy/Git/Util/RemoteUrl.php @@ -25,10 +25,15 @@ final class RemoteUrl { /** - * @throws InvalidArgumentException $url starts with a dash, uses the "ext::" or "fd::" transport, or has a host or a user starting with a dash, encoded or not + * @throws InvalidArgumentException $url is empty, starts with a dash, uses the "ext::" or "fd::" transport, or has a host or a user starting with a dash, encoded or not */ public static function assertSafe(#[\SensitiveParameter] string $url): void { + // git would run in the working directory of PHP, which may be a repository. + if ('' === $url) { + throw new InvalidArgumentException('The URL of a repository can\'t be empty.'); + } + LeadingDash::assertNone($url); // "ext::sh -c ..." runs the command, to talk to it: git refuses it by diff --git a/tests/Gitonomy/Git/Tests/RepositoryTest.php b/tests/Gitonomy/Git/Tests/RepositoryTest.php index 1c58b0c..a03f271 100644 --- a/tests/Gitonomy/Git/Tests/RepositoryTest.php +++ b/tests/Gitonomy/Git/Tests/RepositoryTest.php @@ -339,6 +339,50 @@ public function testIsValidRepositoryReadsANameAsAPath(): void } } + /** + * git reads the configuration of the repository it runs in, like its + * rewrites of URLs: an application may run from a checkout. + */ + public function testTheWorkingDirectoryOfPhpRewritesNoUrl(): void + { + $url = self::createFoobarRepository()->getGitDir(); + $repository = self::createFoobarRepository(false); + self::git($repository, 'config', ['url.'.self::createTempDir().'/missing.git.insteadOf', $url]); + $cwd = getcwd(); + self::assertIsString($cwd); + + chdir($repository->getPath()); + + try { + $this->assertTrue(Repository::isValidRepository($url, self::getDriver())); + $this->assertTrue(Repository::isValidRepositoryAndBranch($url, 'master', self::getDriver())); + } finally { + chdir($cwd); + } + } + + /** + * git ran in the working directory of PHP, which may be a repository. + */ + #[DataProvider('provideEmptyUrlCalls')] + public function testAnEmptyUrlIsRefused(\Closure $call): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The URL of a repository can\'t be empty.'); + + $call(); + } + + /** + * @return iterable + */ + public static function provideEmptyUrlCalls(): iterable + { + yield 'valid repository' => [static fn (): bool => Repository::isValidRepository('', self::getDriver())]; + yield 'valid branch' => [static fn (): bool => Repository::isValidRepositoryAndBranch('', 'master', self::getDriver())]; + yield 'clone' => [static fn (): Repository => Repository::cloneFrom('', self::createTempDir().'/clone', driver: self::getDriver())]; + } + public function testIsValidRepositoryThrowsWhenGitCantRun(): void { $this->skipUnlessCliDriver(); From dbb8bfaa5f0d18d9c182f8d774f2fed94dceb945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 19:41:38 +0200 Subject: [PATCH 20/22] Remove the fields of TagData nothing reads TagData held the object a tag points to, its type and the name of the tag, which nothing reads: the reference gives the object already. A driver implemented outside gitlib had to fill them anyway. TagData keeps the tagger, the message and the signature. --- .../Git/Driver/Cli/Parser/TagParser.php | 22 ++++++------------- src/Gitonomy/Git/Driver/Data/TagData.php | 9 +++----- .../Driver/Ffi/LibGit2RepositoryDriver.php | 3 --- src/Gitonomy/Git/Driver/Ffi/libgit2.h | 3 --- 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php b/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php index 269ffd6..ebffed4 100644 --- a/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php +++ b/src/Gitonomy/Git/Driver/Cli/Parser/TagParser.php @@ -12,9 +12,7 @@ namespace Gitonomy\Git\Driver\Cli\Parser; -use Gitonomy\Git\Driver\Data\ObjectType; use Gitonomy\Git\Driver\Data\TagData; -use Gitonomy\Git\Exception\RuntimeException; use Gitonomy\Git\Parser\ParserBase; /** @@ -28,18 +26,12 @@ final class TagParser extends ParserBase protected function doParse(): void { - $this->consume('object '); - $object = $this->consumeHash(); - $this->consumeNewLine(); - - $this->consume('type '); - $typeName = $this->consumeTo("\n"); - $type = ObjectType::tryFrom($typeName) ?? throw new RuntimeException(\sprintf('The tag points to an object of the unknown type "%s".', $typeName)); - $this->consumeNewLine(); - - $this->consume('tag '); - $tag = $this->consumeTo("\n"); - $this->consumeNewLine(); + // The object the tag points to, which the reference gives already, and the name of the tag. + foreach (['object ', 'type ', 'tag '] as $header) { + $this->consume($header); + $this->consumeTo("\n"); + $this->consumeNewLine(); + } // The oldest tags, like the first ones of git itself, have no tagger. [$taggerName, $taggerEmail, $taggerDate] = [null, null, null]; @@ -50,6 +42,6 @@ protected function doParse(): void $this->consumeNewLine(); - $this->data = TagData::fromSignedMessage($object, $type, $tag, $taggerName, $taggerEmail, $taggerDate, $this->consumeAll()); + $this->data = TagData::fromSignedMessage($taggerName, $taggerEmail, $taggerDate, $this->consumeAll()); } } diff --git a/src/Gitonomy/Git/Driver/Data/TagData.php b/src/Gitonomy/Git/Driver/Data/TagData.php index 87b8044..9635a41 100644 --- a/src/Gitonomy/Git/Driver/Data/TagData.php +++ b/src/Gitonomy/Git/Driver/Data/TagData.php @@ -29,9 +29,6 @@ * @param \DateTimeImmutable|null $taggerDate At the UTC offset of the tagger, like git log shows it */ public function __construct( - public string $objectHash, - public ObjectType $objectType, - public string $tagName, public ?string $taggerName, public ?string $taggerEmail, public ?\DateTimeImmutable $taggerDate, @@ -43,12 +40,12 @@ public function __construct( /** * @param string $signedMessage The message of the tag object, followed by the signature git appends to it, if any */ - public static function fromSignedMessage(string $objectHash, ObjectType $objectType, string $tagName, ?string $taggerName, ?string $taggerEmail, ?\DateTimeImmutable $taggerDate, string $signedMessage): self + public static function fromSignedMessage(?string $taggerName, ?string $taggerEmail, ?\DateTimeImmutable $taggerDate, string $signedMessage): self { if (!preg_match('/^(.*?)'.self::SIGNATURE.'(.*?)-----END \2-----/s', $signedMessage, $vars)) { - return new self($objectHash, $objectType, $tagName, $taggerName, $taggerEmail, $taggerDate, $signedMessage, null); + return new self($taggerName, $taggerEmail, $taggerDate, $signedMessage, null); } - return new self($objectHash, $objectType, $tagName, $taggerName, $taggerEmail, $taggerDate, $vars[1], $vars[3]); + return new self($taggerName, $taggerEmail, $taggerDate, $vars[1], $vars[3]); } } diff --git a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php index c72b6d3..fb569e0 100644 --- a/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php +++ b/src/Gitonomy/Git/Driver/Ffi/LibGit2RepositoryDriver.php @@ -326,9 +326,6 @@ public function getTagData(string $hash): TagData $tagger = $this->ffi->git_tag_tagger($tag); return TagData::fromSignedMessage( - $this->oidToHex($this->ffi->git_tag_target_id($tag)), - $this->objectType($this->ffi->git_tag_target_type($tag)), - $this->ffi->git_tag_name($tag), null === $tagger ? null : \FFI::string($tagger->name), null === $tagger ? null : \FFI::string($tagger->email), null === $tagger ? null : $this->signatureDate($tagger), diff --git a/src/Gitonomy/Git/Driver/Ffi/libgit2.h b/src/Gitonomy/Git/Driver/Ffi/libgit2.h index 029005f..e880474 100644 --- a/src/Gitonomy/Git/Driver/Ffi/libgit2.h +++ b/src/Gitonomy/Git/Driver/Ffi/libgit2.h @@ -168,9 +168,6 @@ uint64_t git_blob_rawsize(const git_blob *blob); int git_tag_lookup(git_tag **out, git_repository *repo, const git_oid *id); void git_tag_free(git_tag *tag); -const git_oid *git_tag_target_id(const git_tag *tag); -git_object_t git_tag_target_type(const git_tag *tag); -const char *git_tag_name(const git_tag *tag); const git_signature *git_tag_tagger(const git_tag *tag); const char *git_tag_message(const git_tag *tag); From 49d974a1f5c950067abeb8f012b840fe930c3104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 19:44:13 +0200 Subject: [PATCH 21/22] Reword the exception messages the review found unclear - A revision starting with a dash names the revision, without the "^{commit}" gitlib adds: Revision::getCommit() checks it first. - A git:// URL whose host starts with a dash names the command of core.gitProxy, not ssh only. - getIncludingBranches(false, false) says what to do, without "a least". - A timeout of one second is "1 second", and a timeout git reaches while it starts is a ProcessTimedOutException, instead of a DriverException holding the command line. - A single unknown option of Repository is "Unknown option". - getLastModification() of a path no commit changes names the path. - The ProcessException of a merge in conflict has the first CONFLICT line git writes on its output, instead of nothing. --- UPGRADE-2.0.md | 9 +++-- src/Gitonomy/Git/Commit.php | 11 +++++- src/Gitonomy/Git/Driver/Cli/GitRunner.php | 10 ++++- src/Gitonomy/Git/Driver/Cli/LegacyOptions.php | 2 +- .../Git/Exception/ProcessException.php | 5 +++ src/Gitonomy/Git/Revision.php | 3 ++ src/Gitonomy/Git/Util/RemoteUrl.php | 2 +- tests/Gitonomy/Git/Tests/CommitTest.php | 9 +++++ .../Git/Tests/ProcessExceptionTest.php | 25 ++++++++++++ tests/Gitonomy/Git/Tests/RemoteUrlTest.php | 2 +- tests/Gitonomy/Git/Tests/RepositoryTest.php | 38 +++++++++++++++++++ tests/Gitonomy/Git/Tests/RevisionTest.php | 12 ++++++ 12 files changed, 118 insertions(+), 10 deletions(-) diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index 8d922de..312308f 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -405,10 +405,11 @@ Errors the first line of its error output starting with `fatal:` or `error:`, after the warnings, or its first line: `The git command "clone" failed (exit 128): fatal: ...`, or `The git command "diff" failed (exit 1).` - when git writes no error. The command line, the rest of the error output - and the output are no longer in the message: `getErrorOutput()` and - `getOutput()` return the outputs of git. Its constructor takes the git - command as second argument, optional. + when git writes no error, but for the conflict of a merge, which git + writes on its output (`CONFLICT (content): ...`). The command line, the + rest of the error output and the output are no longer in the message: + `getErrorOutput()` and `getOutput()` return the outputs of git. Its + constructor takes the git command as second argument, optional. * When git can't start, a missing binary for instance, `CliDriver` throws a `DriverException` instead of a `ProcessException`, with a message of one line, without the command line: `The git command "clone" could not diff --git a/src/Gitonomy/Git/Commit.php b/src/Gitonomy/Git/Commit.php index e57b42f..22b1f29 100644 --- a/src/Gitonomy/Git/Commit.php +++ b/src/Gitonomy/Git/Commit.php @@ -136,13 +136,20 @@ public function getTree(): Tree return $this->tree ??= $this->repository->getTree($this->getTreeHash()); } + /** + * @throws ReferenceNotFoundException No commit up to this one changes $path + */ public function getLastModification(?string $path = null): self { if (null !== $path && str_starts_with($path, '/')) { $path = substr($path, 1); } - return $this->getLog($path, null, 1)->getSingleCommit(); + try { + return $this->getLog($path, null, 1)->getSingleCommit(); + } catch (ReferenceNotFoundException $e) { + throw new ReferenceNotFoundException(\sprintf('No commit up to "%s" changes "%s".', $this->revision, $path), 0, $e); + } } /** @@ -195,7 +202,7 @@ public function getIncludingBranches(bool $local = true, bool $remote = true): a } if (!$local && !$remote) { - throw new InvalidArgumentException('You should a least set one argument to true.'); + throw new InvalidArgumentException('Keep the local branches, the remote ones, or both: $local and $remote can\'t both be false.'); } $fullnames = $this->repository->getRepositoryDriver()->getIncludingBranches($this->revision); diff --git a/src/Gitonomy/Git/Driver/Cli/GitRunner.php b/src/Gitonomy/Git/Driver/Cli/GitRunner.php index 268343e..7cf1bb5 100644 --- a/src/Gitonomy/Git/Driver/Cli/GitRunner.php +++ b/src/Gitonomy/Git/Driver/Cli/GitRunner.php @@ -72,6 +72,9 @@ public function run(string $command, #[\SensitiveParameter] array $args, array $ // command line, with the credentials of its URLs. try { $process->start(); + } catch (SymfonyProcessTimedOutException) { + // A timeout shorter than the start of git. + throw $this->timedOut($command); } catch (SymfonyRuntimeException $e) { // Too many arguments (E2BIG), a working directory removed since the // repository was opened... @@ -88,7 +91,7 @@ public function run(string $command, #[\SensitiveParameter] array $args, array $ try { $process->wait(); } catch (SymfonyProcessTimedOutException) { - throw new ProcessTimedOutException(\sprintf('The git command "%s" ran longer than %s seconds.', $command, $this->timeout)); + throw $this->timedOut($command); } catch (SymfonyRuntimeException $e) { // Killed by a signal... throw new DriverException(Credentials::redact(\sprintf('Unable to run the git command "%s": %s', $command, $e->getMessage()))); @@ -125,4 +128,9 @@ private static function notStarted(string $command, string $error): DriverExcept return new DriverException(Credentials::redact(1 === preg_match('/[.!?]$/', $message) ? $message : $message.'.')); } + + private function timedOut(string $command): ProcessTimedOutException + { + return new ProcessTimedOutException(\sprintf('The git command "%s" ran longer than %s %s.', $command, $this->timeout, 1.0 === $this->timeout ? 'second' : 'seconds')); + } } diff --git a/src/Gitonomy/Git/Driver/Cli/LegacyOptions.php b/src/Gitonomy/Git/Driver/Cli/LegacyOptions.php index 5c9bb75..d4effff 100644 --- a/src/Gitonomy/Git/Driver/Cli/LegacyOptions.php +++ b/src/Gitonomy/Git/Driver/Cli/LegacyOptions.php @@ -52,7 +52,7 @@ public function __construct(array $options) $unknownOptions = array_diff(array_keys($options), [...self::DRIVER_OPTIONS, 'debug', 'working_dir']); if ([] !== $unknownOptions) { - throw new InvalidArgumentException(\sprintf('Unknown options "%s".', implode('", "', $unknownOptions))); + throw new InvalidArgumentException(\sprintf(1 === \count($unknownOptions) ? 'Unknown option "%s".' : 'Unknown options "%s".', implode('", "', $unknownOptions))); } // PHP doesn't check the types of the options, unlike PHPStan. diff --git a/src/Gitonomy/Git/Exception/ProcessException.php b/src/Gitonomy/Git/Exception/ProcessException.php index b766846..44992fd 100644 --- a/src/Gitonomy/Git/Exception/ProcessException.php +++ b/src/Gitonomy/Git/Exception/ProcessException.php @@ -36,6 +36,11 @@ public function __construct( // local changes ... would be overwritten by checkout:". $errorOutput = $process->getErrorOutput(); $error = 1 === preg_match('/^(?:fatal|error):.*$/m', $errorOutput, $matches) ? $matches[0] : explode("\n", trim($errorOutput), 2)[0]; + // A merge writes its conflicts on the standard output, like + // "CONFLICT (content): Merge conflict in README.md". + if ('' === trim($errorOutput) && 1 === preg_match('/^CONFLICT .*$/m', $process->getOutput(), $matches)) { + $error = $matches[0]; + } $error = rtrim(trim($error), ':'); if ('' !== $error) { $message .= ': '.$error; diff --git a/src/Gitonomy/Git/Revision.php b/src/Gitonomy/Git/Revision.php index 3e13120..1fe5b36 100644 --- a/src/Gitonomy/Git/Revision.php +++ b/src/Gitonomy/Git/Revision.php @@ -14,6 +14,7 @@ use Gitonomy\Git\Exception\InvalidArgumentException; use Gitonomy\Git\Exception\ReferenceNotFoundException; +use Gitonomy\Git\Util\LeadingDash; /** * A name git resolves to a commit: a branch, a tag, a hash, "HEAD~2"... @@ -52,6 +53,8 @@ public function getLog(array|string|null $paths = null, ?int $offset = null, ?in public function getCommit(): Commit { $driver = $this->repository->getRepositoryDriver(); + // Before the driver gets "^{commit}", which its message would quote. + LeadingDash::assertNone($this->revision); try { // "^{commit}" would be part of the text of ":/text", the commit diff --git a/src/Gitonomy/Git/Util/RemoteUrl.php b/src/Gitonomy/Git/Util/RemoteUrl.php index dda2434..b29b3c8 100644 --- a/src/Gitonomy/Git/Util/RemoteUrl.php +++ b/src/Gitonomy/Git/Util/RemoteUrl.php @@ -55,7 +55,7 @@ public static function assertSafe(#[\SensitiveParameter] string $url): void // the host of a git URL the same way. A valid host never starts with a dash. $authority = self::authority($url); if (null !== $authority && 1 === preg_match('/(?:^|@)\[?-/', $authority)) { - throw new InvalidArgumentException(\sprintf('"%s" has a host or a user starting with a dash, which ssh would read as an option.', Credentials::redact($url))); + throw new InvalidArgumentException(\sprintf('"%s" has a host or a user starting with a dash, which ssh, or the command of core.gitProxy, would read as an option.', Credentials::redact($url))); } } diff --git a/tests/Gitonomy/Git/Tests/CommitTest.php b/tests/Gitonomy/Git/Tests/CommitTest.php index 5e1800c..0fe8ec0 100644 --- a/tests/Gitonomy/Git/Tests/CommitTest.php +++ b/tests/Gitonomy/Git/Tests/CommitTest.php @@ -376,12 +376,21 @@ public function testGetIncludingBranchesException(bool $bare): void { $repository = self::createFoobarRepository($bare); $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Keep the local branches, the remote ones, or both: $local and $remote can\'t both be false.'); $commit = $repository->getCommit(self::INITIAL_COMMIT); $commit->getIncludingBranches(false, false); } + public function testTheLastModificationOfAPathNoCommitChanges(): void + { + $this->expectException(ReferenceNotFoundException::class); + $this->expectExceptionMessage(\sprintf('No commit up to "%s" changes "nope".', self::LONGFILE_COMMIT)); + + self::createFoobarRepository()->getCommit(self::LONGFILE_COMMIT)->getLastModification('nope'); + } + #[DataProvider('provideBare')] public function testGetIncludingBranches(bool $bare): void { diff --git a/tests/Gitonomy/Git/Tests/ProcessExceptionTest.php b/tests/Gitonomy/Git/Tests/ProcessExceptionTest.php index 512d2c0..652319b 100644 --- a/tests/Gitonomy/Git/Tests/ProcessExceptionTest.php +++ b/tests/Gitonomy/Git/Tests/ProcessExceptionTest.php @@ -89,6 +89,31 @@ public static function provideErrorOutputs(): iterable yield 'no error' => ["\nwarning: something\nmerge: nope - not something we can merge\n", 'The git command "merge" failed (exit 128): warning: something.']; } + /** + * git writes the conflicts of a merge on its standard output. + */ + public function testTheMessageOfAConflictHasIt(): void + { + $this->skipUnlessCliDriver(); + + $repository = self::createFoobarRepository(false); + self::git($repository, 'config', ['user.name', 'Unit Test']); + self::git($repository, 'config', ['user.email', 'unit_test@unit-test.com']); + file_put_contents($repository->getWorkingDir().'/README.md', "mine\n"); + self::git($repository, 'commit', ['-q', '-a', '-m', 'Mine']); + self::git($repository, 'checkout', ['-q', '-b', 'theirs', 'HEAD~1']); + file_put_contents($repository->getWorkingDir().'/README.md', "theirs\n"); + self::git($repository, 'commit', ['-q', '-a', '-m', 'Theirs']); + self::git($repository, 'checkout', ['-q', 'master']); + + try { + $repository->getWorkingCopy()->merge('theirs'); + $this->fail('Both sides change README.md.'); + } catch (ProcessException $e) { + $this->assertSame('The git command "merge" failed (exit 1): CONFLICT (content): Merge conflict in README.md.', $e->getMessage()); + } + } + public function testTheMessageOfAFailureWithoutErrorOutputEndsWithTheExitCode(): void { $this->skipUnlessCliDriver(); diff --git a/tests/Gitonomy/Git/Tests/RemoteUrlTest.php b/tests/Gitonomy/Git/Tests/RemoteUrlTest.php index 5df1f4a..4c05b7e 100644 --- a/tests/Gitonomy/Git/Tests/RemoteUrlTest.php +++ b/tests/Gitonomy/Git/Tests/RemoteUrlTest.php @@ -111,7 +111,7 @@ public function testAHostStartingWithADashIsRefused(\Closure $call, string $url) $call($url, $dir.'/clone'); $this->fail('A host starting with a dash is refused.'); } catch (InvalidArgumentException $e) { - $this->assertSame(\sprintf('"%s" has a host or a user starting with a dash, which ssh would read as an option.', Credentials::redact($url)), $e->getMessage()); + $this->assertSame(\sprintf('"%s" has a host or a user starting with a dash, which ssh, or the command of core.gitProxy, would read as an option.', Credentials::redact($url)), $e->getMessage()); } finally { chdir($previousCwd); } diff --git a/tests/Gitonomy/Git/Tests/RepositoryTest.php b/tests/Gitonomy/Git/Tests/RepositoryTest.php index a03f271..29d048a 100644 --- a/tests/Gitonomy/Git/Tests/RepositoryTest.php +++ b/tests/Gitonomy/Git/Tests/RepositoryTest.php @@ -311,6 +311,34 @@ public function testTimeout(): void $repository->run('commit', ['--allow-empty', '-m', 'Waits for its hook']); } + public function testATimeoutOfOneSecond(): void + { + $repository = self::createFoobarRepository(false); + $repository->getHooks()->set('pre-commit', "#!/bin/sh\nsleep 10\n"); + $repository = new Repository($repository->getPath(), driver: self::getCliDriver(['GIT_AUTHOR_NAME' => 'A', 'GIT_AUTHOR_EMAIL' => 'a@example.com', 'GIT_COMMITTER_NAME' => 'A', 'GIT_COMMITTER_EMAIL' => 'a@example.com'], processTimeout: 1)); + + $this->expectException(ProcessTimedOutException::class); + $this->expectExceptionMessage('The git command "commit" ran longer than 1 second.'); + + $repository->run('commit', ['--allow-empty', '-m', 'Waits for its hook']); + } + + /** + * Symfony Process checks the timeout when git starts too: its exception + * held the command line. + */ + public function testATimeoutWhileGitStarts(): void + { + $repository = new Repository(self::createFoobarRepository()->getPath(), driver: self::getCliDriver(processTimeout: 0.000001)); + + try { + $repository->run('log', ['-n', '1']); + $this->markTestSkipped('git started within the timeout.'); + } catch (ProcessTimedOutException $e) { + $this->assertSame('The git command "log" ran longer than 1.0E-6 seconds.', $e->getMessage()); + } + } + public function testIsValidRepositoryAndBranchMatchesTheNameExactly(): void { $repository = self::createFoobarRepository(); @@ -951,6 +979,16 @@ public function testLegacySetLoggerRequiresCliDriver(): void new Repository(self::createTempDir(), driver: $this->createStub(DriverInterface::class))->setLogger($this->createStub(LoggerInterface::class)); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testAnUnknownOptionIsRefused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Unknown option "foo".'); + + new Repository(self::createTempDir(), ['foo' => 1]); + } + #[Group('legacy')] #[IgnoreDeprecations] public function testUnknownOptionsAreRefused(): void diff --git a/tests/Gitonomy/Git/Tests/RevisionTest.php b/tests/Gitonomy/Git/Tests/RevisionTest.php index b0b267d..05532b3 100644 --- a/tests/Gitonomy/Git/Tests/RevisionTest.php +++ b/tests/Gitonomy/Git/Tests/RevisionTest.php @@ -14,6 +14,7 @@ use Gitonomy\Git\Commit; use Gitonomy\Git\Driver\Cli\CliDriver; +use Gitonomy\Git\Exception\InvalidArgumentException; use Gitonomy\Git\Exception\ReferenceNotFoundException; use PHPUnit\Framework\Attributes\DataProvider; @@ -81,6 +82,17 @@ public function testALogOfARangeFromAt(): void $this->assertSame($hashes('HEAD~2..HEAD'), $hashes('@~2..@')); } + /** + * The message names the revision, without the "^{commit}" gitlib adds. + */ + public function testARevisionStartingWithADashIsRefused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('"--output=/x" starts with a dash, which git would read as an option.'); + + self::createFoobarRepository()->getRevision('--output=/x')->getCommit(); + } + #[DataProvider('provideBare')] public function testGetLog(bool $bare): void { From 2bd07b2f67f1c23573e6f2882c2e8594b417a899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 14 Oct 2026 19:46:42 +0200 Subject: [PATCH 22/22] Test every deprecation Eleven deprecated methods ran in no test: File::addChange(), isChangeMode(), isDelete() and getAnchor(), FileChange::getCount(), Diff::setRepository(), PushReference::getReference() and getRevision(), Commit::getFixedShortHash(), Log::setOffset() and Repository::shell(). Nothing checked the message of the deprecation of CommitReference either. Nothing fixed their behavior until 3.0: each gets a legacy test, CommitReference in a process of its own, as Submodule declares its alias once loaded, and shell() through a command of ~/git-shell-commands. --- tests/Gitonomy/Git/Tests/CommitTest.php | 9 +++ tests/Gitonomy/Git/Tests/DiffTest.php | 64 +++++++++++++++++++ tests/Gitonomy/Git/Tests/LogTest.php | 12 ++++ .../Gitonomy/Git/Tests/PushReferenceTest.php | 18 ++++++ tests/Gitonomy/Git/Tests/RepositoryTest.php | 26 ++++++++ tests/Gitonomy/Git/Tests/TreeTest.php | 15 +++++ 6 files changed, 144 insertions(+) diff --git a/tests/Gitonomy/Git/Tests/CommitTest.php b/tests/Gitonomy/Git/Tests/CommitTest.php index 0fe8ec0..a3dba6e 100644 --- a/tests/Gitonomy/Git/Tests/CommitTest.php +++ b/tests/Gitonomy/Git/Tests/CommitTest.php @@ -321,6 +321,15 @@ public function testGetSignedMessage(bool $bare): void $this->assertSame('signed commit'."\n", $commit->getMessage()); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyGetFixedShortHash(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Commit::getFixedShortHash()" method is deprecated, use "substr($commit->getHash(), 0, $length)" instead.'); + + $this->assertSame(substr(self::LONGFILE_COMMIT, 0, 6), self::createFoobarRepository()->getCommit(self::LONGFILE_COMMIT)->getFixedShortHash()); + } + #[DataProvider('provideBare')] #[Group('legacy')] #[IgnoreDeprecations] diff --git a/tests/Gitonomy/Git/Tests/DiffTest.php b/tests/Gitonomy/Git/Tests/DiffTest.php index 05b95bd..4ce9a8a 100644 --- a/tests/Gitonomy/Git/Tests/DiffTest.php +++ b/tests/Gitonomy/Git/Tests/DiffTest.php @@ -515,6 +515,70 @@ public function testLegacySetRepository(): void $this->assertSame($repository, $file->getRepository()); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyDiffSetRepository(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\Diff::setRepository()" method is deprecated, the diffs gitlib returns have their repository already.'); + + $repository = self::createFoobarRepository(); + $diff = Diff::parse(":100644 100644 1111111111111111111111111111111111111111 2222222222222222222222222222222222222222 M\tREADME.md\n"); + $diff->setRepository($repository); + + $this->assertSame($repository, $diff->getFiles()[0]->getRepository()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyAddChange(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\File::addChange()" method is deprecated, give the changes to the constructor instead.'); + + $file = new File('README.md', 'README.md', '100644', '100644', null, null, false); + $change = new FileChange(1, 1, 1, 1, [new DiffLine(DiffLineType::Addition, 'added')]); + $file->addChange($change); + + $this->assertSame([$change], $file->getChanges()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyIsChangeMode(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\File::isChangeMode()" method is deprecated, use "isModeChange()" instead.'); + + $this->assertTrue(new File('run.sh', 'run.sh', '100644', '100755', null, null, false)->isChangeMode()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyFileIsDelete(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\File::isDelete()" method is deprecated, use "isDeletion()" instead.'); + + $this->assertTrue(new File('README.md', null, '100644', null, null, null, false)->isDelete()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyGetAnchor(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\File::getAnchor()" method is deprecated, use "substr($file->getNewIndex(), 0, 12)" instead.'); + + $this->assertSame('222222222222', new File('README.md', 'README.md', '100644', '100644', str_repeat('1', 40), str_repeat('2', 40), false)->getAnchor()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyGetCount(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Diff\FileChange::getCount()" method is deprecated, use "getAdditions()" or "getDeletions()" instead.'); + + $change = new FileChange(1, 1, 1, 2, [new DiffLine(DiffLineType::Context, 'kept'), new DiffLine(DiffLineType::Addition, 'added')]); + + $this->assertSame(1, $change->getCount(FileChange::LINE_ADD)); + } + public function testMnemonicPrefix(): void { // With `diff.mnemonicPrefix` enabled, git replaces the default "a/" and "b/" diff --git a/tests/Gitonomy/Git/Tests/LogTest.php b/tests/Gitonomy/Git/Tests/LogTest.php index 2c7e684..ebca07c 100644 --- a/tests/Gitonomy/Git/Tests/LogTest.php +++ b/tests/Gitonomy/Git/Tests/LogTest.php @@ -335,6 +335,18 @@ public function testWithersReturnAnotherLog(bool $bare): void $this->assertSame(\array_slice(self::hashes($log), 0, 2), self::hashes($limited)); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacySetOffset(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Log::setOffset()" method is deprecated, use "withOffset()" instead.'); + + $log = self::createFoobarRepository()->getLog(); + $log->setOffset(2); + + $this->assertSame(2, $log->getOffset()); + } + #[Group('legacy')] #[IgnoreDeprecations] public function testLegacySetLimit(): void diff --git a/tests/Gitonomy/Git/Tests/PushReferenceTest.php b/tests/Gitonomy/Git/Tests/PushReferenceTest.php index 0f4c623..191bbfc 100644 --- a/tests/Gitonomy/Git/Tests/PushReferenceTest.php +++ b/tests/Gitonomy/Git/Tests/PushReferenceTest.php @@ -67,6 +67,24 @@ public function testLegacyIsDelete(): void $this->assertTrue(new PushReference(self::createFoobarRepository(), 'foo', self::LONGFILE_COMMIT, PushReference::ZERO)->isDelete()); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyGetReference(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\PushReference::getReference()" method is deprecated, use "getFullname()" instead.'); + + $this->assertSame('refs/heads/foo', new PushReference(self::createFoobarRepository(), 'refs/heads/foo', self::BEFORE_LONGFILE_COMMIT, self::LONGFILE_COMMIT)->getReference()); + } + + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyGetRevision(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\PushReference::getRevision()" method is deprecated, use "getRange()" instead.'); + + $this->assertSame(self::BEFORE_LONGFILE_COMMIT.'..'.self::LONGFILE_COMMIT, new PushReference(self::createFoobarRepository(), 'refs/heads/foo', self::BEFORE_LONGFILE_COMMIT, self::LONGFILE_COMMIT)->getRevision()); + } + public function testUnknownBeforeIsAnErrorNotAForcePush(): void { $reference = new PushReference(self::createFoobarRepository(), 'foo', str_repeat('1', 40), self::LONGFILE_COMMIT); diff --git a/tests/Gitonomy/Git/Tests/RepositoryTest.php b/tests/Gitonomy/Git/Tests/RepositoryTest.php index 29d048a..7a907b1 100644 --- a/tests/Gitonomy/Git/Tests/RepositoryTest.php +++ b/tests/Gitonomy/Git/Tests/RepositoryTest.php @@ -979,6 +979,32 @@ public function testLegacySetLoggerRequiresCliDriver(): void new Repository(self::createTempDir(), driver: $this->createStub(DriverInterface::class))->setLogger($this->createStub(LoggerInterface::class)); } + #[Group('legacy')] + #[IgnoreDeprecations] + public function testLegacyShell(): void + { + if (\defined('PHP_WINDOWS_VERSION_BUILD')) { + $this->markTestSkipped('The command of git shell is a shell script.'); + } + + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\Repository::shell()" method is deprecated, run "git shell" with Symfony Process instead: it ignores the driver.'); + + // git shell runs the commands of ~/git-shell-commands. + $home = self::createTempDir(); + mkdir($home.'/git-shell-commands'); + file_put_contents($home.'/git-shell-commands/record', "#!/bin/sh\n".'echo "$1" > "$HOME/recorded"'."\n"); + chmod($home.'/git-shell-commands/record', 0o700); + $repository = self::createFoobarRepository(); + + $repository->shell('record', ['HOME' => $home]); + + // shell() doesn't wait for git. + for ($i = 0; $i < 100 && !is_file($home.'/recorded'); ++$i) { + usleep(50_000); + } + $this->assertStringEqualsFile($home.'/recorded', $repository->getGitDir()."\n"); + } + #[Group('legacy')] #[IgnoreDeprecations] public function testAnUnknownOptionIsRefused(): void diff --git a/tests/Gitonomy/Git/Tests/TreeTest.php b/tests/Gitonomy/Git/Tests/TreeTest.php index ca21b85..e702aaf 100644 --- a/tests/Gitonomy/Git/Tests/TreeTest.php +++ b/tests/Gitonomy/Git/Tests/TreeTest.php @@ -20,6 +20,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\IgnoreDeprecations; +use PHPUnit\Framework\Attributes\RunInSeparateProcess; class TreeTest extends AbstractTestCase { @@ -75,6 +76,20 @@ public function testGetSubmoduleEntries(bool $bare): void $this->assertSame('160000', $submodules['barbaz']->getMode()); } + /** + * In a process of its own: once Submodule is loaded, it declares the alias, + * whose file isn't loaded anymore. + */ + #[Group('legacy')] + #[IgnoreDeprecations] + #[RunInSeparateProcess] + public function testLegacyCommitReference(): void + { + $this->expectUserDeprecationMessage('Since gitonomy/gitlib 2.0: The "Gitonomy\Git\CommitReference" class is deprecated, use "Gitonomy\Git\Submodule" instead.'); + + $this->assertTrue(class_exists(CommitReference::class)); + } + #[DataProvider('provideBare')] #[Group('legacy')] #[IgnoreDeprecations]