* Re: [PATCH v2] diff: implement config.diff.renames=copies-harder
From: Elijah Newren @ 2023-12-27 2:24 UTC (permalink / raw)
To: Sam James; +Cc: git, Junio C Hamano
In-Reply-To: <20231226202102.3392518-1-sam@gentoo.org>
Hi,
It helps when providing a new iteration of a patch to do two things:
(1) provide a range-diff against the previous version to make it
easier for reviewers to see what has changed
(2) either reply to the previous submission or link to it in your
header so reviewers can find previous comments
In this case, v1 was over at
https://lore.kernel.org/git/pull.1606.git.1699010701704.gitgitgadget@gmail.com/.
On Tue, Dec 26, 2023 at 12:21 PM Sam James <sam@gentoo.org> wrote:
>
> This patch adds a config value for 'diff.renames' called 'copies-harder'
> which make it so '-C -C' is in effect always passed for 'git log -p',
> 'git diff', etc.
>
> This allows specifying that 'git log -p', 'git diff', etc should always act
> as if '-C --find-copies-harder' was passed.
>
> It has proven this especially useful for certain types of repository (like
> Gentoo's ebuild repositories) because files are often copies of a previous
> version:
>
> Suppose a directory 'sys-devel/gcc' contains recipes for building
> GCC, with one file for each supported upstream branch:
> gcc-13.x.build.recipe
> gcc-12.x.build.recipe
> gcc-11.x.build.recipe
> gcc-10.x.build.recipe
>
> gcc-13.x.build.recipe was started as a copy of gcc-12.x.build.recipe
> (which was started as a copy of gcc-11.x.build.recipe, etc.). Previous versions
> are kept around to support parallel installation of multiple versions.
>
> Being able to easily observe the diff relative to other recipes within the
> directory has been a quality of life improvement for such repo layouts.
>
> Cc: Elijah Newren <newren@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Sam James <sam@gentoo.org>
While the "Cc:" lines don't hurt, note that everything above this
point is included in the commit message, so we'd typically rather
those two lines be below the triple-dashed line.
> ---
> v2: Improve the commit message using Elijah Newren's example (it is indeed
> precisely what I was thinking of, but phrased better), and improve the documentation
> to explain better what the new config option actually does & refer to git-diff's
> --find-copies-harder.
>
> Documentation/config/diff.txt | 8 +++++---
> Documentation/config/status.txt | 3 ++-
> diff.c | 12 +++++++++---
> diff.h | 1 +
> diffcore-rename.c | 4 ++--
> merge-ort.c | 2 +-
> merge-recursive.c | 2 +-
> 7 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index bd5ae0c337..cdd8a74ec0 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -131,9 +131,11 @@ diff.renames::
> Whether and how Git detects renames. If set to "false",
> rename detection is disabled. If set to "true", basic rename
> detection is enabled. If set to "copies" or "copy", Git will
> - detect copies, as well. Defaults to true. Note that this
> - affects only 'git diff' Porcelain like linkgit:git-diff[1] and
> - linkgit:git-log[1], and not lower level commands such as
> + detect copies, as well. If set to "copies-harder", Git will spend extra
> + cycles to find more copies even in unmodified paths, see
> + '--find-copies-harder' in linkgit:git-diff[1]. Defaults to true.
> + Note that this affects only 'git diff' Porcelain like linkgit:git-diff[1]
> + and linkgit:git-log[1], and not lower level commands such as
> linkgit:git-diff-files[1].
>
> diff.suppressBlankEmpty::
> diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
> index 2ff8237f8f..7ca7a4becd 100644
> --- a/Documentation/config/status.txt
> +++ b/Documentation/config/status.txt
> @@ -33,7 +33,8 @@ status.renames::
> Whether and how Git detects renames in linkgit:git-status[1] and
> linkgit:git-commit[1] . If set to "false", rename detection is
> disabled. If set to "true", basic rename detection is enabled.
> - If set to "copies" or "copy", Git will detect copies, as well.
> + If set to "copies" or "copy", Git will detect copies, as well. If
> + set to "copies-harder", Git will try harder to detect copies.
It'd be nice to have the improved text from diff.renames here ("If set
to "copies-harder", Git will spend extra cycles to find more copies
even in unmodified paths.").
> Defaults to the value of diff.renames.
>
> status.showStash::
> diff --git a/diff.c b/diff.c
> index a2def45644..585acf9a99 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -206,8 +206,11 @@ int git_config_rename(const char *var, const char *value)
> {
> if (!value)
> return DIFF_DETECT_RENAME;
> + if (!strcasecmp(value, "copies-harder"))
> + return DIFF_DETECT_COPY_HARDER;
> if (!strcasecmp(value, "copies") || !strcasecmp(value, "copy"))
> - return DIFF_DETECT_COPY;
> + return DIFF_DETECT_COPY;
> +
I pointed out in response to v1 that these last couple lines, while a
nice cleanup, should be in a separate patch.
> return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
> }
>
> @@ -4849,8 +4852,11 @@ void diff_setup_done(struct diff_options *options)
> else
> options->flags.diff_from_contents = 0;
>
> - if (options->flags.find_copies_harder)
> + /* Just fold this in as it makes the patch-to-git smaller */
> + if (options->flags.find_copies_harder || options->detect_rename == DIFF_DETECT_COPY_HARDER) {
Again, I already responded to v1 that four of your lines were too long
and needed to be split. All four remain unsplit in your resubmission.
For future reference, when you resubmit a patch, please either (a)
first respond to the review explaining why you don't agree with the
suggested changes, or (b) include the fixes reviewers point out, or
(c) state in your cover letter or explanation after the '---' why you
chose to not include the suggested changes.
> + options->flags.find_copies_harder = 1;
> options->detect_rename = DIFF_DETECT_COPY;
> + }
>
> if (!options->flags.relative_name)
> options->prefix = NULL;
> @@ -5281,7 +5287,7 @@ static int diff_opt_find_copies(const struct option *opt,
> if (*arg != 0)
> return error(_("invalid argument to %s"), opt->long_name);
>
> - if (options->detect_rename == DIFF_DETECT_COPY)
> + if (options->detect_rename == DIFF_DETECT_COPY || options->detect_rename == DIFF_DETECT_COPY_HARDER)
> options->flags.find_copies_harder = 1;
> else
> options->detect_rename = DIFF_DETECT_COPY;
> diff --git a/diff.h b/diff.h
> index 66bd8aeb29..b29e5b777f 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -555,6 +555,7 @@ int git_config_rename(const char *var, const char *value);
>
> #define DIFF_DETECT_RENAME 1
> #define DIFF_DETECT_COPY 2
> +#define DIFF_DETECT_COPY_HARDER 3
>
> #define DIFF_PICKAXE_ALL 1
> #define DIFF_PICKAXE_REGEX 2
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 5a6e2bcac7..856291d66f 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -299,7 +299,7 @@ static int find_identical_files(struct hashmap *srcs,
> }
> /* Give higher scores to sources that haven't been used already */
> score = !source->rename_used;
> - if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY)
> + if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY && options->detect_rename != DIFF_DETECT_COPY_HARDER)
> continue;
> score += basename_same(source, target);
> if (score > best_score) {
> @@ -1405,7 +1405,7 @@ void diffcore_rename_extended(struct diff_options *options,
> trace2_region_enter("diff", "setup", options->repo);
> info.setup = 0;
> assert(!dir_rename_count || strmap_empty(dir_rename_count));
> - want_copies = (detect_rename == DIFF_DETECT_COPY);
> + want_copies = (detect_rename == DIFF_DETECT_COPY || detect_rename == DIFF_DETECT_COPY_HARDER);
> if (dirs_removed && (break_idx || want_copies))
> BUG("dirs_removed incompatible with break/copy detection");
> if (break_idx && relevant_sources)
> diff --git a/merge-ort.c b/merge-ort.c
> index 6491070d96..7749835465 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -4782,7 +4782,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
> * sanity check them anyway.
> */
> assert(opt->detect_renames >= -1 &&
> - opt->detect_renames <= DIFF_DETECT_COPY);
> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
> assert(opt->verbosity >= 0 && opt->verbosity <= 5);
> assert(opt->buffer_output <= 2);
> assert(opt->obuf.len == 0);
> diff --git a/merge-recursive.c b/merge-recursive.c
> index e3beb0801b..d52dd53660 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -3708,7 +3708,7 @@ static int merge_start(struct merge_options *opt, struct tree *head)
> assert(opt->branch1 && opt->branch2);
>
> assert(opt->detect_renames >= -1 &&
> - opt->detect_renames <= DIFF_DETECT_COPY);
> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
> assert(opt->detect_directory_renames >= MERGE_DIRECTORY_RENAMES_NONE &&
> opt->detect_directory_renames <= MERGE_DIRECTORY_RENAMES_TRUE);
> assert(opt->rename_limit >= -1);
> --
> 2.43.0
The patch looks close, but it does continue to violate style
guidelines and make unrelated changes, like v1. And the wording in
one of the documentation blurbs could be improved a little. Looking
forward to a v3 with those fixed.
^ permalink raw reply
* [PATCH] precious-files.txt: new document proposing new precious file type
From: Elijah Newren via GitGitGadget @ 2023-12-27 2:25 UTC (permalink / raw)
To: git; +Cc: Sebastian Thiel, Josh Triplett, Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
We have traditionally considered all ignored files to be expendable, but
users occasionally want ignored files that are not considered
expendable. Add a design document covering how to split ignored files
into two types: 'trashable' (what all ignored files are currently
considered) and 'precious' (the new type of ignored file).
Helped-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
precious-files.txt: new document proposing new precious file type
A couple months ago, we had another in-depth discussion of precious
files[1]. As with previous times, multiple strategies were discussed
(including multiple new ones), meaning we keep making the possible
solution space wider and never nail down an agreed path. I also got the
feeling we were potentially pigeonholing on a subset of the problem
space, and I thought it'd be good to better enumerate what areas of Git
are affected.
So, I went through the exercise of creating a design document to: (1)
provide a specific design proposal and explore it, (2) cover at a high
level the breadth of issues that an implementor needs to at least think
about and which reviewers should be aware of in terms of readiness of a
potential implementation, and (3) provide links to other discussions and
alternative proposals for completeness.
I had some off-list discussions with Sebastian about this proposal, and
he provided some helpful feedback. The idea at this point is that if
folks agree with the general direction, that he is going to be
implementing at least the first cut basic capability. I'll help review
changes, but I'm mostly interested in avoiding unfortunate surprises.
So...does the proposed direction seem reasonable to folks?
[1]
https://lore.kernel.org/git/79901E6C-9839-4AB2-9360-9EBCA1AAE549@icloud.com/
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1627%2Fnewren%2Fprecious-files-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1627/newren/precious-files-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1627
Documentation/technical/precious-files.txt | 540 +++++++++++++++++++++
1 file changed, 540 insertions(+)
create mode 100644 Documentation/technical/precious-files.txt
diff --git a/Documentation/technical/precious-files.txt b/Documentation/technical/precious-files.txt
new file mode 100644
index 00000000000..05c205b57bb
--- /dev/null
+++ b/Documentation/technical/precious-files.txt
@@ -0,0 +1,540 @@
+Precious Files Design Document
+==============================
+
+Table of Contents
+ * Objective
+ * Background
+ * File categorization exceptions
+ * Proposal
+ * Precious file specification
+ * Breakdown of suggested behaviors by command
+ * Backward compatibility notes
+ * Slightly incompatible syntax
+ * Interaction with sparse-checkout parsing
+ * Behavior of traditional flags
+ * Interaction with older Git clients
+ * Commands with modified meaning
+ * Implementation hints
+ * Data structures
+ * Code areas
+ * Minimum
+ * Out of scope
+ * Previous discussions
+ * Alternatives considered
+
+Objective
+---------
+Support "Precious" Files in git, a set of files which are considered
+ignored (e.g. do not show up in "git status" output) but are not expendable
+(thus won't be removed to make room for a file when switching or merging
+branches).
+
+Background
+----------
+In git we have different types of files, with various subdivisions:
+ * tracked
+ * present (i.e. part of sparse checkout)
+ * not present (i.e. not part of sparse checkout)
+ * not tracked
+ * ignored (also treated as expendable)
+ * untracked (more precisely, not-tracked-and-not-ignored, but often
+ referred to as simply "untracked" despite the fact that such a term
+ is easily mistaken as a synonym to "not tracked". However, we haven't
+ been fully consistent, and some places like `git ls-files --others`
+ may use "untracked" to refer to the larger not-tracked category).
+ Not considered expendable.
+
+Over the years, the fact that ignored files are unconditionally treated as
+expendable (so that other operations like git checkout might wipe them out
+to make room for files on the other branch) has occasionally caused
+problems. Many have expressed a desire for subdividing the ignored class,
+so that we have both ignored-and-expendable (possibly referred to as
+"trashable", covering the only type of ignored file we have today) and
+introducing ignored-and-not-expendable (often referred to as "precious").
+
+File categorization exceptions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Our division above into nice categories is actually a bit of a lie.
+
+Once upon a time untracked files were considered expendable[1]. Even after
+that changed, we still had lots of edge cases where untracked files were
+deleted when they shouldn't be, and ignored files weren't deleted when they
+should be[2]. While that has been (mostly) fixed, despite the general
+intent to preserve untracked files, we have special cases that are
+documented as not preserving them[4,5]. There are also a few codepaths
+that have comments about locations that might (or definitely do)
+erroneously delete untracked paths[6]. And at least one code path that is
+known to erroneously delete untracked paths which has not been commented:
+`git checkout <tree> <pathspec>`. And there may be more.
+
+[1] https://lore.kernel.org/git/CABPp-BFyR19ch71W10oJDFuRX1OHzQ3si971pMn6dPtHKxJDXQ@mail.gmail.com/
+[2] https://lore.kernel.org/git/pull.1036.v3.git.1632760428.gitgitgadget@gmail.com/
+[3] https://lore.kernel.org/git/de416f887d7ce24f20ad3ad4cc838394d6523635.1632760428.git.gitgitgadget@gmail.com/
+[4] https://lore.kernel.org/git/xmqqr1e2ejs9.fsf@gitster.g/
+[5] https://lore.kernel.org/git/de416f887d7ce24f20ad3ad4cc838394d6523635.1632760428.git.gitgitgadget@gmail.com/
+[6] https://lore.kernel.org/git/6b42a80bf3d46e16980d0724e8b07101225239d0.1632760428.git.gitgitgadget@gmail.com/
+
+This history and these exceptions matter to this proposal because:
+ * it highlights how much work can be involved in trying to treat a class
+ of files as not expendable
+ * the existing corner cases where untracked files are erroneously
+ treated as expendable will probably also double as corner cases where
+ precious files are treated as expendable
+ * the past fixes for treating untracked files as precious will likely
+ highlight the needed types of code changes to treat ignored files as
+ precious
+
+Proposal
+--------
+We propose adding another class of files: ignored-but-not-expendable,
+referred to by the shorthand of "precious". The proposal is simple at a
+high level, but there are many details to consider:
+ * How to specify precious files (extended .gitignore syntax? attributes?)
+ * Which commands should be modified, and how?
+ * How to handle flags that are essentially a partial implementation of
+ a precious capability (e.g. [--[no-]overwrite-ignore])?
+ * How will older Git clients behave on a repo with precious files?
+The subsequent sections will try to address these questions in more detail.
+
+One thing to highlight here is that the class formerly called
+`ignored` now has two subtypes: (1) the type we already have,
+ignored-and-expendable (sometimes referred to below as "trashable")
+and (2) the new type, ignored-and-not-expendable (referred to as
+"precious").
+
+Precious file specification
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+As per [P2]:
+
+ """
+ Even though I referred to the precious _attribute_ in some of these
+ discussions, between the attribute mechanism and the ignore
+ mechanism, I am actually leaning toward suggesting to extend the
+ exclude/ignore mechanism to introduce the "precious" class. That
+ way, we can avoid possible snafu arising from marking a path in
+ .gitignore as ignored, and in .gitattrbutes as precious, and have to
+ figure out how these two settings are to work together.
+ """
+
+we specify precious files via an extension to .gitignore. In particular,
+lines starting with a '$' character specify that the file is precious.
+For example:
+ $.config
+would say the file `.config` is precious.
+
+Now that there are three types of files specified by .gitignore files --
+untracked, trashable (ignored-and-expendable), and precious
+(ignored-and-not-expendable), the meaning of `!` at the begining of a line
+needs careful clarification. It could be seen as "not ignored" or as "not
+trashable", given the subdivision of ignored files that has occurred. We
+specifically take it to mean "not ignored", i.e. "untracked".
+
+This leaves us with a simple set of rules to provide to users about lines
+in their '.gitignore' file:
+ * No special prefix character => ignored-and-expendable ("trashable")
+ * A '$' prefix character => ignored-and-not-expendable ("precious")
+ * A '!' prefix character => not ignored, i.e. untracked
+
+It's worth noting that the traditional use of '!' as a negation
+character needs updating, given the introduction of a ternary state
+("not trashable" could mean either untracked or precious, which is
+ambiguous). Refrain from referring to '!' as a negation character to
+avoid confusion. To assist users in making this mindset shift, flag
+any line beginning with '!$' as an error. As always,
+backslash-escaping remains an option, allowing users to specify
+entries like '!\$foo' to mark a file named '$foo' as untracked.
+
+Breakdown of suggested behaviors by command
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+See also "Out of Scope" section below, particularly for:
+ * apply, am [without -3]
+ * checkout/restore
+ * checkout-index
+ * additional information on merge backends
+
+Documentation:
+ * audit for references to "ignore" and "ignored", to see which ones need
+ to now replace those with either "ignored-and-expendable" (or
+ "trashable"), and which can remain "ignored".
+ * audit for "exclude" and "excluded" (the older terminology for ignored
+ files) and update them as well.
+ * add references to "precious" (and perhaps "trashable) as needed (don't
+ forget the glossary)
+ * rm: update the documentation:
+ "Ignored files are deemed expendable and won't stop" ->
+ "Ignored files, unless specifically marked precious, are by default
+ deemed expendable and won't stop"
+ * ensure all codepaths touched by 0e29222e0c2 ("Documentation: call out
+ commands that nuke untracked files/directories", 2021-09-27) also call
+ out that they'll nuke precious files in addition to untracked ones.
+ * change the documentation for '!' in gitignore to stop using the term
+ 'negates'; it's potentially misleading now (negating a ternary value
+ yields an ambiguous value). Instead, the prefix is used to mark
+ untracked (or "not ignored") files.
+ * note that the --[no-]overwrite-ignore option is deprecated, and, since
+ it predated the introduction of precious files is also a misnomer. The
+ correct name of the option would actually be --[no-]overwrite-trashable
+ but it is too late to rename.
+ * consider documenting that merge's --no-overwrite-ignore option is
+ virtually worthless (only works with the fast-forwarding backend).
+ * consider auditing the code for 'untracked' and fixing those to be
+ 'not tracked' in cases where both 'untracked' and 'ignored' files
+ are meant
+
+checkout/switch:
+ * will need to not overwrite precious files when they are in the way of
+ switching branches, unless --force/-f is specified.
+
+checkout/restore:
+ * when passed a <tree> as a source, do not overwrite precious files
+ (NOR untracked files!), unless --force/-f is specified. [Could be
+ considered a stretch goal...]
+
+merge:
+ * do not overwrite precious files when they are in the way of merging
+ branches. (Must be handled in each and every merge strategy;
+ user-defined merge strategies may get this wrong.)
+
+read-tree:
+ * -u: do not overwrite precious files when they are in the way, unless...
+ * --reset and -u: overwrite precious files as well as untracked files.
+ Add to the warning under --reset about overwritten untracked files to
+ note that precious files are also overwritten.
+
+am -3, cherry-pick, rebase, revert, : same as above for checkout/switch and
+ merge.
+
+add:
+ * same as today, just make sure when we split the ignored array (ignored &
+ ignored_nr) into multiple categories that it continues working
+
+rm:
+ * make sure submodules are not removed if precious files are present.
+ Currently, rm will remove submodules if only ignored files are present.
+
+check-ignore:
+ * since this command exists for debugging gitignore rules, there needs to
+ be some kind of mechanism for differentiating between trashable and
+ precious files. It is okay if this comes with a new command-line flag,
+ but there should be some tests showing how it behaves both with and
+ without that flag when precious files are present
+
+clean:
+ * clarify the meaning of -x and -X options: -X now means only remove
+ trashable files. -x means remove both untracked and trashable files.
+ (See also [P17])
+ * add a --all option for removing all not-tracked files: untracked,
+ trashable, and precious.
+ * Other than --all, it is not worth adding flags for cleaning subsets of
+ not-tracked files that include precious files (thus, no flag for just
+ precious, or trashable and precious, or untracked and precious)
+ * Patterns with a leading '$' can be passed to --exclude, if wanted.
+
+ls-files:
+ * --ignored/-i: shows every kind of ignored file (thus behaving the same
+ as today, since there is no way to distinguish between the types of
+ ignored in the output)
+ * add new `--ignored=precious` and `--ignored=trashable` flags for
+ differentiating. A plain `--ignored` is like having both
+ `--ignored=precious` and `--ignored=trashable` specified.
+ * --exclude,--exclude-from can now take patterns with a leading '$' and
+ the file will be considered precious rather than trashable.
+
+status:
+ * --ignored (without additional parameters) continues behaving as-is: it
+ prints both trashable and precious files in its "Ignored" category with
+ no distinguishing.
+ * --ignored --short will continue showing trashable files with '!!', but
+ will now show precious files using '$$'.
+ * --ignored --porcelain={v1,v2} will continue showing precious files
+ with the '!' character, since scripts may not be prepared to parse a
+ leading '$'. We can't break those scripts, even if it'd avoid the
+ off chance that those scripts act on the information about "ignored"
+ files and end up nuking precious files.
+ * --ignored --porcelain=v3 will need to be introduced to show precious
+ files with a leading '$'.
+
+sparse-checkout:
+ * the --rules-file option should be tested with a pattern with a leading
+ '$' to make sure it prints an expected error.
+ * it might be worth noting somewhere that sparse-checkout treats
+ ignored files as precious; when sparsifying, it attempts to remove
+ directories that do not match the sparse specification, but will
+ leave them present if any of the tracked files are modified, or if
+ there are any not-tracked files present. That includes ignored
+ files. That means no additional work is needed for precious
+ support; I just mention it for completeness.
+
+Backward compatibility notes
+----------------------------
+There are multiple issues that impinge on backward compatibility (either in
+terms of special care we need to take, or in terms of messaging we may need
+to send out about changes):
+ * Slightly incompatible syntax
+ * Interaction with sparse-checkout parsing
+ * Behavior of traditional flags
+ * Interaction with older Git clients
+ * Commands with modified meaning
+We'll discuss each in its own subsection below.
+
+Slightly incompatible syntax
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+This new syntax obviously breaks backward compatibility in that an ignored
+path named `$.config` would now have to be specified as `\$.config`. This
+is similar to how introducing `!` as a prefix in .gitignore files was a
+backward compatibility break. We expect and hope that the fallout will be
+minor. See also [P10].
+
+Interaction with sparse-checkout parsing
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The $GIT_DIR/info/sparse-checkout file also makes use of gitignore syntax
+and the gitignore parsing to read the file. It differs in that the files
+specified are considered the files to be included (i.e. present in the
+working copy) rather than which files should be excluded, but otherwise
+has until now used identical syntax and parsing.
+
+However, for sparse-checkout there is no third type of file, so the '$'
+prefix makes no sense for it. As such, it should be an error for any
+lines to begin with '$' in a sparse-checkout file.
+
+(This also means that if anyone really did have a path beginning with '$'
+in sparse-checkout files previously, then they now need to backslash escape
+them, the same as with .gitignore files.)
+
+While we could theoretically avoid this small backward compatibility break
+for sparse-checkout parsing by just treating a leading '$' the way it
+traditionally has been done, I am worried about practically maintaining that
+solution:
+ * the gitignore parsing is peppered with references like 'exclude' that
+ are specific to the gitignore case
+ * because of the above, it is _heavily_ confusing to attempt to read and
+ understand the gitignore handling while considering the sparse-checkout
+ case. I've been tripped up by it *many* times.
+ * I think trying to reuse the existing parsing engine and have it handling
+ both old and new syntax is a recipe for failure. It'd be much cleaner
+ to have errors thrown if the processing turns up any "precious" files,
+ or perhaps if any line starts with '$'.
+ * I think making a copy of the existing parsing, and then letting them
+ diverge, means the two will eventually diverge even further, and we
+ would need to make a copy of all the documentation about gitignore rules
+ for sparse-checkout, all for the non-default non-cone case we are
+ already recommending users away from.
+
+Behavior of traditional flags
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+There are two flags to consider here: the --porcelain flag to git-status,
+and the --no-overwrite-ignore command to checkout & merge commands. For
+the --porcelain flag to git-status, see the "Breakdown of suggested
+behaviors by command" and look for git-status there. The rest of this
+section will focus on --[no-]overwrite-ignore.
+
+People have wanted precious files long enough, that they implemented an
+interim kludge of sorts -- a command line option that can be passed to
+various subcommands that treats all ignored files as precious:
+--no-overwrite-ignore.
+
+In particular, this flag can be passed to both git-checkout, and git-merge.
+However, in merge's case, the support depended the flag being passed to the
+backend and the backend supporting it. The builtin/merge.c code only ever
+bothered to pass this flag down to the fast-forwarding merge handling code,
+so it never worked with any backends that actually create a merge commit.
+
+We do need to keep these flags working, at least as much as they did
+previously. However, we don't want to consider them desired features,
+which would lead us to making related equivalents for precious files like
+--overwrite-precious. Instead we will:
+ * Keep --[no-]overwrite-ignore working, as much as it already was.
+ * Recommend users mark precious files in their gitignore files instead of
+ using these flags
+
+Interaction with older Git clients
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Older Git clients will not understand precious files. This means that:
+ * precious files will be considered untracked and not ignored.
+ * most comands will preserve these files, since untracked-and-not-ignored
+ are not considered expendable.
+ * git status will continue listing these files
+ * git add will add these files without requiring -f.
+
+This seems like a reasonable tradeoff that only has minor annoyances. The
+alternative of having the precious files treated as ignored has the very
+risky trade-off of deleting files which the users marked as important for
+us to keep.
+
+Commands with modified meaning
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+In clean, we adjust the meaning of both -x and -X:
+ -X: remove only trashable files
+ -x: remove untracked and trashable files (but preserve precious ones)
+
+Implementation hints
+--------------------
+
+Data structures
+~~~~~~~~~~~~~~~
+ * We will want to add a `precious` and `precious_nr` in dir_struct,
+ similar to the current entries/nr or ignored/ignored_nr.
+ * We may want to rename `ignored` and `ignored_nr` in dir_struct to
+ `trashable` and `trashable_nr`.
+
+Code areas
+~~~~~~~~~~
+ * "preserve_ignored", a flag in the code for handling the
+ --[no-]overwrite-ignore flag, is a very helpful marker about what needs
+ to be tweaked and how to tweak it to preserve more files. In particular,
+ note that --no-overwrite-ignore works by telling the machinery in dir.c
+ to not do the setup_standard_excludes() stuff, so that all ignored files
+ just look like untracked files. We'll need something slightly smarter,
+ which makes precious files look like untracked while trashable files
+ still appear in ignored. Shouldn't be too bad.
+ * we might need to add another entry to the unpack_trees_reset_type
+ enum. Or perhaps we still keep both UNPACK_RESET_PROTECT_UNTRACKED
+ and UNPACK_RESET_OVERWRITE_UNTRACKED but rename them with
+ s/UNTRACKED/NOT_EXPENDABLE/ so it is clear they handle both untracked and
+ precious files. Not sure which is needed yet.
+ * dir_struct->flags _might_ need new entries.
+ * ensure all relevant codepaths touched by 94b7f1563ac ("Comment important
+ codepaths regarding nuking untracked files/dirs", 2021-09-27) are either
+ fixed or also mention precious files
+ * am/rebase/checkout[without -f]: see 480d3d6bf90 ("Change unpack_trees'
+ 'reset' flag into an enum", 2021-09-27)
+ * Merge backends:
+ * (see also "Out of scope" section)
+ * merge-ort can be fixed by fixing the checkout code.
+ * merge-resolve and merge-octopus can probably be fixed by fixing
+ git-reset.
+ * stash:
+ * there is an existing --include-untracked option. There was no reason
+ to add a --include-ignored, because ignored files were trashable. Do
+ we need to add a --include-precious, though?
+ * this is a sad pile of shell-reimplemented-in-C. It's just awful.
+ See b34ab4a43ba ("stash: remove unnecessary process forking",
+ 2020-12-01) and ba359fd5070 ("stash: fix stash application in
+ sparse-checkouts", 2020-12-01) and 94b7f1563ac ("Comment important
+ codepaths regarding nuking untracked files/dirs", 2021-09-27).
+ Fixing stash to not nuke precious files (and to not nuke untracked
+ files either) might mean expunging the stupid
+ shell-reimplemented-in-C design, or at least moving things more in
+ that direction.
+ * rebase (merge backend), revert, cherry-pick, am -3: should automatically
+ be handled by getting merge-ort to work, which should work by making
+ checkout/switch work.
+ * bisect: should work by making checkout work
+
+Minimum
+~~~~~~~
+
+I think for a minimum implementation, we need to ensure that the following
+are handled:
+ * parsing:
+ * parsing of lines starting with '$' in .gitignore
+ * erroring on lines starting with '!$' in .gitignore
+ * erroring on lines starting with '$' in $GIT_DIR/info/sparse-checkout
+ * commands with support:
+ * switch/checkout
+ * merge when using the ort backend
+ * read-tree -u [without --reset] (due to internal use)
+ * ls-files
+
+Out of scope
+------------
+The following tasks are currently out of scope for this proposal:
+
+apply, am [without -3]: apply won't overwrite any file in the working
+ directory even when a new file is in the patch. It should overwrite
+ trashable files. We could log that bug via testcase, but make sure
+ there's a companion testcase that ensures overwriting untracked or
+ precious files continues to make apply throw an error. However, since
+ apply/am don't misbehave for precious files, we can defer this to later.
+
+checkout-index: similar to apply; won't overwrite any existing files, but
+ trashable files should be overwritten
+
+reset --hard:
+ * `git reset --hard` is a little funny and we have thought about changing
+ it[4]. However, that can be left for later and will not be tackled as
+ part of the work of introducing "precious" files as a concept.
+
+merge backends:
+ * it may make sense to try to make --no-overwrite-ignore work with more
+ merge backends, both because it's technically documented behavior, and
+ because doing so may be a step towards getting precious files supported
+ * when multiple merge strategies are specified, builtin/merge.c will
+ stash and restore state between the attempt of different strategies.
+ Since the reset_hard() function invokes `read-tree --reset -u`, there
+ might be a way to cause it to trash untracked files or to trash
+ precious files, depending on what the merge strategies did. It seems
+ unlikely (maybe the strategy handles D/F conflicts or rename
+ conflicts by renaming files in the way, and happens to rename a
+ precious file to a path that is considered either untracked or
+ precious -- merge-recursive certainly did this something like this
+ once upon a time and still might); we can probably ignore it for now.
+ * merge-recursive is a lost cause; it'd be a _huge_ amount of effort to
+ fix, but we intend to deprecate and delete it soon anyway (making all
+ requests for recursive just trigger ort instead).
+ * user-defined merge strategies are up to their authors to get right.
+ Odds are they won't, but odds are they already incorrectly nuke
+ untracked files too because who'd pay attention to a special case
+ like files being in the way of a merge? Anyway, "not our problem". :-)
+
+Previous discussions
+--------------------
+
+A far from exhaustive sampling of various past conversations on the topic:
+
+[P1] https://lore.kernel.org/git/7vipsnar23.fsf@alter.siamese.dyndns.org/
+[P2] https://lore.kernel.org/git/xmqqttqytnqb.fsf@gitster.g/
+[P3] https://lore.kernel.org/git/79901E6C-9839-4AB2-9360-9EBCA1AAE549@icloud.com/
+[P4] https://lore.kernel.org/git/87a6q9kacx.fsf@evledraar.gmail.com/
+[P5] https://lore.kernel.org/git/20190216114938.18843-1-pclouds@gmail.com/
+[P6] https://lore.kernel.org/git/87ftsi68ke.fsf@evledraar.gmail.com/
+[P7] https://lore.kernel.org/git/xmqqo7ub4sfh.fsf@gitster.g/
+[P8] https://lore.kernel.org/git/7v4oepaup7.fsf@alter.siamese.dyndns.org/
+[P9] https://lore.kernel.org/git/20181112232209.GK890086@genre.crustytoothpaste.net/
+[P10] https://lore.kernel.org/git/xmqqttqvg4lw.fsf@gitster.g/
+[P11] https://lore.kernel.org/git/xmqqk1hrr91s.fsf@gitster-ct.c.googlers.com/
+[P12] https://lore.kernel.org/git/9C4A2AFD-AAA2-4ABA-8A8B-2133FD870366@icloud.com/
+[P13] https://lore.kernel.org/git/xmqqfs2e3292.fsf@gitster.g/
+[P14] https://lore.kernel.org/git/0deee2bc-1775-4459-906d-1d44b3103499@gmail.com/
+[P15] https://lore.kernel.org/git/ZSkpOc%2FdcGcrFQNU@ugly/
+[P16] https://lore.kernel.org/git/xmqqil79t82q.fsf@gitster.g/
+[P17] https://lore.kernel.org/git/xmqqo7h6tnib.fsf@gitster.g/
+
+Alternatives considered
+-----------------------
+There have been multiple alternatives considered, along a few different
+axes:
+ * .gitattributes instead of .gitignore
+ * leaving sparse-checkout alone
+ * Trashable [P9,P11]
+ * Alternative gitignore syntax
+
+The choice of .gitattributes vs .gitignore was already addressed in the
+"Precious file specification" section.
+
+The choice to modify or leave alone the parsing of
+$GIT_DIR/info/sparse-checkout was already addressed in the "Interaction
+with sparse-checkout parsing" section.
+
+One alternative raised in the past was treating ignored files as not
+expendable by default, and then introducing a new category of
+ignored-but-expendable. This new category has been dubbed "trashable" in
+the past. That may have been a reasonable solution if Git did not have a
+large userbase already, but moving in this direction would cause severe
+problems for existing builds everywhere[P9] and would require users to
+doubly configure most files (since it is expected that
+ignored-but-expendable is a much larger class of files than
+ignored-but-precious). See also [P11].
+
+There have been multiple alternative suggestions for extending gitignore
+syntax to handle precious files and optionally future extensions as well.
+For example: [P10, P12, P13, P14, P15, P16] However:
+ * There have been on and off requests for precious files for about 14
+ years
+ * We are not aware of other types of extensions needed; there might
+ not be any
+ * The alternatives all seem much more complex to explain to users than
+ the simple proposal here.
+In particular, we like the simplicity of the providing the simple mapping
+to users from the penultimate paragraph of the "Precious file
+specification" section (the one regarding no-prefix vs. '!' vs '$').
base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Junio C Hamano @ 2023-12-27 5:28 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget
Cc: git, Sebastian Thiel, Josh Triplett, Elijah Newren
In-Reply-To: <pull.1627.git.1703643931314.gitgitgadget@gmail.com>
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Elijah Newren <newren@gmail.com>
>
> We have traditionally considered all ignored files to be expendable, but
> users occasionally want ignored files that are not considered
> expendable. Add a design document covering how to split ignored files
> into two types: 'trashable' (what all ignored files are currently
> considered) and 'precious' (the new type of ignored file).
The proposed syntax is a bit different from what I personally prefer
(which is Phillip's [P14] or something like it), but I consider that
the more valuable parts of this document is about how various
commands ought to interact with precious paths, which shouldn't
change regardless of the syntax.
Thanks for putting this together.
^ permalink raw reply
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Elijah Newren @ 2023-12-27 6:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: Elijah Newren via GitGitGadget, git, Sebastian Thiel,
Josh Triplett
In-Reply-To: <xmqq8r5gfc3j.fsf@gitster.g>
On Tue, Dec 26, 2023 at 9:28 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > We have traditionally considered all ignored files to be expendable, but
> > users occasionally want ignored files that are not considered
> > expendable. Add a design document covering how to split ignored files
> > into two types: 'trashable' (what all ignored files are currently
> > considered) and 'precious' (the new type of ignored file).
>
> The proposed syntax is a bit different from what I personally prefer
> (which is Phillip's [P14] or something like it), but I consider that
> the more valuable parts of this document is about how various
> commands ought to interact with precious paths, which shouldn't
> change regardless of the syntax.
I agree that syntax and command behavior are mostly separate issues,
but unfortunately they are not orthogonal. In particular, syntax of
precious file specification is directly tied to fallback behavior for
older Git clients, and it might potentially affect backward
compatibility of non-cone-mode sparse-checkout syntax as well.
I think fallback behavior is of particular importance. There are
precisely two choices in our design for how older Git versions can
treat precious files:
* ignored-and-expendable
* untracked-and-precious
If we pick syntax that causes older Git versions to treat precious
files as ignored-and-expendable, we risk deleting important files.
Alternatively, if we pick syntax that causes older Git versions to
treat precious files as untracked-and-precious, they won't be ignored
by e.g. git-status, and are easier to accidentally add with git-add.
I felt the "precious" bit was much more important than the "ignored"
bit of "precious" files, so I thought untracked-and-precious was a
better fallback. However, to get that, we have to rule out lots of
the syntax proposals, such as Phillip's [P14].
Anyway, I'm open to alternative syntax, but we need to measure it
against the relevant criteria, which I believe are:
(A) ease for users to understand, remember, and use
(B) size of backward compatibility break with .gitignore syntax
(C) appropriateness of implied fallback behavior for older Git
clients with precious files
(D) room for additional extension in .gitignore files
(E) potential affects on backward compatibility of non-cone
sparse-checkout syntax
We probably also need to agree on the relative importance of these
criteria; personally, I would probably order them from most important
to least as C, B, E, A, D.
Phillip's P14 is better for D, and perhaps a little better for B, but
I thought slightly worse for A, and much worse for C. (I think
there's no significant relative difference for E between his proposed
syntax and mine.)
> Thanks for putting this together.
^ permalink raw reply
* [PATCH v3] sideband.c: remove redundant 'NEEDSWORK' tag
From: Chandra Pratap via GitGitGadget @ 2023-12-27 10:20 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1625.v2.git.1703351016486.gitgitgadget@gmail.com>
From: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
sideband.c: replace int with size_t for clarity
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1625%2FChand-ra%2Fdusra-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1625/Chand-ra/dusra-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1625
Range-diff vs v2:
1: fdacc69ae3b ! 1: 273415aa6a4 sideband.c: replace int with size_t for clarity
@@ Metadata
Author: Chandra Pratap <chandrapratap3519@gmail.com>
## Commit message ##
- sideband.c: replace int with size_t for clarity
-
- Replace int with size_t for non-negative values to improve
- clarity and remove the 'NEEDSWORK' tag associated with it.wq
+ sideband.c: remove redundant 'NEEDSWORK' tag
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
@@ sideband.c: void list_config_color_sideband_slots(struct string_list *list, cons
* passed as the first N characters of the SRC array.
*
- * NEEDSWORK: use "size_t n" instead for clarity.
++ * It is fine to use "int n" here instead of "size_t n" as all calls to this
++ * function pass an 'int' parameter.
*/
--static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
-+static void maybe_colorize_sideband(struct strbuf *dest, const char *src, size_t n)
+ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
{
- int i;
-
-@@ sideband.c: static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
-
- for (i = 0; i < ARRAY_SIZE(keywords); i++) {
- struct keyword_entry *p = keywords + i;
-- int len = strlen(p->keyword);
-+ size_t len = strlen(p->keyword);
-
- if (n < len)
- continue;
sideband.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sideband.c b/sideband.c
index 6cbfd391c47..a89201d52ac 100644
--- a/sideband.c
+++ b/sideband.c
@@ -69,7 +69,8 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
* of the line. This should be called for a single line only, which is
* passed as the first N characters of the SRC array.
*
- * NEEDSWORK: use "size_t n" instead for clarity.
+ * It is fine to use "int n" here instead of "size_t n" as all calls to this
+ * function pass an 'int' parameter.
*/
static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
{
base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Christian Couder @ 2023-12-27 10:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Achu Luma, git, Christian Couder, Phillip Wood, Josh Steadmon
In-Reply-To: <xmqqsf3ohkew.fsf@gitster.g>
On Tue, Dec 26, 2023 at 7:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Achu Luma <ach.lumap@gmail.com> writes:
> > +/* Macro to test a character type */
> > +#define TEST_CTYPE_FUNC(func, string) \
> > +static void test_ctype_##func(void) \
> > +{ \
> > + int i; \
> > + for (i = 0; i < 256; i++) \
> > + check_int(func(i), ==, is_in(string, i)); \
> > +}
>
> Now, we let check_int() to do the checking for each and every byte
> value for the class. check_int() uses different reporting and shows
> the problematic value in a way that is more verbose and at the same
> time is a less specific and harder to understand:
>
> test_msg(" left: %"PRIdMAX, a);
> test_msg(" right: %"PRIdMAX, b);
>
> But that is probably the price to pay to use a more generic
> framework, I guess.
I have added Phillip and Josh in Cc: as they might have ideas about this.
Also it might not be a big issue here, but when the new unit test
framework was proposed, I commented on the fact that "left" and
"right" were perhaps a bit less explicit than "actual" and "expected".
> > +int cmd_main(int argc, const char **argv) {
> > + /* Run all character type tests */
> > + TEST(test_ctype_isspace(), "isspace() works as we expect");
> > + TEST(test_ctype_isdigit(), "isdigit() works as we expect");
> > + TEST(test_ctype_isalpha(), "isalpha() works as we expect");
> > + TEST(test_ctype_isalnum(), "isalnum() works as we expect");
> > + TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
> > + TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
> > + TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
> > + TEST(test_ctype_isascii(), "isascii() works as we expect");
> > + TEST(test_ctype_islower(), "islower() works as we expect");
> > + TEST(test_ctype_isupper(), "isupper() works as we expect");
> > + TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
> > + TEST(test_ctype_ispunct(), "ispunct() works as we expect");
> > + TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
> > + TEST(test_ctype_isprint(), "isprint() works as we expect");
> > +
> > + return test_done();
> > +}
>
> As a practice to use the unit-tests framework, the patch looks OK.
> helper/test-ctype.c indeed is an oddball that runs once and checks
> everything it wants to check, for which the unit tests framework is
> much more suited.
Yeah, I agree.
> Let's see how others react and then queue.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: René Scharfe @ 2023-12-27 11:57 UTC (permalink / raw)
To: Christian Couder, Junio C Hamano
Cc: Achu Luma, git, Christian Couder, Phillip Wood, Josh Steadmon
In-Reply-To: <CAP8UFD1vC6=7ESx1w7S9Q9P0Bsc+c03wgHToNBaP+ivvm9BKBg@mail.gmail.com>
Am 27.12.23 um 11:57 schrieb Christian Couder:
> On Tue, Dec 26, 2023 at 7:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Achu Luma <ach.lumap@gmail.com> writes:
>
>>> +/* Macro to test a character type */
>>> +#define TEST_CTYPE_FUNC(func, string) \
>>> +static void test_ctype_##func(void) \
>>> +{ \
>>> + int i; \
>>> + for (i = 0; i < 256; i++) \
>>> + check_int(func(i), ==, is_in(string, i)); \
>>> +}
>>
>> Now, we let check_int() to do the checking for each and every byte
>> value for the class. check_int() uses different reporting and shows
>> the problematic value in a way that is more verbose and at the same
>> time is a less specific and harder to understand:
>>
>> test_msg(" left: %"PRIdMAX, a);
>> test_msg(" right: %"PRIdMAX, b);
>>
>> But that is probably the price to pay to use a more generic
>> framework, I guess.
>
> I have added Phillip and Josh in Cc: as they might have ideas about this.
You can write custom messages for custom tests using test_assert().
> Also it might not be a big issue here, but when the new unit test
> framework was proposed, I commented on the fact that "left" and
> "right" were perhaps a bit less explicit than "actual" and "expected".
True.
>>> +int cmd_main(int argc, const char **argv) {
>>> + /* Run all character type tests */
>>> + TEST(test_ctype_isspace(), "isspace() works as we expect");
>>> + TEST(test_ctype_isdigit(), "isdigit() works as we expect");
>>> + TEST(test_ctype_isalpha(), "isalpha() works as we expect");
>>> + TEST(test_ctype_isalnum(), "isalnum() works as we expect");
>>> + TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
>>> + TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
>>> + TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
>>> + TEST(test_ctype_isascii(), "isascii() works as we expect");
>>> + TEST(test_ctype_islower(), "islower() works as we expect");
>>> + TEST(test_ctype_isupper(), "isupper() works as we expect");
>>> + TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
>>> + TEST(test_ctype_ispunct(), "ispunct() works as we expect");
>>> + TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
>>> + TEST(test_ctype_isprint(), "isprint() works as we expect");
>>> +
>>> + return test_done();
>>> +}
>>
>> As a practice to use the unit-tests framework, the patch looks OK.
The added repetition is a bit grating. With a bit of setup, loop
unrolling and stringification you can retain the property of only having
to mention the class name once. Demo patch below.
>> helper/test-ctype.c indeed is an oddball that runs once and checks
>> everything it wants to check, for which the unit tests framework is
>> much more suited.
>
> Yeah, I agree.
Indeed: test-ctype does unit tests, so the unit test framework should
be a perfect fit. It still feels a bit raw that this point, though,
but that's to be expected.
René
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index 41189ba9f9..7903856cec 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -13,13 +13,23 @@ static int is_in(const char *s, int ch)
return !!strchr(s, ch);
}
-/* Macro to test a character type */
-#define TEST_CTYPE_FUNC(func, string) \
-static void test_ctype_##func(void) \
-{ \
- int i; \
- for (i = 0; i < 256; i++) \
- check_int(func(i), ==, is_in(string, i)); \
+struct ctype {
+ const char *name;
+ const char *expect;
+ int actual[256];
+};
+
+static void test_ctype(const struct ctype *class)
+{
+ for (int i = 0; i < 256; i++) {
+ int expect = is_in(class->expect, i);
+ int actual = class->actual[i];
+ int res = test_assert(TEST_LOCATION(), class->name,
+ actual == expect);
+ if (!res)
+ test_msg("%s classifies char %d (0x%02x) wrongly",
+ class->name, i, i);
+ }
}
#define DIGIT "0123456789"
@@ -40,37 +50,39 @@ static void test_ctype_##func(void) \
"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
"\x7f"
-TEST_CTYPE_FUNC(isdigit, DIGIT)
-TEST_CTYPE_FUNC(isspace, " \n\r\t")
-TEST_CTYPE_FUNC(isalpha, LOWER UPPER)
-TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT)
-TEST_CTYPE_FUNC(is_glob_special, "*?[\\")
-TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|")
-TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~")
-TEST_CTYPE_FUNC(isascii, ASCII)
-TEST_CTYPE_FUNC(islower, LOWER)
-TEST_CTYPE_FUNC(isupper, UPPER)
-TEST_CTYPE_FUNC(iscntrl, CNTRL)
-TEST_CTYPE_FUNC(ispunct, PUNCT)
-TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF")
-TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
+#define APPLY16(f, n) \
+ f(n + 0x0), f(n + 0x1), f(n + 0x2), f(n + 0x3), \
+ f(n + 0x4), f(n + 0x5), f(n + 0x6), f(n + 0x7), \
+ f(n + 0x8), f(n + 0x9), f(n + 0xa), f(n + 0xb), \
+ f(n + 0xc), f(n + 0xd), f(n + 0xe), f(n + 0xf)
+#define APPLY256(f) \
+ APPLY16(f, 0x00), APPLY16(f, 0x10), APPLY16(f, 0x20), APPLY16(f, 0x30),\
+ APPLY16(f, 0x40), APPLY16(f, 0x50), APPLY16(f, 0x60), APPLY16(f, 0x70),\
+ APPLY16(f, 0x80), APPLY16(f, 0x90), APPLY16(f, 0xa0), APPLY16(f, 0xb0),\
+ APPLY16(f, 0xc0), APPLY16(f, 0xd0), APPLY16(f, 0xe0), APPLY16(f, 0xf0),\
+
+#define CTYPE(name, expect) { #name, expect, { APPLY256(name) } }
int cmd_main(int argc, const char **argv) {
+ struct ctype classes[] = {
+ CTYPE(isdigit, DIGIT),
+ CTYPE(isspace, " \n\r\t"),
+ CTYPE(isalpha, LOWER UPPER),
+ CTYPE(isalnum, LOWER UPPER DIGIT),
+ CTYPE(is_glob_special, "*?[\\"),
+ CTYPE(is_regex_special, "$()*+.?[\\^{|"),
+ CTYPE(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~"),
+ CTYPE(isascii, ASCII),
+ CTYPE(islower, LOWER),
+ CTYPE(isupper, UPPER),
+ CTYPE(iscntrl, CNTRL),
+ CTYPE(ispunct, PUNCT),
+ CTYPE(isxdigit, DIGIT "abcdefABCDEF"),
+ CTYPE(isprint, LOWER UPPER DIGIT PUNCT " "),
+ };
/* Run all character type tests */
- TEST(test_ctype_isspace(), "isspace() works as we expect");
- TEST(test_ctype_isdigit(), "isdigit() works as we expect");
- TEST(test_ctype_isalpha(), "isalpha() works as we expect");
- TEST(test_ctype_isalnum(), "isalnum() works as we expect");
- TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
- TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
- TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
- TEST(test_ctype_isascii(), "isascii() works as we expect");
- TEST(test_ctype_islower(), "islower() works as we expect");
- TEST(test_ctype_isupper(), "isupper() works as we expect");
- TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
- TEST(test_ctype_ispunct(), "ispunct() works as we expect");
- TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
- TEST(test_ctype_isprint(), "isprint() works as we expect");
+ for (int i = 0; i < ARRAY_SIZE(classes); i++)
+ TEST(test_ctype(&classes[i]), "%s works", classes[i].name);
return test_done();
}
^ permalink raw reply related
* Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Phillip Wood @ 2023-12-27 14:40 UTC (permalink / raw)
To: René Scharfe, Christian Couder, Junio C Hamano
Cc: Achu Luma, git, Christian Couder, Phillip Wood, Josh Steadmon
In-Reply-To: <f743b473-40f8-423d-bf5b-d42b92e5aa1b@web.de>
On 27/12/2023 11:57, René Scharfe wrote:
> Am 27.12.23 um 11:57 schrieb Christian Couder:
>> On Tue, Dec 26, 2023 at 7:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> Achu Luma <ach.lumap@gmail.com> writes:
>>
>>>> +/* Macro to test a character type */
>>>> +#define TEST_CTYPE_FUNC(func, string) \
>>>> +static void test_ctype_##func(void) \
>>>> +{ \
>>>> + int i; \
>>>> + for (i = 0; i < 256; i++) \
>>>> + check_int(func(i), ==, is_in(string, i)); \
>>>> +}
>>>
>>> Now, we let check_int() to do the checking for each and every byte
>>> value for the class. check_int() uses different reporting and shows
>>> the problematic value in a way that is more verbose and at the same
>>> time is a less specific and harder to understand:
>>>
>>> test_msg(" left: %"PRIdMAX, a);
>>> test_msg(" right: %"PRIdMAX, b);
>>>
>>> But that is probably the price to pay to use a more generic
>>> framework, I guess.
>>
>> I have added Phillip and Josh in Cc: as they might have ideas about this.
>
> You can write custom messages for custom tests using test_assert().
Another possibility is to do
for (int i = 0; i < 256; i++) {
if (!check_int(func(i), ==, is_in(string, i))
test_msg(" i: %02x", i);
}
To print the character code as well as the actual and expected return
values of check_int(). The funny spacing is intended to keep the output
aligned. I did wonder if we should be using
check(func(i) == is_in(string, i))
instead of check_int() but I think it is useful to have the return value
printed on error in case we start returning "53" instead of "1" for
"true" [1]. With the extra test_msg() above we can now see if the test
fails because of a mis-categorization or because func() returned a
different non-zero value when we were expecting "1".
>> Also it might not be a big issue here, but when the new unit test
>> framework was proposed, I commented on the fact that "left" and
>> "right" were perhaps a bit less explicit than "actual" and "expected".
If people are worried about this then it would be possible to change the
check_xxx() macros pass the stringified relational operator into the
various check_xxx_loc() functions and then print "expected" and "actual"
when the operator is "==" and "left" and "right" otherwise.
Best Wishes
Phillip
[1] As an aside I wonder if the ctype functions would make good test
balloons for using _Bool by changing sane_istest() to be
#define sane_istest(x,mask) ((bool)(sane_ctype[(unsigned char)(x)] &
(mask)))
so that we check casting to _Bool coerces non-zero values to "1"
^ permalink raw reply
* Suppress 'info: please complete authentication in browser'
From: Brian Hart @ 2023-12-27 17:11 UTC (permalink / raw)
To: git@vger.kernel.org
Git for Windows version: 2.43.0
Hello,
I am running a nightly shell script that syncs my local and remote Git repos. It kicks off at 7:30 PM each day. When I come into work the following morning, sporadically, I find that part of the script has not been executing for hours because a modal popup displays.
The text in the console is:
`info: please complete authentication in your browser`
At the same time, a modal pop-up appears. The modal popup needs a button clicked to open the browser and sign in. Upon doing so, the browser says "authentication successful," and the script continues -- but this is after I've woken up from being asleep, with the script otherwise sitting suspended all night long.
I've heard various suggestions before about using the Windows Credential Manager (I believe I am already) and such. It seems as if Git is programmed to ignore the Credential Manager periodically and demand the use of the browser to re-authenticate.
Here is the .sh script I am using, e.g., to do a pull on each directory in a root directory:
```
git config --global user.email "XXXXX" --replace-all; git config --global user.name "XXXXXX" --replace-all; git config --global credential.helper wincred --replace-all; for d in */; do cd $d; echo Processing repository in "${d%/}"...; git config credential.helper wincred; git remote update; git pull --all -v --no-rebase; git add --renormalize .; cd ..; done
```
As you can see I am using `git config --global credential.helper wincred --replace-all` and I am not sure why this should not work. NOTE: The 'XXXX' are for privacy.
How can I suppress the modal pop-up so I can run my overnight Git scripts?
Regards,
Brian Hart
^ permalink raw reply
* Re: Suppress 'info: please complete authentication in browser'
From: brian m. carlson @ 2023-12-27 17:47 UTC (permalink / raw)
To: Brian Hart; +Cc: git@vger.kernel.org
In-Reply-To: <1858825158.1864122.1703697091856@mailbusiness.ionos.com>
[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]
On 2023-12-27 at 17:11:31, Brian Hart wrote:
> Hello,
Hey,
> Here is the .sh script I am using, e.g., to do a pull on each directory in a root directory:
>
> ```
> git config --global user.email "XXXXX" --replace-all; git config --global user.name "XXXXXX" --replace-all; git config --global credential.helper wincred --replace-all; for d in */; do cd $d; echo Processing repository in "${d%/}"...; git config credential.helper wincred; git remote update; git pull --all -v --no-rebase; git add --renormalize .; cd ..; done
> ```
If your goal is to use a specific configuration, you may find it easier
to use the GIT_CONFIG_GLOBAL environment variable to set a custom global
configuration file and then GIT_CONFIG_NOSYSTEM=1 to unset the system
configuration from being used. My guess is that there's a
`credential.helper=manager` setting in some config file, which you'd be
able to see with `git config -l --show-origin`, and that using this
configuration would fix that.
However, if you see this problem after that, I'd recommend contacting
the Git for Windows team on their issue tracker at
https://github.com/git-for-windows/git/issues/ or the Git Credential
Manager Core team at
https://github.com/git-ecosystem/git-credential-manager/issues/. The
Git project itself doesn't distribute any binaries or anything but the
core Git code itself, so if you have a problem with Git for Windows or
Git Credential Manager Core, you should contact those folks directly.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* [PATCH 0/1] doc: git-bisect: change plural form to singular
From: Britton Leo Kerin @ 2023-12-27 20:53 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
Correct the usage in git-bisect documentation to use singular form for a
repeatable argument as other commands do.
I reported this tiny issue previously but didn't see a response so I
thought I'd use it as a chance to get up to speed on the patch
submission process. Sorry if no response meant no interest on this
issue.
Britton Leo Kerin (1):
doc: use singular form of repeatable path arg
Documentation/git-bisect.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
base-commit: 055bb6e9969085777b7fab83e3fee0017654f134
--
2.43.0
^ permalink raw reply
* [PATCH 1/1] doc: use singular form of repeatable path arg
From: Britton Leo Kerin @ 2023-12-27 20:53 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin, Britton L Kerin
In-Reply-To: <20231227205340.9886-1-britton.kerin@gmail.com>
This is more correct because the <path>... doc syntax already indicates
that the arg is "array-type". It's how other tools do it. Finally, the
later document text mentions 'path' arguments, while it doesn't mention
'paths'.
Signed-off-by: Britton L Kerin <britton.kergin@gmail.com>
---
Documentation/git-bisect.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 191b4a42b6..58821f7e11 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
on the subcommand:
git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
+ [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<path>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-good | --term-bad]
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Junio C Hamano @ 2023-12-27 22:15 UTC (permalink / raw)
To: Elijah Newren
Cc: Elijah Newren via GitGitGadget, git, Sebastian Thiel,
Josh Triplett
In-Reply-To: <CABPp-BGSTYDUR1oYYXkCSh-1i2zwxBM=-gnoe-ezNbtPi5CV2A@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> There are
> precisely two choices in our design for how older Git versions can
> treat precious files:
> * ignored-and-expendable
> * untracked-and-precious
> If we pick syntax that causes older Git versions to treat precious
> files as ignored-and-expendable, we risk deleting important files.
Yes but not really. I'd expect the adoption of precious feature and
the adoption of versions of Git that supports that feature will go
more or less hand in hand. Projects that, for any reason, need to
keep their participants at pre-precious versions of Git would
naturally refrain from marking the "precious" paths in their "ignore"
mechanism before their participants are ready, so even if we chose
syntax that will make the precious ones mistaken as merely ignored,
the damage would be fairly small.
^ permalink raw reply
* Re: [PATCH v3] sideband.c: remove redundant 'NEEDSWORK' tag
From: Junio C Hamano @ 2023-12-27 22:22 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget
Cc: git, Torsten Bögershausen, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1625.v3.git.1703672407895.gitgitgadget@gmail.com>
"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:
> - * NEEDSWORK: use "size_t n" instead for clarity.
> ++ * It is fine to use "int n" here instead of "size_t n" as all calls to this
> ++ * function pass an 'int' parameter.
This does not sound like a sufficient justification, though.
We should also explain why "int" is good enough for these callers.
Otherwise, using size_t throughout the callchain would become
another viable solution.
^ permalink raw reply
* Re: [PATCH 1/1] doc: use singular form of repeatable path arg
From: Eric Sunshine @ 2023-12-27 22:29 UTC (permalink / raw)
To: Britton Leo Kerin; +Cc: git, Britton L Kerin
In-Reply-To: <3d46bca1-96d4-43ba-a912-1f7c76942287@smtp-relay.sendinblue.com>
On Wed, Dec 27, 2023 at 3:55 PM Britton Leo Kerin
<britton.kerin@gmail.com> wrote:
> This is more correct because the <path>... doc syntax already indicates
> that the arg is "array-type". It's how other tools do it. Finally, the
> later document text mentions 'path' arguments, while it doesn't mention
> 'paths'.
Yep, makes sense.
> Signed-off-by: Britton L Kerin <britton.kergin@gmail.com>
> ---
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
> git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
> - [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
> + [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<path>...]
Looking good.
In builtin/bisect.c, the "usage" string says "[<pathspec>...]" rather
than "[<path>...]". Perhaps it makes sense to unify these?
Also, there are a few more documentation files that could use the
"<paths>" to "<path>..." fixup (though not always in the synopsis). A
'grep' indicates that git-checkout.txt, git-diff.txt, and
git-rev-list-options.txt also mention "<paths>". Those may be outside
the scope of this patch, although they could easily be included, as
well, or made part of a patch series if you feel inclined.
^ permalink raw reply
* Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Junio C Hamano @ 2023-12-27 23:48 UTC (permalink / raw)
To: René Scharfe
Cc: Christian Couder, Achu Luma, git, Christian Couder, Phillip Wood,
Josh Steadmon
In-Reply-To: <f743b473-40f8-423d-bf5b-d42b92e5aa1b@web.de>
René Scharfe <l.s.r@web.de> writes:
>> Also it might not be a big issue here, but when the new unit test
>> framework was proposed, I commented on the fact that "left" and
>> "right" were perhaps a bit less explicit than "actual" and "expected".
>
> True.
> ...
> The added repetition is a bit grating. With a bit of setup, loop
> unrolling and stringification you can retain the property of only having
> to mention the class name once. Demo patch below.
Nice.
This (and your mempool thing) being one of the early efforts to
adopt the unit-test framework outside the initial set of sample
tests, it is understandable that we might find what framework offers
is still lacking. But at the same time, while the macro tricks
demonstrated here are all amusing to read and admire, it feels a bit
too much to expect that the test writers are willing to invent
something like these every time they want to test.
Being a relatively faithful conversion of the original ctype tests,
with its thorough enumeration of test samples and expected output,
is what makes this test program require these macro tricks, and it
does not have much to do with the features (or lack thereof) of the
framework, I guess.
> +struct ctype {
> + const char *name;
> + const char *expect;
> + int actual[256];
> +};
> +
> +static void test_ctype(const struct ctype *class)
> +{
> + for (int i = 0; i < 256; i++) {
> + int expect = is_in(class->expect, i);
> + int actual = class->actual[i];
> + int res = test_assert(TEST_LOCATION(), class->name,
> + actual == expect);
> + if (!res)
> + test_msg("%s classifies char %d (0x%02x) wrongly",
> + class->name, i, i);
> + }
> }
Somehow, the "test_assert" does not seem to be adding much value
here (i.e. we can do "res = (actual == expect)" there). Is this
because we want to be able to report success, too?
... goes and looks at test_assert() ...
Ah, is it because we want to be able to "skip" (which pretends that
the assert() was satisified). OK, but then the error reporting from
it is redundant with our own test_msg().
Everything below this line was a fun read ;-)
Thanks.
> ...
> +#define APPLY16(f, n) \
> + f(n + 0x0), f(n + 0x1), f(n + 0x2), f(n + 0x3), \
> + f(n + 0x4), f(n + 0x5), f(n + 0x6), f(n + 0x7), \
> + f(n + 0x8), f(n + 0x9), f(n + 0xa), f(n + 0xb), \
> + f(n + 0xc), f(n + 0xd), f(n + 0xe), f(n + 0xf)
> +#define APPLY256(f) \
> + APPLY16(f, 0x00), APPLY16(f, 0x10), APPLY16(f, 0x20), APPLY16(f, 0x30),\
> + APPLY16(f, 0x40), APPLY16(f, 0x50), APPLY16(f, 0x60), APPLY16(f, 0x70),\
> + APPLY16(f, 0x80), APPLY16(f, 0x90), APPLY16(f, 0xa0), APPLY16(f, 0xb0),\
> + APPLY16(f, 0xc0), APPLY16(f, 0xd0), APPLY16(f, 0xe0), APPLY16(f, 0xf0),\
> +
> +#define CTYPE(name, expect) { #name, expect, { APPLY256(name) } }
>
> int cmd_main(int argc, const char **argv) {
> + struct ctype classes[] = {
> + CTYPE(isdigit, DIGIT),
> + CTYPE(isspace, " \n\r\t"),
> + CTYPE(isalpha, LOWER UPPER),
> + CTYPE(isalnum, LOWER UPPER DIGIT),
> + CTYPE(is_glob_special, "*?[\\"),
> + CTYPE(is_regex_special, "$()*+.?[\\^{|"),
> + CTYPE(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~"),
> + CTYPE(isascii, ASCII),
> + CTYPE(islower, LOWER),
> + CTYPE(isupper, UPPER),
> + CTYPE(iscntrl, CNTRL),
> + CTYPE(ispunct, PUNCT),
> + CTYPE(isxdigit, DIGIT "abcdefABCDEF"),
> + CTYPE(isprint, LOWER UPPER DIGIT PUNCT " "),
> + };
> /* Run all character type tests */
> - TEST(test_ctype_isspace(), "isspace() works as we expect");
> - TEST(test_ctype_isdigit(), "isdigit() works as we expect");
> - TEST(test_ctype_isalpha(), "isalpha() works as we expect");
> - TEST(test_ctype_isalnum(), "isalnum() works as we expect");
> - TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
> - TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
> - TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
> - TEST(test_ctype_isascii(), "isascii() works as we expect");
> - TEST(test_ctype_islower(), "islower() works as we expect");
> - TEST(test_ctype_isupper(), "isupper() works as we expect");
> - TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
> - TEST(test_ctype_ispunct(), "ispunct() works as we expect");
> - TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
> - TEST(test_ctype_isprint(), "isprint() works as we expect");
> + for (int i = 0; i < ARRAY_SIZE(classes); i++)
> + TEST(test_ctype(&classes[i]), "%s works", classes[i].name);
>
> return test_done();
> }
^ permalink raw reply
* Re: [PATCH 1/1] doc: use singular form of repeatable path arg
From: Junio C Hamano @ 2023-12-28 0:10 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Britton Leo Kerin, git, Britton L Kerin
In-Reply-To: <CAPig+cShsSSd-jpvSW_sq3-R++zjtHU-m2PmTsz-Nx9YVRStug@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Wed, Dec 27, 2023 at 3:55 PM Britton Leo Kerin
> <britton.kerin@gmail.com> wrote:
>> This is more correct because the <path>... doc syntax already indicates
>> that the arg is "array-type". It's how other tools do it. Finally, the
>> later document text mentions 'path' arguments, while it doesn't mention
>> 'paths'.
>
> Yep, makes sense.
>
>> Signed-off-by: Britton L Kerin <britton.kergin@gmail.com>
Micronit. This should be identical to how From: line identifies the
author. s/L/Leo/ should be sufficient, I presume?
>> ---
>> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
>> @@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
>> git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
>> - [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
>> + [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<path>...]
>
> Looking good.
>
> In builtin/bisect.c, the "usage" string says "[<pathspec>...]" rather
> than "[<path>...]". Perhaps it makes sense to unify these?
Yup, and that would be a good thing to do in a single patch, and
also it would be a good place to stop. Further clean-up you
suggested below are very much worth doing, but it probably is good
to leave out of this single focused fix we are reviewing here and
instead be done as separate patch(es).
Thanks.
> Also, there are a few more documentation files that could use the
> "<paths>" to "<path>..." fixup (though not always in the synopsis). A
> 'grep' indicates that git-checkout.txt, git-diff.txt, and
> git-rev-list-options.txt also mention "<paths>". Those may be outside
> the scope of this patch, although they could easily be included, as
> well, or made part of a patch series if you feel inclined.
^ permalink raw reply
* Re: [PATCH 2/3] sparse-checkout: use default patterns for 'set' only !stdin
From: Junio C Hamano @ 2023-12-28 0:18 UTC (permalink / raw)
To: Elijah Newren; +Cc: git
In-Reply-To: <CABPp-BF3Vqq8Y8+2pQyO7-StUVPR3as+Q_aL+o92ydoJY-8zEw@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
>> index 8f55127202..04ae81fce8 100644
>> --- a/builtin/sparse-checkout.c
>> +++ b/builtin/sparse-checkout.c
>> @@ -837,7 +837,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>> * non-cone mode, if nothing is specified, manually select just the
>> * top-level directory (much as 'init' would do).
>> */
>> - if (!core_sparse_checkout_cone && argc == 0) {
>> + if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
>> argv = default_patterns;
>> argc = default_patterns_nr;
>> } else {
>> --
>> 2.43.0-174-g055bb6e996
>
> Thanks for catching this; the fix looks good to me.
Actually I am not so sure.
An obvious alternative would be to collect the patterns, either from
the command line or from the standard input, and then use the
default when we collected nothing.
But I guess those who use the standard input should be able to
specify everything fully, so it would probably be OK.
^ permalink raw reply
* What's cooking in git.git (Dec 2023, #05; Wed, 27)
From: Junio C Hamano @ 2023-12-28 4:25 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
The RelNotes symbolic link says we are now working towards Git 2.44.
It may not be a bad idea to reflect on what technical debt and UI
warts we have accumulated so far to see if we have enough of them to
start planning for a breaking Git 3.0 release (or, of course, keep
incrementally improve the system, which is much more preferrable---
continuity and stability is good). End of year being a relatively
quiet period, it may be a good time to think about your favorite pet
peeve, to be discussed early next year.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* es/add-doc-list-short-form-of-all-in-synopsis (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at a4f20da2bf)
+ git-add.txt: add missing short option -A to synopsis
Doc update.
source: <20231215204333.1253-1-ericsunshine@charter.net>
* jc/checkout-B-branch-in-use (2023-12-13) 2 commits
(merged to 'next' on 2023-12-14 at 0a3998619e)
+ checkout: forbid "-B <branch>" from touching a branch used elsewhere
+ checkout: refactor die_if_checked_out() caller
"git checkout -B <branch> [<start-point>]" allowed a branch that is
in use in another worktree to be updated and checked out, which
might be a bit unexpected. The rule has been tightened, which is a
breaking change. "--ignore-other-worktrees" option is required to
unbreak you, if you are used to the current behaviour that "-B"
overrides the safety.
source: <xmqqjzq9cl70.fsf@gitster.g>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
(merged to 'next' on 2023-12-15 at 4aa7596593)
+ diff-lib: fix check_removed() when fsmonitor is active
+ Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
+ Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqr0n0h0tw.fsf@gitster.g>
* jc/doc-misspelt-refs-fix (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at e7799fd5c9)
+ doc: format.notes specify a ref under refs/notes/ hierarchy
Doc update.
source: <xmqqjzpfje33.fsf_-_@gitster.g>
* jc/doc-most-refs-are-not-that-special (2023-12-15) 5 commits
(merged to 'next' on 2023-12-18 at aead30fcc8)
+ docs: MERGE_AUTOSTASH is not that special
+ docs: AUTO_MERGE is not that special
+ refs.h: HEAD is not that special
+ git-bisect.txt: BISECT_HEAD is not that special
+ git.txt: HEAD is not that special
Doc updates.
source: <20231215203245.3622299-1-gitster@pobox.com>
* jc/fake-lstat (2023-09-15) 1 commit
(merged to 'next' on 2023-12-15 at 48e34cc0b4)
+ cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
cf. <e5295dbe-94d2-3186-5663-2466eba4bdde@jeffhostetler.com>
source: <xmqqcyykig1l.fsf@gitster.g>
* jk/mailinfo-iterative-unquote-comment (2023-12-14) 2 commits
(merged to 'next' on 2023-12-18 at 92363605fd)
+ mailinfo: avoid recursion when unquoting From headers
+ t5100: make rfc822 comment test more careful
(this branch uses jk/mailinfo-oob-read-fix.)
The code to parse the From e-mail header has been updated to avoid
recursion.
source: <20231214214444.GB2297853@coredump.intra.peff.net>
* jk/mailinfo-oob-read-fix (2023-12-12) 1 commit
(merged to 'next' on 2023-12-14 at 0dcfcb0d02)
+ mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()
(this branch is used by jk/mailinfo-iterative-unquote-comment.)
OOB read fix.
source: <20231212221243.GA1656116@coredump.intra.peff.net>
* jx/fetch-atomic-error-message-fix (2023-12-18) 2 commits
(merged to 'next' on 2023-12-18 at a1988b00e5)
+ fetch: no redundant error message for atomic fetch
+ t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
cf. <ZX__e7VjyLXIl-uV@tanuki>
source: <cover.1702821462.git.zhiyou.jx@alibaba-inc.com>
* ps/chainlint-self-check-update (2023-12-15) 1 commit
(merged to 'next' on 2023-12-18 at 0de2e1807f)
+ tests: adjust whitespace in chainlint expectations
Test framework update.
source: <fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im>
* ps/clone-into-reftable-repository (2023-12-12) 7 commits
(merged to 'next' on 2023-12-19 at adf7eb1f84)
+ builtin/clone: create the refdb with the correct object format
+ builtin/clone: skip reading HEAD when retrieving remote
+ builtin/clone: set up sparse checkout later
+ builtin/clone: fix bundle URIs with mismatching object formats
+ remote-curl: rediscover repository when fetching refs
+ setup: allow skipping creation of the refdb
+ setup: extract function to create the refdb
(this branch is used by ps/refstorage-extension.)
"git clone" has been prepared to allow cloning a repository with
non-default hash function into a repository that uses the reftable
backend.
source: <cover.1702361370.git.ps@pks.im>
* ps/reftable-fixes (2023-12-11) 11 commits
(merged to 'next' on 2023-12-15 at ebba966016)
+ reftable/block: reuse buffer to compute record keys
+ reftable/block: introduce macro to initialize `struct block_iter`
+ reftable/merged: reuse buffer to compute record keys
+ reftable/stack: fix use of unseeded randomness
+ reftable/stack: fix stale lock when dying
+ reftable/stack: reuse buffers when reloading stack
+ reftable/stack: perform auto-compaction with transactional interface
+ reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
+ reftable: handle interrupted writes
+ reftable: handle interrupted reads
+ reftable: wrap EXPECT macros in do/while
(this branch is used by ps/reftable-fixes-and-optims.)
Bunch of small fix-ups to the reftable code.
source: <cover.1702285387.git.ps@pks.im>
* rs/c99-stdbool-test-balloon (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at 5a62aaa127)
+ git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
Test balloon to use C99 "bool" type from <stdbool.h>.
source: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>
* rs/show-ref-incompatible-options (2023-12-11) 1 commit
(merged to 'next' on 2023-12-18 at 5a092285f7)
+ show-ref: use die_for_incompatible_opt3()
Code clean-up for sanity checking of command line options for "git
show-ref".
source: <e5304253-3347-4900-bbf2-d3c6ee3fb976@web.de>
* rs/t6300-compressed-size-fix (2023-12-12) 1 commit
(merged to 'next' on 2023-12-19 at 37ed09549c)
+ t6300: avoid hard-coding object sizes
Test fix.
source: <9feeb6cf-aabf-4002-917f-3f6c27547bc8@web.de>
* sp/test-i18ngrep (2023-12-18) 1 commit
(merged to 'next' on 2023-12-18 at d54442693a)
+ test-lib-functions.sh: fix test_grep fail message wording
Error message fix in the test framework.
source: <20231203171956.771-1-shreyanshpaliwalcmsmn@gmail.com>
--------------------------------------------------
[New Topics]
* ml/doc-merge-updates (2023-12-20) 2 commits
- Documentation/git-merge.txt: use backticks for command wrapping
- Documentation/git-merge.txt: fix reference to synopsis
Doc update.
Will merge to 'next'.
source: <20231220195342.17590-1-mi.al.lohmann@gmail.com>
* cp/apply-core-filemode (2023-12-26) 3 commits
- apply: code simplification
- apply: correctly reverse patch's pre- and post-image mode bits
- apply: ignore working tree filemode when !core.filemode
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
Needs review.
source: <20231226233218.472054-1-gitster@pobox.com>
* jc/archive-list-with-extra-args (2023-12-21) 1 commit
- archive: "--list" does not take further options
"git archive --list extra garbage" silently ignored excess command
line parameters, which has been corrected.
Will merge to 'next'.
source: <xmqqmsu3mnix.fsf@gitster.g>
* jk/t1006-cat-file-objectsize-disk (2023-12-21) 1 commit
- t1006: add tests for %(objectsize:disk)
Test update.
Will merge to 'next'.
source: <20231221094722.GA570888@coredump.intra.peff.net>
* js/contributor-docs-updates (2023-12-21) 9 commits
- SubmittingPatches: hyphenate non-ASCII
- SubmittingPatches: clarify GitHub artifact format
- SubmittingPatches: clarify GitHub visual
- SubmittingPatches: improve extra tags advice
- SubmittingPatches: update extra tags list
- SubmittingPatches: discourage new trailers
- SubmittingPatches: drop ref to "What's in git.git"
- CodingGuidelines: write punctuation marks
- CodingGuidelines: move period inside parentheses
Doc update.
Expecting a reroll, but basically looking good.
source: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
* al/unit-test-ctype (2023-12-26) 1 commit
- unit-tests: rewrite t/helper/test-ctype.c as a unit test.
Move test-ctype helper to the unit-test framework.
Expecting a reroll.
source: <20231221231527.8130-1-ach.lumap@gmail.com>
* bk/bisect-doc-fix (2023-12-27) 1 commit
- doc: use singular form of repeatable path arg
Synopsis fix.
Expecting a reroll.
source: <3d46bca1-96d4-43ba-a912-1f7c76942287@smtp-relay.sendinblue.com>
* en/sparse-checkout-eoo (2023-12-26) 2 commits
- sparse-checkout: be consistent with end of options markers
- Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Will merge to 'next'.
source: <pull.1625.v2.git.git.1703619562639.gitgitgadget@gmail.com>
* ja/doc-placeholders-fix (2023-12-26) 2 commits
- doc: enforce placeholders in documentation
- doc: enforce dashes in placeholders
Docfix.
Needs review.
source: <pull.1626.git.1703539287.gitgitgadget@gmail.com>
* jc/sparse-checkout-set-default-fix (2023-12-26) 1 commit
- sparse-checkout: use default patterns for 'set' only !stdin
"git sparse-checkout set" added default patterns even when the
patterns are being fed from the standard input, which has been
corrected.
Will merge to 'next'.
source: <20231221065925.3234048-3-gitster@pobox.com>
* rs/fast-import-simplify-mempool-allocation (2023-12-26) 1 commit
- fast-import: use mem_pool_calloc()
Code simplification.
Will merge to 'next'.
source: <50c1f410-ca37-4c1c-a28b-3e9fad49f2b4@web.de>
* rs/mem-pool-improvements (2023-12-26) 2 commits
- mem-pool: simplify alignment calculation
- mem-pool: fix big allocations
MemPool allocator fixes.
Will merge to 'next'.
source: <3e15d11a-bd19-49ca-b674-9b50e0ba7fc2@web.de>
--------------------------------------------------
[Cooking]
* jc/retire-cas-opt-name-constant (2023-12-19) 1 commit
(merged to 'next' on 2023-12-21 at 39ef057c8b)
+ remote.h: retire CAS_OPT_NAME
Code clean-up.
Will merge to 'master'.
source: <xmqq5y0uc7tq.fsf@gitster.g>
* rs/rebase-use-strvec-pushf (2023-12-20) 1 commit
(merged to 'next' on 2023-12-20 at ecb190973c)
+ rebase: use strvec_pushf() for format-patch revisions
Code clean-up.
Will merge to 'master'.
source: <4ab7431c-6c1b-448c-b4d2-e8b9be0e4eef@web.de>
* ps/refstorage-extension (2023-12-20) 13 commits
- t9500: write "extensions.refstorage" into config
- builtin/clone: introduce `--ref-format=` value flag
- builtin/init: introduce `--ref-format=` value flag
- builtin/rev-parse: introduce `--show-ref-format` flag
- t: introduce GIT_TEST_DEFAULT_REF_FORMAT envvar
- setup: introduce GIT_DEFAULT_REF_FORMAT envvar
- setup: introduce "extensions.refStorage" extension
- setup: set repository's formats on init
- setup: start tracking ref storage format when
- refs: refactor logic to look up storage backends
- worktree: skip reading HEAD when repairing worktrees
- t: introduce DEFAULT_REPO_FORMAT prereq
- Merge branch 'ps/clone-into-reftable-repository' into ps/refstorage-extension
Introduce a new extension "refstorage" so that we can mark a
repository that uses a non-default ref backend, like reftable.
Needs review.
source: <cover.1703067989.git.ps@pks.im>
* ps/reftable-fixes-and-optims (2023-12-20) 9 commits
- SQUASH??? make "make hdr-check" pass
- reftable/merged: transfer ownership of records when iterating
- reftable/merged: really reuse buffers to compute record keys
- reftable/record: store "val2" hashes as static arrays
- reftable/record: store "val1" hashes as static arrays
- reftable/record: constify some parts of the interface
- reftable/writer: fix index corruption when writing multiple indices
- reftable/stack: do not overwrite errors when compacting
- Merge branch 'ps/reftable-fixes' into ps/reftable-fixes-and-optims
More fixes and optimizations to the reftable backend.
Needs review.
source: <cover.1703063544.git.ps@pks.im>
* ps/pseudo-refs (2023-12-14) 4 commits
(merged to 'next' on 2023-12-21 at 3460e3d667)
+ bisect: consistently write BISECT_EXPECTED_REV via the refdb
+ refs: complete list of special refs
+ refs: propagate errno when reading special refs fails
+ wt-status: read HEAD and ORIG_HEAD via the refdb
Assorted changes around pseudoref handling.
Will merge to 'master'.
source: <cover.1702560829.git.ps@pks.im>
* tb/multi-pack-verbatim-reuse (2023-12-14) 26 commits
- t/perf: add performance tests for multi-pack reuse
- pack-bitmap: enable reuse from all bitmapped packs
- pack-objects: allow setting `pack.allowPackReuse` to "single"
- t/test-lib-functions.sh: implement `test_trace2_data` helper
- pack-objects: add tracing for various packfile metrics
- pack-bitmap: prepare to mark objects from multiple packs for reuse
- pack-revindex: implement `midx_pair_to_pack_pos()`
- pack-revindex: factor out `midx_key_to_pack_pos()` helper
- midx: implement `midx_preferred_pack()`
- git-compat-util.h: implement checked size_t to uint32_t conversion
- pack-objects: include number of packs reused in output
- pack-objects: prepare `write_reused_pack_verbatim()` for multi-pack reuse
- pack-objects: prepare `write_reused_pack()` for multi-pack reuse
- pack-objects: pass `bitmapped_pack`'s to pack-reuse functions
- pack-objects: keep track of `pack_start` for each reuse pack
- pack-objects: parameterize pack-reuse routines over a single pack
- pack-bitmap: return multiple packs via `reuse_partial_packfile_from_bitmap()`
- pack-bitmap: simplify `reuse_partial_packfile_from_bitmap()` signature
- ewah: implement `bitmap_is_empty()`
- pack-bitmap: pass `bitmapped_pack` struct to pack-reuse functions
- midx: implement `midx_locate_pack()`
- midx: implement `BTMP` chunk
- midx: factor out `fill_pack_info()`
- pack-bitmap: plug leak in find_objects()
- pack-bitmap-write: deep-clear the `bb_commit` slab
- pack-objects: free packing_data in more places
Streaming spans of packfile data used to be done only from a
single, primary, pack in a repository with multiple packfiles. It
has been extended to allow reuse from other packfiles, too.
Will merge to 'next'?
cf. <ZXurD1NTZ4TAs7WZ@nand.local>
source: <cover.1702592603.git.me@ttaylorr.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* sh/completion-with-reftable (2023-12-19) 2 commits
(merged to 'next' on 2023-12-20 at 7957d4aa5b)
+ completion: support pseudoref existence checks for reftables
+ completion: refactor existence checks for pseudorefs
Command line completion script (in contrib/) learned to work better
with the reftable backend.
Will merge to 'master'.
source: <cover.1703022850.git.stanhu@gmail.com>
* en/header-cleanup (2023-12-26) 12 commits
- treewide: remove unnecessary includes in source files
- treewide: add direct includes currently only pulled in transitively
- trace2/tr2_tls.h: remove unnecessary include
- submodule-config.h: remove unnecessary include
- pkt-line.h: remove unnecessary include
- line-log.h: remove unnecessary include
- http.h: remove unnecessary include
- fsmonitor--daemon.h: remove unnecessary includes
- blame.h: remove unnecessary includes
- archive.h: remove unnecessary include
- treewide: remove unnecessary includes in source files
- treewide: remove unnecessary includes from header files
Remove unused header "#include".
Will merge to 'next'.
source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>
* jc/orphan-unborn (2023-11-24) 2 commits
(merged to 'next' on 2023-12-21 at 030300487a)
+ orphan/unborn: fix use of 'orphan' in end-user facing messages
+ orphan/unborn: add to the glossary and use them consistently
Doc updates to clarify what an "unborn branch" means.
Will merge to 'master'.
source: <xmqq4jhb977x.fsf@gitster.g>
* jw/builtin-objectmode-attr (2023-12-12) 2 commits
- SQUASH??? - leakfix
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
Needs to get leakfix reviewed.
cf. <xmqq5y0ssknj.fsf@gitster.g>
source: <20231116054437.2343549-1-jojwang@google.com>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Expecting a reroll.
cf. <20231023202212.GA5470@szeder.dev>
source: <cover.1697653929.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* la/trailer-cleanups (2023-12-20) 3 commits
(merged to 'next' on 2023-12-21 at e26ede5f55)
+ trailer: use offsets for trailer_start/trailer_end
+ trailer: find the end of the log message
+ commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Will merge to 'master'.
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-12-14) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1702562879.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-12-18) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Will merge to 'next'?
source: <cover.1702823801.git.zhiyou.jx@alibaba-inc.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
(merged to 'next' on 2023-12-21 at 929a027fb7)
+ status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Will merge to 'master'.
cf. <xmqqil76kyov.fsf@gitster.g>
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/sparse-checkout-eoo (2023-12-21) 5 commits
. sparse-checkout: tighten add_patterns_from_input()
. sparse-checkout: use default patterns for 'set' only !stdin
. SQUASH??? end-of-options test
. sparse-checkout: take care of "--end-of-options" in set/add/check-rules
- Merge branch 'jk/end-of-options' into jc/sparse-checkout-set-add-end-of-options
"git sparse-checkout (add|set) --[no-]cone --end-of-options" did
not handle "--end-of-options" correctly after a recent update.
Superseded by the en/sparse-checkout-eoo topic.
source: <20231221065925.3234048-1-gitster@pobox.com>
^ permalink raw reply
* [PATCH v3 0/9] Minor improvements to CodingGuidelines and SubmittingPatches
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref
In-Reply-To: <pull.1623.v2.git.1703176865.gitgitgadget@gmail.com>
These are a bunch of things I've run into over my past couple of attempts to
contribute to Git.
* Incremental punctuation/grammatical improvements
* Update extra tags suggestions based on common usage
* drop reference to an article that was discontinued over a decade ago
* update GitHub references
* harmonize non-ASCII while I'm here
Note that I'm trying to do things "in the neighborhood". It'll be slower
than me replacing things topically, but hopefully easier for others to
digest. My current estimate is a decade or two :).
Josh Soref (9):
CodingGuidelines: move period inside parentheses
CodingGuidelines: write punctuation marks
SubmittingPatches: drop ref to "What's in git.git"
SubmittingPatches: discourage new trailers
SubmittingPatches: update extra tags list
SubmittingPatches: provide tag naming advice
SubmittingPatches: clarify GitHub visual
SubmittingPatches: clarify GitHub artifact format
SubmittingPatches: hyphenate non-ASCII
Documentation/CodingGuidelines | 4 ++--
Documentation/SubmittingPatches | 33 +++++++++++++++++++++++----------
2 files changed, 25 insertions(+), 12 deletions(-)
base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1623%2Fjsoref%2Fdocumentation-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1623/jsoref/documentation-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1623
Range-diff vs v2:
1: b9a8eb6aa4e = 1: b9a8eb6aa4e CodingGuidelines: move period inside parentheses
2: c0db8336e51 = 2: c0db8336e51 CodingGuidelines: write punctuation marks
3: 22d66c5b78a = 3: 22d66c5b78a SubmittingPatches: drop ref to "What's in git.git"
4: eac2211332f = 4: eac2211332f SubmittingPatches: discourage new trailers
5: 8848572fe2c = 5: 8848572fe2c SubmittingPatches: update extra tags list
6: 8f16c7caa73 ! 6: f28c1011ba9 SubmittingPatches: improve extra tags advice
@@ Metadata
Author: Josh Soref <jsoref@gmail.com>
## Commit message ##
- SubmittingPatches: improve extra tags advice
+ SubmittingPatches: provide tag naming advice
Current statistics show a strong preference to only capitalize the first
letter in a hyphenated tag, but that some guidance would be helpful:
@@ Documentation/SubmittingPatches: While you can also create your own trailer if t
encourage you to instead use one of the common trailers in this project
highlighted above.
-+Extra tags should only capitalize the very first letter, i.e. favor
++Only capitalize the very first letter of tags, i.e. favor
+"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
+
[[git-tools]]
7: cdb5fd0957f < -: ----------- SubmittingPatches: clarify GitHub visual
8: 77576327df8 ! 7: 49cef6f7c20 SubmittingPatches: clarify GitHub artifact format
@@ Metadata
Author: Josh Soref <jsoref@gmail.com>
## Commit message ##
- SubmittingPatches: clarify GitHub artifact format
+ SubmittingPatches: clarify GitHub visual
- GitHub wraps artifacts generated by workflows in a .zip file.
+ GitHub has two general forms for its states, sometimes they're a simple
+ colored object (e.g. green check or red x), and sometimes there's also a
+ colored container (e.g. green box or red circle) which contains that
+ object (e.g. check or x).
- Internally, workflows can package anything they like in them.
-
- A recently generated failure artifact had the form:
-
- windows-artifacts.zip
- Length Date Time Name
- --------- ---------- ----- ----
- 76001695 12-19-2023 01:35 artifacts.tar.gz
- 11005650 12-19-2023 01:35 tracked.tar.gz
- --------- -------
- 87007345 2 files
+ That's a lot of words to try to describe things, but in general, the key
+ for a failure is that it's recognized as an `x` and that it's associated
+ with the color red -- the color of course is problematic for people who
+ are red-green color-blind, but that's why they are paired with distinct
+ shapes.
Signed-off-by: Josh Soref <jsoref@gmail.com>
## Documentation/SubmittingPatches ##
-@@ Documentation/SubmittingPatches: branches here: `https://github.com/<Your GitHub handle>/git/actions/workflows/ma
- If a branch did not pass all test cases then it is marked with a red
- +x+. In that case you can click on the failing job and navigate to
- "ci/run-build-and-tests.sh" and/or "ci/print-test-failures.sh". You
+@@ Documentation/SubmittingPatches: After the initial setup, CI will run whenever you push new changes
+ to your fork of Git on GitHub. You can monitor the test state of all your
+ branches here: `https://github.com/<Your GitHub handle>/git/actions/workflows/main.yml`
+
+-If a branch did not pass all test cases then it is marked with a red
+-cross. In that case you can click on the failing job and navigate to
+-"ci/run-build-and-tests.sh" and/or "ci/print-test-failures.sh". You
-can also download "Artifacts" which are tarred (or zipped) archives
-+can also download "Artifacts" which are zip archives containing
-+tarred (or zipped) archives
- with test data relevant for debugging.
+-with test data relevant for debugging.
++If a branch does not pass all test cases then it will be marked with a
++red +x+, instead of a green check. In that case, you can click on the
++failing job and navigate to "ci/run-build-and-tests.sh" and/or
++"ci/print-test-failures.sh". You can also download "Artifacts" which
++are tarred (or zipped) archives with test data relevant for debugging.
Then fix the problem and push your fix to your GitHub fork. This will
+ trigger a new CI build to ensure all tests pass.
-: ----------- > 8: deb1bf02f3a SubmittingPatches: clarify GitHub artifact format
9: a4878f58fe4 = 9: b1b75cc6a3e SubmittingPatches: hyphenate non-ASCII
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/9] CodingGuidelines: move period inside parentheses
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
The contents within parenthesis should be omittable without resulting
in broken text.
Eliding the parenthesis left a period to end a run without any content.
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/CodingGuidelines | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 8ed517a5ca0..af94ed3a75d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -450,7 +450,7 @@ For C programs:
one of the approved headers that includes it first for you. (The
approved headers currently include "builtin.h",
"t/helper/test-tool.h", "xdiff/xinclude.h", or
- "reftable/system.h"). You do not have to include more than one of
+ "reftable/system.h".) You do not have to include more than one of
these.
- A C file must directly include the header files that declare the
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/9] CodingGuidelines: write punctuation marks
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
- Match style in Release Notes
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/CodingGuidelines | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index af94ed3a75d..578587a4715 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -578,7 +578,7 @@ Externally Visible Names
. The variable name describes the effect of tweaking this knob.
The section and variable names that consist of multiple words are
- formed by concatenating the words without punctuations (e.g. `-`),
+ formed by concatenating the words without punctuation marks (e.g. `-`),
and are broken using bumpyCaps in documentation as a hint to the
reader.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 3/9] SubmittingPatches: drop ref to "What's in git.git"
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
"What's in git.git" was last seen in 2010:
https://lore.kernel.org/git/?q=%22what%27s+in+git.git%22
https://lore.kernel.org/git/7vaavikg72.fsf@alter.siamese.dyndns.org/
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index bce7f97815c..32e90238777 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -570,7 +570,7 @@ their trees themselves.
master).
* Read the Git mailing list, the maintainer regularly posts messages
- entitled "What's cooking in git.git" and "What's in git.git" giving
+ entitled "What's cooking in git.git" giving
the status of various proposed changes.
== GitHub CI[[GHCI]]
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 4/9] SubmittingPatches: discourage new trailers
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
There seems to be consensus amongst the core Git community on a working
set of common trailers, and there are non-trivial costs to people
inventing new trailers (research to discover what they mean/how they
differ from existing trailers) such that inventing new ones is generally
unwarranted and not something to be recommended to new contributors.
Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 32e90238777..58dfe405049 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -356,8 +356,9 @@ If you like, you can put extra tags at the end:
. `Tested-by:` is used to indicate that the person applied the patch
and found it to have the desired effect.
-You can also create your own tag or use one that's in common usage
-such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
+While you can also create your own trailer if the situation warrants it, we
+encourage you to instead use one of the common trailers in this project
+highlighted above.
[[git-tools]]
=== Generate your patch using Git tools out of your commits.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/9] SubmittingPatches: update extra tags list
From: Josh Soref via GitGitGadget @ 2023-12-28 4:55 UTC (permalink / raw)
To: git
Cc: Elijah Newren, René Scharfe, Phillip Wood, Josh Soref,
Dragan Simic, Josh Soref, Josh Soref
In-Reply-To: <pull.1623.v3.git.1703739324.gitgitgadget@gmail.com>
From: Josh Soref <jsoref@gmail.com>
Add items with at least 100 uses in the past three years:
- Co-authored-by
- Helped-by
- Mentored-by
- Suggested-by
git log --since=3.years|
perl -ne 'next unless /^\s+[A-Z][a-z]+-\S+:/;s/^\s+//;s/:.*/:/;print'|
sort|uniq -c|sort -n|grep '[0-9][0-9] '
14 Based-on-patch-by:
14 Original-patch-by:
17 Tested-by:
100 Suggested-by:
121 Co-authored-by:
163 Mentored-by:
274 Reported-by:
290 Acked-by:
450 Helped-by:
602 Reviewed-by:
14111 Signed-off-by:
Signed-off-by: Josh Soref <jsoref@gmail.com>
---
Documentation/SubmittingPatches | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 58dfe405049..31878cb70b7 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -355,6 +355,14 @@ If you like, you can put extra tags at the end:
patch after a detailed analysis.
. `Tested-by:` is used to indicate that the person applied the patch
and found it to have the desired effect.
+. `Co-authored-by:` is used to indicate that people exchanged drafts
+ of a patch before submitting it.
+. `Helped-by:` is used to credit someone who suggested ideas for
+ changes without providing the precise changes in patch form.
+. `Mentored-by:` is used to credit someone with helping develop a
+ patch as part of a mentorship program (e.g., GSoC or Outreachy).
+. `Suggested-by:` is used to credit someone with suggesting the idea
+ for a patch.
While you can also create your own trailer if the situation warrants it, we
encourage you to instead use one of the common trailers in this project
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox