* Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"
From: Uwe Kleine-König @ 2023-10-20 10:04 UTC (permalink / raw)
To: git; +Cc: Luben Tuikov, Michael Strawbridge, entwicklung
In-Reply-To: <20231013141437.ywrhw65xdapmev7d@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]
hello,
On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> $ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
> Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
> error: unable to extract a valid address from:
>
> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
>
> Bisection points at
>
> a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
>
> I didn't try to understand that change and fix the problem.
Another (similar?) problem with non-ascii-chars:
$ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
error: unable to extract a valid address from:
Bisection points to the same commit, when dropping ń in Krzysztof's
name, it works fine.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH 0/3] some send-email --compose fixes
From: Jeff King @ 2023-10-20 10:03 UTC (permalink / raw)
To: Michael Strawbridge; +Cc: Junio C Hamano, Bagas Sanjaya, Git Mailing List
In-Reply-To: <20231020071402.GC1642714@coredump.intra.peff.net>
[culling the rather large cc, as we moving off the original topic]
On Fri, Oct 20, 2023 at 03:14:03AM -0400, Jeff King wrote:
> 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):
So here's the fix in a cleaned up form, guided by my own comments from
earlier. ;) I think this is actually all orthogonal to the patch you are
working on, so yours could either go on top or just be applied
separately.
[1/3]: doc/send-email: mention handling of "reply-to" with --compose
[2/3]: Revert "send-email: extract email-parsing code into a subroutine"
[3/3]: send-email: handle to/cc/bcc from --compose message
Documentation/git-send-email.txt | 10 +--
git-send-email.perl | 132 ++++++++++++-------------------
t/t9001-send-email.sh | 41 ++++++++++
3 files changed, 98 insertions(+), 85 deletions(-)
-Peff
^ permalink raw reply
* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Jeff King @ 2023-10-20 10:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git, Karthik Nayak, Taylor Blau
In-Reply-To: <xmqqzg0ey1kn.fsf@gitster.g>
On Thu, Oct 19, 2023 at 10:16:56AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > There's another way to handle this, which is to conditionally enable the
> > object existence check. This would be less of a performance hit compared
> > to disabling commit graphs altogether with `--missing`, but still ensure
> > that repository corruption was detected. Second, it would not regress
> > performance for all preexisting users of `repo_parse_commit_gently()`.
>
> The above was what I meant to suggest when you demonstrated that the
> code with additional check is still much more performant than
> running without the commit-graph optimization, yet has observable
> impact on performance for normal codepaths that do not need the
> extra carefulness.
>
> But I wasn't sure if it is easy to plumb the "do we want to double
> check? in other words, are we running something like --missing that
> care the correctness a bit more than usual cases?" bit down from the
> caller, because this check is so deep in the callchain.
I wonder if we would want a "be extra careful" flag that is read from
the environment? That is largely how GIT_REF_PARANOIA works, and then
particular operations set it (though actually it is the default these
days, so they no longer do so explicitly).
I guess that is really like a global variable but even more gross. ;)
But it is nice that it can cross process boundaries, because "how
careful do we want to be" may be in the eye of the caller, especially
for plumbing commands. E.g., even without --missing, you may want
"rev-list" to be extra careful about checking the existence of commits.
The caller in check_connected() could arguably turn that on by default
(the way we used to turn on GIT_REF_PARANOIA for pruning repacks before
it became the default).
-Peff
^ permalink raw reply
* Re: [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Christian Couder @ 2023-10-20 9:41 UTC (permalink / raw)
To: Isoken Ibizugbe; +Cc: git, gitster
In-Reply-To: <CAJHH8bFXVnFgjoCD+JU2uw77JAWUiKU+G=ub9Xg7xYJ_MwXPQQ@mail.gmail.com>
Hi Isoken,
On Fri, Oct 20, 2023 at 6:28 AM Isoken Ibizugbe <isokenjune@gmail.com> wrote:
>
> 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.
Please don't expect Junio to give direct feedback on all the patches
sent to the mailing list. This patch has already been reviewed by
Rubén. If Junio doesn't review it and if this patch is not mentioned
in the next "What's cooking in git.git" email from Junio, it likely
means that you should send a version 3 addressing Rubén's feedback
about the commit message.
Thanks,
Christian.
^ permalink raw reply
* [PATCH v3 1/3] rebase: simplify code related to imply_merge()
From: Oswald Buddenhagen @ 2023-10-20 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood
In-Reply-To: <20231020093654.922890-1-oswald.buddenhagen@gmx.de>
The code's evolution left in some bits surrounding enum rebase_type that
don't really make sense any more. In particular, it makes no sense to
invoke imply_merge() if the type is already known not to be
REBASE_APPLY, and it makes no sense to assign the type after calling
imply_merge().
enum rebase_type had more values until commit a74b35081c ("rebase: drop
support for `--preserve-merges`") and commit 10cdb9f38a ("rebase: rename
the two primary rebase backends"). The latter commit also renamed
imply_interactive() to imply_merge().
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v2:
- more verbose commit message
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>
---
builtin/rebase.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 50cb85751f..44cc1eed12 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -386,7 +386,6 @@ static int parse_opt_keep_empty(const struct option *opt, const char *arg,
imply_merge(opts, unset ? "--no-keep-empty" : "--keep-empty");
opts->keep_empty = !unset;
- opts->type = REBASE_MERGE;
return 0;
}
@@ -1505,9 +1504,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
}
}
- if (options.type == REBASE_MERGE)
- imply_merge(&options, "--merge");
-
if (options.root && !options.onto_name)
imply_merge(&options, "--root without --onto");
@@ -1552,7 +1548,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
- imply_merge(&options, "--merge");
+ options.type = REBASE_MERGE;
else if (!strcmp(options.default_backend, "apply"))
options.type = REBASE_APPLY;
else
--
2.42.0.419.g70bf8a5751
^ permalink raw reply related
* [PATCH v3 3/3] rebase: move parse_opt_keep_empty() down
From: Oswald Buddenhagen @ 2023-10-20 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood
In-Reply-To: <20231020093654.922890-1-oswald.buddenhagen@gmx.de>
This moves it right next to parse_opt_empty(), which is a much more
logical place. As a side effect, this removes the need for a forward
declaration of imply_merge().
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>
---
builtin/rebase.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4a093bb125..13ca5a644b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -376,19 +376,6 @@ static int run_sequencer_rebase(struct rebase_options *opts)
return ret;
}
-static void imply_merge(struct rebase_options *opts, const char *option);
-static int parse_opt_keep_empty(const struct option *opt, const char *arg,
- int unset)
-{
- struct rebase_options *opts = opt->value;
-
- BUG_ON_OPT_ARG(arg);
-
- imply_merge(opts, unset ? "--no-keep-empty" : "--keep-empty");
- opts->keep_empty = !unset;
- return 0;
-}
-
static int is_merge(struct rebase_options *opts)
{
return opts->type == REBASE_MERGE;
@@ -982,6 +969,18 @@ static enum empty_type parse_empty_value(const char *value)
die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"ask\"."), value);
}
+static int parse_opt_keep_empty(const struct option *opt, const char *arg,
+ int unset)
+{
+ struct rebase_options *opts = opt->value;
+
+ BUG_ON_OPT_ARG(arg);
+
+ imply_merge(opts, unset ? "--no-keep-empty" : "--keep-empty");
+ opts->keep_empty = !unset;
+ return 0;
+}
+
static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
{
struct rebase_options *options = opt->value;
--
2.42.0.419.g70bf8a5751
^ permalink raw reply related
* [PATCH v3 2/3] rebase: handle --strategy via imply_merge() as well
From: Oswald Buddenhagen @ 2023-10-20 9:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood
In-Reply-To: <20231020093654.922890-1-oswald.buddenhagen@gmx.de>
At least after the successive trimming of enum rebase_type mentioned in
the previous commit, this code did exactly what imply_merge() does, so
just call it instead.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
Cc: Phillip Wood <phillip.wood123@gmail.com>
---
builtin/rebase.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 44cc1eed12..4a093bb125 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1490,18 +1490,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.strategy) {
options.strategy = xstrdup(options.strategy);
- switch (options.type) {
- case REBASE_APPLY:
- die(_("--strategy requires --merge or --interactive"));
- case REBASE_MERGE:
- /* compatible */
- break;
- case REBASE_UNSPECIFIED:
- options.type = REBASE_MERGE;
- break;
- default:
- BUG("unhandled rebase type (%d)", options.type);
- }
+ imply_merge(&options, "--strategy");
}
if (options.root && !options.onto_name)
--
2.42.0.419.g70bf8a5751
^ permalink raw reply related
* [PATCH v3 0/3] rebase refactoring
From: Oswald Buddenhagen @ 2023-10-20 9:36 UTC (permalink / raw)
To: git
In-Reply-To: <20230809171531.2564844-1-oswald.buddenhagen@gmx.de>
broken out of the bigger series, as the aggregation just unnecessarily holds it
up.
v3: removed "stray" footer. so more of a RESEND than an actual new version.
Oswald Buddenhagen (3):
rebase: simplify code related to imply_merge()
rebase: handle --strategy via imply_merge() as well
rebase: move parse_opt_keep_empty() down
builtin/rebase.c | 44 ++++++++++++++------------------------------
1 file changed, 14 insertions(+), 30 deletions(-)
--
2.42.0.419.g70bf8a5751
^ permalink raw reply
* [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
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