diff options
47 files changed, 961 insertions, 578 deletions
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 683a2d633e..3428773b09 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -159,7 +159,7 @@ jobs: if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v4 with: - name: failed-tests-windows + name: failed-tests-windows-${{ matrix.nr }} path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: name: win+VS build @@ -250,7 +250,7 @@ jobs: if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v4 with: - name: failed-tests-windows + name: failed-tests-windows-vs-${{ matrix.nr }} path: ${{env.FAILED_TEST_ARTIFACTS}} regular: name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}}) diff --git a/Documentation/RelNotes/2.45.0.txt b/Documentation/RelNotes/2.45.0.txt index c7c1ff45e9..b780bb89c6 100644 --- a/Documentation/RelNotes/2.45.0.txt +++ b/Documentation/RelNotes/2.45.0.txt @@ -37,6 +37,14 @@ UI, Workflows & Features * Platform specific tweaks for OS/390 has been added to config.mak.uname. + * Users with safe.bareRepository=explicit can still work from within + $GIT_DIR of a seconary worktree (which resides at .git/worktrees/$name/) + of the primary worktree without explicitly specifying the $GIT_DIR + environment variable or the --git-dir=<path> option. + + * The output format for dates "iso-strict" has been tweaked to show + a time in the Zulu timezone with "Z" suffix, instead of "+00:00". + Performance, Internal Implementation, Development Support etc. @@ -67,6 +75,24 @@ Performance, Internal Implementation, Development Support etc. * Uses of xwrite() helper have been audited and updated for better error checking and simpler code. + * Some trace2 events that lacked def_param have learned to show it, + enriching the output. + + * The parse-options code that deals with abbreviated long option + names have been cleaned up. + + * The code in reftable backend that creates new table files works + better with the tempfile framework to avoid leaving cruft after a + failure. + + * The reftable code has its own custom binary search function whose + comparison callback has an unusual interface, which caused the + binary search to degenerate into a linear search, which has been + corrected. + + * The code to iterate over reflogs in the reftable has been optimized + to reduce memory allocation and deallocation. + Fixes since v2.44 ----------------- @@ -173,6 +199,21 @@ Fixes since v2.44 the user to exact naming rules. (merge 8fbd903e58 kh/branch-ref-syntax-advice later to maint). + * Code simplification by getting rid of code that sets an environment + variable that is no longer used. + (merge 72a8d3f027 pw/rebase-i-ignore-cherry-pick-help-environment later to maint). + + * The code to find the effective end of log message can fall into an + endless loop, which has been corrected. + (merge 2541cba2d6 fs/find-end-of-log-message-fix later to maint). + + * Mark-ups used in the documentation has been improved for + consistency. + (merge 45d5ed3e50 ja/doc-markup-fixes later to maint). + + * The status.showUntrackedFiles configuration variable was + incorrectly documented to accept "false", which has been corrected. + * Other code cleanup, docfix, build fix, etc. (merge f0e578c69c rs/use-xstrncmpz later to maint). (merge 83e6eb7d7a ba/credential-test-clean-fix later to maint). @@ -190,3 +231,5 @@ Fixes since v2.44 (merge 3223204456 eg/add-uflags later to maint). (merge 5f78d52dce es/config-doc-sort-sections later to maint). (merge 781fb7b4c2 as/option-names-in-messages later to maint). + (merge 51d41dc243 jk/doc-remote-helpers-markup-fix later to maint). + (merge e1aaf309db pb/ci-win-artifact-names-fix later to maint). diff --git a/Documentation/config/init.txt b/Documentation/config/init.txt index 79c79d6617..dd1d833273 100644 --- a/Documentation/config/init.txt +++ b/Documentation/config/init.txt @@ -1,7 +1,10 @@ -init.templateDir:: - Specify the directory from which templates will be copied. - (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].) +:see-git-init: +ifndef::git-init[] +:see-git-init: (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].) +endif::[] +init.templateDir:: + Specify the directory from which templates will be copied. {see-git-init} init.defaultBranch:: Allows overriding the default branch name e.g. when initializing a new repository. diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index ca626f7fc6..112658b3c3 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -8,7 +8,8 @@ git-bugreport - Collect information for user to file a bug report SYNOPSIS -------- [verse] -'git bugreport' [(-o | --output-directory) <path>] [(-s | --suffix) <format>] +'git bugreport' [(-o | --output-directory) <path>] + [(-s | --suffix) <format> | --no-suffix] [--diagnose[=<mode>]] DESCRIPTION @@ -51,9 +52,12 @@ OPTIONS -s <format>:: --suffix <format>:: +--no-suffix:: Specify an alternate suffix for the bugreport name, to create a file named 'git-bugreport-<formatted-suffix>'. This should take the form of a strftime(3) format string; the current local time will be used. + `--no-suffix` disables the suffix and the file is just named + `git-bugreport` without any disambiguation measure. --no-diagnose:: --diagnose[=<mode>]:: diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 0c07720c6f..f90977a851 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -102,9 +102,9 @@ its source repository, you can simply run `git repack -a` to copy all objects from the source repository into a pack in the cloned repository. --reference[-if-able] <repository>:: - If the reference repository is on the local machine, + If the reference _<repository>_ is on the local machine, automatically setup `.git/objects/info/alternates` to - obtain objects from the reference repository. Using + obtain objects from the reference _<repository>_. Using an already existing repository as an alternate will require fewer objects to be copied from the repository being cloned, reducing network and local storage costs. @@ -156,13 +156,13 @@ objects from the source repository into a pack in the cloned repository. --[no-]reject-shallow:: Fail if the source repository is a shallow repository. - The 'clone.rejectShallow' configuration variable can be used to + The `clone.rejectShallow` configuration variable can be used to specify the default. --bare:: Make a 'bare' Git repository. That is, instead of - creating `<directory>` and placing the administrative - files in `<directory>/.git`, make the `<directory>` + creating _<directory>_ and placing the administrative + files in `<directory>/.git`, make the _<directory>_ itself the `$GIT_DIR`. This obviously implies the `--no-checkout` because there is nowhere to check out the working tree. Also the branch heads at the remote are copied directly @@ -180,11 +180,11 @@ objects from the source repository into a pack in the cloned repository. --filter=<filter-spec>:: Use the partial clone feature and request that the server sends a subset of reachable objects according to a given object filter. - When using `--filter`, the supplied `<filter-spec>` is used for + When using `--filter`, the supplied _<filter-spec>_ is used for the partial clone filter. For example, `--filter=blob:none` will filter out all blobs (file contents) until needed by Git. Also, `--filter=blob:limit=<size>` will filter out all blobs of size - at least `<size>`. For more details on filter specifications, see + at least _<size>_. For more details on filter specifications, see the `--filter` option in linkgit:git-rev-list[1]. --also-filter-submodules:: @@ -203,13 +203,13 @@ objects from the source repository into a pack in the cloned repository. -o <name>:: --origin <name>:: Instead of using the remote name `origin` to keep track of the upstream - repository, use `<name>`. Overrides `clone.defaultRemoteName` from the + repository, use _<name>_. Overrides `clone.defaultRemoteName` from the config. -b <name>:: --branch <name>:: Instead of pointing the newly created HEAD to the branch pointed - to by the cloned repository's HEAD, point to `<name>` branch + to by the cloned repository's HEAD, point to _<name>_ branch instead. In a non-bare repository, this is the branch that will be checked out. `--branch` can also take tags and detaches the HEAD at that commit @@ -230,7 +230,7 @@ objects from the source repository into a pack in the cloned repository. Set a configuration variable in the newly-created repository; this takes effect immediately after the repository is initialized, but before the remote history is fetched or any - files checked out. The key is in the same format as expected by + files checked out. The _<key>_ is in the same format as expected by linkgit:git-config[1] (e.g., `core.eol=true`). If multiple values are given for the same key, each value will be written to the config file. This makes it safe, for example, to add @@ -263,7 +263,7 @@ corresponding `--mirror` and `--no-tags` options instead. branch remote's `HEAD` points at. Further fetches into the resulting repository will only update the remote-tracking branch for the branch this option was used for the - initial cloning. If the HEAD at the remote did not point at any + initial cloning. If the `HEAD` at the remote did not point at any branch when `--single-branch` clone was made, no remote-tracking branch is created. @@ -281,7 +281,7 @@ branch of some repository for search indexing. --recurse-submodules[=<pathspec>]:: After the clone is created, initialize and clone submodules - within based on the provided pathspec. If no pathspec is + within based on the provided _<pathspec>_. If no _=<pathspec>_ is provided, all submodules are initialized and cloned. This option can be given multiple times for pathspecs consisting of multiple entries. The resulting clone has `submodule.active` set to @@ -323,20 +323,20 @@ include::ref-storage-format.txt[] Defaults to the `submodule.fetchJobs` option. <repository>:: - The (possibly remote) repository to clone from. See the + The (possibly remote) _<repository>_ to clone from. See the <<URLS,GIT URLS>> section below for more information on specifying repositories. <directory>:: The name of a new directory to clone into. The "humanish" - part of the source repository is used if no directory is + part of the source repository is used if no _<directory>_ is explicitly given (`repo` for `/path/to/repo.git` and `foo` for `host.xz:foo/.git`). Cloning into an existing directory is only allowed if the directory is empty. --bundle-uri=<uri>:: Before fetching from the remote, fetch a bundle from the given - `<uri>` and unbundle the data into the local repository. The refs + _<uri>_ and unbundle the data into the local repository. The refs in the bundle will be stored under the hidden `refs/bundle/*` namespace. This option is incompatible with `--depth`, `--shallow-since`, and `--shallow-exclude`. diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index e8dc645bb5..2f864e11ed 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -33,10 +33,10 @@ If the object storage directory is specified via the are created underneath; otherwise, the default `$GIT_DIR/objects` directory is used. -Running 'git init' in an existing repository is safe. It will not +Running `git init` in an existing repository is safe. It will not overwrite things that are already there. The primary reason for -rerunning 'git init' is to pick up newly added templates (or to move -the repository to another place if --separate-git-dir is given). +rerunning `git init` is to pick up newly added templates (or to move +the repository to another place if `--separate-git-dir` is given). OPTIONS ------- @@ -53,14 +53,14 @@ current working directory. --object-format=<format>:: -Specify the given object format (hash algorithm) for the repository. The valid -values are 'sha1' and (if enabled) 'sha256'. 'sha1' is the default. +Specify the given object _<format>_ (hash algorithm) for the repository. The valid +values are `sha1` and (if enabled) `sha256`. `sha1` is the default. + include::object-format-disclaimer.txt[] --ref-format=<format>:: -Specify the given ref storage format for the repository. The valid values are: +Specify the given ref storage _<format>_ for the repository. The valid values are: + include::ref-storage-format.txt[] @@ -81,7 +81,7 @@ If this is a reinitialization, the repository will be moved to the specified pat -b <branch-name>:: --initial-branch=<branch-name>:: -Use the specified name for the initial branch in the newly created +Use _<branch-name>_ for the initial branch in the newly created repository. If not specified, fall back to the default name (currently `master`, but this is subject to change in the future; the name can be customized via the `init.defaultBranch` configuration variable). @@ -90,40 +90,44 @@ customized via the `init.defaultBranch` configuration variable). Specify that the Git repository is to be shared amongst several users. This allows users belonging to the same group to push into that -repository. When specified, the config variable "core.sharedRepository" is +repository. When specified, the config variable `core.sharedRepository` is set so that files and directories under `$GIT_DIR` are created with the requested permissions. When not specified, Git will use permissions reported -by umask(2). +by `umask(2)`. + -The option can have the following values, defaulting to 'group' if no value +The option can have the following values, defaulting to `group` if no value is given: + -- -'umask' (or 'false'):: +umask:: +false:: Use permissions reported by umask(2). The default, when `--shared` is not specified. -'group' (or 'true'):: +group:: +true:: Make the repository group-writable, (and g+sx, since the git group may not be the primary group of all users). This is used to loosen the permissions of an otherwise safe umask(2) value. Note that the umask still applies to the other -permission bits (e.g. if umask is '0022', using 'group' will not remove read -privileges from other (non-group) users). See '0xxx' for how to exactly specify +permission bits (e.g. if umask is `0022`, using `group` will not remove read +privileges from other (non-group) users). See `0xxx` for how to exactly specify the repository permissions. -'all' (or 'world' or 'everybody'):: +all:: +world:: +everybody:: -Same as 'group', but make the repository readable by all users. +Same as `group`, but make the repository readable by all users. -'<perm>':: +<perm>:: -'<perm>' is a 3-digit octal number prefixed with `0` and each file -will have mode '<perm>'. '<perm>' will override users' umask(2) -value (and not only loosen permissions as 'group' and 'all' -do). '0640' will create a repository which is group-readable, but -not group-writable or accessible to others. '0660' will create a repo +_<perm>_ is a 3-digit octal number prefixed with `0` and each file +will have mode _<perm>_. _<perm>_ will override users'`umask(2)` +value (and not only loosen permissions as `group` and `all` +do). `0640` will create a repository which is group-readable, but +not group-writable or accessible to others. `0660` will create a repo that is readable and writable to the current user and group, but inaccessible to others (directories and executable files get their `x` bit from the `r` bit for corresponding classes of users). @@ -133,7 +137,7 @@ By default, the configuration flag `receive.denyNonFastForwards` is enabled in shared repositories, so that you cannot force a non fast-forwarding push into it. -If you provide a 'directory', the command is run inside it. If this directory +If you provide a _<directory>_, the command is run inside it. If this directory does not exist, it will be created. TEMPLATE DIRECTORY @@ -172,7 +176,7 @@ $ git add . <2> $ git commit <3> ---------------- + -<1> Create a /path/to/my/codebase/.git directory. +<1> Create a `/path/to/my/codebase/.git` directory. <2> Add all existing files to the index. <3> Record the pristine state as the first commit in the history. @@ -181,6 +185,8 @@ CONFIGURATION include::includes/cmd-config-section-all.txt[] +:git-init: + include::config/init.txt[] GIT diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 4dbb88373b..b0f36fabfb 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -472,7 +472,7 @@ again, because your configuration may already be caching `git status` results, so it could be faster on subsequent runs. * The `--untracked-files=no` flag or the - `status.showUntrackedfiles=false` config (see above for both): + `status.showUntrackedFiles=no` config (see above for both): indicate that `git status` should not report untracked files. This is the fastest option. `git status` will not list the untracked files, so you need to be careful to remember if diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ed8da428c9..07c8439a6f 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -526,7 +526,7 @@ set by Git if the remote helper has the 'option' capability. 'option pushcert' {'true'|'false'}:: GPG sign pushes. -'option push-option <string>:: +'option push-option' <string>:: Transmit <string> as a push option. As the push option must not contain LF or NUL characters, the string is not encoded. diff --git a/Documentation/urls.txt b/Documentation/urls.txt index ce671f812d..0b9e0c4302 100644 --- a/Documentation/urls.txt +++ b/Documentation/urls.txt @@ -44,26 +44,26 @@ syntaxes may be used: ifndef::git-clone[] These two syntaxes are mostly equivalent, except when cloning, when -the former implies --local option. See linkgit:git-clone[1] for +the former implies `--local` option. See linkgit:git-clone[1] for details. endif::git-clone[] ifdef::git-clone[] These two syntaxes are mostly equivalent, except the former implies ---local option. +`--local` option. endif::git-clone[] -'git clone', 'git fetch' and 'git pull', but not 'git push', will also +`git clone`, `git fetch` and `git pull`, but not `git push`, will also accept a suitable bundle file. See linkgit:git-bundle[1]. When Git doesn't know how to handle a certain transport protocol, it -attempts to use the 'remote-<transport>' remote helper, if one +attempts to use the `remote-<transport>` remote helper, if one exists. To explicitly request a remote helper, the following syntax may be used: -- <transport>::<address> +- _<transport>_::_<address>_ -where <address> may be a path, a server and path, or an arbitrary +where _<address>_ may be a path, a server and path, or an arbitrary URL-like string recognized by the specific remote helper being invoked. See linkgit:gitremote-helpers[7] for details. diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 6433903491..90a4189358 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -4093,7 +4093,38 @@ that not only specifies their type, but also provides size information about the data in the object. It's worth noting that the SHA-1 hash that is used to name the object is the hash of the original data plus this header, so `sha1sum` 'file' does not match the object name -for 'file'. +for 'file' (the earliest versions of Git hashed slightly differently +but the conclusion is still the same). + +The following is a short example that demonstrates how these hashes +can be generated manually: + +Let's assume a small text file with some simple content: + +------------------------------------------------- +$ echo "Hello world" >hello.txt +------------------------------------------------- + +We can now manually generate the hash Git would use for this file: + +- The object we want the hash for is of type "blob" and its size is + 12 bytes. + +- Prepend the object header to the file content and feed this to + `sha1sum`: + +------------------------------------------------- +$ { printf "blob 12\0"; cat hello.txt; } | sha1sum +802992c4220de19a90767f3000a79a31b98d0df7 - +------------------------------------------------- + +This manually constructed hash can be verified using `git hash-object` +which of course hides the addition of the header: + +------------------------------------------------- +$ git hash-object hello.txt +802992c4220de19a90767f3000a79a31b98d0df7 +------------------------------------------------- As a result, the general consistency of an object can always be tested independently of the contents or the type of the object: all objects can @@ -4123,7 +4154,8 @@ $ git switch --detach e83c5163 ---------------------------------------------------- The initial revision lays the foundation for almost everything Git has -today, but is small enough to read in one sitting. +today (even though details may differ in a few places), but is small +enough to read in one sitting. Note that terminology has changed since that revision. For example, the README in that revision uses the word "changeset" to describe what we diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 3106e56a13..25f860a0d9 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -64,7 +64,8 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) } static const char * const bugreport_usage[] = { - N_("git bugreport [(-o | --output-directory) <path>] [(-s | --suffix) <format>]\n" + N_("git bugreport [(-o | --output-directory) <path>]\n" + " [(-s | --suffix) <format> | --no-suffix]\n" " [--diagnose[=<mode>]]"), NULL }; @@ -138,8 +139,11 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix) strbuf_complete(&report_path, '/'); output_path_len = report_path.len; - strbuf_addstr(&report_path, "git-bugreport-"); - strbuf_addftime(&report_path, option_suffix, localtime_r(&now, &tm), 0, 0); + strbuf_addstr(&report_path, "git-bugreport"); + if (option_suffix) { + strbuf_addch(&report_path, '-'); + strbuf_addftime(&report_path, option_suffix, localtime_r(&now, &tm), 0, 0); + } strbuf_addstr(&report_path, ".txt"); switch (safe_create_leading_directories(report_path.buf)) { diff --git a/builtin/checkout.c b/builtin/checkout.c index 15293a3013..902c97ab23 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1702,10 +1702,11 @@ static char cb_option = 'b'; static int checkout_main(int argc, const char **argv, const char *prefix, struct checkout_opts *opts, struct option *options, - const char * const usagestr[], - struct branch_info *new_branch_info) + const char * const usagestr[]) { int parseopt_flags = 0; + struct branch_info new_branch_info = { 0 }; + int ret; opts->overwrite_ignore = 1; opts->prefix = prefix; @@ -1821,7 +1822,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, opts->track == BRANCH_TRACK_UNSPECIFIED && !opts->new_branch; int n = parse_branchname_arg(argc, argv, dwim_ok, - new_branch_info, opts, &rev); + &new_branch_info, opts, &rev); argv += n; argc -= n; } else if (!opts->accept_ref && opts->from_treeish) { @@ -1830,7 +1831,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, if (repo_get_oid_mb(the_repository, opts->from_treeish, &rev)) die(_("could not resolve %s"), opts->from_treeish); - setup_new_branch_info_and_source_tree(new_branch_info, + setup_new_branch_info_and_source_tree(&new_branch_info, opts, &rev, opts->from_treeish); @@ -1850,7 +1851,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, * Try to give more helpful suggestion. * new_branch && argc > 1 will be caught later. */ - if (opts->new_branch && argc == 1 && !new_branch_info->commit) + if (opts->new_branch && argc == 1 && !new_branch_info.commit) die(_("'%s' is not a commit and a branch '%s' cannot be created from it"), argv[0], opts->new_branch); @@ -1900,9 +1901,16 @@ static int checkout_main(int argc, const char **argv, const char *prefix, } if (opts->patch_mode || opts->pathspec.nr) - return checkout_paths(opts, new_branch_info); + ret = checkout_paths(opts, &new_branch_info); else - return checkout_branch(opts, new_branch_info); + ret = checkout_branch(opts, &new_branch_info); + + branch_info_release(&new_branch_info); + clear_pathspec(&opts->pathspec); + free(opts->pathspec_from_file); + free(options); + + return ret; } int cmd_checkout(int argc, const char **argv, const char *prefix) @@ -1920,8 +1928,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.dwim_new_local_branch = 1; @@ -1951,13 +1957,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) options = add_common_switch_branch_options(&opts, options); options = add_checkout_path_options(&opts, options); - ret = checkout_main(argc, argv, prefix, &opts, - options, checkout_usage, &new_branch_info); - branch_info_release(&new_branch_info); - clear_pathspec(&opts.pathspec); - free(opts.pathspec_from_file); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + checkout_usage); } int cmd_switch(int argc, const char **argv, const char *prefix) @@ -1975,8 +1976,6 @@ int cmd_switch(int argc, const char **argv, const char *prefix) N_("throw away local modifications")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.dwim_new_local_branch = 1; @@ -1995,11 +1994,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix) cb_option = 'c'; - ret = checkout_main(argc, argv, prefix, &opts, - options, switch_branch_usage, &new_branch_info); - branch_info_release(&new_branch_info); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + switch_branch_usage); } int cmd_restore(int argc, const char **argv, const char *prefix) @@ -2018,8 +2014,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.accept_ref = 0; @@ -2034,9 +2028,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix) options = add_common_options(&opts, options); options = add_checkout_path_options(&opts, options); - ret = checkout_main(argc, argv, prefix, &opts, - options, restore_usage, &new_branch_info); - branch_info_release(&new_branch_info); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + restore_usage); } diff --git a/builtin/rebase.c b/builtin/rebase.c index be787690bd..e444ab102d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -567,13 +567,6 @@ static int move_to_original_branch(struct rebase_options *opts) return ret; } -static const char *resolvemsg = -N_("Resolve all conflicts manually, mark them as resolved with\n" -"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n" -"You can instead skip this commit: run \"git rebase --skip\".\n" -"To abort and get back to the state before \"git rebase\", run " -"\"git rebase --abort\"."); - static int run_am(struct rebase_options *opts) { struct child_process am = CHILD_PROCESS_INIT; @@ -587,7 +580,7 @@ static int run_am(struct rebase_options *opts) opts->reflog_action); if (opts->action == ACTION_CONTINUE) { strvec_push(&am.args, "--resolved"); - strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg); + strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg); if (opts->gpg_sign_opt) strvec_push(&am.args, opts->gpg_sign_opt); status = run_command(&am); @@ -598,7 +591,7 @@ static int run_am(struct rebase_options *opts) } if (opts->action == ACTION_SKIP) { strvec_push(&am.args, "--skip"); - strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg); + strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg); status = run_command(&am); if (status) return status; @@ -672,7 +665,7 @@ static int run_am(struct rebase_options *opts) strvec_pushv(&am.args, opts->git_am_opts.v); strvec_push(&am.args, "--rebasing"); - strvec_pushf(&am.args, "--resolvemsg=%s", resolvemsg); + strvec_pushf(&am.args, "--resolvemsg=%s", rebase_resolvemsg); strvec_push(&am.args, "--patch-format=mboxrd"); if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) strvec_push(&am.args, "--rerere-autoupdate"); @@ -700,7 +693,6 @@ static int run_specific_rebase(struct rebase_options *opts) if (opts->type == REBASE_MERGE) { /* Run sequencer-based rebase */ - setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1); if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) setenv("GIT_SEQUENCE_EDITOR", ":", 1); if (opts->gpg_sign_opt) { @@ -342,14 +342,18 @@ const char *show_date(timestamp_t time, int tz, const struct date_mode *mode) tm->tm_hour, tm->tm_min, tm->tm_sec, tz); else if (mode->type == DATE_ISO8601_STRICT) { - char sign = (tz >= 0) ? '+' : '-'; - tz = abs(tz); - strbuf_addf(&timebuf, "%04d-%02d-%02dT%02d:%02d:%02d%c%02d:%02d", + strbuf_addf(&timebuf, "%04d-%02d-%02dT%02d:%02d:%02d", tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, - tm->tm_hour, tm->tm_min, tm->tm_sec, - sign, tz / 100, tz % 100); + tm->tm_hour, tm->tm_min, tm->tm_sec); + if (tz == 0) { + strbuf_addch(&timebuf, 'Z'); + } else { + strbuf_addch(&timebuf, tz >= 0 ? '+' : '-'); + tz = abs(tz); + strbuf_addf(&timebuf, "%02d:%02d", tz / 100, tz % 100); + } } else if (mode->type == DATE_RFC2822) strbuf_addf(&timebuf, "%.3s, %d %.3s %d %02d:%02d:%02d %+05d", weekday_names[tm->tm_wday], tm->tm_mday, @@ -379,8 +379,6 @@ static int handle_alias(int *argcp, const char ***argv) strvec_pushv(&child.args, (*argv) + 1); trace2_cmd_alias(alias_command, child.args.v); - trace2_cmd_list_config(); - trace2_cmd_list_env_vars(); trace2_cmd_name("_run_shell_alias_"); ret = run_command(&child); @@ -417,8 +415,6 @@ static int handle_alias(int *argcp, const char ***argv) COPY_ARRAY(new_argv + count, *argv + 1, *argcp); trace2_cmd_alias(alias_command, new_argv); - trace2_cmd_list_config(); - trace2_cmd_list_env_vars(); *argv = new_argv; *argcp += count - 1; @@ -468,8 +464,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) trace_argv_printf(argv, "trace: built-in: git"); trace2_cmd_name(p->cmd); - trace2_cmd_list_config(); - trace2_cmd_list_env_vars(); validate_cache_entries(the_repository->index); status = p->fn(argc, argv, prefix); diff --git a/lockfile.h b/lockfile.h index 90af4e66b2..1bb9926497 100644 --- a/lockfile.h +++ b/lockfile.h @@ -321,11 +321,11 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path) * Roll back `lk`: close the file descriptor and/or file pointer and * remove the lockfile. It is a NOOP to call `rollback_lock_file()` * for a `lock_file` object that has already been committed or rolled - * back. + * back. No error will be returned in this case. */ -static inline void rollback_lock_file(struct lock_file *lk) +static inline int rollback_lock_file(struct lock_file *lk) { - delete_tempfile(&lk->tempfile); + return delete_tempfile(&lk->tempfile); } #endif /* LOCKFILE_H */ diff --git a/parse-options.c b/parse-options.c index 63a99dea6e..30b9e68f8a 100644 --- a/parse-options.c +++ b/parse-options.c @@ -350,98 +350,107 @@ static int is_alias(struct parse_opt_ctx_t *ctx, return 0; } +struct parsed_option { + const struct option *option; + enum opt_parsed flags; +}; + +static void register_abbrev(struct parse_opt_ctx_t *p, + const struct option *option, enum opt_parsed flags, + struct parsed_option *abbrev, + struct parsed_option *ambiguous) +{ + if (p->flags & PARSE_OPT_KEEP_UNKNOWN_OPT) + return; + if (abbrev->option && + !(abbrev->flags == flags && is_alias(p, abbrev->option, option))) { + /* + * If this is abbreviated, it is + * ambiguous. So when there is no + * exact match later, we need to + * error out. + */ + ambiguous->option = abbrev->option; + ambiguous->flags = abbrev->flags; + } + abbrev->option = option; + abbrev->flags = flags; +} + static enum parse_opt_result parse_long_opt( struct parse_opt_ctx_t *p, const char *arg, const struct option *options) { const char *arg_end = strchrnul(arg, '='); - const struct option *abbrev_option = NULL, *ambiguous_option = NULL; - enum opt_parsed abbrev_flags = OPT_LONG, ambiguous_flags = OPT_LONG; - int allow_abbrev = !(p->flags & PARSE_OPT_KEEP_UNKNOWN_OPT); + const char *arg_start = arg; + enum opt_parsed flags = OPT_LONG; + int arg_starts_with_no_no = 0; + struct parsed_option abbrev = { .option = NULL, .flags = OPT_LONG }; + struct parsed_option ambiguous = { .option = NULL, .flags = OPT_LONG }; + + if (skip_prefix(arg_start, "no-", &arg_start)) { + if (skip_prefix(arg_start, "no-", &arg_start)) + arg_starts_with_no_no = 1; + else + flags |= OPT_UNSET; + } for (; options->type != OPTION_END; options++) { const char *rest, *long_name = options->long_name; - enum opt_parsed flags = OPT_LONG, opt_flags = OPT_LONG; + enum opt_parsed opt_flags = OPT_LONG; + int allow_unset = !(options->flags & PARSE_OPT_NONEG); if (options->type == OPTION_SUBCOMMAND) continue; if (!long_name) continue; - if (!starts_with(arg, "no-") && - !(options->flags & PARSE_OPT_NONEG) && - skip_prefix(long_name, "no-", &long_name)) + if (skip_prefix(long_name, "no-", &long_name)) opt_flags |= OPT_UNSET; + else if (arg_starts_with_no_no) + continue; - if (!skip_prefix(arg, long_name, &rest)) - rest = NULL; - if (!rest) { - /* abbreviated? */ - if (allow_abbrev && - !strncmp(long_name, arg, arg_end - arg)) { -is_abbreviated: - if (abbrev_option && - !is_alias(p, abbrev_option, options)) { - /* - * If this is abbreviated, it is - * ambiguous. So when there is no - * exact match later, we need to - * error out. - */ - ambiguous_option = abbrev_option; - ambiguous_flags = abbrev_flags; - } - if (!(flags & OPT_UNSET) && *arg_end) - p->opt = arg_end + 1; - abbrev_option = options; - abbrev_flags = flags ^ opt_flags; - continue; - } - /* negation allowed? */ - if (options->flags & PARSE_OPT_NONEG) - continue; - /* negated and abbreviated very much? */ - if (allow_abbrev && starts_with("no-", arg)) { - flags |= OPT_UNSET; - goto is_abbreviated; - } - /* negated? */ - if (!starts_with(arg, "no-")) - continue; - flags |= OPT_UNSET; - if (!skip_prefix(arg + 3, long_name, &rest)) { - /* abbreviated and negated? */ - if (allow_abbrev && - starts_with(long_name, arg + 3)) - goto is_abbreviated; - else - continue; - } - } - if (*rest) { - if (*rest != '=') + if (((flags ^ opt_flags) & OPT_UNSET) && !allow_unset) + continue; + + if (skip_prefix(arg_start, long_name, &rest)) { + if (*rest == '=') + p->opt = rest + 1; + else if (*rest) continue; - p->opt = rest + 1; + return get_value(p, options, flags ^ opt_flags); } - return get_value(p, options, flags ^ opt_flags); + + /* abbreviated? */ + if (!strncmp(long_name, arg_start, arg_end - arg_start)) + register_abbrev(p, options, flags ^ opt_flags, + &abbrev, &ambiguous); + + /* negated and abbreviated very much? */ + if (allow_unset && starts_with("no-", arg)) + register_abbrev(p, options, OPT_UNSET ^ opt_flags, + &abbrev, &ambiguous); } - if (disallow_abbreviated_options && (ambiguous_option || abbrev_option)) + if (disallow_abbreviated_options && (ambiguous.option || abbrev.option)) die("disallowed abbreviated or ambiguous option '%.*s'", (int)(arg_end - arg), arg); - if (ambiguous_option) { + if (ambiguous.option) { error(_("ambiguous option: %s " "(could be --%s%s or --%s%s)"), arg, - (ambiguous_flags & OPT_UNSET) ? "no-" : "", - ambiguous_option->long_name, - (abbrev_flags & OPT_UNSET) ? "no-" : "", - abbrev_option->long_name); + (ambiguous.flags & OPT_UNSET) ? "no-" : "", + ambiguous.option->long_name, + (abbrev.flags & OPT_UNSET) ? "no-" : "", + abbrev.option->long_name); return PARSE_OPT_HELP; } - if (abbrev_option) - return get_value(p, abbrev_option, abbrev_flags); + if (abbrev.option) { + if (*arg_end) + p->opt = arg_end + 1; + return get_value(p, abbrev.option, abbrev.flags); + } return PARSE_OPT_UNKNOWN; } diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 74dab18eda..e206d5a073 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -171,23 +171,6 @@ static int should_write_log(struct ref_store *refs, const char *refname) } } -static void clear_reftable_log_record(struct reftable_log_record *log) -{ - switch (log->value_type) { - case REFTABLE_LOG_UPDATE: - /* - * When we write log records, the hashes are owned by the - * caller and thus shouldn't be free'd. - */ - log->value.update.old_hash = NULL; - log->value.update.new_hash = NULL; - break; - case REFTABLE_LOG_DELETION: - break; - } - reftable_log_record_release(log); -} - static void fill_reftable_log_record(struct reftable_log_record *log) { const char *info = git_committer_info(0); @@ -1106,8 +1089,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data fill_reftable_log_record(log); log->update_index = ts; log->refname = xstrdup(u->refname); - log->value.update.new_hash = u->new_oid.hash; - log->value.update.old_hash = tx_update->current_oid.hash; + memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ); + memcpy(log->value.update.old_hash, tx_update->current_oid.hash, GIT_MAX_RAWSZ); log->value.update.message = xstrndup(u->msg, arg->refs->write_options.block_size / 2); } @@ -1162,7 +1145,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data done: assert(ret != REFTABLE_API_ERROR); for (i = 0; i < logs_nr; i++) - clear_reftable_log_record(&logs[i]); + reftable_log_record_release(&logs[i]); free(logs); return ret; } @@ -1279,13 +1262,13 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da log.update_index = ts; log.value.update.message = xstrndup(create->logmsg, create->refs->write_options.block_size / 2); - log.value.update.new_hash = new_oid.hash; + memcpy(log.value.update.new_hash, new_oid.hash, GIT_MAX_RAWSZ); if (refs_resolve_ref_unsafe(&create->refs->base, create->refname, RESOLVE_REF_READING, &old_oid, NULL)) - log.value.update.old_hash = old_oid.hash; + memcpy(log.value.update.old_hash, old_oid.hash, GIT_MAX_RAWSZ); ret = reftable_writer_add_log(writer, &log); - clear_reftable_log_record(&log); + reftable_log_record_release(&log); return ret; } @@ -1424,7 +1407,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) logs[logs_nr].update_index = deletion_ts; logs[logs_nr].value.update.message = xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2); - logs[logs_nr].value.update.old_hash = old_ref.value.val1; + memcpy(logs[logs_nr].value.update.old_hash, old_ref.value.val1, GIT_MAX_RAWSZ); logs_nr++; ret = read_ref_without_reload(arg->stack, "HEAD", &head_oid, &head_referent, &head_type); @@ -1456,7 +1439,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) logs[logs_nr].update_index = creation_ts; logs[logs_nr].value.update.message = xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2); - logs[logs_nr].value.update.new_hash = old_ref.value.val1; + memcpy(logs[logs_nr].value.update.new_hash, old_ref.value.val1, GIT_MAX_RAWSZ); logs_nr++; /* @@ -1519,10 +1502,6 @@ done: for (i = 0; i < logs_nr; i++) { if (!strcmp(logs[i].refname, "HEAD")) continue; - if (logs[i].value.update.old_hash == old_ref.value.val1) - logs[i].value.update.old_hash = NULL; - if (logs[i].value.update.new_hash == old_ref.value.val1) - logs[i].value.update.new_hash = NULL; logs[i].refname = NULL; reftable_log_record_release(&logs[i]); } @@ -1600,7 +1579,7 @@ struct reftable_reflog_iterator { struct reftable_ref_store *refs; struct reftable_iterator iter; struct reftable_log_record log; - char *last_name; + struct strbuf last_name; int err; }; @@ -1619,15 +1598,15 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) * we've already produced this name. This could be faster by * seeking directly to reflog@update_index==0. */ - if (iter->last_name && !strcmp(iter->log.refname, iter->last_name)) + if (!strcmp(iter->log.refname, iter->last_name.buf)) continue; if (check_refname_format(iter->log.refname, REFNAME_ALLOW_ONELEVEL)) continue; - free(iter->last_name); - iter->last_name = xstrdup(iter->log.refname); + strbuf_reset(&iter->last_name); + strbuf_addstr(&iter->last_name, iter->log.refname); iter->base.refname = iter->log.refname; break; @@ -1660,7 +1639,7 @@ static int reftable_reflog_iterator_abort(struct ref_iterator *ref_iterator) (struct reftable_reflog_iterator *)ref_iterator; reftable_log_record_release(&iter->log); reftable_iterator_destroy(&iter->iter); - free(iter->last_name); + strbuf_release(&iter->last_name); free(iter); return ITER_DONE; } @@ -1680,13 +1659,14 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl iter = xcalloc(1, sizeof(*iter)); base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable); + strbuf_init(&iter->last_name, 0); iter->refs = refs; ret = refs->err; if (ret) goto done; - ret = reftable_stack_reload(refs->main_stack); + ret = reftable_stack_reload(stack); if (ret < 0) goto done; @@ -2184,7 +2164,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, dest->value_type = REFTABLE_LOG_DELETION; } else { if ((flags & EXPIRE_REFLOGS_REWRITE) && last_hash) - dest->value.update.old_hash = last_hash; + memcpy(dest->value.update.old_hash, last_hash, GIT_MAX_RAWSZ); last_hash = logs[i].value.update.new_hash; } } diff --git a/reftable/block.c b/reftable/block.c index ad9074dba6..e2a2cee58d 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -301,7 +301,7 @@ static int restart_key_less(size_t idx, void *args) result = strbuf_cmp(&a->key, &rkey); strbuf_release(&rkey); - return result; + return result < 0; } void block_iter_copy_from(struct block_iter *dest, struct block_iter *src) @@ -332,7 +332,8 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec) return REFTABLE_FORMAT_ERROR; string_view_consume(&in, n); - n = reftable_record_decode(rec, it->last_key, extra, in, it->br->hash_size); + n = reftable_record_decode(rec, it->last_key, extra, in, it->br->hash_size, + &it->scratch); if (n < 0) return -1; string_view_consume(&in, n); @@ -369,6 +370,7 @@ int block_iter_seek(struct block_iter *it, struct strbuf *want) void block_iter_close(struct block_iter *it) { strbuf_release(&it->last_key); + strbuf_release(&it->scratch); } int block_reader_seek(struct block_reader *br, struct block_iter *it, diff --git a/reftable/block.h b/reftable/block.h index 51699af233..47acc62c0a 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -84,10 +84,12 @@ struct block_iter { /* key for last entry we read. */ struct strbuf last_key; + struct strbuf scratch; }; #define BLOCK_ITER_INIT { \ .last_key = STRBUF_INIT, \ + .scratch = STRBUF_INIT, \ } /* initializes a block reader. */ diff --git a/reftable/merged_test.c b/reftable/merged_test.c index d0f77a3b8f..530fc82d1c 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -289,16 +289,13 @@ merged_table_from_log_records(struct reftable_log_record **logs, static void test_merged_logs(void) { - uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 }; - uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 }; - uint8_t hash3[GIT_SHA1_RAWSZ] = { 3 }; struct reftable_log_record r1[] = { { .refname = "a", .update_index = 2, .value_type = REFTABLE_LOG_UPDATE, .value.update = { - .old_hash = hash2, + .old_hash = { 2 }, /* deletion */ .name = "jane doe", .email = "jane@invalid", @@ -310,8 +307,8 @@ static void test_merged_logs(void) .update_index = 1, .value_type = REFTABLE_LOG_UPDATE, .value.update = { - .old_hash = hash1, - .new_hash = hash2, + .old_hash = { 1 }, + .new_hash = { 2 }, .name = "jane doe", .email = "jane@invalid", .message = "message1", @@ -324,7 +321,7 @@ static void test_merged_logs(void) .update_index = 3, .value_type = REFTABLE_LOG_UPDATE, .value.update = { - .new_hash = hash3, + .new_hash = { 3 }, .name = "jane doe", .email = "jane@invalid", .message = "message3", diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 363fe0f998..a6dbd214c5 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -77,18 +77,15 @@ static void write_table(char ***names, struct strbuf *buf, int N, } for (i = 0; i < N; i++) { - uint8_t hash[GIT_SHA256_RAWSZ] = { 0 }; char name[100]; int n; - set_test_hash(hash, i); - snprintf(name, sizeof(name), "refs/heads/branch%02d", i); log.refname = name; log.update_index = update_index; log.value_type = REFTABLE_LOG_UPDATE; - log.value.update.new_hash = hash; + set_test_hash(log.value.update.new_hash, i); log.value.update.message = "message"; n = reftable_writer_add_log(w, &log); @@ -137,13 +134,10 @@ static void test_log_buffer_size(void) /* This tests buffer extension for log compression. Must use a random hash, to ensure that the compressed part is larger than the original. */ - uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ]; for (i = 0; i < GIT_SHA1_RAWSZ; i++) { - hash1[i] = (uint8_t)(git_rand() % 256); - hash2[i] = (uint8_t)(git_rand() % 256); + log.value.update.old_hash[i] = (uint8_t)(git_rand() % 256); + log.value.update.new_hash[i] = (uint8_t)(git_rand() % 256); } - log.value.update.old_hash = hash1; - log.value.update.new_hash = hash2; reftable_writer_set_limits(w, update_index, update_index); err = reftable_writer_add_log(w, &log); EXPECT_ERR(err); @@ -161,25 +155,26 @@ static void test_log_overflow(void) .block_size = ARRAY_SIZE(msg), }; int err; - struct reftable_log_record - log = { .refname = "refs/heads/master", - .update_index = 0xa, - .value_type = REFTABLE_LOG_UPDATE, - .value = { .update = { - .name = "Han-Wen Nienhuys", - .email = "hanwen@google.com", - .tz_offset = 100, - .time = 0x5e430672, - .message = msg, - } } }; + struct reftable_log_record log = { + .refname = "refs/heads/master", + .update_index = 0xa, + .value_type = REFTABLE_LOG_UPDATE, + .value = { + .update = { + .old_hash = { 1 }, + .new_hash = { 2 }, + .name = "Han-Wen Nienhuys", + .email = "hanwen@google.com", + .tz_offset = 100, + .time = 0x5e430672, + .message = msg, + }, + }, + }; struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); - uint8_t hash1[GIT_SHA1_RAWSZ] = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 }; - memset(msg, 'x', sizeof(msg) - 1); - log.value.update.old_hash = hash1; - log.value.update.new_hash = hash2; reftable_writer_set_limits(w, update_index, update_index); err = reftable_writer_add_log(w, &log); EXPECT(err == REFTABLE_ENTRY_TOO_BIG_ERROR); @@ -219,16 +214,13 @@ static void test_log_write_read(void) EXPECT_ERR(err); } for (i = 0; i < N; i++) { - uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ]; struct reftable_log_record log = { NULL }; - set_test_hash(hash1, i); - set_test_hash(hash2, i + 1); log.refname = names[i]; log.update_index = i; log.value_type = REFTABLE_LOG_UPDATE; - log.value.update.old_hash = hash1; - log.value.update.new_hash = hash2; + set_test_hash(log.value.update.old_hash, i); + set_test_hash(log.value.update.new_hash, i + 1); err = reftable_writer_add_log(w, &log); EXPECT_ERR(err); @@ -298,18 +290,15 @@ static void test_log_zlib_corruption(void) struct reftable_writer *w = reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts); const struct reftable_stats *stats = NULL; - uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 }; - uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 }; char message[100] = { 0 }; int err, i, n; - struct reftable_log_record log = { .refname = "refname", .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { - .new_hash = hash1, - .old_hash = hash2, + .new_hash = { 1 }, + .old_hash = { 2 }, .name = "My Name", .email = "myname@invalid", .message = message, @@ -821,13 +810,12 @@ static void test_write_multiple_indices(void) } for (i = 0; i < 100; i++) { - unsigned char hash[GIT_SHA1_RAWSZ] = {i}; struct reftable_log_record log = { .update_index = 1, .value_type = REFTABLE_LOG_UPDATE, .value.update = { - .old_hash = hash, - .new_hash = hash, + .old_hash = { i }, + .new_hash = { i }, }, }; diff --git a/reftable/record.c b/reftable/record.c index 367de04600..23b497adab 100644 --- a/reftable/record.c +++ b/reftable/record.c @@ -374,7 +374,7 @@ static int reftable_ref_record_encode(const void *rec, struct string_view s, static int reftable_ref_record_decode(void *rec, struct strbuf key, uint8_t val_type, struct string_view in, - int hash_size) + int hash_size, struct strbuf *scratch) { struct reftable_ref_record *r = rec; struct string_view start = in; @@ -425,13 +425,12 @@ static int reftable_ref_record_decode(void *rec, struct strbuf key, break; case REFTABLE_REF_SYMREF: { - struct strbuf dest = STRBUF_INIT; - int n = decode_string(&dest, in); + int n = decode_string(scratch, in); if (n < 0) { return -1; } string_view_consume(&in, n); - r->value.symref = dest.buf; + r->value.symref = strbuf_detach(scratch, NULL); } break; case REFTABLE_REF_DELETION: @@ -579,7 +578,7 @@ static int reftable_obj_record_encode(const void *rec, struct string_view s, static int reftable_obj_record_decode(void *rec, struct strbuf key, uint8_t val_type, struct string_view in, - int hash_size) + int hash_size, struct strbuf *scratch UNUSED) { struct string_view start = in; struct reftable_obj_record *r = rec; @@ -588,6 +587,8 @@ static int reftable_obj_record_decode(void *rec, struct strbuf key, uint64_t last; int j; + reftable_obj_record_release(r); + REFTABLE_ALLOC_ARRAY(r->hash_prefix, key.len); memcpy(r->hash_prefix, key.buf, key.len); r->hash_prefix_len = key.len; @@ -763,16 +764,10 @@ static void reftable_log_record_copy_from(void *rec, const void *src_rec, xstrdup(dst->value.update.message); } - if (dst->value.update.new_hash) { - REFTABLE_ALLOC_ARRAY(dst->value.update.new_hash, hash_size); - memcpy(dst->value.update.new_hash, - src->value.update.new_hash, hash_size); - } - if (dst->value.update.old_hash) { - REFTABLE_ALLOC_ARRAY(dst->value.update.old_hash, hash_size); - memcpy(dst->value.update.old_hash, - src->value.update.old_hash, hash_size); - } + memcpy(dst->value.update.new_hash, + src->value.update.new_hash, hash_size); + memcpy(dst->value.update.old_hash, + src->value.update.old_hash, hash_size); break; } } @@ -790,8 +785,6 @@ void reftable_log_record_release(struct reftable_log_record *r) case REFTABLE_LOG_DELETION: break; case REFTABLE_LOG_UPDATE: - reftable_free(r->value.update.new_hash); - reftable_free(r->value.update.old_hash); reftable_free(r->value.update.name); reftable_free(r->value.update.email); reftable_free(r->value.update.message); @@ -808,33 +801,20 @@ static uint8_t reftable_log_record_val_type(const void *rec) return reftable_log_record_is_deletion(log) ? 0 : 1; } -static uint8_t zero[GIT_SHA256_RAWSZ] = { 0 }; - static int reftable_log_record_encode(const void *rec, struct string_view s, int hash_size) { const struct reftable_log_record *r = rec; struct string_view start = s; int n = 0; - uint8_t *oldh = NULL; - uint8_t *newh = NULL; if (reftable_log_record_is_deletion(r)) return 0; - oldh = r->value.update.old_hash; - newh = r->value.update.new_hash; - if (!oldh) { - oldh = zero; - } - if (!newh) { - newh = zero; - } - if (s.len < 2 * hash_size) return -1; - memcpy(s.buf, oldh, hash_size); - memcpy(s.buf + hash_size, newh, hash_size); + memcpy(s.buf, r->value.update.old_hash, hash_size); + memcpy(s.buf + hash_size, r->value.update.new_hash, hash_size); string_view_consume(&s, 2 * hash_size); n = encode_string(r->value.update.name ? r->value.update.name : "", s); @@ -870,19 +850,18 @@ static int reftable_log_record_encode(const void *rec, struct string_view s, static int reftable_log_record_decode(void *rec, struct strbuf key, uint8_t val_type, struct string_view in, - int hash_size) + int hash_size, struct strbuf *scratch) { struct string_view start = in; struct reftable_log_record *r = rec; uint64_t max = 0; uint64_t ts = 0; - struct strbuf dest = STRBUF_INIT; int n; if (key.len <= 9 || key.buf[key.len - 9] != 0) return REFTABLE_FORMAT_ERROR; - r->refname = reftable_realloc(r->refname, key.len - 8); + REFTABLE_ALLOC_GROW(r->refname, key.len - 8, r->refname_cap); memcpy(r->refname, key.buf, key.len - 8); ts = get_be64(key.buf + key.len - 8); @@ -891,9 +870,8 @@ static int reftable_log_record_decode(void *rec, struct strbuf key, if (val_type != r->value_type) { switch (r->value_type) { case REFTABLE_LOG_UPDATE: - FREE_AND_NULL(r->value.update.old_hash); - FREE_AND_NULL(r->value.update.new_hash); FREE_AND_NULL(r->value.update.message); + r->value.update.message_cap = 0; FREE_AND_NULL(r->value.update.email); FREE_AND_NULL(r->value.update.name); break; @@ -909,36 +887,43 @@ static int reftable_log_record_decode(void *rec, struct strbuf key, if (in.len < 2 * hash_size) return REFTABLE_FORMAT_ERROR; - r->value.update.old_hash = - reftable_realloc(r->value.update.old_hash, hash_size); - r->value.update.new_hash = - reftable_realloc(r->value.update.new_hash, hash_size); - memcpy(r->value.update.old_hash, in.buf, hash_size); memcpy(r->value.update.new_hash, in.buf + hash_size, hash_size); string_view_consume(&in, 2 * hash_size); - n = decode_string(&dest, in); + n = decode_string(scratch, in); if (n < 0) goto done; string_view_consume(&in, n); - r->value.update.name = - reftable_realloc(r->value.update.name, dest.len + 1); - memcpy(r->value.update.name, dest.buf, dest.len); - r->value.update.name[dest.len] = 0; + /* + * In almost all cases we can expect the reflog name to not change for + * reflog entries as they are tied to the local identity, not to the + * target commits. As an optimization for this common case we can thus + * skip copying over the name in case it's accurate already. + */ + if (!r->value.update.name || + strcmp(r->value.update.name, scratch->buf)) { + r->value.update.name = + reftable_realloc(r->value.update.name, scratch->len + 1); + memcpy(r->value.update.name, scratch->buf, scratch->len); + r->value.update.name[scratch->len] = 0; + } - strbuf_reset(&dest); - n = decode_string(&dest, in); + n = decode_string(scratch, in); if (n < 0) goto done; string_view_consume(&in, n); - r->value.update.email = - reftable_realloc(r->value.update.email, dest.len + 1); - memcpy(r->value.update.email, dest.buf, dest.len); - r->value.update.email[dest.len] = 0; + /* Same as above, but for the reflog email. */ + if (!r->value.update.email || + strcmp(r->value.update.email, scratch->buf)) { + r->value.update.email = + reftable_realloc(r->value.update.email, scratch->len + 1); + memcpy(r->value.update.email, scratch->buf, scratch->len); + r->value.update.email[scratch->len] = 0; + } ts = 0; n = get_var_int(&ts, &in); @@ -952,22 +937,19 @@ static int reftable_log_record_decode(void *rec, struct strbuf key, r->value.update.tz_offset = get_be16(in.buf); string_view_consume(&in, 2); - strbuf_reset(&dest); - n = decode_string(&dest, in); + n = decode_string(scratch, in); if (n < 0) goto done; string_view_consume(&in, n); - r->value.update.message = - reftable_realloc(r->value.update.message, dest.len + 1); - memcpy(r->value.update.message, dest.buf, dest.len); - r->value.update.message[dest.len] = 0; + REFTABLE_ALLOC_GROW(r->value.update.message, scratch->len + 1, + r->value.update.message_cap); + memcpy(r->value.update.message, scratch->buf, scratch->len); + r->value.update.message[scratch->len] = 0; - strbuf_release(&dest); return start.len - in.len; done: - strbuf_release(&dest); return REFTABLE_FORMAT_ERROR; } @@ -983,17 +965,6 @@ static int null_streq(char *a, char *b) return 0 == strcmp(a, b); } -static int zero_hash_eq(uint8_t *a, uint8_t *b, int sz) -{ - if (!a) - a = zero; - - if (!b) - b = zero; - - return !memcmp(a, b, sz); -} - static int reftable_log_record_equal_void(const void *a, const void *b, int hash_size) { @@ -1037,10 +1008,10 @@ int reftable_log_record_equal(const struct reftable_log_record *a, b->value.update.email) && null_streq(a->value.update.message, b->value.update.message) && - zero_hash_eq(a->value.update.old_hash, - b->value.update.old_hash, hash_size) && - zero_hash_eq(a->value.update.new_hash, - b->value.update.new_hash, hash_size); + !memcmp(a->value.update.old_hash, + b->value.update.old_hash, hash_size) && + !memcmp(a->value.update.new_hash, + b->value.update.new_hash, hash_size); } abort(); @@ -1118,7 +1089,7 @@ static int reftable_index_record_encode(const void *rec, struct string_view out, static int reftable_index_record_decode(void *rec, struct strbuf key, uint8_t val_type, struct string_view in, - int hash_size) + int hash_size, struct strbuf *scratch UNUSED) { struct string_view start = in; struct reftable_index_record *r = rec; @@ -1199,10 +1170,12 @@ uint8_t reftable_record_val_type(struct reftable_record *rec) } int reftable_record_decode(struct reftable_record *rec, struct strbuf key, - uint8_t extra, struct string_view src, int hash_size) + uint8_t extra, struct string_view src, int hash_size, + struct strbuf *scratch) { return reftable_record_vtable(rec)->decode(reftable_record_data(rec), - key, extra, src, hash_size); + key, extra, src, hash_size, + scratch); } void reftable_record_release(struct reftable_record *rec) diff --git a/reftable/record.h b/reftable/record.h index 5e8304e052..826ee1c55c 100644 --- a/reftable/record.h +++ b/reftable/record.h @@ -55,7 +55,8 @@ struct reftable_record_vtable { /* decode data from `src` into the record. */ int (*decode)(void *rec, struct strbuf key, uint8_t extra, - struct string_view src, int hash_size); + struct string_view src, int hash_size, + struct strbuf *scratch); /* deallocate and null the record. */ void (*release)(void *rec); @@ -138,7 +139,7 @@ int reftable_record_encode(struct reftable_record *rec, struct string_view dest, int hash_size); int reftable_record_decode(struct reftable_record *rec, struct strbuf key, uint8_t extra, struct string_view src, - int hash_size); + int hash_size, struct strbuf *scratch); int reftable_record_is_deletion(struct reftable_record *rec); static inline uint8_t reftable_record_type(struct reftable_record *rec) diff --git a/reftable/record_test.c b/reftable/record_test.c index 89209894d8..c158ee79ff 100644 --- a/reftable/record_test.c +++ b/reftable/record_test.c @@ -99,6 +99,7 @@ static void set_hash(uint8_t *h, int j) static void test_reftable_ref_record_roundtrip(void) { + struct strbuf scratch = STRBUF_INIT; int i = 0; for (i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) { @@ -140,7 +141,7 @@ static void test_reftable_ref_record_roundtrip(void) EXPECT(n > 0); /* decode into a non-zero reftable_record to test for leaks. */ - m = reftable_record_decode(&out, key, i, dest, GIT_SHA1_RAWSZ); + m = reftable_record_decode(&out, key, i, dest, GIT_SHA1_RAWSZ, &scratch); EXPECT(n == m); EXPECT(reftable_ref_record_equal(&in.u.ref, &out.u.ref, @@ -150,6 +151,8 @@ static void test_reftable_ref_record_roundtrip(void) strbuf_release(&key); reftable_record_release(&out); } + + strbuf_release(&scratch); } static void test_reftable_log_record_equal(void) @@ -175,7 +178,6 @@ static void test_reftable_log_record_equal(void) static void test_reftable_log_record_roundtrip(void) { int i; - struct reftable_log_record in[] = { { .refname = xstrdup("refs/heads/master"), @@ -183,8 +185,6 @@ static void test_reftable_log_record_roundtrip(void) .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { - .old_hash = reftable_malloc(GIT_SHA1_RAWSZ), - .new_hash = reftable_malloc(GIT_SHA1_RAWSZ), .name = xstrdup("han-wen"), .email = xstrdup("hanwen@google.com"), .message = xstrdup("test"), @@ -202,15 +202,10 @@ static void test_reftable_log_record_roundtrip(void) .refname = xstrdup("branch"), .update_index = 33, .value_type = REFTABLE_LOG_UPDATE, - .value = { - .update = { - .old_hash = reftable_malloc(GIT_SHA1_RAWSZ), - .new_hash = reftable_malloc(GIT_SHA1_RAWSZ), - /* rest of fields left empty. */ - }, - }, } }; + struct strbuf scratch = STRBUF_INIT; + set_test_hash(in[0].value.update.new_hash, 1); set_test_hash(in[0].value.update.old_hash, 2); set_test_hash(in[2].value.update.new_hash, 3); @@ -231,8 +226,6 @@ static void test_reftable_log_record_roundtrip(void) .value_type = REFTABLE_LOG_UPDATE, .value = { .update = { - .new_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1), - .old_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1), .name = xstrdup("old name"), .email = xstrdup("old@email"), .message = xstrdup("old message"), @@ -252,7 +245,7 @@ static void test_reftable_log_record_roundtrip(void) EXPECT(n >= 0); valtype = reftable_record_val_type(&rec); m = reftable_record_decode(&out, key, valtype, dest, - GIT_SHA1_RAWSZ); + GIT_SHA1_RAWSZ, &scratch); EXPECT(n == m); EXPECT(reftable_log_record_equal(&in[i], &out.u.log, @@ -261,6 +254,8 @@ static void test_reftable_log_record_roundtrip(void) strbuf_release(&key); reftable_record_release(&out); } + + strbuf_release(&scratch); } static void test_u24_roundtrip(void) @@ -310,23 +305,27 @@ static void test_reftable_obj_record_roundtrip(void) { uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 }; uint64_t till9[] = { 1, 2, 3, 4, 500, 600, 700, 800, 9000 }; - struct reftable_obj_record recs[3] = { { - .hash_prefix = testHash1, - .hash_prefix_len = 5, - .offsets = till9, - .offset_len = 3, - }, - { - .hash_prefix = testHash1, - .hash_prefix_len = 5, - .offsets = till9, - .offset_len = 9, - }, - { - .hash_prefix = testHash1, - .hash_prefix_len = 5, - } }; + struct reftable_obj_record recs[3] = { + { + .hash_prefix = testHash1, + .hash_prefix_len = 5, + .offsets = till9, + .offset_len = 3, + }, + { + .hash_prefix = testHash1, + .hash_prefix_len = 5, + .offsets = till9, + .offset_len = 9, + }, + { + .hash_prefix = testHash1, + .hash_prefix_len = 5, + }, + }; + struct strbuf scratch = STRBUF_INIT; int i = 0; + for (i = 0; i < ARRAY_SIZE(recs); i++) { uint8_t buffer[1024] = { 0 }; struct string_view dest = { @@ -350,13 +349,15 @@ static void test_reftable_obj_record_roundtrip(void) EXPECT(n > 0); extra = reftable_record_val_type(&in); m = reftable_record_decode(&out, key, extra, dest, - GIT_SHA1_RAWSZ); + GIT_SHA1_RAWSZ, &scratch); EXPECT(n == m); EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); strbuf_release(&key); reftable_record_release(&out); } + + strbuf_release(&scratch); } static void test_reftable_index_record_roundtrip(void) @@ -373,6 +374,7 @@ static void test_reftable_index_record_roundtrip(void) .buf = buffer, .len = sizeof(buffer), }; + struct strbuf scratch = STRBUF_INIT; struct strbuf key = STRBUF_INIT; struct reftable_record out = { .type = BLOCK_TYPE_INDEX, @@ -390,13 +392,15 @@ static void test_reftable_index_record_roundtrip(void) EXPECT(n > 0); extra = reftable_record_val_type(&in); - m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ); + m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ, + &scratch); EXPECT(m == n); EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); reftable_record_release(&out); strbuf_release(&key); + strbuf_release(&scratch); strbuf_release(&in.u.idx.last_key); } diff --git a/reftable/reftable-record.h b/reftable/reftable-record.h index e657001d42..2a2943cd13 100644 --- a/reftable/reftable-record.h +++ b/reftable/reftable-record.h @@ -74,6 +74,7 @@ int reftable_ref_record_equal(const struct reftable_ref_record *a, /* reftable_log_record holds a reflog entry */ struct reftable_log_record { char *refname; + size_t refname_cap; uint64_t update_index; /* logical timestamp of a transactional update. */ @@ -88,13 +89,14 @@ struct reftable_log_record { union { struct { - uint8_t *new_hash; - uint8_t *old_hash; + unsigned char new_hash[GIT_MAX_RAWSZ]; + unsigned char old_hash[GIT_MAX_RAWSZ]; char *name; char *email; uint64_t time; int16_t tz_offset; char *message; + size_t message_cap; } update; } value; }; diff --git a/reftable/stack.c b/reftable/stack.c index b64e55648a..1ecf1b9751 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -737,8 +737,9 @@ int reftable_addition_add(struct reftable_addition *add, struct strbuf tab_file_name = STRBUF_INIT; struct strbuf next_name = STRBUF_INIT; struct reftable_writer *wr = NULL; + struct tempfile *tab_file = NULL; int err = 0; - int tab_fd = 0; + int tab_fd; strbuf_reset(&next_name); format_name(&next_name, add->next_update_index, add->next_update_index); @@ -746,17 +747,20 @@ int reftable_addition_add(struct reftable_addition *add, stack_filename(&temp_tab_file_name, add->stack, next_name.buf); strbuf_addstr(&temp_tab_file_name, ".temp.XXXXXX"); - tab_fd = mkstemp(temp_tab_file_name.buf); - if (tab_fd < 0) { + tab_file = mks_tempfile(temp_tab_file_name.buf); + if (!tab_file) { err = REFTABLE_IO_ERROR; goto done; } if (add->stack->config.default_permissions) { - if (chmod(temp_tab_file_name.buf, add->stack->config.default_permissions)) { + if (chmod(get_tempfile_path(tab_file), + add->stack->config.default_permissions)) { err = REFTABLE_IO_ERROR; goto done; } } + tab_fd = get_tempfile_fd(tab_file); + wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &add->stack->config); err = write_table(wr, arg); @@ -771,14 +775,13 @@ int reftable_addition_add(struct reftable_addition *add, if (err < 0) goto done; - err = close(tab_fd); - tab_fd = 0; + err = close_tempfile_gently(tab_file); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } - err = stack_check_addition(add->stack, temp_tab_file_name.buf); + err = stack_check_addition(add->stack, get_tempfile_path(tab_file)); if (err < 0) goto done; @@ -789,14 +792,13 @@ int reftable_addition_add(struct reftable_addition *add, format_name(&next_name, wr->min_update_index, wr->max_update_index); strbuf_addstr(&next_name, ".ref"); - stack_filename(&tab_file_name, add->stack, next_name.buf); /* On windows, this relies on rand() picking a unique destination name. Maybe we should do retry loop as well? */ - err = rename(temp_tab_file_name.buf, tab_file_name.buf); + err = rename_tempfile(&tab_file, tab_file_name.buf); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; @@ -806,14 +808,7 @@ int reftable_addition_add(struct reftable_addition *add, add->new_tables_cap); add->new_tables[add->new_tables_len++] = strbuf_detach(&next_name, NULL); done: - if (tab_fd > 0) { - close(tab_fd); - tab_fd = 0; - } - if (temp_tab_file_name.len > 0) { - unlink(temp_tab_file_name.buf); - } - + delete_tempfile(&tab_file); strbuf_release(&temp_tab_file_name); strbuf_release(&tab_file_name); strbuf_release(&next_name); @@ -832,51 +827,56 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st) static int stack_compact_locked(struct reftable_stack *st, size_t first, size_t last, - struct strbuf *temp_tab, - struct reftable_log_expiry_config *config) + struct reftable_log_expiry_config *config, + struct tempfile **tab_file_out) { struct strbuf next_name = STRBUF_INIT; - int tab_fd = -1; + struct strbuf tab_file_path = STRBUF_INIT; struct reftable_writer *wr = NULL; - int err = 0; + struct tempfile *tab_file; + int tab_fd, err = 0; format_name(&next_name, reftable_reader_min_update_index(st->readers[first]), reftable_reader_max_update_index(st->readers[last])); + stack_filename(&tab_file_path, st, next_name.buf); + strbuf_addstr(&tab_file_path, ".temp.XXXXXX"); - stack_filename(temp_tab, st, next_name.buf); - strbuf_addstr(temp_tab, ".temp.XXXXXX"); + tab_file = mks_tempfile(tab_file_path.buf); + if (!tab_file) { + err = REFTABLE_IO_ERROR; + goto done; + } + tab_fd = get_tempfile_fd(tab_file); - tab_fd = mkstemp(temp_tab->buf); if (st->config.default_permissions && - chmod(temp_tab->buf, st->config.default_permissions) < 0) { + chmod(get_tempfile_path(tab_file), st->config.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; } - wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &st->config); - + wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, + &tab_fd, &st->config); err = stack_write_compact(st, wr, first, last, config); if (err < 0) goto done; + err = reftable_writer_close(wr); if (err < 0) goto done; - err = close(tab_fd); - tab_fd = 0; + err = close_tempfile_gently(tab_file); + if (err < 0) + goto done; + + *tab_file_out = tab_file; + tab_file = NULL; done: + delete_tempfile(&tab_file); reftable_writer_free(wr); - if (tab_fd > 0) { - close(tab_fd); - tab_fd = 0; - } - if (err != 0 && temp_tab->len > 0) { - unlink(temp_tab->buf); - strbuf_release(temp_tab); - } strbuf_release(&next_name); + strbuf_release(&tab_file_path); return err; } @@ -983,212 +983,200 @@ static int stack_compact_range(struct reftable_stack *st, size_t first, size_t last, struct reftable_log_expiry_config *expiry) { - char **delete_on_success = NULL, **subtable_locks = NULL, **listp = NULL; - struct strbuf temp_tab_file_name = STRBUF_INIT; + struct strbuf tables_list_buf = STRBUF_INIT; struct strbuf new_table_name = STRBUF_INIT; - struct strbuf lock_file_name = STRBUF_INIT; - struct strbuf ref_list_contents = STRBUF_INIT; struct strbuf new_table_path = STRBUF_INIT; - size_t i, j, compact_count; - int err = 0; - int have_lock = 0; - int lock_file_fd = -1; - int is_empty_table = 0; + struct strbuf table_name = STRBUF_INIT; + struct lock_file tables_list_lock = LOCK_INIT; + struct lock_file *table_locks = NULL; + struct tempfile *new_table = NULL; + int is_empty_table = 0, err = 0; + size_t i; if (first > last || (!expiry && first == last)) { err = 0; goto done; } - compact_count = last - first + 1; - REFTABLE_CALLOC_ARRAY(delete_on_success, compact_count + 1); - REFTABLE_CALLOC_ARRAY(subtable_locks, compact_count + 1); - st->stats.attempts++; - strbuf_reset(&lock_file_name); - strbuf_addstr(&lock_file_name, st->list_file); - strbuf_addstr(&lock_file_name, ".lock"); - - lock_file_fd = - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666); - if (lock_file_fd < 0) { - if (errno == EEXIST) { + /* + * Hold the lock so that we can read "tables.list" and lock all tables + * which are part of the user-specified range. + */ + err = hold_lock_file_for_update(&tables_list_lock, st->list_file, + LOCK_NO_DEREF); + if (err < 0) { + if (errno == EEXIST) err = 1; - } else { + else err = REFTABLE_IO_ERROR; - } goto done; } - /* Don't want to write to the lock for now. */ - close(lock_file_fd); - lock_file_fd = -1; - have_lock = 1; err = stack_uptodate(st); - if (err != 0) + if (err) goto done; - for (i = first, j = 0; i <= last; i++) { - struct strbuf subtab_file_name = STRBUF_INIT; - struct strbuf subtab_lock = STRBUF_INIT; - int sublock_file_fd = -1; - - stack_filename(&subtab_file_name, st, - reader_name(st->readers[i])); - - strbuf_reset(&subtab_lock); - strbuf_addbuf(&subtab_lock, &subtab_file_name); - strbuf_addstr(&subtab_lock, ".lock"); + /* + * Lock all tables in the user-provided range. This is the slice of our + * stack which we'll compact. + */ + REFTABLE_CALLOC_ARRAY(table_locks, last - first + 1); + for (i = first; i <= last; i++) { + stack_filename(&table_name, st, reader_name(st->readers[i])); - sublock_file_fd = open(subtab_lock.buf, - O_EXCL | O_CREAT | O_WRONLY, 0666); - if (sublock_file_fd >= 0) { - close(sublock_file_fd); - } else if (sublock_file_fd < 0) { - if (errno == EEXIST) { + err = hold_lock_file_for_update(&table_locks[i - first], + table_name.buf, LOCK_NO_DEREF); + if (err < 0) { + if (errno == EEXIST) err = 1; - } else { + else err = REFTABLE_IO_ERROR; - } + goto done; } - subtable_locks[j] = subtab_lock.buf; - delete_on_success[j] = subtab_file_name.buf; - j++; - - if (err != 0) + /* + * We need to close the lockfiles as we might otherwise easily + * run into file descriptor exhaustion when we compress a lot + * of tables. + */ + err = close_lock_file_gently(&table_locks[i - first]); + if (err < 0) { + err = REFTABLE_IO_ERROR; goto done; + } } - err = unlink(lock_file_name.buf); - if (err < 0) + /* + * We have locked all tables in our range and can thus release the + * "tables.list" lock while compacting the locked tables. This allows + * concurrent updates to the stack to proceed. + */ + err = rollback_lock_file(&tables_list_lock); + if (err < 0) { + err = REFTABLE_IO_ERROR; goto done; - have_lock = 0; - - err = stack_compact_locked(st, first, last, &temp_tab_file_name, - expiry); - /* Compaction + tombstones can create an empty table out of non-empty - * tables. */ - is_empty_table = (err == REFTABLE_EMPTY_TABLE_ERROR); - if (is_empty_table) { - err = 0; } - if (err < 0) - goto done; - lock_file_fd = - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666); - if (lock_file_fd < 0) { - if (errno == EEXIST) { + /* + * Compact the now-locked tables into a new table. Note that compacting + * these tables may end up with an empty new table in case tombstones + * end up cancelling out all refs in that range. + */ + err = stack_compact_locked(st, first, last, expiry, &new_table); + if (err < 0) { + if (err != REFTABLE_EMPTY_TABLE_ERROR) + goto done; + is_empty_table = 1; + } + + /* + * Now that we have written the new, compacted table we need to re-lock + * "tables.list". We'll then replace the compacted range of tables with + * the new table. + */ + err = hold_lock_file_for_update(&tables_list_lock, st->list_file, + LOCK_NO_DEREF); + if (err < 0) { + if (errno == EEXIST) err = 1; - } else { + else err = REFTABLE_IO_ERROR; - } goto done; } - have_lock = 1; + if (st->config.default_permissions) { - if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) { + if (chmod(get_lock_file_path(&tables_list_lock), + st->config.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; } } - format_name(&new_table_name, st->readers[first]->min_update_index, - st->readers[last]->max_update_index); - strbuf_addstr(&new_table_name, ".ref"); - - stack_filename(&new_table_path, st, new_table_name.buf); - + /* + * If the resulting compacted table is not empty, then we need to move + * it into place now. + */ if (!is_empty_table) { - /* retry? */ - err = rename(temp_tab_file_name.buf, new_table_path.buf); + format_name(&new_table_name, st->readers[first]->min_update_index, + st->readers[last]->max_update_index); + strbuf_addstr(&new_table_name, ".ref"); + stack_filename(&new_table_path, st, new_table_name.buf); + + err = rename_tempfile(&new_table, new_table_path.buf); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } } - for (i = 0; i < first; i++) { - strbuf_addstr(&ref_list_contents, st->readers[i]->name); - strbuf_addstr(&ref_list_contents, "\n"); - } - if (!is_empty_table) { - strbuf_addbuf(&ref_list_contents, &new_table_name); - strbuf_addstr(&ref_list_contents, "\n"); - } - for (i = last + 1; i < st->merged->stack_len; i++) { - strbuf_addstr(&ref_list_contents, st->readers[i]->name); - strbuf_addstr(&ref_list_contents, "\n"); - } - - err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len); - if (err < 0) { - err = REFTABLE_IO_ERROR; - unlink(new_table_path.buf); - goto done; - } - - err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd); + /* + * Write the new "tables.list" contents with the compacted table we + * have just written. In case the compacted table became empty we + * simply skip writing it. + */ + for (i = 0; i < first; i++) + strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name); + if (!is_empty_table) + strbuf_addf(&tables_list_buf, "%s\n", new_table_name.buf); + for (i = last + 1; i < st->merged->stack_len; i++) + strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name); + + err = write_in_full(get_lock_file_fd(&tables_list_lock), + tables_list_buf.buf, tables_list_buf.len); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); goto done; } - err = close(lock_file_fd); - lock_file_fd = -1; + err = fsync_component(FSYNC_COMPONENT_REFERENCE, get_lock_file_fd(&tables_list_lock)); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); goto done; } - err = rename(lock_file_name.buf, st->list_file); + err = commit_lock_file(&tables_list_lock); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); goto done; } - have_lock = 0; - /* Reload the stack before deleting. On windows, we can only delete the - files after we closed them. - */ + /* + * Reload the stack before deleting the compacted tables. We can only + * delete the files after we closed them on Windows, so this needs to + * happen first. + */ err = reftable_stack_reload_maybe_reuse(st, first < last); + if (err < 0) + goto done; - listp = delete_on_success; - while (*listp) { - if (strcmp(*listp, new_table_path.buf)) { - unlink(*listp); - } - listp++; + /* + * Delete the old tables. They may still be in use by concurrent + * readers, so it is expected that unlinking tables may fail. + */ + for (i = first; i <= last; i++) { + struct lock_file *table_lock = &table_locks[i - first]; + char *table_path = get_locked_file_path(table_lock); + unlink(table_path); + free(table_path); } done: - free_names(delete_on_success); + rollback_lock_file(&tables_list_lock); + for (i = first; table_locks && i <= last; i++) + rollback_lock_file(&table_locks[i - first]); + reftable_free(table_locks); - if (subtable_locks) { - listp = subtable_locks; - while (*listp) { - unlink(*listp); - listp++; - } - free_names(subtable_locks); - } - if (lock_file_fd >= 0) { - close(lock_file_fd); - lock_file_fd = -1; - } - if (have_lock) { - unlink(lock_file_name.buf); - } + delete_tempfile(&new_table); strbuf_release(&new_table_name); strbuf_release(&new_table_path); - strbuf_release(&ref_list_contents); - strbuf_release(&temp_tab_file_name); - strbuf_release(&lock_file_name); + + strbuf_release(&tables_list_buf); + strbuf_release(&table_name); return err; } diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 509f486623..7336757cf5 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -468,8 +468,6 @@ static void test_reftable_stack_add(void) logs[i].refname = xstrdup(buf); logs[i].update_index = N + i + 1; logs[i].value_type = REFTABLE_LOG_UPDATE; - - logs[i].value.update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ); logs[i].value.update.email = xstrdup("identity@invalid"); set_test_hash(logs[i].value.update.new_hash, i); } @@ -547,16 +545,17 @@ static void test_reftable_stack_log_normalize(void) }; struct reftable_stack *st = NULL; char *dir = get_tmp_dir(__LINE__); - - uint8_t h1[GIT_SHA1_RAWSZ] = { 0x01 }, h2[GIT_SHA1_RAWSZ] = { 0x02 }; - - struct reftable_log_record input = { .refname = "branch", - .update_index = 1, - .value_type = REFTABLE_LOG_UPDATE, - .value = { .update = { - .new_hash = h1, - .old_hash = h2, - } } }; + struct reftable_log_record input = { + .refname = "branch", + .update_index = 1, + .value_type = REFTABLE_LOG_UPDATE, + .value = { + .update = { + .new_hash = { 1 }, + .old_hash = { 2 }, + }, + }, + }; struct reftable_log_record dest = { .update_index = 0, }; @@ -627,8 +626,6 @@ static void test_reftable_stack_tombstone(void) logs[i].update_index = 42; if (i % 2 == 0) { logs[i].value_type = REFTABLE_LOG_UPDATE; - logs[i].value.update.new_hash = - reftable_malloc(GIT_SHA1_RAWSZ); set_test_hash(logs[i].value.update.new_hash, i); logs[i].value.update.email = xstrdup("identity@invalid"); @@ -810,7 +807,6 @@ static void test_reflog_expire(void) logs[i].update_index = i; logs[i].value_type = REFTABLE_LOG_UPDATE; logs[i].value.update.time = i; - logs[i].value.update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ); logs[i].value.update.email = xstrdup("identity@invalid"); set_test_hash(logs[i].value.update.new_hash, i); } diff --git a/reftable/system.h b/reftable/system.h index 6b74a81514..5d8b6dede5 100644 --- a/reftable/system.h +++ b/reftable/system.h @@ -12,7 +12,9 @@ https://developers.google.com/open-source/licenses/bsd /* This header glues the reftable library to the rest of Git */ #include "git-compat-util.h" +#include "lockfile.h" #include "strbuf.h" +#include "tempfile.h" #include "hash-ll.h" /* hash ID, sizes.*/ #include "dir.h" /* remove_dir_recursively, for tests.*/ diff --git a/sequencer.c b/sequencer.c index ea1441e617..4e14fa6541 100644 --- a/sequencer.c +++ b/sequencer.c @@ -461,10 +461,22 @@ static void free_message(struct commit *commit, struct commit_message *msg) repo_unuse_commit_buffer(the_repository, commit, msg->message); } +const char *rebase_resolvemsg = +N_("Resolve all conflicts manually, mark them as resolved with\n" +"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n" +"You can instead skip this commit: run \"git rebase --skip\".\n" +"To abort and get back to the state before \"git rebase\", run " +"\"git rebase --abort\"."); + static void print_advice(struct repository *r, int show_hint, struct replay_opts *opts) { - char *msg = getenv("GIT_CHERRY_PICK_HELP"); + const char *msg; + + if (is_rebase_i(opts)) + msg = rebase_resolvemsg; + else + msg = getenv("GIT_CHERRY_PICK_HELP"); if (msg) { advise("%s\n", msg); diff --git a/sequencer.h b/sequencer.h index dcef7bb99c..437eabd38a 100644 --- a/sequencer.h +++ b/sequencer.h @@ -14,6 +14,8 @@ const char *rebase_path_todo(void); const char *rebase_path_todo_backup(void); const char *rebase_path_dropped(void); +extern const char *rebase_resolvemsg; + #define APPEND_SIGNOFF_DEDUP (1u << 0) enum replay_action { @@ -1243,6 +1243,32 @@ static const char *allowed_bare_repo_to_string( return NULL; } +static int is_implicit_bare_repo(const char *path) +{ + /* + * what we found is a ".git" directory at the root of + * the working tree. + */ + if (ends_with_path_components(path, ".git")) + return 1; + + /* + * we are inside $GIT_DIR of a secondary worktree of a + * non-bare repository. + */ + if (strstr(path, "/.git/worktrees/")) + return 1; + + /* + * we are inside $GIT_DIR of a worktree of a non-embedded + * submodule, whose superproject is not a bare repository. + */ + if (strstr(path, "/.git/modules/")) + return 1; + + return 0; +} + /* * We cannot decide in this function whether we are in the work tree or * not, since the config can only be read _after_ this function was called. @@ -1372,7 +1398,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, if (is_git_directory(dir->buf)) { trace2_data_string("setup", NULL, "implicit-bare-repository", dir->buf); if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT && - !ends_with_path_components(dir->buf, ".git")) + !is_implicit_bare_repo(dir->buf)) return GIT_DIR_DISALLOWED_BARE; if (!ensure_valid_ownership(NULL, NULL, dir->buf, report)) return GIT_DIR_INVALID_OWNERSHIP; diff --git a/t/t0006-date.sh b/t/t0006-date.sh index e18b160286..3031256d14 100755 --- a/t/t0006-date.sh +++ b/t/t0006-date.sh @@ -46,6 +46,7 @@ check_show () { TIME='1466000000 +0200' check_show iso8601 "$TIME" '2016-06-15 16:13:20 +0200' check_show iso8601-strict "$TIME" '2016-06-15T16:13:20+02:00' +check_show iso8601-strict "$(echo "$TIME" | sed 's/+0200$/+0000/')" '2016-06-15T14:13:20Z' check_show rfc2822 "$TIME" 'Wed, 15 Jun 2016 16:13:20 +0200' check_show short "$TIME" '2016-06-15' check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200' @@ -69,6 +70,14 @@ check_show 'format:%s' '123456789 +1234' 123456789 check_show 'format:%s' '123456789 -1234' 123456789 check_show 'format-local:%s' '123456789 -1234' 123456789 +# negative TZ offset +TIME='1466000000 -0200' +check_show iso8601 "$TIME" '2016-06-15 12:13:20 -0200' +check_show iso8601-strict "$TIME" '2016-06-15T12:13:20-02:00' +check_show rfc2822 "$TIME" 'Wed, 15 Jun 2016 12:13:20 -0200' +check_show default "$TIME" 'Wed Jun 15 12:13:20 2016 -0200' +check_show raw "$TIME" '1466000000 -0200' + # arbitrary time absurdly far in the future FUTURE="5758122296 -0400" check_show iso "$FUTURE" "2152-06-19 18:24:56 -0400" TIME_IS_64BIT,TIME_T_IS_64BIT diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh index 8048856379..d3cb2a1cb9 100755 --- a/t/t0035-safe-bare-repository.sh +++ b/t/t0035-safe-bare-repository.sh @@ -29,9 +29,20 @@ expect_rejected () { grep -F "implicit-bare-repository:$pwd" "$pwd/trace.perf" } -test_expect_success 'setup bare repo in worktree' ' +test_expect_success 'setup an embedded bare repo, secondary worktree and submodule' ' git init outer-repo && - git init --bare outer-repo/bare-repo + git init --bare --initial-branch=main outer-repo/bare-repo && + git -C outer-repo worktree add ../outer-secondary && + test_path_is_dir outer-secondary && + ( + cd outer-repo && + test_commit A && + git push bare-repo +HEAD:refs/heads/main && + git -c protocol.file.allow=always \ + submodule add --name subn -- ./bare-repo subd + ) && + test_path_is_dir outer-repo/.git/worktrees/outer-secondary && + test_path_is_dir outer-repo/.git/modules/subn ' test_expect_success 'safe.bareRepository unset' ' @@ -53,8 +64,7 @@ test_expect_success 'safe.bareRepository in the repository' ' # safe.bareRepository must not be "explicit", otherwise # git config fails with "fatal: not in a git directory" (like # safe.directory) - test_config -C outer-repo/bare-repo safe.bareRepository \ - all && + test_config -C outer-repo/bare-repo safe.bareRepository all && test_config_global safe.bareRepository explicit && expect_rejected -C outer-repo/bare-repo ' @@ -86,4 +96,12 @@ test_expect_success 'no trace when "bare repository" is a subdir of .git' ' expect_accepted_implicit -C outer-repo/.git/objects ' +test_expect_success 'no trace in $GIT_DIR of secondary worktree' ' + expect_accepted_implicit -C outer-repo/.git/worktrees/outer-secondary +' + +test_expect_success 'no trace in $GIT_DIR of a submodule' ' + expect_accepted_implicit -C outer-repo/.git/modules/subn +' + test_done diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index ec974867e4..8bb2a8b453 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -210,6 +210,22 @@ test_expect_success 'superfluous value provided: boolean' ' test_cmp expect actual ' +test_expect_success 'superfluous value provided: boolean, abbreviated' ' + cat >expect <<-\EOF && + error: option `yes'\'' takes no value + EOF + test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \ + test-tool parse-options --ye=hi 2>actual && + test_cmp expect actual && + + cat >expect <<-\EOF && + error: option `no-yes'\'' takes no value + EOF + test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \ + test-tool parse-options --no-ye=hi 2>actual && + test_cmp expect actual +' + test_expect_success 'superfluous value provided: cmdmode' ' cat >expect <<-\EOF && error: option `mode1'\'' takes no value diff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh index 290b6eaaab..13ef69b92f 100755 --- a/t/t0211-trace2-perf.sh +++ b/t/t0211-trace2-perf.sh @@ -287,4 +287,235 @@ test_expect_success 'unsafe URLs are redacted by default' ' grep "d0|main|def_param|.*|remote.origin.url:https://user:pwd@example.com" actual ' +# Confirm that the requested command produces a "cmd_name" and a +# set of "def_param" events. +# +try_simple () { + test_when_finished "rm prop.perf actual" && + + cmd=$1 && + cmd_name=$2 && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + $cmd && + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + grep "d0|main|cmd_name|.*|$cmd_name" actual && + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual +} + +# Representative mainstream builtin Git command dispatched +# in run_builtin() in git.c +# +test_expect_success 'expect def_params for normal builtin command' ' + try_simple "git version" "version" +' + +# Representative query command dispatched in handle_options() +# in git.c +# +test_expect_success 'expect def_params for query command' ' + try_simple "git --man-path" "_query_" +' + +# remote-curl.c does not use the builtin setup in git.c, so confirm +# that executables built from remote-curl.c emit def_params. +# +# Also tests the dashed-command handling where "git foo" silently +# spawns "git-foo". Make sure that both commands should emit +# def_params. +# +# Pass bogus arguments to remote-https and allow the command to fail +# because we don't actually have a remote to fetch from. We just want +# to see the run-dashed code run an executable built from +# remote-curl.c rather than git.c. Confirm that we get def_param +# events from both layers. +# +test_expect_success 'expect def_params for remote-curl and _run_dashed_' ' + test_when_finished "rm prop.perf actual" && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + test_might_fail env \ + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + git remote-http x y && + + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + + grep "d0|main|cmd_name|.*|_run_dashed_" actual && + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual && + + grep "d1|main|cmd_name|.*|remote-curl" actual && + grep "d1|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual +' + +# Similarly, `git-http-fetch` is not built from git.c so do a +# trivial fetch so that the main git.c run-dashed code spawns +# an executable built from http-fetch.c. Confirm that we get +# def_param events from both layers. +# +test_expect_success 'expect def_params for http-fetch and _run_dashed_' ' + test_when_finished "rm prop.perf actual" && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + test_might_fail env \ + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + git http-fetch --stdin file:/// <<-EOF && + EOF + + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + + grep "d0|main|cmd_name|.*|_run_dashed_" actual && + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual && + + grep "d1|main|cmd_name|.*|http-fetch" actual && + grep "d1|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual +' + +# Historically, alias expansion explicitly emitted the def_param +# events (independent of whether the command was a builtin, a Git +# command or arbitrary shell command) so that it wasn't dependent +# upon the unpeeling of the alias. Let's make sure that we preserve +# the net effect. +# +test_expect_success 'expect def_params during git alias expansion' ' + test_when_finished "rm prop.perf actual" && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + test_config_global "alias.xxx" "version" && + + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + git xxx && + + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + + # "git xxx" is first mapped to "git-xxx" and the child will fail. + grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual && + + # We unpeel that and substitute "version" into "xxx" (giving + # "git version") and update the cmd_name event. + grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_git_alias_)" actual && + + # These def_param events could be associated with either of the + # above cmd_name events. It does not matter. + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual && + + # The "git version" child sees a different cmd_name hierarchy. + # Also test the def_param (only for completeness). + grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_git_alias_/version)" actual && + grep "d1|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual +' + +test_expect_success 'expect def_params during shell alias expansion' ' + test_when_finished "rm prop.perf actual" && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + test_config_global "alias.xxx" "!git version" && + + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + git xxx && + + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + + # "git xxx" is first mapped to "git-xxx" and the child will fail. + grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual && + + # We unpeel that and substitute "git version" for "git xxx" (as a + # shell command. Another cmd_name event is emitted as we unpeel. + grep "d0|main|cmd_name|.*|_run_shell_alias_ (_run_dashed_/_run_shell_alias_)" actual && + + # These def_param events could be associated with either of the + # above cmd_name events. It does not matter. + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual && + + # We get the following only because we used a git command for the + # shell command. In general, it could have been a shell script and + # we would see nothing. + # + # The child knows the cmd_name hierarchy so it includes it. + grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_shell_alias_/version)" actual && + grep "d1|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual +' + +test_expect_success 'expect def_params during nested git alias expansion' ' + test_when_finished "rm prop.perf actual" && + + test_config_global "trace2.configParams" "cfg.prop.*" && + test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" && + + test_config_global "cfg.prop.foo" "red" && + + test_config_global "alias.xxx" "yyy" && + test_config_global "alias.yyy" "version" && + + ENV_PROP_FOO=blue \ + GIT_TRACE2_PERF="$(pwd)/prop.perf" \ + git xxx && + + perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual && + + # "git xxx" is first mapped to "git-xxx" and try to spawn "git-xxx" + # and the child will fail. + grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual && + grep "d0|main|child_start|.*|.* class:dashed argv:\[git-xxx\]" actual && + + # We unpeel that and substitute "yyy" into "xxx" (giving "git yyy") + # and spawn "git-yyy" and the child will fail. + grep "d0|main|alias|.*|alias:xxx argv:\[yyy\]" actual && + grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_/_run_dashed_)" actual && + grep "d0|main|child_start|.*|.* class:dashed argv:\[git-yyy\]" actual && + + # We unpeel that and substitute "version" into "xxx" (giving + # "git version") and update the cmd_name event. + grep "d0|main|alias|.*|alias:yyy argv:\[version\]" actual && + grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_dashed_/_run_git_alias_)" actual && + + # These def_param events could be associated with any of the + # above cmd_name events. It does not matter. + grep "d0|main|def_param|.*|cfg.prop.foo:red" actual >actual.matches && + grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual && + + # However, we do not want them repeated each time we unpeel. + test_line_count = 1 actual.matches && + + # The "git version" child sees a different cmd_name hierarchy. + # Also test the def_param (only for completeness). + grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_dashed_/_run_git_alias_/version)" actual && + grep "d1|main|def_param|.*|cfg.prop.foo:red" actual && + grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual +' + test_done diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh index f0737593c3..b754b9fd74 100755 --- a/t/t1502-rev-parse-parseopt.sh +++ b/t/t1502-rev-parse-parseopt.sh @@ -322,4 +322,15 @@ check_invalid_long_option optionspec-neg --no-positive-only check_invalid_long_option optionspec-neg --negative check_invalid_long_option optionspec-neg --no-no-negative +test_expect_success 'ambiguous: --no matches both --noble and --no-noble' ' + cat >spec <<-\EOF && + some-command [options] + -- + noble The feudal switch. + EOF + test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \ + git rev-parse --parseopt -- <spec 2>err --no && + grep "error: ambiguous option: no (could be --noble or --no-noble)" err +' + test_done diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh index 16d6348b69..ac404945d4 100755 --- a/t/t2070-restore.sh +++ b/t/t2070-restore.sh @@ -5,6 +5,7 @@ test_description='restore basic functionality' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh index 27e85be40a..42d5522119 100755 --- a/t/t2071-restore-patch.sh +++ b/t/t2071-restore-patch.sh @@ -2,6 +2,7 @@ test_description='git restore --patch' +TEST_PASSES_SANITIZE_LEAK=true . ./lib-patch-mode.sh test_expect_success 'setup' ' diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh index 8198a1e578..86c9c88788 100755 --- a/t/t2072-restore-pathspec-file.sh +++ b/t/t2072-restore-pathspec-file.sh @@ -2,6 +2,7 @@ test_description='restore --pathspec-from-file' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_tick diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index a58f91035d..61e2be2903 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -465,7 +465,7 @@ test_with_bad_commit () { must_pass_arg="$2" && ( cd strict && - test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack" + test_must_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack" && git index-pack "$must_pass_arg" "test-$(cat pack-name).pack" ) } diff --git a/t/t6418-merge-text-auto.sh b/t/t6418-merge-text-auto.sh index 41288a60ce..48a62cb855 100755 --- a/t/t6418-merge-text-auto.sh +++ b/t/t6418-merge-text-auto.sh @@ -15,6 +15,7 @@ test_description='CRLF merge conflict across text=auto change GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index ec9c6de114..3d3e13ccf8 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -1935,4 +1935,18 @@ test_expect_success 'suppressing --- does not disable cut-line handling' ' test_cmp expected actual ' +test_expect_success 'handling of --- lines in conjunction with cut-lines' ' + echo "my-trailer: here" >expected && + + git interpret-trailers --parse >actual <<-\EOF && + subject + + my-trailer: here + --- + # ------------------------ >8 ------------------------ + EOF + + test_cmp expected actual +' + test_done diff --git a/tempfile.c b/tempfile.c index ecdebf1afb..ed88cf8431 100644 --- a/tempfile.c +++ b/tempfile.c @@ -50,15 +50,17 @@ static VOLATILE_LIST_HEAD(tempfile_list); -static void remove_template_directory(struct tempfile *tempfile, +static int remove_template_directory(struct tempfile *tempfile, int in_signal_handler) { if (tempfile->directory) { if (in_signal_handler) - rmdir(tempfile->directory); + return rmdir(tempfile->directory); else - rmdir_or_warn(tempfile->directory); + return rmdir_or_warn(tempfile->directory); } + + return 0; } static void remove_tempfiles(int in_signal_handler) @@ -353,16 +355,19 @@ int rename_tempfile(struct tempfile **tempfile_p, const char *path) return 0; } -void delete_tempfile(struct tempfile **tempfile_p) +int delete_tempfile(struct tempfile **tempfile_p) { struct tempfile *tempfile = *tempfile_p; + int err = 0; if (!is_tempfile_active(tempfile)) - return; + return 0; - close_tempfile_gently(tempfile); - unlink_or_warn(tempfile->filename.buf); - remove_template_directory(tempfile, 0); + err |= close_tempfile_gently(tempfile); + err |= unlink_or_warn(tempfile->filename.buf); + err |= remove_template_directory(tempfile, 0); deactivate_tempfile(tempfile); *tempfile_p = NULL; + + return err ? -1 : 0; } diff --git a/tempfile.h b/tempfile.h index d0413af733..2d2ae5b657 100644 --- a/tempfile.h +++ b/tempfile.h @@ -269,7 +269,7 @@ int reopen_tempfile(struct tempfile *tempfile); * `delete_tempfile()` for a `tempfile` object that has already been * deleted or renamed. */ -void delete_tempfile(struct tempfile **tempfile_p); +int delete_tempfile(struct tempfile **tempfile_p); /* * Close the file descriptor and/or file pointer if they are still @@ -433,6 +433,9 @@ void trace2_cmd_name_fl(const char *file, int line, const char *name) for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_command_name_fl) tgt_j->pfn_command_name_fl(file, line, name, hierarchy); + + trace2_cmd_list_config(); + trace2_cmd_list_env_vars(); } void trace2_cmd_mode_fl(const char *file, int line, const char *mode) @@ -464,17 +467,29 @@ void trace2_cmd_alias_fl(const char *file, int line, const char *alias, void trace2_cmd_list_config_fl(const char *file, int line) { + static int emitted = 0; + if (!trace2_enabled) return; + if (emitted) + return; + emitted = 1; + tr2_cfg_list_config_fl(file, line); } void trace2_cmd_list_env_vars_fl(const char *file, int line) { + static int emitted = 0; + if (!trace2_enabled) return; + if (emitted) + return; + emitted = 1; + tr2_list_env_vars_fl(file, line); } diff --git a/wt-status.c b/wt-status.c index 7108a92b52..2db4bb3a12 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1093,8 +1093,11 @@ size_t wt_status_locate_end(const char *s, size_t len) strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line); if (starts_with(s, pattern.buf + 1)) len = 0; - else if ((p = strstr(s, pattern.buf))) - len = p - s + 1; + else if ((p = strstr(s, pattern.buf))) { + size_t newlen = p - s + 1; + if (newlen < len) + len = newlen; + } strbuf_release(&pattern); return len; } |