Fix what the last review before 2.0 found

.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
 

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.
@@ -55,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));
     ```
@@ -114,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));
     ```
@@ -170,6 +174,8 @@ References
     After:
 
     ```php
+    use Gitonomy\Git\Reference\Branch;
+
     $branches = array_filter($commit->getIncludingBranches(), static fn (Branch $branch): bool => $branch->isLocal());
     ```
 
@@ -200,6 +206,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
@@ -322,23 +329,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
 ------------------------
 
@@ -406,14 +396,20 @@ 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
     (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
@@ -428,9 +424,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.
@@ -442,8 +440,10 @@ 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.
+    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,
@@ -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
 -----
@@ -521,16 +523,28 @@ 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
     ["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`. `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`
@@ -558,13 +572,20 @@ 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
     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
 -------------

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": {

doc/admin.md

@@ -101,13 +101,17 @@ 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:
 
+-   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
     `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

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
 --------------

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.

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`.
@@ -165,7 +167,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();
@@ -237,9 +240,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
@@ -260,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
 -----------------
@@ -352,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
@@ -492,6 +499,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 +513,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
 ------
@@ -575,3 +593,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.

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
 ------------------------------