Git development
 help / color / mirror / Atom feed
* [RESEND] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Oswald Buddenhagen @ 2023-10-20  9:27 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Phillip Wood, Christian Couder, Charvi Mendiratta,
	Marc Branchaud

Create a clear top-down structure which makes it hopefully unambiguous
what happens when.

Also mention the timestamp along with the author - this is primarily
meant to include the keywords somebody might be searching for, like I
did a year ago.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

---
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>
Cc: Christian Couder <christian.couder@gmail.com>
Cc: Charvi Mendiratta <charvi077@gmail.com>
Cc: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-rebase.txt | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..857e025361 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -890,20 +890,21 @@ command "pick" with the command "reword".
 To drop a commit, replace the command "pick" with "drop", or just
 delete the matching line.
 
-If you want to fold two or more commits into one, replace the command
-"pick" for the second and subsequent commits with "squash" or "fixup".
-If the commits had different authors, the folded commit will be
-attributed to the author of the first commit.  The suggested commit
-message for the folded commit is the concatenation of the first
-commit's message with those identified by "squash" commands, omitting the
-messages of commits identified by "fixup" commands, unless "fixup -c"
-is used.  In that case the suggested commit message is only the message
-of the "fixup -c" commit, and an editor is opened allowing you to edit
-the message.  The contents (patch) of the "fixup -c" commit are still
-incorporated into the folded commit. If there is more than one "fixup -c"
-commit, the message from the final one is used.  You can also use
-"fixup -C" to get the same behavior as "fixup -c" except without opening
-an editor.
+If you want to fold two or more commits into one (that is, to combine
+their contents/patches), replace the command "pick" for the second and
+subsequent commits with "squash" or "fixup".
+The commit message for the folded commit is the concatenation of the
+first commit's message with those identified by "squash" commands,
+omitting the messages of commits identified by "fixup" commands, unless
+"fixup -c" is used.  In the latter case, the message is obtained only
+from the "fixup -c" commit (having more than one "fixup -c" commit
+makes no sense, and only the message from the last one is used).
+If the resulting commit message is a concatenation of multiple messages,
+an editor is opened allowing you to edit it. This is also the case for a
+message obtained via "fixup -c", while using "fixup -C" instead skips
+the editor; this is analogous to the behavior of `git commit`.
+The first commit which contributes to the suggested commit message also
+determines the author, along with the date/timestamp.
 
 `git rebase` will stop when "pick" has been replaced with "edit" or
 when a command fails due to merge errors. When you are done editing
-- 
2.42.0.419.g70bf8a5751


^ permalink raw reply related

* Re: [PATCH] typo: fix the typo 'neeed' into 'needed' in the comment under merge-ort.c
From: Bagas Sanjaya @ 2023-10-20  7:41 UTC (permalink / raw)
  To: 王常新, Junio C Hamano; +Cc: git
In-Reply-To: <42D0A6FD-1314-4977-92E6-BF6535D82E8A@gmail.com>

On 20/10/2023 09:14, 王常新 wrote:
> It is my official name. But the email address is not a valid one. Should I rewrite the commit message?
> 

Please don't top-post, reply inline with appropriate context instead.

Did you mean that you can't receive ML traffic on your @qq.com address?
If so, resend with your @gmail.com address as patch author (you need
to set user.name and user.email accordingly). 

Thanks.

-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply

* Re: [PATCH v4 1/7] bulk-checkin: extract abstract `bulk_checkin_source`
From: Jeff King @ 2023-10-20  7:35 UTC (permalink / raw)
  To: Taylor Blau
  Cc: git, Elijah Newren, Eric W. Biederman, Junio C Hamano,
	Patrick Steinhardt
In-Reply-To: <97bb6e9f59e5092f0519c7d1141d0673313fdc33.1697736516.git.me@ttaylorr.com>

On Thu, Oct 19, 2023 at 01:28:42PM -0400, Taylor Blau wrote:

> +struct bulk_checkin_source {
> +	enum { SOURCE_FILE } type;
> +
> +	/* SOURCE_FILE fields */
> +	int fd;
> +
> +	/* common fields */
> +	size_t size;
> +	const char *path;
> +};

This is a pretty minor nit, but we may find that "SOURCE_FILE" is not
sufficiently name-spaced. Enum tags are in the global namespace, so
the compiler will barf if there are any conflicts.

It might be OK here, since this is local to a single C file (so we at
least are not hurting other code), but we may be in trouble if code in a
header file is less careful. There is already a near-miss here with
GREP_SOURCE_FILE, but fortunately grep.h is indeed careful. :)

(I notice that ref-filter.c similarly uses SOURCE_* for an internal
enum).

-Peff

^ permalink raw reply

* Re: [PATCH v4 0/7] merge-ort: implement support for packing objects together
From: Jeff King @ 2023-10-20  7:29 UTC (permalink / raw)
  To: Taylor Blau
  Cc: git, Elijah Newren, Eric W. Biederman, Junio C Hamano,
	Patrick Steinhardt
In-Reply-To: <cover.1697736516.git.me@ttaylorr.com>

On Thu, Oct 19, 2023 at 01:28:38PM -0400, Taylor Blau wrote:

> This is a new round that is significantly simplified thanks to
> another very helpful suggestion[1] from Junio. By factoring out a common
> "deflate object to pack" that takes an abstract bulk_checkin_source as a
> parameter, all of the earlier refactorings can be dropped since we
> retain only a single caller instead of multiple.

FWIW, I gave this a read-over and did not see anything wrong (on the
contrary, I think the new abstractions make it quite easy to follow).

Two thoughts that occurred to me while reading it (but neither
actionable for your series):

  - Having not worked with the bulk-checkin code much before, I thought
    it only put in one blob per pack, and thus you'd have to teach it to
    handle multiple objects, too. But fortunately I was wrong, and it
    handles this case already. (I mention this mainly because we had
    talked about it off-list a few weeks ago, and some of my confusion
    may have seeped into my comments then).

  - I did briefly wonder if we need this SOURCE abstraction at all. The
    file source assumes we can seek() and read(), so it must be a
    regular file. We could probably just mmap() it and treat it as
    in-core, too (this is what index_fd() and index_path() do under the
    hood!). But it would probably be a disservice to any platforms that
    do not support mmap, as the fallback is to read into a full buffer
    (and the whole point of the bulk checkin code is to avoid that).

-Peff

^ permalink raw reply

* Re: [PATCH] send-email: move process_address_list earlier to avoid, uninitialized address error
From: Jeff King @ 2023-10-20  7:14 UTC (permalink / raw)
  To: Michael Strawbridge
  Cc: Junio C Hamano, Bagas Sanjaya, Todd Zullinger, Luben Tuikov,
	Ævar Arnfjörð Bjarmason, Taylor Blau,
	Git Mailing List, Uwe Kleine-König
In-Reply-To: <20231020064525.GB1642714@coredump.intra.peff.net>

On Fri, Oct 20, 2023 at 02:45:25AM -0400, Jeff King wrote:

> > I have been looking into handling the interactive input cases while
> > solving this issue, but have yet to make a breakthrough.  Simply
> > moving the validation code below the original process_address_list
> > code results in a a scenario where I get the email address being seen
> > as something like "ARRAY (0x55ddb951d768)" rather than the email
> > address I wrote in the compose buffer.
> 
> Sounds like something is making a perl ref that shouldn't (or something
> that should be dereferencing it not doing so). If you post your patch
> and a reproduction command, I might be able to help debug.

Ah, your "address I wrote in the compose buffer" was the clue I needed.

I think this is actually an existing bug. If I use --compose and write:

  To: foo@example.com

in the editor, we read that back in and handle it in parse_header_line()
like:

        my $addr_pat = join "|", qw(To Cc Bcc);

        foreach (split(/\n/, $lines)) {
                if (/^($addr_pat):\s*(.+)$/i) {
                        $parsed_line->{$1} = [ parse_address_line($2) ];
                } elsif (/^([^:]*):\s*(.+)\s*$/i) {
                        $parsed_line->{$1} = $2;
                }
        }

and there's your perl array ref (from the square brackets, which are
necessary because we're sticking it in a hash value). But even before
your patch, this seems to end up as garbage. The code which reads
$parsed_line does not dereference the array.

The patch to fix it is only a few lines (well, more than that with some
light editorializing in the comments):

diff --git a/git-send-email.perl b/git-send-email.perl
index 76589c7827..46a30088c9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -918,7 +918,28 @@ sub get_patch_subject {
 	# Preserve unknown headers
 	foreach my $key (keys %parsed_email) {
 		next if $key eq 'body';
-		print $c2 "$key: $parsed_email{$key}";
+
+		# it seems like it would be easier to just look for
+		# $parsed_email{'To'} and so on. But we actually match
+		# these case-insenstively and preserve the user's spelling, so
+		# we might see $parsed_email{'to'}. Of course, the same bug
+		# exists for Subject, etc, above. Anyway, a "/i" regex here
+		# handles all cases.
+		#
+		# It kind of feels like all of this code would be much simpler
+		# if we just handled all of the headers while reading back the
+		# input, rather than stuffing them all into $parsed_email and
+		# then picking them out of it.
+		#
+		# It also really feels like these to/cc/bcc lines should be
+		# added to the regular ones? It is silly to make a cover letter
+		# that goes to some addresses, and then not send the patches to
+		# them, too.
+		if ($key =~ /^(To|Cc|Bcc)$/i) {
+			print $c2 "$key: ", join(', ', @{$parsed_email{$key}});
+		} else {
+			print $c2 "$key: $parsed_email{$key}";
+		}
 	}
 
 	if ($parsed_email{'body'}) {

I don't really think your patch makes things worse here. But it is
probably worth fixing it while we are here.

-Peff

^ permalink raw reply related

* Re: [PATCH] send-email: move process_address_list earlier to avoid, uninitialized address error
From: Jeff King @ 2023-10-20  6:45 UTC (permalink / raw)
  To: Michael Strawbridge
  Cc: Junio C Hamano, Bagas Sanjaya, Todd Zullinger, Luben Tuikov,
	Ævar Arnfjörð Bjarmason, Taylor Blau,
	Git Mailing List, Uwe Kleine-König
In-Reply-To: <b4385543-bee0-473b-ab2d-df0d7847ddf3@amd.com>

On Fri, Oct 13, 2023 at 04:25:57PM -0400, Michael Strawbridge wrote:

> > I did not look carefully at the flow of send-email, so this may or may
> > not be an issue. But what I think would be _really_ annoying is if you
> > asked to write a cover letter, went through the trouble of writing it,
> > and then send-email bailed due to some validation failure that could
> > have been checked earlier.
> >
> > There is probably a way to recover your work (presumably we leave it in
> > a temporary file somewhere), but it may not be entirely trivial,
> > especially for users who are not comfortable with advanced usage of
> > their editor. ;)
>
> As I was looking at covering the case of interactive input (--compose)
> to the fix I noticed that this seems to be at least partly handled by
> the $compose_filename code.  There is a nice output message telling
> you exactly where the intermediate version of the email you are
> composing is located if there are errors.  I took a quick look inside
> and can verify that any lost work should be minimal as long as someone
> knows how to edit files with their editor of choice.

OK, that makes me feel better about just moving the validation code. A
more complicated solution could be do to do _some_ basic checks up
front, and then more complete validation later. But even if we wanted to
do that, moving the bulk of the validation (as discussed in this thread)
would probably be the first step anyway (and if nobody complains, maybe
we can avoid doing the extra work).

I do wonder if we might find other interesting corner cases where
the validation code (or somebody's hook) isn't happy with seeing the
more "full" picture (i.e., with the extra addresses from interactive and
command-line input). But arguably any such case would be indicative of a
bug, and smoking it out would be a good thing.

> I have been looking into handling the interactive input cases while
> solving this issue, but have yet to make a breakthrough.  Simply
> moving the validation code below the original process_address_list
> code results in a a scenario where I get the email address being seen
> as something like "ARRAY (0x55ddb951d768)" rather than the email
> address I wrote in the compose buffer.

Sounds like something is making a perl ref that shouldn't (or something
that should be dereferencing it not doing so). If you post your patch
and a reproduction command, I might be able to help debug.

But just blindly moving the validation code down, like:

diff --git a/git-send-email.perl b/git-send-email.perl
index 288ea1ae80..76589c7827 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,30 +799,6 @@ sub is_format_patch_arg {
 
 $time = time - scalar $#files;
 
-if ($validate) {
-	# FIFOs can only be read once, exclude them from validation.
-	my @real_files = ();
-	foreach my $f (@files) {
-		unless (-p $f) {
-			push(@real_files, $f);
-		}
-	}
-
-	# Run the loop once again to avoid gaps in the counter due to FIFO
-	# arguments provided by the user.
-	my $num = 1;
-	my $num_files = scalar @real_files;
-	$ENV{GIT_SENDEMAIL_FILE_TOTAL} = "$num_files";
-	foreach my $r (@real_files) {
-		$ENV{GIT_SENDEMAIL_FILE_COUNTER} = "$num";
-		pre_process_file($r, 1);
-		validate_patch($r, $target_xfer_encoding);
-		$num += 1;
-	}
-	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
-	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
-}
-
 @files = handle_backup_files(@files);
 
 if (@files) {
@@ -1121,6 +1097,30 @@ sub expand_one_alias {
 	$reply_to = sanitize_address($reply_to);
 }
 
+if ($validate) {
+	# FIFOs can only be read once, exclude them from validation.
+	my @real_files = ();
+	foreach my $f (@files) {
+		unless (-p $f) {
+			push(@real_files, $f);
+		}
+	}
+
+	# Run the loop once again to avoid gaps in the counter due to FIFO
+	# arguments provided by the user.
+	my $num = 1;
+	my $num_files = scalar @real_files;
+	$ENV{GIT_SENDEMAIL_FILE_TOTAL} = "$num_files";
+	foreach my $r (@real_files) {
+		$ENV{GIT_SENDEMAIL_FILE_COUNTER} = "$num";
+		pre_process_file($r, 1);
+		validate_patch($r, $target_xfer_encoding);
+		$num += 1;
+	}
+	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
+	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
+}
+
 if (!defined $sendmail_cmd && !defined $smtp_server) {
 	my @sendmail_paths = qw( /usr/sbin/sendmail /usr/lib/sendmail );
 	push @sendmail_paths, map {"$_/sendmail"} split /:/, $ENV{PATH};

seems to fix the problem from this thread and passes the existing tests.
Manually inspecting the result (and what's fed to the validation hook) I
don't see anything odd (like "ARRAY (...)").

-Peff

^ permalink raw reply related

* Re: Pulling from a linux box to a Solaris 9 OS
From: Jeff King @ 2023-10-20  6:27 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Daniel Santos, git
In-Reply-To: <ZTHK7o5LXvMB5EeJ@tapette.crustytoothpaste.net>

On Fri, Oct 20, 2023 at 12:33:50AM +0000, brian m. carlson wrote:

> By default, if the SSH binary is the default ("ssh"), Git assumes that
> it's OpenSSH and sends certain options to enable protocol v2, including
> -o SendEnv.
> 
> If you don't want that, you can set "ssh.variant" to "simple", in which
> case Git will send only the username and the host, but not -p port, -4,
> -6, or -o.  If you do need a different port, then you're out of luck,
> and will either have to install Putty (in which case, the ssh.variant
> value would need to be "putty") or upgrade OpenSSH.  Otherwise, the
> simple value should work fine.

I think your suggestion is the most straight-forward one, but just in
case the "out of luck" part is a problem, you should also be able to
side-step the issue with:

  git -c protocol.version=0 fetch ...

That would allow other features (assuming this older ssh version
supports them!) without triggering the SendEnv option.

-Peff

^ permalink raw reply

* Re: [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Isoken Ibizugbe @ 2023-10-20  4:31 UTC (permalink / raw)
  To: git, Christian Couder
In-Reply-To: <CAJHH8bEfM8KmwhHX_Fmcb0A2zpr8L75vgNhfvZy-uitpSXNUvQ@mail.gmail.com>

On Thu, Oct 19, 2023 at 10:25 AM Isoken Ibizugbe <isokenjune@gmail.com> wrote:
>
> Dear Git Community and Mentors,
>
> I hope you're doing well. I'm excited to share my application draft
> for the Outreachy program with the Git project. Your feedback is
> invaluable, and I'm eager to align the project with the community's
> needs. Please review the attached draft and share your insights.
>
> Thank you for your support.
Hello Christian, I would appreciate a review from you.
>
> Project Application
> ----
>
> About Me:
>
> My name is Isoken June Ibizugbe, my language is primarily English, and
> I am a resident of Nigeria. I am a student at an online coding school
> called African Leadership Xcelerator (ALX), participating in the
> software engineering program.
>
> What project am I applying for?
>
> Moving Existing Tests to a Unit Testing Framework
>
> Why am I interested in working with the Git chosen project?
>
> Git has been a cornerstone for software development, enabling
> developers worldwide to collaborate, innovate, and create exceptional
> software. I would say without Git, my journey to pursuing my software
> engineering career would be impossible, as I use it almost every day.
> Yet, in this constantly evolving landscape, there is always room for
> improvement, even in a well-established project. The Git project
> currently relies on end-to-end tests, and this is where I see an
> opportunity to make a profound impact. Being able to test libraries in
> isolation via unit tests or mocks speeds up determining the root cause
> of bugs. I am deeply passionate about contributing to this project and
> firmly believe in the power of open-source software and the collective
> intelligence of the community. A successful completion of this project
> will significantly improve Git's testing capabilities and bring the
> benefits of fewer errors, faster work and better testing for all
> parts.
>
> My motivation for joining the Git community stemmed from my desire to
> immerse myself in the world of open-source software and, ultimately,
> to become a part of the Outreachy program. My time spent contributing
> to Git has been nothing short of transformative. It has been a
> remarkable learning experience that has introduced me to a new form of
> collaboration using a mailing list and contributing through patches
> rather than the typical Pull Request (PR). This collaborative
> atmosphere has been pivotal in my growth as a developer, and I am
> eager to continue this journey, making meaningful contributions to
> this remarkable open-source project.
>
> Contributions to Git
>
> I have actively participated in Git's mailing list discussions and
> contributed to a micro-project;
>
> - builtin/branch.c: Adjust error messages such as die(), error(), and
> warning() messages used in branch, to conform to coding guidelines
> (https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
> - Implemented changes to fix broken tests based on reviews from the
> community (https://lore.kernel.org/git/20231019084052.567922-1-isokenjune@gmail.com/)
> - In review.
>
> Project Goals:
>
> - Improve Testing Efficiency: Transitioning from end-to-end tests to
> unit tests will enable more efficient testing of error conditions.
> - Codebase Stability: Unit tests enhance code stability and facilitate
> easier debugging through isolation.
> - Simplify Testing: Writing unit tests in pure C simplifies test
> setup, data passing, and reduces testing runtime by eliminating
> separate processes for each test.
>
> Project Milestones:
>
> - Add useful tests of library-like code
> - Integrate with stdlib work
> - Run alongside regular make test target
>
> Project Timeline:
>
> 1. Oct 2 - Nov 20: Community Bonding
>
> - Understanding the structure of Git
> - Getting familiar with the code
>
> 2. Dec 4 - Jan 15: Add useful tests of library-like code
>
> - Identify and document the current state of the tests in the Git
> t/helper directory.
> - Confirm the licensing and compatibility requirements for the chosen
> unit testing framework.
> - Develop unit tests for these library-like components.
> - Execute the tests and ensure they cover various scenarios, including
> error conditions.
> - Run the tests and address any initial issues or bugs to ensure they
> work as intended.
> - Document the new tests and their coverage.
> - Seek feedback  and support from mentors and the Git community
>
> 3. Jan 15 - Feb 15: Integrate with Stdlib Work
>
> - Collaborate with the team working on standard library integration.
> - Ensure that the tests for library-like code align with stdlib work.
> - Verify that the tests effectively check the compatibility and
> interaction of the code with standard libraries.
> - Gather feedback and insights from the Git community on the
> integrated tests, addressing any concerns or suggestions.
>
> 4. Feb 15 - March 1: Run Alongside Regular 'make test' Target and finalize
>
> - Configure the testing framework to run alongside the regular 'make
> test' target.
> - Ensure that the new tests are included in the standard testing suite.
> - Execute 'make test' with the new tests and verify that they pass successfully.
> - Document the integration process and how the new tests are included
> in the standard testing procedure.
> - Perform comprehensive testing of the entire unit testing framework.
> - Ensure all migrated tests are working correctly within the new framework.
> - Document the entire process of migrating Git's tests
> - Prepare a final project report
>
> Technical Requirements
>
> According to the documentation on the unit test project
> (https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc),
> the suggested best framework for the Git project is the "Custom TAP
> framework" (Phillip Wood's TAP implementation), as it aligns with
> Git's licensing requirements, is vendorable, and can be customized by
> Git's developers as needed, but it may require some additional
> development work for features like parallel execution and mock
> support, but it offers a strong foundation for unit testing within the
> Git project.
>
> Relevant Projects
>
> Simple shell -  A project based on emulating a shell. It was a
> collaborative project which we managed using Git.
> (https://github.com/Junie06/simple_shell).
> This project was written in C, which allowed me to apply my C language
> knowledge, essential for Git projects.
> I'm proficient in using Shell for scripting, redirections, and
> permissions, as shown in my work
> (https://github.com/Junie06/alx-system_engineering-devops).
> Creating the simple shell project deepened my understanding of how
> shells work, and I even attempted to replicate a shell environment.
> Collaborating on the Simple Shell project reinforced my Git skills.

^ permalink raw reply

* Re: [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken Ibizugbe @ 2023-10-20  4:26 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <20231019084052.567922-1-isokenjune@gmail.com>

On Thu, Oct 19, 2023 at 9:41 AM Isoken June Ibizugbe
<isokenjune@gmail.com> wrote:
>
> As per the CodingGuidelines document, it is recommended that a single-line
> message provided to error messages such as die(), error() and warning(),
> should start with a lowercase letter and should not end with a period.
> Also this patch fixes the tests broken by the changes.
>
> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> ---
>  builtin/branch.c          | 66 +++++++++++++++++++--------------------
>  t/t2407-worktree-heads.sh |  2 +-
>  t/t3200-branch.sh         | 16 +++++-----
>  t/t3202-show-branch.sh    | 10 +++---
>  4 files changed, 47 insertions(+), 47 deletions(-)
Hello Junio, I would appreciate your comment on this.
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 2ec190b14a..e7ee9bd0f1 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -173,11 +173,11 @@ static int branch_merged(int kind, const char *name,
>             (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
>                 if (merged)
>                         warning(_("deleting branch '%s' that has been merged to\n"
> -                               "         '%s', but not yet merged to HEAD."),
> +                               "         '%s', but not yet merged to HEAD"),
>                                 name, reference_name);
>                 else
>                         warning(_("not deleting branch '%s' that is not yet merged to\n"
> -                               "         '%s', even though it is merged to HEAD."),
> +                               "         '%s', even though it is merged to HEAD"),
>                                 name, reference_name);
>         }
>         free(reference_name_to_free);
> @@ -190,13 +190,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
>  {
>         struct commit *rev = lookup_commit_reference(the_repository, oid);
>         if (!force && !rev) {
> -               error(_("Couldn't look up commit object for '%s'"), refname);
> +               error(_("couldn't look up commit object for '%s'"), refname);
>                 return -1;
>         }
>         if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> -               error(_("The branch '%s' is not fully merged.\n"
> +               error(_("the branch '%s' is not fully merged.\n"
>                       "If you are sure you want to delete it, "
> -                     "run 'git branch -D %s'."), branchname, branchname);
> +                     "run 'git branch -D %s'"), branchname, branchname);
>                 return -1;
>         }
>         return 0;
> @@ -207,7 +207,7 @@ static void delete_branch_config(const char *branchname)
>         struct strbuf buf = STRBUF_INIT;
>         strbuf_addf(&buf, "branch.%s", branchname);
>         if (git_config_rename_section(buf.buf, NULL) < 0)
> -               warning(_("Update of config-file failed"));
> +               warning(_("update of config-file failed"));
>         strbuf_release(&buf);
>  }
>
> @@ -260,7 +260,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>                 if (kinds == FILTER_REFS_BRANCHES) {
>                         const char *path;
>                         if ((path = branch_checked_out(name))) {
> -                               error(_("Cannot delete branch '%s' "
> +                               error(_("cannot delete branch '%s' "
>                                         "used by worktree at '%s'"),
>                                       bname.buf, path);
>                                 ret = 1;
> @@ -275,7 +275,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>                                         &oid, &flags);
>                 if (!target) {
>                         if (remote_branch) {
> -                               error(_("remote-tracking branch '%s' not found."), bname.buf);
> +                               error(_("remote-tracking branch '%s' not found"), bname.buf);
>                         } else {
>                                 char *virtual_name = mkpathdup(fmt_remotes, bname.buf);
>                                 char *virtual_target = resolve_refdup(virtual_name,
> @@ -290,7 +290,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>                                                 "Did you forget --remote?"),
>                                                 bname.buf);
>                                 else
> -                                       error(_("branch '%s' not found."), bname.buf);
> +                                       error(_("branch '%s' not found"), bname.buf);
>                                 FREE_AND_NULL(virtual_target);
>                         }
>                         ret = 1;
> @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
>                         continue;
>
>                 if (is_worktree_being_rebased(wt, target))
> -                       die(_("Branch %s is being rebased at %s"),
> +                       die(_("branch %s is being rebased at %s"),
>                             target, wt->path);
>
>                 if (is_worktree_being_bisected(wt, target))
> -                       die(_("Branch %s is being bisected at %s"),
> +                       die(_("branch %s is being bisected at %s"),
>                             target, wt->path);
>         }
>  }
> @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>                 if (ref_exists(oldref.buf))
>                         recovery = 1;
>                 else
> -                       die(_("Invalid branch name: '%s'"), oldname);
> +                       die(_("invalid branch name: '%s'"), oldname);
>         }
>
>         for (int i = 0; worktrees[i]; i++) {
> @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
>         if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
>                 if (oldref_usage & IS_HEAD)
> -                       die(_("No commit on branch '%s' yet."), oldname);
> +                       die(_("no commit on branch '%s' yet"), oldname);
>                 else
> -                       die(_("No branch named '%s'."), oldname);
> +                       die(_("no branch named '%s'"), oldname);
>         }
>
>         /*
> @@ -624,32 +624,32 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>
>         if (!copy && !(oldref_usage & IS_ORPHAN) &&
>             rename_ref(oldref.buf, newref.buf, logmsg.buf))
> -               die(_("Branch rename failed"));
> +               die(_("branch rename failed"));
>         if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
> -               die(_("Branch copy failed"));
> +               die(_("branch copy failed"));
>
>         if (recovery) {
>                 if (copy)
> -                       warning(_("Created a copy of a misnamed branch '%s'"),
> +                       warning(_("created a copy of a misnamed branch '%s'"),
>                                 interpreted_oldname);
>                 else
> -                       warning(_("Renamed a misnamed branch '%s' away"),
> +                       warning(_("renamed a misnamed branch '%s' away"),
>                                 interpreted_oldname);
>         }
>
>         if (!copy && (oldref_usage & IS_HEAD) &&
>             replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
>                                               logmsg.buf))
> -               die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> +               die(_("branch renamed to %s, but HEAD is not updated"), newname);
>
>         strbuf_release(&logmsg);
>
>         strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
>         strbuf_addf(&newsection, "branch.%s", interpreted_newname);
>         if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
> -               die(_("Branch is renamed, but update of config-file failed"));
> +               die(_("branch is renamed, but update of config-file failed"));
>         if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
> -               die(_("Branch is copied, but update of config-file failed"));
> +               die(_("branch is copied, but update of config-file failed"));
>         strbuf_release(&oldref);
>         strbuf_release(&newref);
>         strbuf_release(&oldsection);
> @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
>         head = resolve_refdup("HEAD", 0, &head_oid, NULL);
>         if (!head)
> -               die(_("Failed to resolve HEAD as a valid ref."));
> +               die(_("failed to resolve HEAD as a valid ref"));
>         if (!strcmp(head, "HEAD"))
>                 filter.detached = 1;
>         else if (!skip_prefix(head, "refs/heads/", &head))
> @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
>                 if (!argc) {
>                         if (filter.detached)
> -                               die(_("Cannot give description to detached HEAD"));
> +                               die(_("cannot give description to detached HEAD"));
>                         branch_name = head;
>                 } else if (argc == 1) {
>                         strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
> @@ -878,8 +878,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
>                 if (!ref_exists(branch_ref.buf))
>                         error((!argc || branch_checked_out(branch_ref.buf))
> -                             ? _("No commit on branch '%s' yet.")
> -                             : _("No branch named '%s'."),
> +                             ? _("no commit on branch '%s' yet")
> +                             : _("no branch named '%s'"),
>                               branch_name);
>                 else if (!edit_branch_description(branch_name))
>                         ret = 0; /* happy */
> @@ -892,8 +892,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 if (!argc)
>                         die(_("branch name required"));
>                 else if ((argc == 1) && filter.detached)
> -                       die(copy? _("cannot copy the current branch while not on any.")
> -                               : _("cannot rename the current branch while not on any."));
> +                       die(copy? _("cannot copy the current branch while not on any")
> +                               : _("cannot rename the current branch while not on any"));
>                 else if (argc == 1)
>                         copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
>                 else if (argc == 2)
> @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 if (!branch) {
>                         if (!argc || !strcmp(argv[0], "HEAD"))
>                                 die(_("could not set upstream of HEAD to %s when "
> -                                     "it does not point to any branch."),
> +                                     "it does not point to any branch"),
>                                     new_upstream);
>                         die(_("no such branch '%s'"), argv[0]);
>                 }
>
>                 if (!ref_exists(branch->refname)) {
>                         if (!argc || branch_checked_out(branch->refname))
> -                               die(_("No commit on branch '%s' yet."), branch->name);
> +                               die(_("no commit on branch '%s' yet"), branch->name);
>                         die(_("branch '%s' does not exist"), branch->name);
>                 }
>
> @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 if (!branch) {
>                         if (!argc || !strcmp(argv[0], "HEAD"))
>                                 die(_("could not unset upstream of HEAD when "
> -                                     "it does not point to any branch."));
> +                                     "it does not point to any branch"));
>                         die(_("no such branch '%s'"), argv[0]);
>                 }
>
>                 if (!branch_has_merge_config(branch))
> -                       die(_("Branch '%s' has no upstream information"), branch->name);
> +                       die(_("branch '%s' has no upstream information"), branch->name);
>
>                 strbuf_reset(&buf);
>                 strbuf_addf(&buf, "branch.%s.remote", branch->name);
> @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 const char *start_name = argc == 2 ? argv[1] : head;
>
>                 if (filter.kind != FILTER_REFS_BRANCHES)
> -                       die(_("The -a, and -r, options to 'git branch' do not take a branch name.\n"
> +                       die(_("the -a, and -r, options to 'git branch' do not take a branch name.\n"
>                                   "Did you mean to use: -a|-r --list <pattern>?"));
>
>                 if (track == BRANCH_TRACK_OVERRIDE)
> -                       die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
> +                       die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
>
>                 if (recurse_submodules) {
>                         create_branches_recursively(the_repository, branch_name,
> diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
> index 469443d8ae..f6835c91dc 100755
> --- a/t/t2407-worktree-heads.sh
> +++ b/t/t2407-worktree-heads.sh
> @@ -45,7 +45,7 @@ test_expect_success 'refuse to overwrite: checked out in worktree' '
>                 grep "cannot force update the branch" err &&
>
>                 test_must_fail git branch -D wt-$i 2>err &&
> -               grep "Cannot delete branch" err || return 1
> +               grep "cannot delete branch" err || return 1
>         done
>  '
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 080e4f24a6..3182abde27 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -291,10 +291,10 @@ test_expect_success 'git branch -M topic topic should work when main is checked
>  test_expect_success 'git branch -M and -C fail on detached HEAD' '
>         git checkout HEAD^{} &&
>         test_when_finished git checkout - &&
> -       echo "fatal: cannot rename the current branch while not on any." >expect &&
> +       echo "fatal: cannot rename the current branch while not on any" >expect &&
>         test_must_fail git branch -M must-fail 2>err &&
>         test_cmp expect err &&
> -       echo "fatal: cannot copy the current branch while not on any." >expect &&
> +       echo "fatal: cannot copy the current branch while not on any" >expect &&
>         test_must_fail git branch -C must-fail 2>err &&
>         test_cmp expect err
>  '
> @@ -943,7 +943,7 @@ test_expect_success 'deleting currently checked out branch fails' '
>         git worktree add -b my7 my7 &&
>         test_must_fail git -C my7 branch -d my7 &&
>         test_must_fail git branch -d my7 2>actual &&
> -       grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
> +       grep "^error: cannot delete branch .my7. used by worktree at " actual &&
>         rm -r my7 &&
>         git worktree prune
>  '
> @@ -954,7 +954,7 @@ test_expect_success 'deleting in-use branch fails' '
>         git -C my7 bisect start HEAD HEAD~2 &&
>         test_must_fail git -C my7 branch -d my7 &&
>         test_must_fail git branch -d my7 2>actual &&
> -       grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
> +       grep "^error: cannot delete branch .my7. used by worktree at " actual &&
>         rm -r my7 &&
>         git worktree prune
>  '
> @@ -1024,7 +1024,7 @@ test_expect_success '--set-upstream-to fails on multiple branches' '
>  test_expect_success '--set-upstream-to fails on detached HEAD' '
>         git checkout HEAD^{} &&
>         test_when_finished git checkout - &&
> -       echo "fatal: could not set upstream of HEAD to main when it does not point to any branch." >expect &&
> +       echo "fatal: could not set upstream of HEAD to main when it does not point to any branch" >expect &&
>         test_must_fail git branch --set-upstream-to main 2>err &&
>         test_cmp expect err
>  '
> @@ -1072,7 +1072,7 @@ test_expect_success 'use --set-upstream-to modify a particular branch' '
>  '
>
>  test_expect_success '--unset-upstream should fail if given a non-existent branch' '
> -       echo "fatal: Branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
> +       echo "fatal: branch '"'"'i-dont-exist'"'"' has no upstream information" >expect &&
>         test_must_fail git branch --unset-upstream i-dont-exist 2>err &&
>         test_cmp expect err
>  '
> @@ -1094,7 +1094,7 @@ test_expect_success 'test --unset-upstream on HEAD' '
>         test_must_fail git config branch.main.remote &&
>         test_must_fail git config branch.main.merge &&
>         # fail for a branch without upstream set
> -       echo "fatal: Branch '"'"'main'"'"' has no upstream information" >expect &&
> +       echo "fatal: branch '"'"'main'"'"' has no upstream information" >expect &&
>         test_must_fail git branch --unset-upstream 2>err &&
>         test_cmp expect err
>  '
> @@ -1108,7 +1108,7 @@ test_expect_success '--unset-upstream should fail on multiple branches' '
>  test_expect_success '--unset-upstream should fail on detached HEAD' '
>         git checkout HEAD^{} &&
>         test_when_finished git checkout - &&
> -       echo "fatal: could not unset upstream of HEAD when it does not point to any branch." >expect &&
> +       echo "fatal: could not unset upstream of HEAD when it does not point to any branch" >expect &&
>         test_must_fail git branch --unset-upstream 2>err &&
>         test_cmp expect err
>  '
> diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
> index b17f388f56..2cdb834b37 100755
> --- a/t/t3202-show-branch.sh
> +++ b/t/t3202-show-branch.sh
> @@ -10,7 +10,7 @@ GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
>  test_expect_success 'error descriptions on empty repository' '
>         current=$(git branch --show-current) &&
>         cat >expect <<-EOF &&
> -       error: No commit on branch '\''$current'\'' yet.
> +       error: no commit on branch '\''$current'\'' yet
>         EOF
>         test_must_fail git branch --edit-description 2>actual &&
>         test_cmp expect actual &&
> @@ -21,7 +21,7 @@ test_expect_success 'error descriptions on empty repository' '
>  test_expect_success 'fatal descriptions on empty repository' '
>         current=$(git branch --show-current) &&
>         cat >expect <<-EOF &&
> -       fatal: No commit on branch '\''$current'\'' yet.
> +       fatal: no commit on branch '\''$current'\'' yet
>         EOF
>         test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
>         test_cmp expect actual &&
> @@ -224,7 +224,7 @@ done
>
>  test_expect_success 'error descriptions on non-existent branch' '
>         cat >expect <<-EOF &&
> -       error: No branch named '\''non-existent'\'.'
> +       error: no branch named '\''non-existent'\''
>         EOF
>         test_must_fail git branch --edit-description non-existent 2>actual &&
>         test_cmp expect actual
> @@ -238,7 +238,7 @@ test_expect_success 'fatal descriptions on non-existent branch' '
>         test_cmp expect actual &&
>
>         cat >expect <<-EOF &&
> -       fatal: No branch named '\''non-existent'\''.
> +       fatal: no branch named '\''non-existent'\''
>         EOF
>         test_must_fail git branch -c non-existent new-branch 2>actual &&
>         test_cmp expect actual &&
> @@ -253,7 +253,7 @@ test_expect_success 'error descriptions on orphan branch' '
>         test_branch_op_in_wt() {
>                 test_orphan_error() {
>                         test_must_fail git $* 2>actual &&
> -                       test_i18ngrep "No commit on branch .orphan-branch. yet.$" actual
> +                       test_i18ngrep "no commit on branch .orphan-branch. yet$" actual
>                 } &&
>                 test_orphan_error -C wt branch $1 $2 &&                # implicit branch
>                 test_orphan_error -C wt branch $1 orphan-branch $2 &&  # explicit branch
> --
> 2.42.0.346.g24618a8a3e.dirty
>

^ permalink raw reply

* Re: [PATCH] send-email: move process_address_list earlier to avoid, uninitialized address error
From: Bagas Sanjaya @ 2023-10-20  2:50 UTC (permalink / raw)
  To: Michael Strawbridge, Jeff King, Todd Zullinger
  Cc: Junio C Hamano, Luben Tuikov,
	Ævar Arnfjörð Bjarmason, Taylor Blau,
	Git Mailing List
In-Reply-To: <7e2c92ff-b42c-4b3f-a509-9d0785448262@amd.com>

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

On Wed, Oct 11, 2023 at 04:22:18PM -0400, Michael Strawbridge wrote:
> Move processing of email address lists before the sendemail-validate
> hook code.  This fixes email address validation errors when the optional
> perl module Email::Valid is installed and multiple addresses are passed
> in on a single to/cc argument like --to=foo@example.com,bar@example.com.
> ---
>  git-send-email.perl | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 288ea1ae80..cfd80c9d8b 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -799,6 +799,10 @@ sub is_format_patch_arg {
>  
>  $time = time - scalar $#files;
>  
> +@initial_to = process_address_list(@initial_to);
> +@initial_cc = process_address_list(@initial_cc);
> +@initial_bcc = process_address_list(@initial_bcc);
> +
>  if ($validate) {
>         # FIFOs can only be read once, exclude them from validation.
>         my @real_files = ();
> @@ -1099,10 +1103,6 @@ sub expand_one_alias {
>         return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
>  }
>  
> -@initial_to = process_address_list(@initial_to);
> -@initial_cc = process_address_list(@initial_cc);
> -@initial_bcc = process_address_list(@initial_bcc);
> -
>  if ($thread && !defined $initial_in_reply_to && $prompting) {
>         $initial_in_reply_to = ask(
>                 __("Message-ID to be used as In-Reply-To for the first email (if any)? "),

Thanks for the fixup! The patch itself is whitespace-damaged, though, so
I have to manually apply it. Regardless,

Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH] typo: fix the typo 'neeed' into 'needed' in the comment under merge-ort.c
From: 王常新 @ 2023-10-20  2:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq5y32zgnk.fsf@gitster.g>

It is my official name. But the email address is not a valid one. Should I rewrite the commit message?

Best regards

> 2023年10月20日 01:05,Junio C Hamano <gitster@pobox.com> 写道:
> 
> 王常新 <wchangxin824@gmail.com> writes:
> 
>> From: foril <1571825323@qq.com>
>> 
>> Signed-off-by: 王常新 (Wang Changxin) <foril@foril.space>
>> ---
> 
> Thanks.  
> 
> We want to make sure that the "Name <e-mail-address>" on the From:
> and Signed-off-by: lines match.  Is your official name/address the
> one on the Singed-off-by: line?
> 


^ permalink raw reply

* Re: [PATCH v4 2/4] trailer: find the end of the log message
From: Junio C Hamano @ 2023-10-20  0:36 UTC (permalink / raw)
  To: Linus Arver
  Cc: Jonathan Tan, Linus Arver via GitGitGadget, git, Glen Choo,
	Christian Couder, Phillip Wood
In-Reply-To: <owlymsweqgx4.fsf@fine.c.googlers.com>

Linus Arver <linusa@google.com> writes:

> Hi Jonathan, it's been a while because I was on vacation. I've forgotten
> about most of the intricacies of this patch (I think this was a good
> thing, read on below).

Welcome back ;-).

> Will update, thanks.

Thanks.


^ permalink raw reply

* Re: Pulling from a linux box to a Solaris 9 OS
From: brian m. carlson @ 2023-10-20  0:33 UTC (permalink / raw)
  To: Daniel Santos; +Cc: git
In-Reply-To: <20A47898-41F6-4E94-822C-10509EC1E3B9@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

On 2023-10-19 at 21:49:53, Daniel Santos wrote:
> Hello,
> 
> I have a bare repo on a linux box. I can push and pull from a modern macOS X.
> But I also have a Solaris 9 box with a checkout of that same repo. when I try to pull I get the messages:
> 
> bash-2.05$ git pull
> command-line: line 0: Bad configuration option: SendEnv
> fatal: Could not read from remote repository.
> 
> Please make sure you have the correct access rights
> and the repository exists.
> 
> The cause maybe the ‘old-ness’ of solaris software.
> Is there any way to get around this ?

By default, if the SSH binary is the default ("ssh"), Git assumes that
it's OpenSSH and sends certain options to enable protocol v2, including
-o SendEnv.

If you don't want that, you can set "ssh.variant" to "simple", in which
case Git will send only the username and the host, but not -p port, -4,
-6, or -o.  If you do need a different port, then you're out of luck,
and will either have to install Putty (in which case, the ssh.variant
value would need to be "putty") or upgrade OpenSSH.  Otherwise, the
simple value should work fine.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Re: [PATCH v4 2/4] trailer: find the end of the log message
From: Linus Arver @ 2023-10-20  0:24 UTC (permalink / raw)
  To: Jonathan Tan, Linus Arver via GitGitGadget
  Cc: Jonathan Tan, git, Glen Choo, Christian Couder, Phillip Wood
In-Reply-To: <20230928231644.3529127-1-jonathantanmy@google.com>

Hi Jonathan, it's been a while because I was on vacation. I've forgotten
about most of the intricacies of this patch (I think this was a good
thing, read on below).

Jonathan Tan <jonathantanmy@google.com> writes:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> From: Linus Arver <linusa@google.com>
>> 
>> Previously, trailer_info_get() computed the trailer block end position
>> by
>> 
>> (1) checking for the opts->no_divider flag and optionally calling
>>     find_patch_start() to find the "patch start" location (patch_start), and
>> (2) calling find_trailer_end() to find the end of the trailer block
>>     using patch_start as a guide, saving the return value into
>>     "trailer_end".
>> 
>> The logic in (1) was awkward because the variable "patch_start" is
>> misleading if there is no patch in the input. The logic in (2) was
>> misleading because it could be the case that no trailers are in the
>> input (yet we are setting a "trailer_end" variable before even searching
>> for trailers, which happens later in find_trailer_start()). The name
>> "find_trailer_end" was misleading because that function did not look for
>> any trailer block itself --- instead it just computed the end position
>> of the log message in the input where the end of the trailer block (if
>> it exists) would be (because trailer blocks must always come after the
>> end of the log message).
>
> [...]
>
> As it is, the reader still needs to know that trailer_start is where
> the trailer would start if it was present, and then I think it's quite
> natural to have trailer_end be where the trailer would end if it was
> present.
>
> I believe the code is simpler this way, because trailer absence now no
> longer needs to be special-cased when we use these variables (or maybe
> they sometimes do, but not as often, since code that writes to the end
> of the trailers, for example, can now just write at trailer_end instead
> of having to check whether trailers exist). Same comment for patch 4
> regarding using the special value 0 if no trailers are found (I think
> the existing code makes more sense).

I think the root cause of my confusion with this codebase is due to the
variables being named as if the things they refer to exist, but without
any guarantees that they indeed exist. This applies to "patch_start"
(the patch part might not be present in the input),
"trailer_{start,end}" (trailers block might not exist (yet)). IOW these
variables are named as if the intent is to always add new trailers into
the input, which may not be the case (we have "--parse", after all).

Looking again at patch 4, I'm now leaning toward dropping it. Other
than the reasons you cited, we also add a new struct field which by
itself does not add new information (the information can already be
deduced from the other fields). In the near future I want to simplify
the data structures as much as possible, and adding a new field seems to
go against this desire of mine.

>> Combine the logic in (1) and (2) together into find_patch_start() by
>> renaming it to find_end_of_log_message(). The end of the log message is
>> the starting point which find_trailer_start() needs to start searching
>> backward to parse individual trailers (if any).
>
> Having said that, if patch_start is too confusing for whatever reason,
> this refactoring makes sense. (Avoid the confusing name by avoiding
> needing to name it in the first place.)

I think the existing code is confusing, and would prefer to keep this
patch.

>> -static size_t find_patch_start(const char *str)
>> +static size_t find_end_of_log_message(const char *input, int no_divider)
>>  {
>> +	size_t end;
>> +
>>  	const char *s;
>>  
>> -	for (s = str; *s; s = next_line(s)) {
>> +	/* Assume the naive end of the input is already what we want. */
>> +	end = strlen(input);
>> +
>> +	/* Optionally skip over any patch part ("---" line and below). */
>> +	for (s = input; *s; s = next_line(s)) {
>>  		const char *v;
>>  
>> -		if (skip_prefix(s, "---", &v) && isspace(*v))
>> -			return s - str;
>> +		if (!no_divider && skip_prefix(s, "---", &v) && isspace(*v)) {
>> +			end = s - input;
>> +			break;
>> +		}
>>  	}
>>  
>> -	return s - str;
>> +	/* Skip over other ignorable bits. */
>> +	return end - ignored_log_message_bytes(input, end);
>>  }
>
> This sometimes redundantly calls strlen and sometimes redundantly loops.
> I think it's better to do as the code currently does - so, have a big
> if/else at the beginning of this function that checks no_divider.

Will update, thanks.

^ permalink raw reply

* Re: [PATCH v3 3/3] rev-list: add commit object support in `--missing` option
From: Junio C Hamano @ 2023-10-19 23:35 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git, ps
In-Reply-To: <xmqq4jimuv26.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Do we really need to allocate a new bit in the object flags, which
> is already a scarce resource?

Clarification.  I was *not* wondering if we can steal and (re|ab)use
a bit that is used for other purposes, in order to avoid allocating
a new bit.

Rather, I was wondering if we need to use object flags to mark these
objects, or can do what we want to do without using any object flags
at all.  For the purpose of reporting "missing" objects, wouldn't it
be sufficient to walk the object graph and report our findings as we
go?  To avoid reporting the same object twice, as we reasonably can
expect that the missing objects are minority (compared to the total
number of objects), perhaps the codepath that makes such a report
can use a hashmap of object_ids or something, for example.

Thanks.

^ permalink raw reply

* Re: [PATCH v3 3/3] rev-list: add commit object support in `--missing` option
From: Junio C Hamano @ 2023-10-19 22:05 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git, ps
In-Reply-To: <20231019121024.194317-4-karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

> diff --git a/object.h b/object.h
> index 114d45954d..b76830fce1 100644
> --- a/object.h
> +++ b/object.h
> @@ -62,7 +62,7 @@ void object_array_init(struct object_array *array);
>  
>  /*
>   * object flag allocation:
> - * revision.h:               0---------10         15             23------27
> + * revision.h:               0---------10         15             22------28
>   * fetch-pack.c:             01    67
>   * negotiator/default.c:       2--5
>   * walker.c:                 0-2
> @@ -82,7 +82,7 @@ void object_array_init(struct object_array *array);
>   * builtin/show-branch.c:    0-------------------------------------------26
>   * builtin/unpack-objects.c:                                 2021
>   */
> -#define FLAG_BITS  28
> +#define FLAG_BITS  29
>  
>  #define TYPE_BITS 3

I am afraid that this is not a good direction to go, given that the
way FLAG_BITS is used is like this:

    /*
     * The object type is stored in 3 bits.
     */
    struct object {
            unsigned parsed : 1;
            unsigned type : TYPE_BITS;
            unsigned flags : FLAG_BITS;
            struct object_id oid;
    };

28 was there, not as a random number of bits we happen to be using.
It was derived by (32 - 3 - 1), i.e. ensure the bitfields above are
stored within a single word.

sizeof(struct object) is 40 bytes on x86-64, with offsetof(oid)
being 4 (i.e. the bitfields fit in a single 4-byte word).  If we
make FLAG_BITS 29, we will add 4 bytes to the structure and waste
31-bit per each and every in-core objects.

Do we really need to allocate a new bit in the object flags, which
is already a scarce resource?

^ permalink raw reply

* Pulling from a linux box to a Solaris 9 OS
From: Daniel Santos @ 2023-10-19 21:49 UTC (permalink / raw)
  To: git

Hello,

I have a bare repo on a linux box. I can push and pull from a modern macOS X.
But I also have a Solaris 9 box with a checkout of that same repo. when I try to pull I get the messages:

bash-2.05$ git pull
command-line: line 0: Bad configuration option: SendEnv
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

The cause maybe the ‘old-ness’ of solaris software.
Is there any way to get around this ?

Thanks,
Regards
Daniel Santos




^ permalink raw reply

* Re: [PATCH v4 0/7] merge-ort: implement support for packing objects together
From: Junio C Hamano @ 2023-10-19 21:47 UTC (permalink / raw)
  To: Taylor Blau
  Cc: git, Elijah Newren, Eric W. Biederman, Jeff King,
	Patrick Steinhardt
In-Reply-To: <cover.1697736516.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> By factoring out a common
> "deflate object to pack" that takes an abstract bulk_checkin_source as a
> parameter, all of the earlier refactorings can be dropped since we
> retain only a single caller instead of multiple.

Ah, that is how we managed to lose the preparatory split of new
helper functions.  The names given to these functions that represent
steps of checkpointing were well thought out, and they served their
documentation purposes very well, but if they all have a single
caller, then that is fine ;-).

I _think_ I carefully followed the code/data flow of the new code to
see that we do identical things when streaming from a file to a pack
(in other words, this does not introduce regression), and nothing
objectionable stood out.  The order of presentation was a good way
to let readers to do so, and the actual "now we have everything
working as we want, let's teach the machinery to also read from an
in-core buffer" step being very small gives us extra confidence with
the result.  Very nicely done.

^ permalink raw reply

* Re: Is there any interest in localizing term delimiters in git messages?
From: Junio C Hamano @ 2023-10-19 20:16 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Jiang Xin, Jeff Hostetler, Alexander Shopov, Git List, jmas,
	alexhenrie24, ralf.thielow, matthias.ruester, phillip.szelat,
	vyruss, christopher.diaz.riv, jn.avila, flashcode, bagasdotme,
	Ævar Arnfjörð Bjarmason, alessandro.menti,
	elongbug, cwryu, uneedsihyeon, arek_koz, dacs.git,
	insolor@gmail.com, peter, bitigchi, ark, kate,
	vnwildman@gmail.com, pclouds, dyroneteng@gmail.com,
	oldsharp@gmail.com, lilydjwg@gmail.com, pan93412@gmail.com,
	franklin@goodhorse.idv.tw
In-Reply-To: <20231019194747.GC25301@tb-raspi4>

Torsten Bögershausen <tboegi@web.de> writes:

> The switch case covers all 8 values of "enum crlf_action",
> and removing these 2 lines
>  -	warning("Illegal crlf_action %d\n", (int)crlf_action);
>  -	return core_eol;
> does still compile without a compiler warning.
> So yes, a BUG is more appropriate here.

Yeah, and if our expectation is whenever we add a new value to enum
convert_crlf_action, we will handle in and return from the switch
statement, so I agree with you that BUG() is more appropriate.

Thanks for a quick response.  Note that this was something we
discovered while talking about i18n and no immediate action is
required---it is not like we saw a report that tells us that end
users are actively getting confused by this message.

Thanks.

^ permalink raw reply

* Re: [PATCH v5 11/14] replay: use standard revision ranges
From: Linus Arver @ 2023-10-19 19:49 UTC (permalink / raw)
  To: Christian Couder, git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Christian Couder
In-Reply-To: <20231010123847.2777056-12-christian.couder@gmail.com>

This patch's documentation bits LGTM. The comments I have below are all
nits and I don't think they are enough to require a re-roll. If v5 is
accepted as-is, then this stuff could be #leftoverbits for a future
(minor) cleanup.

Thanks!

Christian Couder <christian.couder@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
> 
> +DESCRIPTION
> +-----------
> +
> +Takes a range of commits and replays them onto a new location. Leaves
> +the working tree and the index untouched, and updates no
> +references.

Nit: in v4 you had

     Does
    +not touch the working tree or index, and does not update any
    +references. 

and the "does not update any references" sounds more natural than
"updates no references".

> The output of this command is meant to be used as input to
> +`git update-ref --stdin`, which would update the relevant branches
> +(see the OUTPUT section below).
> +
> +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.

Nit: add "IN THE FUTURE" at the end.

> +EXAMPLES
> +--------
> +
> +To simply rebase `mybranch` onto `target`:

Nit: s/To simply rebase/Rebase

or, remove "simply" because it doesn't add much value.

^ permalink raw reply

* Re: Is there any interest in localizing term delimiters in git messages?
From: Torsten Bögershausen @ 2023-10-19 19:47 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Jeff Hostetler, Alexander Shopov, Git List, jmas,
	alexhenrie24, ralf.thielow, matthias.ruester, phillip.szelat,
	vyruss, christopher.diaz.riv, jn.avila, flashcode, bagasdotme,
	Ævar Arnfjörð Bjarmason, alessandro.menti,
	elongbug, cwryu, uneedsihyeon, arek_koz, dacs.git,
	insolor@gmail.com, peter, bitigchi, ark, kate,
	vnwildman@gmail.com, pclouds, dyroneteng@gmail.com,
	oldsharp@gmail.com, lilydjwg@gmail.com, pan93412@gmail.com,
	franklin@goodhorse.idv.tw
In-Reply-To: <xmqqcyxaxzxw.fsf@gitster.g>

On Thu, Oct 19, 2023 at 10:52:11AM -0700, Junio C Hamano wrote:

>
> Also some of them might better be a BUG(), instead of die(_()).
>
> >  * crlf_action in convert.c:
> >
> >     warning(_("illegal crlf_action %d"), (int)crlf_action);
>
> [jch: cc'ed Torsten for area expertise].
>
> For example, can convert.c::output_eol() be called with an illegal
> crlf_action that is not covered by the switch() statement due to
> data error, not a programming error?  From my quick scan, it looks
> like that the error should never happen no matter what end-user
> mistakes (e.g., misspelt attribute and configuration variable names
> in their files) are fed to convert_attrs(), and can come only from a
> bug in that function (e.g., long and convoluted if/else cascade fails
> to assign any value to ca->crlf_action and leaves an undefined and
> "illegal" value there).

The switch case covers all 8 values of "enum crlf_action",
and removing these 2 lines
 -	warning("Illegal crlf_action %d\n", (int)crlf_action);
 -	return core_eol;
does still compile without a compiler warning.
So yes, a BUG is more appropriate here.
I hopefully find some time to send a patch the next days.

>
> Thanks.
>

^ permalink raw reply

* [PATCH 7/7] log: show pseudorefs in decorations
From: Andy Koppe @ 2023-10-19 19:39 UTC (permalink / raw)
  To: git; +Cc: Andy Koppe
In-Reply-To: <20231003205442.22963-1-andy.koppe@gmail.com>

Show various pseudorefs in log decorations. This includes pseudorefs for
ongoing operations such as MERGE_HEAD and REBASE_HEAD, as well as
ORIG_HEAD for the HEAD position before any "drastic" operations.

Do not include FETCH_HEAD in the default decoration filter though,
because it would appear a lot in user's logs, often right alongside
HEAD, while providing little value as usually a remote-tracking branch
already points at what's been fetched.

To implement this, introduce decoration type DECORATION_REF_PSEUDO with
corresponding color.decorate.pseudoref setting that defaults to bold
blue. (This makes it similar to but not the same as HEAD, which defaults
to bold cyan.)

Add entries for each pseudoref to the ref_namespace array in refs.c.
Process them in new function add_pseudoref_decorations(). They also get
picked up by set_default_decoration_filter().

Document the showing of pseudorefs on the git-log page and the
color.decorate.pseudoref setting on the git-config page.

Amend t4207-log-decoration-colors.sh to test color.decorate.pseudoref,
and tweak various other tests to reflect the appearance of ORIG_HEAD in
decorations.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 Documentation/config/color.txt                |  2 +
 Documentation/git-log.txt                     |  7 ++--
 commit.h                                      |  1 +
 log-tree.c                                    | 29 +++++++++++++
 refs.c                                        | 41 +++++++++++++++++++
 refs.h                                        |  7 ++++
 t/t4013/diff.log_--decorate=full_--all        |  2 +-
 ..._--decorate=full_--clear-decorations_--all |  4 +-
 t/t4013/diff.log_--decorate_--all             |  2 +-
 ...f.log_--decorate_--clear-decorations_--all |  4 +-
 t/t4202-log.sh                                | 23 ++++++-----
 t/t4207-log-decoration-colors.sh              | 13 ++++--
 12 files changed, 113 insertions(+), 22 deletions(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 086efebbe5d..139fb90498a 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -95,6 +95,8 @@ color.decorate.<slot>::
 `ref`;;
 	any other refs (not shown unless enabled with one of the decoration
 	filter options such as `--decorate-refs=<pattern>`)
+`pseudoref`;;
+	pseudorefs such as ORIG_HEAD or MERGE_HEAD
 `symbol`;;
 	punctuation surrounding the other elements
 --
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf88807..f217d16c439 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -52,9 +52,10 @@ OPTIONS
 	the decorations, but an explicit `--decorate-refs` pattern will
 	override a match in `log.excludeDecoration`.
 +
-If none of these options or config settings are given, then references are
-used as decoration if they match `HEAD`, `refs/heads/`, `refs/remotes/`,
-`refs/stash/`, or `refs/tags/`.
+If none of these options or config settings are given, then refs matching
+`refs/heads/`, `refs/remotes/`, `refs/stash/`, or `refs/tags/`, as well as
+the `HEAD` ref and the pseudorefs `ORIG_HEAD`, `MERGE_HEAD`, `REBASE_HEAD`,
+`CHERRY_PICK_HEAD`, `REVERT_HEAD` and `BISECT_HEAD` are shown.
 
 --clear-decorations::
 	When specified, this option clears all previous `--decorate-refs`
diff --git a/commit.h b/commit.h
index f6b2125fc42..44dd3ce19b4 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
 	DECORATION_REF_STASH,
 	DECORATION_REF,
 	DECORATION_REF_HEAD,
+	DECORATION_REF_PSEUDO,
 	DECORATION_GRAFTED,
 	DECORATION_SYMBOL,
 };
diff --git a/log-tree.c b/log-tree.c
index fd99eb88d95..eae288599d0 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	[DECORATION_REF_STASH]	= GIT_COLOR_BOLD_MAGENTA,
 	[DECORATION_REF]	= GIT_COLOR_BOLD_MAGENTA,
 	[DECORATION_REF_HEAD]	= GIT_COLOR_BOLD_CYAN,
+	[DECORATION_REF_PSEUDO]	= GIT_COLOR_BOLD_BLUE,
 	[DECORATION_GRAFTED]	= GIT_COLOR_BOLD_BLUE,
 	[DECORATION_SYMBOL]	= GIT_COLOR_NIL,
 };
@@ -52,6 +53,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_STASH]	= "stash",
 	[DECORATION_REF]	= "ref",
 	[DECORATION_REF_HEAD]	= "HEAD",
+	[DECORATION_REF_PSEUDO]	= "pseudoref",
 	[DECORATION_GRAFTED]	= "grafted",
 	[DECORATION_SYMBOL]	= "symbol",
 };
@@ -146,6 +148,32 @@ static int ref_filter_match(const char *refname,
 	return 1;
 }
 
+static void add_pseudoref_decorations(const struct decoration_filter *filter)
+{
+	struct ref_store *store = get_main_ref_store(the_repository);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
+		struct object_id oid;
+		struct object *obj;
+		enum object_type objtype;
+		const struct ref_namespace_info *info = &ref_namespace[i];
+
+		if (info->decoration != DECORATION_REF_PSEUDO ||
+		    !refs_resolve_ref_unsafe(store, info->ref,
+					     RESOLVE_REF_READING, &oid, NULL) ||
+		    (filter && !ref_filter_match(info->ref, filter)))
+			continue;
+
+		objtype = oid_object_info(the_repository, &oid, NULL);
+		if (objtype < 0)
+			continue;
+
+		obj = lookup_object_by_type(the_repository, &oid, objtype);
+		add_name_decoration(DECORATION_REF_PSEUDO, info->ref, obj);
+	}
+}
+
 static int add_ref_decoration(const char *refname, const struct object_id *oid,
 			      int flags UNUSED,
 			      void *cb_data)
@@ -236,6 +264,7 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)
 		decoration_loaded = 1;
 		decoration_flags = flags;
 		for_each_ref(add_ref_decoration, filter);
+		add_pseudoref_decorations(filter);
 		head_ref(add_ref_decoration, filter);
 		for_each_commit_graft(add_graft_decoration, filter);
 	}
diff --git a/refs.c b/refs.c
index 492b9f7d4e5..d960997aed1 100644
--- a/refs.c
+++ b/refs.c
@@ -149,6 +149,47 @@ struct ref_namespace_info ref_namespace[] = {
 		.ref = "refs/",
 		.decoration = DECORATION_REF,
 	},
+	[NAMESPACE_ORIG_HEAD] = {
+		.ref = "ORIG_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_MERGE_HEAD] = {
+		.ref = "MERGE_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_REBASE_HEAD] = {
+		.ref = "REBASE_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_CHERRY_PICK_HEAD] = {
+		.ref = "CHERRY_PICK_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_REVERT_HEAD] = {
+		.ref = "REVERT_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_BISECT_HEAD] = {
+		.ref = "BISECT_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+		.include = 1,
+	},
+	[NAMESPACE_FETCH_HEAD] = {
+		.ref = "FETCH_HEAD",
+		.exact = 1,
+		.decoration = DECORATION_REF_PSEUDO,
+	},
 };
 
 void update_ref_namespace(enum ref_namespace namespace, char *ref)
diff --git a/refs.h b/refs.h
index b77691d787a..5ac1f308d07 100644
--- a/refs.h
+++ b/refs.h
@@ -1011,6 +1011,13 @@ enum ref_namespace {
 	NAMESPACE_PREFETCH,
 	NAMESPACE_REWRITTEN,
 	NAMESPACE_REFS,
+	NAMESPACE_ORIG_HEAD,
+	NAMESPACE_MERGE_HEAD,
+	NAMESPACE_REBASE_HEAD,
+	NAMESPACE_CHERRY_PICK_HEAD,
+	NAMESPACE_REVERT_HEAD,
+	NAMESPACE_BISECT_HEAD,
+	NAMESPACE_FETCH_HEAD,
 
 	/* Must be last */
 	NAMESPACE__COUNT
diff --git a/t/t4013/diff.log_--decorate=full_--all b/t/t4013/diff.log_--decorate=full_--all
index 6b0b334a5d6..9ebdd9b14fa 100644
--- a/t/t4013/diff.log_--decorate=full_--all
+++ b/t/t4013/diff.log_--decorate=full_--all
@@ -39,7 +39,7 @@ Date:   Mon Jun 26 00:03:00 2006 +0000
 
     Side
 
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:02:00 2006 +0000
 
diff --git a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
index 1c030a6554e..7d16978e7f6 100644
--- a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
 
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, refs/heads/side)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:03:00 2006 +0000
 
     Side
 
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:02:00 2006 +0000
 
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
index c7df1f58141..46dde03cffa 100644
--- a/t/t4013/diff.log_--decorate_--all
+++ b/t/t4013/diff.log_--decorate_--all
@@ -39,7 +39,7 @@ Date:   Mon Jun 26 00:03:00 2006 +0000
 
     Side
 
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:02:00 2006 +0000
 
diff --git a/t/t4013/diff.log_--decorate_--clear-decorations_--all b/t/t4013/diff.log_--decorate_--clear-decorations_--all
index 88be82cce31..4f9be50ce02 100644
--- a/t/t4013/diff.log_--decorate_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date:   Mon Jun 26 00:04:00 2006 +0000
 
     Merge branch 'side'
 
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, side)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:03:00 2006 +0000
 
     Side
 
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:02:00 2006 +0000
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd22..ea048cb4a29 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -927,7 +927,7 @@ test_expect_success 'multiple decorate-refs' '
 test_expect_success 'decorate-refs-exclude with glob' '
 	cat >expect.decorate <<-\EOF &&
 	Merge-tag-reach (HEAD -> main)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	seventh (tag: seventh)
 	octopus-b (tag: octopus-b)
 	octopus-a (tag: octopus-a)
@@ -944,7 +944,7 @@ test_expect_success 'decorate-refs-exclude with glob' '
 test_expect_success 'decorate-refs-exclude without globs' '
 	cat >expect.decorate <<-\EOF &&
 	Merge-tag-reach (HEAD -> main)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	seventh (tag: seventh)
 	octopus-b (tag: octopus-b, octopus-b)
 	octopus-a (tag: octopus-a, octopus-a)
@@ -961,7 +961,7 @@ test_expect_success 'decorate-refs-exclude without globs' '
 test_expect_success 'multiple decorate-refs-exclude' '
 	cat >expect.decorate <<-\EOF &&
 	Merge-tag-reach (HEAD -> main)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	seventh (tag: seventh)
 	octopus-b (tag: octopus-b)
 	octopus-a (tag: octopus-a)
@@ -1022,10 +1022,12 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
 	EOF
 	git log -n6 --decorate=short --pretty="tformat:%f%d" \
 		--decorate-refs-exclude="*octopus*" \
+		--decorate-refs-exclude="ORIG_HEAD" \
 		--simplify-by-decoration >actual &&
 	test_cmp expect.decorate actual &&
-	git -c log.excludeDecoration="*octopus*" log \
-		-n6 --decorate=short --pretty="tformat:%f%d" \
+	git -c log.excludeDecoration="*octopus*" \
+	    -c log.excludeDecoration="ORIG_HEAD" \
+	    log -n6 --decorate=short --pretty="tformat:%f%d" \
 		--simplify-by-decoration >actual &&
 	test_cmp expect.decorate actual
 '
@@ -1067,9 +1069,10 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
 	test_cmp expect actual
 '
 
-test_expect_success 'decorate-refs-exclude HEAD' '
+test_expect_success 'decorate-refs-exclude HEAD ORIG_HEAD' '
 	git log --decorate=full --oneline \
-		--decorate-refs-exclude="HEAD" >actual &&
+		--decorate-refs-exclude="HEAD" \
+		--decorate-refs-exclude="ORIG_HEAD" >actual &&
 	! grep HEAD actual
 '
 
@@ -1082,7 +1085,7 @@ test_expect_success 'decorate-refs focus from default' '
 test_expect_success '--clear-decorations overrides defaults' '
 	cat >expect.default <<-\EOF &&
 	Merge-tag-reach (HEAD -> refs/heads/main)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	seventh (tag: refs/tags/seventh)
 	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
 	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
@@ -1107,7 +1110,7 @@ test_expect_success '--clear-decorations overrides defaults' '
 
 	cat >expect.all <<-\EOF &&
 	Merge-tag-reach (HEAD -> refs/heads/main)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	seventh (tag: refs/tags/seventh)
 	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
 	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
@@ -1139,7 +1142,7 @@ test_expect_success '--clear-decorations clears previous exclusions' '
 	cat >expect.all <<-\EOF &&
 	Merge-tag-reach (HEAD -> refs/heads/main)
 	reach (tag: refs/tags/reach, refs/heads/reach)
-	Merge-tags-octopus-a-and-octopus-b
+	Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
 	octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
 	octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
 	seventh (tag: refs/tags/seventh)
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 4b51e34f8b2..0b32e0bb8e9 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
 	git config color.decorate.ref blue &&
+	git config color.decorate.pseudoref "bold cyan" &&
 	git config color.decorate.grafted black &&
 	git config color.decorate.symbol white &&
 	git config color.decorate.HEAD cyan &&
@@ -30,6 +31,7 @@ test_expect_success setup '
 	c_tag="<BOLD;REVERSE;YELLOW>" &&
 	c_stash="<MAGENTA>" &&
 	c_ref="<BLUE>" &&
+	c_pseudoref="<BOLD;CYAN>" &&
 	c_HEAD="<CYAN>" &&
 	c_grafted="<BLACK>" &&
 	c_symbol="<WHITE>" &&
@@ -46,7 +48,10 @@ test_expect_success setup '
 	test_commit B &&
 	git tag v1.0 &&
 	echo >>A.t &&
-	git stash save Changes to A.t
+	git stash save Changes to A.t &&
+	git reset other/main &&
+	git reset ORIG_HEAD &&
+	git revert --no-commit @~
 '
 
 cmp_filtered_decorations () {
@@ -63,17 +68,19 @@ ${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
 ${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
 ${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
 ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}ORIG_HEAD${c_reset}${c_symbol}, ${c_reset}\
 ${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
 ${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
 ${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}REVERT_HEAD${c_reset}${c_symbol}, ${c_reset}\
 ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
 ${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
 	EOF
 
-	git log --first-parent --no-abbrev --decorate --clear-decorations \
-		--oneline --color=always --all >actual &&
+	git log --first-parent --no-abbrev --decorate --color=always \
+		--decorate-refs-exclude=FETCH_HEAD --oneline --all >actual &&
 	cmp_filtered_decorations
 '
 
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH 6/7] refs: exempt pseudoref patterns from prefixing
From: Andy Koppe @ 2023-10-19 19:39 UTC (permalink / raw)
  To: git; +Cc: Andy Koppe
In-Reply-To: <20231003205442.22963-1-andy.koppe@gmail.com>

In normalize_glob_ref(), don't prefix patterns that look like pseudorefs
with "refs/". This is in preparation for showing pseudorefs such as
ORIG_HEAD in log decorations, as they are not matched as intended in
decoration filters otherwise.

Use existing function is_pseudoref_syntax() to do this, declaring it in
refs.h to make it available to normalize_glob_ref() and anything else
that might need it. As the function returns true for HEAD as well, the
special case for that is no longer needed.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 refs.c | 8 ++------
 refs.h | 2 ++
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index b03cccab909..492b9f7d4e5 100644
--- a/refs.c
+++ b/refs.c
@@ -564,12 +564,8 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
 	if (prefix)
 		strbuf_addstr(&normalized_pattern, prefix);
 	else if (!starts_with(pattern, "refs/") &&
-		   strcmp(pattern, "HEAD"))
+		 !is_pseudoref_syntax(pattern))
 		strbuf_addstr(&normalized_pattern, "refs/");
-	/*
-	 * NEEDSWORK: Special case other symrefs such as REBASE_HEAD,
-	 * MERGE_HEAD, etc.
-	 */
 
 	strbuf_addstr(&normalized_pattern, pattern);
 	strbuf_strip_suffix(&normalized_pattern, "/");
@@ -840,7 +836,7 @@ int is_per_worktree_ref(const char *refname)
 	       starts_with(refname, "refs/rewritten/");
 }
 
-static int is_pseudoref_syntax(const char *refname)
+int is_pseudoref_syntax(const char *refname)
 {
 	const char *c;
 
diff --git a/refs.h b/refs.h
index 82aecd37a2b..b77691d787a 100644
--- a/refs.h
+++ b/refs.h
@@ -123,6 +123,8 @@ int should_autocreate_reflog(const char *refname);
 
 int is_branch(const char *refname);
 
+int is_pseudoref_syntax(const char *refname);
+
 int refs_init_db(struct strbuf *err);
 
 /*
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH 5/7] log: add color.decorate.ref option for other refs
From: Andy Koppe @ 2023-10-19 19:39 UTC (permalink / raw)
  To: git; +Cc: Andy Koppe
In-Reply-To: <20231003205442.22963-1-andy.koppe@gmail.com>

Refs other than those with builtin special meaning do not appear in log
decorations by default, but they can be made to appear using the
decoration filter options. However, they would do so without coloring.

Add config option color.decorate.ref to address this, defaulting to bold
magenta, which is the same as refs/stash.

To implement this, add NAMESPACE_REF with pattern "refs/", but do not
set its .include bit, so as to leave it out of the default decoration
filter.

Document the color.decorate slot on the git-config page and amend
t4207-log-decoration-colors.sh to test it.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 Documentation/config/color.txt   | 3 +++
 commit.h                         | 1 +
 log-tree.c                       | 2 ++
 refs.c                           | 7 +++++++
 refs.h                           | 1 +
 t/t4207-log-decoration-colors.sh | 9 +++++++--
 6 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index ba9f56885e3..086efebbe5d 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,9 @@ color.decorate.<slot>::
 	the stash ref
 `grafted`;;
 	grafted commits (used to implement shallow clones)
+`ref`;;
+	any other refs (not shown unless enabled with one of the decoration
+	filter options such as `--decorate-refs=<pattern>`)
 `symbol`;;
 	punctuation surrounding the other elements
 --
diff --git a/commit.h b/commit.h
index cb13e4d5baa..f6b2125fc42 100644
--- a/commit.h
+++ b/commit.h
@@ -54,6 +54,7 @@ enum decoration_type {
 	DECORATION_REF_REMOTE,
 	DECORATION_REF_TAG,
 	DECORATION_REF_STASH,
+	DECORATION_REF,
 	DECORATION_REF_HEAD,
 	DECORATION_GRAFTED,
 	DECORATION_SYMBOL,
diff --git a/log-tree.c b/log-tree.c
index 890024f205b..fd99eb88d95 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -39,6 +39,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	[DECORATION_REF_REMOTE]	= GIT_COLOR_BOLD_RED,
 	[DECORATION_REF_TAG]	= GIT_COLOR_BOLD_YELLOW,
 	[DECORATION_REF_STASH]	= GIT_COLOR_BOLD_MAGENTA,
+	[DECORATION_REF]	= GIT_COLOR_BOLD_MAGENTA,
 	[DECORATION_REF_HEAD]	= GIT_COLOR_BOLD_CYAN,
 	[DECORATION_GRAFTED]	= GIT_COLOR_BOLD_BLUE,
 	[DECORATION_SYMBOL]	= GIT_COLOR_NIL,
@@ -49,6 +50,7 @@ static const char *color_decorate_slots[] = {
 	[DECORATION_REF_REMOTE] = "remoteBranch",
 	[DECORATION_REF_TAG]	= "tag",
 	[DECORATION_REF_STASH]	= "stash",
+	[DECORATION_REF]	= "ref",
 	[DECORATION_REF_HEAD]	= "HEAD",
 	[DECORATION_GRAFTED]	= "grafted",
 	[DECORATION_SYMBOL]	= "symbol",
diff --git a/refs.c b/refs.c
index 416c35f0c2f..b03cccab909 100644
--- a/refs.c
+++ b/refs.c
@@ -142,6 +142,13 @@ struct ref_namespace_info ref_namespace[] = {
 		 */
 		.ref = "refs/rewritten/",
 	},
+	[NAMESPACE_REFS] = {
+		/*
+		 * Catch-all for any other refs.
+		 */
+		.ref = "refs/",
+		.decoration = DECORATION_REF,
+	},
 };
 
 void update_ref_namespace(enum ref_namespace namespace, char *ref)
diff --git a/refs.h b/refs.h
index 4b054d30fe5..82aecd37a2b 100644
--- a/refs.h
+++ b/refs.h
@@ -1008,6 +1008,7 @@ enum ref_namespace {
 	NAMESPACE_NOTES,
 	NAMESPACE_PREFETCH,
 	NAMESPACE_REWRITTEN,
+	NAMESPACE_REFS,
 
 	/* Must be last */
 	NAMESPACE__COUNT
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f4173b61141..4b51e34f8b2 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -17,6 +17,7 @@ test_expect_success setup '
 	git config color.decorate.remoteBranch red &&
 	git config color.decorate.tag "reverse bold yellow" &&
 	git config color.decorate.stash magenta &&
+	git config color.decorate.ref blue &&
 	git config color.decorate.grafted black &&
 	git config color.decorate.symbol white &&
 	git config color.decorate.HEAD cyan &&
@@ -28,11 +29,13 @@ test_expect_success setup '
 	c_remoteBranch="<RED>" &&
 	c_tag="<BOLD;REVERSE;YELLOW>" &&
 	c_stash="<MAGENTA>" &&
+	c_ref="<BLUE>" &&
 	c_HEAD="<CYAN>" &&
 	c_grafted="<BLACK>" &&
 	c_symbol="<WHITE>" &&
 
 	test_commit A &&
+	git update-ref refs/foo A &&
 	git clone . other &&
 	(
 		cd other &&
@@ -65,10 +68,12 @@ ${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
 ${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
 	${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
+${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
 	EOF
 
-	git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
+	git log --first-parent --no-abbrev --decorate --clear-decorations \
+		--oneline --color=always --all >actual &&
 	cmp_filtered_decorations
 '
 
-- 
2.42.GIT


^ permalink raw reply related

* [PATCH 4/7] refs: separate decoration type from default filter
From: Andy Koppe @ 2023-10-19 19:39 UTC (permalink / raw)
  To: git; +Cc: Andy Koppe
In-Reply-To: <20231003205442.22963-1-andy.koppe@gmail.com>

Add 'include' bit to struct ref_namespace_info to determine whether a
ref namespace is to be included in the default decoration filters,
instead of using the decoration type for the purpose.

This is to allow adding ref namespaces that do have a decoration type
but that are not shown by default.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
 builtin/log.c | 6 ++----
 refs.c        | 6 ++++++
 refs.h        | 4 ++++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index ba775d7b5cf..25d73c25697 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -227,10 +227,8 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f
 	 * populate with sensible defaults.
 	 */
 	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) {
-		if (!ref_namespace[i].decoration)
-			continue;
-
-		string_list_append(include, ref_namespace[i].ref);
+		if (ref_namespace[i].include)
+			string_list_append(include, ref_namespace[i].ref);
 	}
 }
 
diff --git a/refs.c b/refs.c
index fcae5dddc60..416c35f0c2f 100644
--- a/refs.c
+++ b/refs.c
@@ -70,14 +70,17 @@ struct ref_namespace_info ref_namespace[] = {
 		.ref = "HEAD",
 		.decoration = DECORATION_REF_HEAD,
 		.exact = 1,
+		.include = 1,
 	},
 	[NAMESPACE_BRANCHES] = {
 		.ref = "refs/heads/",
 		.decoration = DECORATION_REF_LOCAL,
+		.include = 1,
 	},
 	[NAMESPACE_TAGS] = {
 		.ref = "refs/tags/",
 		.decoration = DECORATION_REF_TAG,
+		.include = 1,
 	},
 	[NAMESPACE_REMOTE_REFS] = {
 		/*
@@ -87,6 +90,7 @@ struct ref_namespace_info ref_namespace[] = {
 		 */
 		.ref = "refs/remotes/",
 		.decoration = DECORATION_REF_REMOTE,
+		.include = 1,
 	},
 	[NAMESPACE_STASH] = {
 		/*
@@ -96,6 +100,7 @@ struct ref_namespace_info ref_namespace[] = {
 		.ref = "refs/stash",
 		.exact = 1,
 		.decoration = DECORATION_REF_STASH,
+		.include = 1,
 	},
 	[NAMESPACE_REPLACE] = {
 		/*
@@ -107,6 +112,7 @@ struct ref_namespace_info ref_namespace[] = {
 		 */
 		.ref = "refs/replace/",
 		.decoration = DECORATION_GRAFTED,
+		.include = 1,
 	},
 	[NAMESPACE_NOTES] = {
 		/*
diff --git a/refs.h b/refs.h
index 23211a5ea1c..4b054d30fe5 100644
--- a/refs.h
+++ b/refs.h
@@ -987,10 +987,14 @@ struct ref_namespace_info {
 	 * If 'exact' is true, then we must match the 'ref' exactly.
 	 * Otherwise, use a prefix match.
 	 *
+	 * If 'include' is true, the namespace is included in the
+	 * default decoration filters.
+	 *
 	 * 'ref_updated' is for internal use. It represents whether the
 	 * 'ref' value was replaced from its original literal version.
 	 */
 	unsigned exact:1,
+		 include:1,
 		 ref_updated:1;
 };
 
-- 
2.42.GIT


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox