* [PATCH 1/2] strbuf_commented_addf(): drop the comment_line_char parameter
From: Junio C Hamano @ 2023-10-30 5:10 UTC (permalink / raw)
To: git
In-Reply-To: <20231030051034.2295242-1-gitster@pobox.com>
All the callers of this function supply the global variable
comment_line_char as an argument to its second parameter. Remove
the parameter to allow us in the future to change the reference to
the global variable with something else, like a function call.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
add-patch.c | 8 ++++----
builtin/branch.c | 2 +-
builtin/merge.c | 8 ++++----
builtin/tag.c | 4 ++--
rebase-interactive.c | 2 +-
sequencer.c | 4 ++--
strbuf.c | 3 ++-
strbuf.h | 4 ++--
wt-status.c | 2 +-
9 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index bfe19876cd..471a0037be 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1106,11 +1106,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
size_t i;
strbuf_reset(&s->buf);
- strbuf_commented_addf(&s->buf, comment_line_char,
+ strbuf_commented_addf(&s->buf,
_("Manual hunk edit mode -- see bottom for "
"a quick guide.\n"));
render_hunk(s, hunk, 0, 0, &s->buf);
- strbuf_commented_addf(&s->buf, comment_line_char,
+ strbuf_commented_addf(&s->buf,
_("---\n"
"To remove '%c' lines, make them ' ' lines "
"(context).\n"
@@ -1119,13 +1119,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
s->mode->is_reverse ? '+' : '-',
s->mode->is_reverse ? '-' : '+',
comment_line_char);
- strbuf_commented_addf(&s->buf, comment_line_char, "%s",
+ strbuf_commented_addf(&s->buf, "%s",
_(s->mode->edit_hunk_hint));
/*
* TRANSLATORS: 'it' refers to the patch mentioned in the previous
* messages.
*/
- strbuf_commented_addf(&s->buf, comment_line_char,
+ strbuf_commented_addf(&s->buf,
_("If it does not apply cleanly, you will be "
"given an opportunity to\n"
"edit again. If all lines of the hunk are "
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..b2f171e10b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -668,7 +668,7 @@ static int edit_branch_description(const char *branch_name)
exists = !read_branch_desc(&buf, branch_name);
if (!buf.len || buf.buf[buf.len-1] != '\n')
strbuf_addch(&buf, '\n');
- strbuf_commented_addf(&buf, comment_line_char,
+ strbuf_commented_addf(&buf,
_("Please edit the description for the branch\n"
" %s\n"
"Lines starting with '%c' will be stripped.\n"),
diff --git a/builtin/merge.c b/builtin/merge.c
index d748d46e13..8f0e8be7c3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -857,15 +857,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)
strbuf_addch(&msg, '\n');
if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
wt_status_append_cut_line(&msg);
- strbuf_commented_addf(&msg, comment_line_char, "\n");
+ strbuf_commented_addf(&msg, "\n");
}
- strbuf_commented_addf(&msg, comment_line_char,
+ strbuf_commented_addf(&msg,
_(merge_editor_comment));
if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
- strbuf_commented_addf(&msg, comment_line_char,
+ strbuf_commented_addf(&msg,
_(scissors_editor_comment));
else
- strbuf_commented_addf(&msg, comment_line_char,
+ strbuf_commented_addf(&msg,
_(no_scissors_editor_comment), comment_line_char);
}
if (signoff)
diff --git a/builtin/tag.c b/builtin/tag.c
index 3918eacbb5..a85a0d8def 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -314,10 +314,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,
struct strbuf buf = STRBUF_INIT;
strbuf_addch(&buf, '\n');
if (opt->cleanup_mode == CLEANUP_ALL)
- strbuf_commented_addf(&buf, comment_line_char,
+ strbuf_commented_addf(&buf,
_(tag_template), tag, comment_line_char);
else
- strbuf_commented_addf(&buf, comment_line_char,
+ strbuf_commented_addf(&buf,
_(tag_template_nocleanup), tag, comment_line_char);
write_or_die(fd, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index d9718409b3..3f33da7f03 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -71,7 +71,7 @@ void append_todo_help(int command_count,
if (!edit_todo) {
strbuf_addch(buf, '\n');
- strbuf_commented_addf(buf, comment_line_char,
+ strbuf_commented_addf(buf,
Q_("Rebase %s onto %s (%d command)",
"Rebase %s onto %s (%d commands)",
command_count),
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed..5d348a3f12 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -675,11 +675,11 @@ void append_conflicts_hint(struct index_state *istate,
}
strbuf_addch(msgbuf, '\n');
- strbuf_commented_addf(msgbuf, comment_line_char, "Conflicts:\n");
+ strbuf_commented_addf(msgbuf, "Conflicts:\n");
for (i = 0; i < istate->cache_nr;) {
const struct cache_entry *ce = istate->cache[i++];
if (ce_stage(ce)) {
- strbuf_commented_addf(msgbuf, comment_line_char,
+ strbuf_commented_addf(msgbuf,
"\t%s\n", ce->name);
while (i < istate->cache_nr &&
!strcmp(ce->name, istate->cache[i]->name))
diff --git a/strbuf.c b/strbuf.c
index 7827178d8e..15550b2619 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,4 +1,5 @@
#include "git-compat-util.h"
+#include "environment.h"
#include "gettext.h"
#include "hex-ll.h"
#include "strbuf.h"
@@ -372,7 +373,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
add_lines(out, prefix1, prefix2, buf, size);
}
-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,
+void strbuf_commented_addf(struct strbuf *sb,
const char *fmt, ...)
{
va_list params;
diff --git a/strbuf.h b/strbuf.h
index e959caca87..981617dc77 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -378,8 +378,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
* Add a formatted string prepended by a comment character and a
* blank to the buffer.
*/
-__attribute__((format (printf, 3, 4)))
-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...);
+__attribute__((format (printf, 2, 3)))
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
__attribute__((format (printf,2,0)))
void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..54b2775730 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1102,7 +1102,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
{
const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
- strbuf_commented_addf(buf, comment_line_char, "%s", cut_line);
+ strbuf_commented_addf(buf, "%s", cut_line);
strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);
}
--
2.42.0-526-g3130c155df
^ permalink raw reply related
* [PATCH 0/2] Avoid passing global comment_line_char repeatedly
From: Junio C Hamano @ 2023-10-30 5:10 UTC (permalink / raw)
To: git
Two strbuf functions used to produce commented lines take the
comment_line_char as their parameter, but in practice, all callers
feed the global variable comment_line_char from environment.[ch].
Dropping the parameter from the callchain will make the interface
less flexible, and less error prone. If we choose to change the
implementation of the customizable comment line character (e.g., we
may want to stop referencing the global variable and instead use a
getter function), we will have fewer places we need to modify.
Junio C Hamano (2):
strbuf_commented_addf(): drop the comment_line_char parameter
strbuf_add_commented_lines(): drop the comment_line_char parameter
add-patch.c | 8 ++++----
builtin/branch.c | 2 +-
builtin/merge.c | 8 ++++----
builtin/notes.c | 9 ++++-----
builtin/stripspace.c | 2 +-
builtin/tag.c | 4 ++--
fmt-merge-msg.c | 9 +++------
rebase-interactive.c | 8 ++++----
sequencer.c | 14 ++++++--------
strbuf.c | 9 +++++----
strbuf.h | 7 +++----
wt-status.c | 6 +++---
12 files changed, 40 insertions(+), 46 deletions(-)
--
2.42.0-526-g3130c155df
^ permalink raw reply
* Re: [PATCH] sequencer: remove use of comment character
From: Junio C Hamano @ 2023-10-30 4:00 UTC (permalink / raw)
To: Tony Tung via GitGitGadget; +Cc: git, Tony Tung
In-Reply-To: <pull.1603.git.1698635292629.gitgitgadget@gmail.com>
"Tony Tung via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Tony Tung <tonytung@merly.org>
>
> Instead of using the hardcoded `# `, use the
> user-defined comment_line_char. Adds a test
> to prevent regressions.
Good spotting.
Two observations.
(1) There are a few more places that need similar treatment in the
same file; you may want to fix them all while at it.
(2) The second argument to strbuf_commented_addf() is always the
comment_line_char global variable, not just inside this file
but all callers across the codebase. We probably should drop
it and have the strbuf_commented_addf() helper itself refer to
the global. That way, if we ever want to change the global
variable reference to something else (e.g. function call), we
only have to touch a single place.
The latter is meant as #leftoverbits and will be a lot wider
clean-up that we may want to do long after this patch hits out
codebase. The "other places" I spotted for the former are the
following, but needs to be taken with a huge grain of salt, as it
has not even been compile tested.
Thanks.
sequencer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git c/sequencer.c w/sequencer.c
index d584cac8ed..33208b1660 100644
--- c/sequencer.c
+++ w/sequencer.c
@@ -1893,8 +1893,8 @@ static void update_squash_message_for_fixup(struct strbuf *msg)
size_t orig_msg_len;
int i = 1;
- strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str));
- strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str));
+ strbuf_addf(&buf1, comment_line_char, "%s\n", _(first_commit_msg_str));
+ strbuf_addf(&buf2, comment_line_char, "%s\n", _(skip_first_commit_msg_str));
s = start = orig_msg = strbuf_detach(msg, &orig_msg_len);
while (s) {
const char *next;
@@ -2269,8 +2269,8 @@ static int do_pick_commit(struct repository *r,
next = parent;
next_label = msg.parent_label;
if (opts->commit_use_reference) {
- strbuf_addstr(&msgbuf,
- "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
+ strbuf_commented_addf(&msgbuf, comment_line_char, "%s",
+ "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
} else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
/*
* We don't touch pre-existing repeated reverts, because
^ permalink raw reply related
* [PATCH] sequencer: remove use of comment character
From: Tony Tung via GitGitGadget @ 2023-10-30 3:08 UTC (permalink / raw)
To: git; +Cc: Tony Tung, Tony Tung
From: Tony Tung <tonytung@merly.org>
Instead of using the hardcoded `# `, use the
user-defined comment_line_char. Adds a test
to prevent regressions.
Signed-off-by: Tony Tung <tonytung@merly.org>
---
sequencer: remove use of hardcoded comment char
Instead of using the hardcoded # , use the user-defined
comment_line_char. Adds a test to prevent regressions.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1603%2Fttung%2Fttung%2Fcommentchar-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1603/ttung/ttung/commentchar-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1603
sequencer.c | 5 +++--
t/t3404-rebase-interactive.sh | 39 +++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed9..8c6666d5e43 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6082,8 +6082,9 @@ static int add_decorations_to_list(const struct commit *commit,
/* If the branch is checked out, then leave a comment instead. */
if ((path = branch_checked_out(decoration->name))) {
item->command = TODO_COMMENT;
- strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n",
- decoration->name, path);
+ strbuf_commented_addf(ctx->buf, comment_line_char,
+ "Ref %s checked out at '%s'\n",
+ decoration->name, path);
} else {
struct string_list_item *sti;
item->command = TODO_UPDATE_REF;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8ea2bf13026..076dca87871 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1839,6 +1839,45 @@ test_expect_success '--update-refs adds label and update-ref commands' '
)
'
+test_expect_success '--update-refs works with core.commentChar' '
+ git checkout -b update-refs-with-commentchar no-conflict-branch &&
+ test_config core.commentChar : &&
+ git branch -f base HEAD~4 &&
+ git branch -f first HEAD~3 &&
+ git branch -f second HEAD~3 &&
+ git branch -f third HEAD~1 &&
+ git commit --allow-empty --fixup=third &&
+ git branch -f is-not-reordered &&
+ git commit --allow-empty --fixup=HEAD~4 &&
+ git branch -f shared-tip &&
+ git checkout update-refs &&
+ (
+ write_script fake-editor.sh <<-\EOF &&
+ grep "^[^:]" "$1"
+ exit 1
+ EOF
+ test_set_editor "$(pwd)/fake-editor.sh" &&
+
+ cat >expect <<-EOF &&
+ pick $(git log -1 --format=%h J) J
+ fixup $(git log -1 --format=%h update-refs) fixup! J : empty
+ update-ref refs/heads/second
+ update-ref refs/heads/first
+ pick $(git log -1 --format=%h K) K
+ pick $(git log -1 --format=%h L) L
+ fixup $(git log -1 --format=%h is-not-reordered) fixup! L : empty
+ update-ref refs/heads/third
+ pick $(git log -1 --format=%h M) M
+ update-ref refs/heads/no-conflict-branch
+ update-ref refs/heads/is-not-reordered
+ update-ref refs/heads/update-refs-with-commentchar
+ EOF
+
+ test_must_fail git rebase -i --autosquash --update-refs primary shared-tip >todo &&
+ test_cmp expect todo
+ )
+'
+
test_expect_success '--update-refs adds commands with --rebase-merges' '
git checkout -b update-refs-with-merge no-conflict-branch &&
git branch -f base HEAD~4 &&
base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3] bugreport: reject positional arguments
From: Junio C Hamano @ 2023-10-30 1:59 UTC (permalink / raw)
To: Phillip Wood
Cc: Eric Sunshine, emilyshaffer, git, Emily Shaffer, Sheik,
Dragan Simic
In-Reply-To: <xmqqpm0xeyp9.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>> It is rather unfortunate that test_i18ngrep was deprecated without
>>> providing an alternative that offers the same debugging
>>> experience.
>> ...
>> We could rename test_i18ngrep to test_grep (and make test_i18ngrep
>> into a thin wrapper with warnings).
>>
>> test_grep -e must-exist file &&
>> test_grep ! -e must-not-exist file
>
> ... as the only remaining part in test_18ngrep has no hack to work
> around the tainted localization tests, so "was deprecated without"
> is a bit too strong. There is nothing we have lost yet.
Having said all that, when re-reading the test_i18ngrep with a fresh
pair of eyes, I somehow doubt there was much upside in "debugging
experience" with test_i18ngrep in the first place, and I doubt if
retaining it with a new name test_grep has much value.
Given that test_i18ngrep (hence test_grep) requires you to have the
haystack in a file, between
test_i18ngrep must-exist file &&
test_i18ngrep ! must-not-exist file
and
grep must-exist file &&
! grep must-not-exist file
I do not see any difference in "debugging experience" when you run
the test with "-i [-v] -d". The two cases you care about are
(1) the test expects the string "must-exist" in the file "file" but
the string is not there.
(2) the test expects the string "must-not-exist" missing from the
file "file", but the string is there.
The latter can clearly be seen in output from "-i -v -d" (the "grep"
outputs a line with "must-not-exist" on it). The former will show
silence but since you are debugging with "-d", and your haystack is
in a file, after such a step fails, the test stops, and without
removing the "file" even if the test piece had test_when_finished
to remove it (i.e. running tests in debugging mode "-d" and
immediately stopping upon failure "-i" behaves this way exactly to
help you debugging), so you can go there to the TRASH_DIRECTORY
yourself and inspect "file" to see what is going on anyway.
So, I dunno. Surely with a long &&-chain of steps, where a grep
that expects lack of something is in the middle, it is hard to see
if the lack of hit is because an earlier step failed (and the
control did not reach "grep must-exist file") or because the
haystack lacked the "must-exist" needle, so from that point of view,
it may be nicer that "did not find an expected match" is explicitly
stated.
^ permalink raw reply
* Re: [RFC PATCH v2 1/6] status: add noob format from status.noob config
From: Dragan Simic @ 2023-10-30 1:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Stopak, git
In-Reply-To: <xmqqjzr4gaie.fsf@gitster.g>
On 2023-10-30 02:32, Junio C Hamano wrote:
> Jacob Stopak <jacob@initialcommit.io> writes:
>> diff --git a/table.h b/table.h
>> new file mode 100644
>> index 0000000000..c9e8c386de
>> --- /dev/null
>> +++ b/table.h
>> @@ -0,0 +1,6 @@
>> +#ifndef TABLE_H
>> +#define TABLE_H
>> +
>> +void print_noob_status(struct wt_status *s);
>> +
>> +#endif /* TABLE_H */
>
> I am guessing that your plan is to add other "distim_noob_add()" and
> other "noob" variant of operations for various Git subcommands here,
> but I really do not think you want to add table.[ch] that has logic
> for such random set of Git subcommands copied and tweaked from all
> over the place, as the only trait being shared among them will
> become "they are written by Jacob Stopak", that is not a very useful
> grouping of the functions. It is not even "this file collects all
> the code that produce tabular output from Git"---"git status -s"
> already gives tabular output, for example, without using any of the
> "I only want to draw a table with three columns of equal width"
> logic. Adding code that are necessary to add yet another output
> mode for "git status" directly to where various output modes of "git
> status" are implemented, i.e. wt-status.c, and do similar changes for
> each command would make more sense, I would think.
Furthermore, "extended" should perhaps be used instead of "noob"
throughout, to reflect the planned naming of the configuration option
values.
^ permalink raw reply
* Re: [RFC PATCH v2 1/6] status: add noob format from status.noob config
From: Junio C Hamano @ 2023-10-30 1:32 UTC (permalink / raw)
To: Jacob Stopak; +Cc: git
In-Reply-To: <20231026224615.675172-2-jacob@initialcommit.io>
Jacob Stopak <jacob@initialcommit.io> writes:
> diff --git a/table.c b/table.c
> new file mode 100644
> index 0000000000..15600e117f
> --- /dev/null
> +++ b/table.c
Yuck, do we need an entirely new file? What trait are the things
that are thrown into this file together supposed to share [*]? It
is not very clear to me what the focus of this file is.
Side note: for example, stuff in wt-status.c are to compute
per-path status of the working tree and in-index files.
> @@ -0,0 +1,117 @@
> +#define USE_THE_INDEX_VARIABLE
I personally do not mind, but I suspect many people hate to see this
compatibility set of macros used in a newly written source file.
> +static const char *color(int slot, struct wt_status *s)
> +{
> + const char *c = "";
> + if (want_color(s->use_color))
> + c = s->color_palette[slot];
> + if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
> + c = s->color_palette[WT_STATUS_HEADER];
> + return c;
> +}
Do we need to duplicate this from other files? If this is about
"git status", perhaps some parts of this patch, the truly new things
(rather than what was copied, like this one) can be added to
wt-status.c instead of adding a new file with unclear focus?
> +static void build_table_border(struct strbuf *buf, int cols)
> +{
> + strbuf_reset(buf);
> + strbuf_addchars(buf, '-', cols);
> +}
This seems to be horizontal border; do we need a separate vertical
border?
> +static void build_table_entry(struct strbuf *buf, char *entry, int cols)
> +{
> + strbuf_reset(buf);
> + strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> + strbuf_addstr(buf, entry);
> +
> + /* Bump right padding if entry length is odd */
> + if (!(strlen(entry) % 2))
> + strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
> + else
> + strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
> +}
The code assumes that one byte in string "entry" occupies one and
only one display columns, which is so 20th centry assumption that
does not care about i18n. Often what takes 3 bytes in a UTF-8
string occupies 2 display columns, for example. In addition, if you
plan to color entries in the table, some substring would end up to
be 0-width. Your pathname may be so long that 1/3 of a window
width may not be sufficient to show it in its entirety, you might
need to show it truncated in the middle.
utf8.c has support for measuring the display width of UTF-8 string,
which is used elsewhere in our code. You may want to study it if
you want to do a "tabular" output. The code in diff.c that shows
diffstat has many gems to help what this code wants to do, including
measuring display columns of a string, chomping a long string to fit
in a desired display columns, etc., by using helpers defined in
utf8.c
A potential excuse I can think of to have these outside wt-status.c
and in a separate new file is to have a generic "table" layout
machinery that is independent from "git status" or what each column
of the table is showing (in other words, they may not be pathnames),
and reusable by other subcommands that want to show things in the
"table" layout. But even as a candidate for such a generic table
mechanism, the above falls far short by hardcoding that it can only
show 3-col table whose columns are evenly distributed and nothing
else.
> +static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
> +{
> + printf(_("|"));
> + color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
> + printf(_("|"));
> + color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
> + printf(_("|"));
> + color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
> + printf(_("|\n"));
> +}
How does the code deal with unknown display width of translated
version of "|" emitted here? Are you assuming that no matter how
these are translated, they will always occupy one display column
each?
> +void print_noob_status(struct wt_status *s)
> +{
> + struct winsize w;
> + int cols;
> + struct strbuf table_border = STRBUF_INIT;
> + struct strbuf table_col_entry_1 = STRBUF_INIT;
> + struct strbuf table_col_entry_2 = STRBUF_INIT;
> + struct strbuf table_col_entry_3 = STRBUF_INIT;
> + struct string_list_item *item;
> +
> + /* Get terminal width */
> + ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
> + cols = w.ws_col;
Let's not reinvent an incomplete solution before studying and
finding out what we already have in our codebase. Immediately after
you got tempted to type TIOCGWINSZ, you can "git grep" the codebase
for that particular constant to see if we already use it, as that is
one very reasonable way to achieve what this piece of code wants to
do (i.e. find out what the display width would be). You'd find
pager.c:term_columns() and also learned that we want to prepare for
the case where the ioctl() is not available.
> + build_table_entry(&table_col_entry_1, "Untracked files", cols);
> + build_table_entry(&table_col_entry_2, "Unstaged changes", cols);
> + build_table_entry(&table_col_entry_3, "Staging area", cols);
Shouldn't these three strings be translatable?
What shoudl happen when these labels are wider than cols/3?
> diff --git a/table.h b/table.h
> new file mode 100644
> index 0000000000..c9e8c386de
> --- /dev/null
> +++ b/table.h
> @@ -0,0 +1,6 @@
> +#ifndef TABLE_H
> +#define TABLE_H
> +
> +void print_noob_status(struct wt_status *s);
> +
> +#endif /* TABLE_H */
I am guessing that your plan is to add other "distim_noob_add()" and
other "noob" variant of operations for various Git subcommands here,
but I really do not think you want to add table.[ch] that has logic
for such random set of Git subcommands copied and tweaked from all
over the place, as the only trait being shared among them will
become "they are written by Jacob Stopak", that is not a very useful
grouping of the functions. It is not even "this file collects all
the code that produce tabular output from Git"---"git status -s"
already gives tabular output, for example, without using any of the
"I only want to draw a table with three columns of equal width"
logic. Adding code that are necessary to add yet another output
mode for "git status" directly to where various output modes of "git
status" are implemented, i.e. wt-status.c, and do similar changes for
each command would make more sense, I would think.
Thanks.
^ permalink raw reply
* Re: [PATCH v3] bugreport: reject positional arguments
From: Junio C Hamano @ 2023-10-30 0:33 UTC (permalink / raw)
To: Phillip Wood
Cc: Eric Sunshine, emilyshaffer, git, Emily Shaffer, Sheik,
Dragan Simic
In-Reply-To: <xmqqv8apez0o.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> It is rather unfortunate that test_i18ngrep was deprecated without
>> providing an alternative that offers the same debugging
>> experience.
>
> The primary thing test_i18ngrep did was to _SKIP_ certain test that
> looked for an expected string in "C" locale from the output when the
> tests were run under a tainted localization mode. The tests that
> looked for strings in messages that are *not* to be localized used
> "grep". Tests that (unfortunately) had to match human-readable
> messages had to work around the tainted localization test to use
> test_i18ngrep.
>
>> When test_i18ngrep fails it prints a message with the
>> pattern and text that failed to match so it is easy to see where the
>> test failed. If grep fails there is no output and so unless the test
>> is run with "-x" it can be hard to see which command caused the test
>> to fail.
>
> We could rename test_i18ngrep to test_grep (and make test_i18ngrep
> into a thin wrapper with warnings).
>
> test_grep -e must-exist file &&
> test_grep ! -e must-not-exist file
... as the only remaining part in test_18ngrep has no hack to work
around the tainted localization tests, so "was deprecated without"
is a bit too strong. There is nothing we have lost yet.
t/test-lib-functions.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git c/t/test-lib-functions.sh w/t/test-lib-functions.sh
index 2f8868caa1..c50bc18861 100644
--- c/t/test-lib-functions.sh
+++ w/t/test-lib-functions.sh
@@ -1208,14 +1208,16 @@ test_cmp_bin () {
cmp "$@"
}
-# Wrapper for grep which used to be used for
-# GIT_TEST_GETTEXT_POISON=false. Only here as a shim for other
-# in-flight changes. Should not be used and will be removed soon.
+# Deprecated - do not use this in new code
test_i18ngrep () {
+ test_grep "$@"
+}
+
+test_grep () {
eval "last_arg=\${$#}"
test -f "$last_arg" ||
- BUG "test_i18ngrep requires a file to read as the last parameter"
+ BUG "test_grep requires a file to read as the last parameter"
if test $# -lt 2 ||
{ test "x!" = "x$1" && test $# -lt 3 ; }
^ permalink raw reply related
* Re: [PATCH v3] bugreport: reject positional arguments
From: Junio C Hamano @ 2023-10-30 0:26 UTC (permalink / raw)
To: Phillip Wood
Cc: Eric Sunshine, emilyshaffer, git, Emily Shaffer, Sheik,
Dragan Simic
In-Reply-To: <3e15f266-c790-4b71-84b6-1328339425c1@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> It is rather unfortunate that test_i18ngrep was deprecated without
> providing an alternative that offers the same debugging
> experience.
The primary thing test_i18ngrep did was to _SKIP_ certain test that
looked for an expected string in "C" locale from the output when the
tests were run under a tainted localization mode. The tests that
looked for strings in messages that are *not* to be localized used
"grep". Tests that (unfortunately) had to match human-readable
messages had to work around the tainted localization test to use
test_i18ngrep.
> When test_i18ngrep fails it prints a message with the
> pattern and text that failed to match so it is easy to see where the
> test failed. If grep fails there is no output and so unless the test
> is run with "-x" it can be hard to see which command caused the test
> to fail.
We could rename test_i18ngrep to test_grep (and make test_i18ngrep
into a thin wrapper with warnings).
test_grep -e must-exist file &&
test_grep ! -e must-not-exist file
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Junio C Hamano @ 2023-10-30 0:22 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git, Oswald Buddenhagen
In-Reply-To: <0d889da1-7fd8-4e21-965f-6222e4433ecf@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> I agree we don't want to support too many CI platforms but I think
> adding support for GitLab is good as it helps to stop us being too
> tied to GitHub Actions (which should make it easier if we ever need to
> transition to a different platform in the future) and provides an
> alternative for contributors who want to use a different platform.
> ...
> Having someone committed to on-going maintenance is great.
Yeah, it is great to see that stakeholder companies are helping Git
in ways they can.
Will queue.
^ permalink raw reply
* Re: [PATCH v3] bugreport: reject positional arguments
From: Junio C Hamano @ 2023-10-30 0:15 UTC (permalink / raw)
To: Phillip Wood
Cc: Eric Sunshine, emilyshaffer, git, Emily Shaffer, Sheik,
Dragan Simic
In-Reply-To: <3e15f266-c790-4b71-84b6-1328339425c1@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] parse-options: make CMDMODE errors more precise
From: Junio C Hamano @ 2023-10-30 0:12 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <cefdba32-db0b-4f68-954e-9d31fc12b1a0@web.de>
René Scharfe <l.s.r@web.de> writes:
> Store the argument of PARSE_OPT_CMDMODE options of type OPTION_CALLBACK
> as well to allow taking over the responsibility for compatibility
> checking from the callback function. The next patch will use this
> capability to fix the messages for git am --show-current-patch.
>
> Use a linked list for storing the PARSE_OPT_CMDMODE variables. This
> somewhat outdated data structure is simple and suffices, as the number
> of elements per command is currently only zero or one. We do support
> multiple different command modes variables per command, but I don't
> expect that we'd ever use a significant number of them. Once we do we
> can switch to a hashmap.
Makes quite a lot of sense. I would have expected to see this in
the parse_options_check() function, where other sanity checks are
done, but I think the reason you added the call to its caller
because parse_options_check() does not take the context.
It is not like we want to perform a sanity check that is independent
from a particular invocation of parse_options() on the options[]
array only just once, and want to reuse the array that is known to
be sane multiple times. The check is called once for every
invocation, so with or without this change, I do not see a good
reason for parse_options_check() not to take context, though.
> +static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
> + const struct option *opt,
> + enum opt_parsed flags)
> +{
> + const char *arg = NULL;
> + enum parse_opt_result result = do_get_value(p, opt, flags, &arg);
> + struct parse_opt_cmdmode_list *elem = p->cmdmode_list;
> + char *opt_name, *other_opt_name;
> +
> + for (; elem; elem = elem->next) {
> + if (*elem->value_ptr == elem->value)
> + continue;
> +
> + if (elem->opt &&
> + (elem->opt->flags | opt->flags) & PARSE_OPT_CMDMODE)
> + break;
> +
> + elem->opt = opt;
> + elem->arg = arg;
> + elem->flags = flags;
> + elem->value = *elem->value_ptr;
> + }
> +
> + if (result || !elem)
> + return result;
> +
> + opt_name = optnamearg(opt, arg, flags);
> + other_opt_name = optnamearg(elem->opt, elem->arg, elem->flags);
> + error(_("%s is incompatible with %s"), opt_name, other_opt_name);
> + free(opt_name);
> + free(other_opt_name);
> + return -1;
> +}
Looks quite involved but the overhead is number of supported cmdmode
options per each command line option, and the problems outlined in
the proposed log message are worth addressing. OK.
> @@ -1006,6 +1041,11 @@ int parse_options(int argc, const char **argv,
> precompose_argv_prefix(argc, argv, NULL);
> free_preprocessed_options(real_options);
> free(ctx.alias_groups);
> + for (struct parse_opt_cmdmode_list *elem = ctx.cmdmode_list; elem;) {
> + struct parse_opt_cmdmode_list *next = elem->next;
> + free(elem);
> + elem = next;
> + }
> return parse_options_end(&ctx);
> }
OK.
^ permalink raw reply
* Re: [PATCH v4 1/2] t0091-bugreport: stop using i18ngrep
From: Junio C Hamano @ 2023-10-29 23:59 UTC (permalink / raw)
To: emilyshaffer; +Cc: git, Emily Shaffer
In-Reply-To: <20231026182231.3369370-2-nasamuffin@google.com>
emilyshaffer@google.com writes:
> From: Emily Shaffer <nasamuffin@google.com>
>
> Since e6545201ad (Merge branch 'ab/detox-config-gettext', 2021-04-13),
> test_i18ngrep is no longer required. Quit using it in the bugreport
> tests, since it's setting a bad example for tests added later.
>
> Signed-off-by: Emily Shaffer <nasamuffin@google.com>
> ---
Makes sense. Thanks.
> t/t0091-bugreport.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> index f6998269be..e1588f71b7 100755
> --- a/t/t0091-bugreport.sh
> +++ b/t/t0091-bugreport.sh
> @@ -65,7 +65,7 @@ test_expect_success '--output-directory puts the report in the provided dir' '
>
> test_expect_success 'incorrect arguments abort with usage' '
> test_must_fail git bugreport --false 2>output &&
> - test_i18ngrep usage output &&
> + grep usage output &&
> test_path_is_missing git-bugreport-*
> '
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Junio C Hamano @ 2023-10-29 23:53 UTC (permalink / raw)
To: Jeff King; +Cc: Kousik Sanagavarapu, Liam Beguin, git
In-Reply-To: <20231028021301.GA35796@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Oct 28, 2023 at 09:12:06AM +0900, Junio C Hamano wrote:
>
>> Grouping @gmail.com addresses do not smell all that useful, though.
>> ...
> One way you could directly use this is in shortlog, which these days
> lets you group by specific formats. So:
>
> git shortlog -ns --group=format:%aA
>
> is potentially useful.
Exactly. That is what I meant by "Grouping", and I agree with you
about "potentially" part, too ;-) Throwing all @gmail.com addresses
into a single bin would not be very useful.
> ... If we could spell it as
> %(authoremail:domain) that would remove the question. But given the
> existence of "%al", I'm not too sad to see another letter allocated to
> this purpose in the meantime.
Another line of thought is perhaps it is potentially useful to teach
the --format= machinery to be a bit more programmable, e.g. allowing
to compute a substring of an existing field %{%aE#*@} without having
to waste a letter each for the local part and domain part. But as I
already said, we are now talking about "postprocessing", and adding
complexity to our codebase only to have incomplete flexibility may
not be worth it. A more specific %(authoremail:localpart) and its
domain counterpart may be easier to explain and understand.
In any case, it is a bit too late to say "let's not waste the
precious single letter namespace to add useless features", as we
have come way too far, so I do not mind too much using a currently
unused letter $X for yet another author and committer trait.
^ permalink raw reply
* What's cooking in git.git (Oct 2023, #09; Mon, 30)
From: Junio C Hamano @ 2023-10-29 23:52 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* bc/racy-4gb-files (2023-10-13) 2 commits
(merged to 'next' on 2023-10-16 at c60962dfee)
+ Prevent git from rehashing 4GiB files
+ t: add a test helper to truncate files
The index file has room only for lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
source: <20231012160930.330618-1-sandals@crustytoothpaste.net>
* en/docfixes (2023-10-09) 25 commits
(merged to 'next' on 2023-10-17 at 1e3cdeb427)
+ documentation: add missing parenthesis
+ documentation: add missing quotes
+ documentation: add missing fullstops
+ documentation: add some commas where they are helpful
+ documentation: fix whitespace issues
+ documentation: fix capitalization
+ documentation: fix punctuation
+ documentation: use clearer prepositions
+ documentation: add missing hyphens
+ documentation: remove unnecessary hyphens
+ documentation: add missing article
+ documentation: fix choice of article
+ documentation: whitespace is already generally plural
+ documentation: fix singular vs. plural
+ documentation: fix verb vs. noun
+ documentation: fix adjective vs. noun
+ documentation: fix verb tense
+ documentation: employ consistent verb tense for a list
+ documentation: fix subject/verb agreement
+ documentation: remove extraneous words
+ documentation: add missing words
+ documentation: fix apostrophe usage
+ documentation: fix typos
+ documentation: fix small error
+ documentation: wording improvements
Documentation typo and grammo fixes.
source: <pull.1595.git.1696747527.gitgitgadget@gmail.com>
* jc/fail-stash-to-store-non-stash (2023-10-11) 1 commit
(merged to 'next' on 2023-10-16 at e26db57315)
+ stash: be careful what we store
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
source: <xmqqbkd4lwj0.fsf_-_@gitster.g>
* jk/chunk-bounds (2023-10-14) 21 commits
(merged to 'next' on 2023-10-16 at 68c9e37980)
+ t5319: make corrupted large-offset test more robust
(merged to 'next' on 2023-10-10 at 21139603ce)
+ chunk-format: drop pair_chunk_unsafe()
+ commit-graph: detect out-of-order BIDX offsets
+ commit-graph: check bounds when accessing BIDX chunk
+ commit-graph: check bounds when accessing BDAT chunk
+ commit-graph: bounds-check generation overflow chunk
+ commit-graph: check size of generations chunk
+ commit-graph: bounds-check base graphs chunk
+ commit-graph: detect out-of-bounds extra-edges pointers
+ commit-graph: check size of commit data chunk
+ midx: check size of revindex chunk
+ midx: bounds-check large offset chunk
+ midx: check size of object offset chunk
+ midx: enforce chunk alignment on reading
+ midx: check size of pack names chunk
+ commit-graph: check consistency of fanout table
+ midx: check size of oid lookup chunk
+ commit-graph: check size of oid fanout chunk
+ midx: stop ignoring malformed oid fanout chunk
+ t: add library for munging chunk-format files
+ chunk-format: note that pair_chunk() is unsafe
(this branch is used by tb/pair-chunk-expect-size.)
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
source: <20231009205544.GA3281950@coredump.intra.peff.net>
* so/diff-merges-dd (2023-10-09) 3 commits
(merged to 'next' on 2023-10-16 at 71b5e29625)
+ completion: complete '--dd'
+ diff-merges: introduce '--dd' option
+ diff-merges: improve --diff-merges documentation
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
source: <20231009160535.236523-1-sorganov@gmail.com>
--------------------------------------------------
[New Topics]
* ii/branch-error-messages-update (2023-10-23) 1 commit
(merged to 'next' on 2023-10-23 at 3d00599173)
+ builtin/branch.c: adjust error messages to coding guidelines
Error message update.
Will merge to 'master'.
source: <20231023160656.4341-1-isokenjune@gmail.com>
* jm/bisect-run-synopsis-fix (2023-10-23) 1 commit
(merged to 'next' on 2023-10-23 at 8dfa3ed356)
+ doc/git-bisect: clarify `git bisect run` syntax
Doc and usage message update.
Will merge to 'master'.
source: <pull.1602.v2.git.1698088990478.gitgitgadget@gmail.com>
* ar/submitting-patches-doc-update (2023-10-24) 1 commit
- SubmittingPatches: call gitk's command "Copy commit reference"
Doc update.
Will merge to 'next'.
source: <20231024195123.911431-1-rybak.a.v@gmail.com>
* es/bugreport-no-extra-arg (2023-10-29) 2 commits
- bugreport: reject positional arguments
- t0091-bugreport: stop using i18ngrep
source: <20231026155459.2234929-1-nasamuffin@google.com>
* js/my-first-contribution-update (2023-10-28) 1 commit
- Include gettext.h in MyFirstContribution tutorial
source: <20231017041503.3249-1-jacob@initialcommit.io>
* ms/send-email-validate-fix (2023-10-26) 1 commit
- send-email: move validation code below process_address_list
source: <ddd4bfdd-ed14-44f4-89d3-192332bbc1c4@amd.com>
* ps/ci-gitlab (2023-10-29) 5 commits
- ci: add support for GitLab CI
- ci: split out logic to set up failed test artifacts
- ci: group installation of Docker dependencies
- ci: make grouping setup more generic
- ci: reorder definitions for grouping functions
source: <cover.1698398590.git.ps@pks.im>
* ps/ref-tests-update (2023-10-24) 9 commits
- t: mark several tests that assume the files backend with REFFILES
- t7900: assert the absence of refs via git-for-each-ref(1)
- t7300: assert exact states of repo
- t4207: delete replace references via git-update-ref(1)
- t1450: convert tests to remove worktrees via git-worktree(1)
- t: convert tests to not access reflog via the filesystem
- t: convert tests to not access symrefs via the filesystem
- t: convert tests to not write references via the filesystem
- t: allow skipping expected object ID in `ref-store update-ref`
source: <cover.1698156169.git.ps@pks.im>
* rs/fix-arghelp (2023-10-29) 1 commit
- am, rebase: fix arghelp syntax of --empty
source: <10e09b2d-15d7-4af1-b24c-217f9e2f457a@web.de>
* rs/parse-options-cmdmode (2023-10-29) 2 commits
- am: simplify --show-current-patch handling
- parse-options: make CMDMODE errors more precise
source: <4520156b-9418-493c-a50c-e61b42e805b3@web.de>
* rs/reflog-expire-single-worktree-fix (2023-10-29) 1 commit
- reflog: fix expire --single-worktree
source: <63eade0e-bf2c-4906-8b4c-689797cff737@web.de>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* jc/am-doc-whitespace-action-fix (2023-10-18) 1 commit
(merged to 'next' on 2023-10-20 at 9200d39c08)
+ am: align placeholder for --whitespace option with apply
Docfix.
Will merge to 'master'.
source: <xmqqwmvjzeqd.fsf@gitster.g>
* da/t7601-style-fix (2023-10-18) 1 commit
(merged to 'next' on 2023-10-20 at 8e7326458c)
+ t7601: use "test_path_is_file" etc. instead of "test -f"
Coding style update.
Will merge to 'master'.
source: <20231018124538.68549-2-anonolitunya@gmail.com>
* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
- fetch: no redundant error message for atomic fetch
- t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Needs review.
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* mm/p4-symlink-with-lfs (2023-10-19) 1 commit
(merged to 'next' on 2023-10-20 at 9c05ce7e85)
+ git-p4 shouldn't attempt to store symlinks in LFS
"git p4" tried to store symlinks to LFS when told, but has been
fixed not to do so, because it does not make sense.
Will merge to 'master'.
source: <20231019002558.867830-1-mmcclain@noprivs.com>
* jk/send-email-fix-addresses-from-composed-messages (2023-10-20) 3 commits
(merged to 'next' on 2023-10-22 at 43221cc3a4)
+ send-email: handle to/cc/bcc from --compose message
+ Revert "send-email: extract email-parsing code into a subroutine"
+ doc/send-email: mention handling of "reply-to" with --compose
The codepath to handle recipient addresses `git send-email
--compose` learns from the user was completely broken, which has
been corrected.
Will merge to 'master'.
source: <20231020100343.GA2194322@coredump.intra.peff.net>
* ms/doc-push-fix (2023-10-20) 1 commit
(merged to 'next' on 2023-10-22 at 7ce3cef56b)
+ git-push doc: more visibility for -q option
Docfix.
Will merge to 'master'.
source: <20231020184627.14336-1-msuchanek@suse.de>
* ob/rebase-cleanup (2023-10-20) 3 commits
(merged to 'next' on 2023-10-22 at 05e14ca4fc)
+ rebase: move parse_opt_keep_empty() down
+ rebase: handle --strategy via imply_merge() as well
+ rebase: simplify code related to imply_merge()
Code clean-up.
Will merge to 'master'.
source: <20231020093654.922890-1-oswald.buddenhagen@gmx.de>
* wx/merge-ort-comment-typofix (2023-10-21) 1 commit
(merged to 'next' on 2023-10-22 at ad1e33883a)
+ merge-ort.c: fix typo 'neeed' to 'needed'
Typofix.
Will merge to 'master'.
source: <pull.1592.v3.git.git.1697942768555.gitgitgadget@gmail.com>
* jc/commit-new-underscore-index-fix (2023-10-17) 1 commit
(merged to 'next' on 2023-10-22 at 0e4787303d)
+ commit: do not use cryptic "new_index" in end-user facing messages
Message fix.
Will merge to 'master'.
source: <xmqqo7gwvd8c.fsf_-_@gitster.g>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/pathspec-error-wo-repository-fix (2023-10-20) 1 commit
(merged to 'next' on 2023-10-22 at 4f77af1e40)
+ grep: die gracefully when outside repository
The pathspec code carelessly dereferenced NULL while emitting an
error message, which has been corrected.
Will merge to 'master'.
source: <5c8ef6bec1c99e0fae7ada903885a8e77f8137f9.1697819838.git.code@khaugsbakk.name>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Needs review.
source: <cover.1697319294.git.code@khaugsbakk.name>
* ni/die-message-fix-for-git-add (2023-10-17) 1 commit
(merged to 'next' on 2023-10-22 at f46c5dfd63)
+ builtin/add.c: clean up die() messages
Message updates.
Will merge to 'master'.
source: <20231017113946.747-1-naomi.ibeh69@gmail.com>
* ps/git-repack-doc-fixes (2023-10-16) 2 commits
(merged to 'next' on 2023-10-22 at df64849f26)
+ doc/git-repack: don't mention nonexistent "--unpacked" option
+ doc/git-repack: fix syntax for `-g` shorthand option
Doc updates.
Will merge to 'master'.
source: <cover.1697440686.git.ps@pks.im>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Will merge to 'next'?
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/format-pack-doc-update (2023-10-12) 2 commits
- Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
- Documentation/gitformat-pack.txt: fix typo
Doc update.
Expecting a reroll.
cf. <xmqq5y3b4id2.fsf@gitster.g>
source: <cover.1697144959.git.me@ttaylorr.com>
* ps/do-not-trust-commit-graph-blindly-for-existence (2023-10-24) 2 commits
- commit: detect commits that exist in commit-graph but not in the ODB
- commit-graph: introduce envvar to disable commit existence checks
(this branch is used by kn/rev-list-missing-fix.)
The codepath to traverse the commit-graph learned to notice that a
commit is missing (e.g., corrupt repository lost an object), even
though it knows something about the commit (like its parents) from
what is in commit-graph.
Comments?
source: <cover.1698060036.git.ps@pks.im>
* tb/pair-chunk-expect-size (2023-10-14) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- midx: read `OIDF` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDF` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
Code clean-up for jk/chunk-bounds topic.
Comments?
source: <45cac29403e63483951f7766c6da3c022c68d9f0.1697225110.git.me@ttaylorr.com>
source: <cover.1697225110.git.me@ttaylorr.com>
* jc/grep-f-relative-to-cwd (2023-10-12) 1 commit
- grep: -f <path> is relative to $cwd
"cd sub && git grep -f patterns" tried to read "patterns" file at
the top level of the working tree; it has been corrected to read
"sub/patterns" instead.
Needs review.
source: <xmqqedhzg37z.fsf@gitster.g>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* kn/rev-list-missing-fix (2023-10-29) 4 commits
- rev-list: add commit object support in `--missing` option
- rev-list: move `show_commit()` to the bottom
- revision: rename bit to `do_not_die_on_missing_objects`
- Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into kn/rev-list-missing-fix
(this branch uses ps/do-not-trust-commit-graph-blindly-for-existence.)
"git rev-list --missing" did not work for missing commit objects,
which has been corrected.
Will merge to 'next'.
source: <20231026101109.43110-1-karthik.188@gmail.com>
* sn/typo-grammo-phraso-fixes (2023-10-05) 5 commits
(merged to 'next' on 2023-10-18 at 575d767f9a)
+ t/README: fix multi-prerequisite example
+ doc/gitk: s/sticked/stuck/
+ git-jump: admit to passing merge mode args to ls-files
+ doc/diff-options: improve wording of the log.diffMerges mention
+ doc: fix some typos, grammar and wording issues
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.
Will merge to 'master'.
source: <20231003082107.3002173-1-stepnem@smrk.net>
* jc/update-list-references-to-lore (2023-10-06) 1 commit
(merged to 'next' on 2023-10-19 at 83a721a137)
+ doc: update list archive reference to use lore.kernel.org
Doc update.
Will merge to 'master'.
source: <xmqq7cnz741s.fsf@gitster.g>
* cc/git-replay (2023-10-10) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Expectting a (hopefully final and quick) reroll.
cf. <bd872b81-80a9-5e4e-dcb6-faebc9671848@gmx.de>
source: <20231010123847.2777056-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* jc/attr-tree-config (2023-10-13) 2 commits
(merged to 'next' on 2023-10-19 at 202dc1c453)
+ attr: add attr.tree for setting the treeish to read attributes from
+ attr: read attributes from HEAD when bare repo
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
Will merge to 'master'.
source: <pull.1577.v5.git.git.1697218770.gitgitgadget@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Will merge to 'next'?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* js/doc-unit-tests (2023-10-16) 5 commits
- fixup! ci: run unit tests in CI
- fixup! unit tests: add TAP unit test framework
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Expecting a (hopefully final and minor) reroll.
cf. <20231016134421.21659-1-phillip.wood123@gmail.com>
source: <cover.1696889529.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-10-19) 7 commits
- cmake: handle also unit tests
- cmake: use test names instead of full paths
- cmake: fix typo in variable name
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Needs review.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* rs/parse-options-value-int (2023-09-18) 2 commits
. parse-options: use and require int pointer for OPT_CMDMODE
. parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Retracted.
cf. <4014e490-c6c1-453d-b0ed-645220e3e614@web.de>
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
^ permalink raw reply
* please add link / url to remote - when - git push
From: Alexander Mills @ 2023-10-29 23:15 UTC (permalink / raw)
To: git
When a feature branch is pushed, we get a link in the console to go to
remote, however when I push directly to main/master, no such link, eg:
```
alex.mills@alex node-be % git push
Enumerating objects: 20, done.
Counting objects: 100% (20/20), done.
Delta compression using up to 12 threads
Compressing objects: 100% (10/10), done.
Writing objects: 100% (11/11), 1.56 KiB | 799.00 KiB/s, done.
Total 11 (delta 7), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (7/7), completed with 7 local objects.
remote: Bypassed rule violations for refs/heads/main:
remote:
remote: - Changes must be made through a pull request.
remote:
To github.com:elx-onlinx/beautychat-chatcards.git
ffe1e05..bb7b0ef main -> main
```
Having the link in the console saves me tremendous time and is
extremely effective/efficient. Can we get links in the console plz?
-alex
--
Alexander D. Mills
Mobile phone (737) 281-4824
https://linkedin.com/in/alexanderdmills
^ permalink raw reply
* Re: [PATCH] reflog: fix expire --single-worktree
From: Junio C Hamano @ 2023-10-29 22:31 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, John Cai
In-Reply-To: <63eade0e-bf2c-4906-8b4c-689797cff737@web.de>
René Scharfe <l.s.r@web.de> writes:
> ... and added a non-printable short flag for it, presumably by
> accident.
Very well spotted.
FWIW, with the following patch on top of this patch, all tests pass
(and without your fix, of course this notices the "\001" and breaks
numerous tests that use "git reflog"). So you seem to have found
the only one broken instance (among those that are tested, anyway).
parse-options.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git i/parse-options.c w/parse-options.c
index 093eaf2db8..be8bedba29 100644
--- i/parse-options.c
+++ w/parse-options.c
@@ -469,7 +469,8 @@ static void parse_options_check(const struct option *opts)
optbug(opts, "uses incompatible flags "
"LASTARG_DEFAULT and OPTARG");
if (opts->short_name) {
- if (0x7F <= opts->short_name)
+ if (opts->short_name &&
+ (opts->short_name < 0x21 || 0x7F <= opts->short_name))
optbug(opts, "invalid short name");
else if (short_opts[opts->short_name]++)
optbug(opts, "short name already used");
^ permalink raw reply related
* [PATCH] chore: fix typo in .clang-format comment
From: Aditya Neelamraju via GitGitGadget @ 2023-10-29 20:23 UTC (permalink / raw)
To: git; +Cc: Aditya Neelamraju, Aditya Neelamraju
From: Aditya Neelamraju <adityanv97@gmail.com>
Signed-off-by: Aditya Neelamraju <adityanv97@gmail.com>
---
chore: fix typo in .clang-format comment
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1602%2Faneelamr%2Fclang-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1602/aneelamr/clang-v1
Pull-Request: https://github.com/git/git/pull/1602
.clang-format | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/.clang-format b/.clang-format
index c592dda681f..3ed4fac753a 100644
--- a/.clang-format
+++ b/.clang-format
@@ -83,9 +83,9 @@ BinPackParameters: true
BreakBeforeBraces: Linux
# Break after operators
-# int valuve = aaaaaaaaaaaaa +
-# bbbbbb -
-# ccccccccccc;
+# int value = aaaaaaaaaaaaa +
+# bbbbbb -
+# ccccccccccc;
BreakBeforeBinaryOperators: None
BreakBeforeTernaryOperators: false
base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14
--
gitgitgadget
^ permalink raw reply related
* Re: [EXT] Re: ls-remote bug
From: René Scharfe @ 2023-10-29 18:56 UTC (permalink / raw)
To: Lior Zeltzer, Bagas Sanjaya, Git Mailing List
Cc: Andrzej Hunt, Ævar Arnfjörð Bjarmason,
Junio C Hamano
In-Reply-To: <BL0PR18MB213007901229C84D348D4CB8BADCA@BL0PR18MB2130.namprd18.prod.outlook.com>
Am 27.10.23 um 13:16 schrieb Lior Zeltzer:
> The reproduction ,as I wrote in the code, should be done with few threads in parallel
> Each working on a list of ~10 repos with each repo containing a lot of refs/tags (~1000)
> All this should be against gerrit (my gerrit is 3.8.0)
>
> Also read the notes below regarding the code that was moved between 2.31.8 and 2.32.0 in ls-remote.c file
> I can elaborate more, in a zoom meeting.
>
> 10x
> Lior.
>
>
> -----Original Message-----
> From: Bagas Sanjaya <bagasdotme@gmail.com>
> Sent: Friday, October 27, 2023 4:08 AM
> To: Lior Zeltzer <liorz@marvell.com>; Git Mailing List <git@vger.kernel.org>
> Cc: Andrzej Hunt <ajrhunt@google.com>; Ævar Arnfjörð Bjarmason <avarab@gmail.com>; Junio C Hamano <gitster@pobox.com>
> Subject: [EXT] Re: ls-remote bug
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, Oct 24, 2023 at 10:55:24AM +0000, Lior Zeltzer wrote:
>>
>>> uname -a
>> Linux dc3lp-veld0045 3.10.0-1160.21.1.el7.x86_64 #1 SMP Tue Mar 16
>> 18:28:22 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
>>
>> Gerrit version :
>> 3.8.0
>>
>> Bug description :
>> When running ls-remote : sometime data gets cut in the middle
>>
>> Reproducing :
>> You need a few files with a few repo names (I used 4 files with 10
>> repos each) Call then l1..l4 And the code below just cd into each of
>> them does ls-remote twice and compares the data Doing it in parallel
>> on all lists.
>> Data received in both ls-remotes should be the same , if not, it
>> prints *** Repos should contain a lot of tags and refs
>
> What repo did you find this regression? Did you mean linux.git (Linux kernel)?
>
>>
>> Note :
>> 1. without stderr redirection (2>&1) all works well 2. On local repos
>> (not through gerrit) all works well
>>
>> I compared various git vers and found the bug to be between 2.31.8 and
>> 2.32.0 Comparing ls-remote.c file between those vers gave me :
>>
>> Lines :
>> if (transport_disconnect(transport))
>> return 1;
>>
>> moved to end of sub
>>
>> copying ls-remote.c from 2.31.8 to 2.32.0 - fixed the bug
This partly undoes 68ffe095a2 (transport: also free remote_refs in
transport_disconnect(), 2021-03-21). With that patch connections are
kept open during ref sorting and printing.
Perhaps the other side gets tired of waiting and aborts? Maybe
splitting transport_disconnect() into two functions -- one for
disconnecting and and for cleaning up -- would make sense? And
disconnecting as soon as possible? Just guessing -- didn't actually
reproduce the bug. Still, demo patch below.
>>
>>
>>
>> Code reproducing bug :
>>
>> #!/proj/mislcad/areas/DAtools/tools/perl/5.10.1/bin/perl -w use
>> strict; use Cwd qw(cwd);
>>
>> my $count = 4;
>> for my $f (1..$count) {
>> my $child = fork();
>> if (!$child) {
>> my $curr = cwd();
>>
>> my @repos = `cat l$f`;
>> foreach my $repo (@repos) {
>> chomp $repo;
>> print "$repo\n";
>> chdir($repo);
>> my $remote_tags_str = `git ls-remote 2>&1`;
>> my $remote_tags_str2 = `git ls-remote 2>&1 `;
>> chdir($curr);
>> if ( $remote_tags_str ne $remote_tags_str2) {
>> print "***\n";
>> }
>> }
>>
>> exit(0);
>> }
>> }
>> while (wait != -1) {}
>> 1;
>>
>
> I tried reproducing this regression by:
>
> ```
> $ cd /path/to/git.git
> $ git ls-remote 2>&1 > /tmp/root.list
> $ cd builtin/
> $ git ls-remote 2>&1 > /tmp/builtin.list $ cd ../ $ git diff --no-index /tmp/root.list /tmp/builtin.list ```
>
> And indeed, the diff was empty (which meant that both listings are same).
>
> Confused...
>
> --
> An old man doll... just what I always wanted! - Clara
---
builtin/ls-remote.c | 8 +++++---
builtin/remote.c | 3 ++-
transport.c | 15 +++++++++++++--
transport.h | 2 ++
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fc76575430..4c1daa0f92 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -128,6 +128,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
repo_set_hash_algo(the_repository, hash_algo);
}
+ if (transport_disconnect_raw(transport))
+ status = 1;
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
@@ -154,12 +156,12 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
if (show_symref_target && ref->symref)
printf("ref: %s\t%s\n", ref->symref, ref->refname);
printf("%s\t%s\n", oid_to_hex(&ref->objectname), ref->refname);
- status = 0; /* we found something */
+ if (status != 1)
+ status = 0; /* we found something */
}
ref_array_clear(&ref_array);
- if (transport_disconnect(transport))
- status = 1;
+ transport_clear(transport);
transport_ls_refs_options_release(&transport_options);
return status;
}
diff --git a/builtin/remote.c b/builtin/remote.c
index d91bbe728d..055a221942 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1000,6 +1000,7 @@ static int get_remote_ref_states(const char *name,
transport = transport_get(states->remote, states->remote->url_nr > 0 ?
states->remote->url[0] : NULL);
remote_refs = transport_get_remote_refs(transport, NULL);
+ transport_disconnect_raw(transport);
states->queried = 1;
if (query & GET_REF_STATES)
@@ -1008,7 +1009,7 @@ static int get_remote_ref_states(const char *name,
get_head_names(remote_refs, states);
if (query & GET_PUSH_REF_STATES)
get_push_ref_states(remote_refs, states);
- transport_disconnect(transport);
+ transport_clear(transport);
} else {
for_each_ref(append_ref_to_tracked_list, states);
string_list_sort(&states->tracked);
diff --git a/transport.c b/transport.c
index 219af8fd50..c71dab75e9 100644
--- a/transport.c
+++ b/transport.c
@@ -1584,17 +1584,28 @@ int transport_connect(struct transport *transport, const char *name,
die(_("operation not supported by protocol"));
}
-int transport_disconnect(struct transport *transport)
+int transport_disconnect_raw(struct transport *transport)
{
int ret = 0;
if (transport->vtable->disconnect)
ret = transport->vtable->disconnect(transport);
+ return ret;
+}
+
+void transport_clear(struct transport *transport)
+{
if (transport->got_remote_refs)
free_refs((void *)transport->remote_refs);
clear_bundle_list(transport->bundles);
free(transport->bundles);
free(transport);
- return ret;
+}
+
+int transport_disconnect(struct transport *transport)
+{
+ int ret = transport_disconnect_raw(transport);
+ transport_clear(transport);
+ return 0;
}
/*
diff --git a/transport.h b/transport.h
index 6393cd9823..fd75905568 100644
--- a/transport.h
+++ b/transport.h
@@ -320,6 +320,8 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs);
void transport_unlock_pack(struct transport *transport, unsigned int flags);
int transport_disconnect(struct transport *transport);
+void transport_clear(struct transport *transport);
+int transport_disconnect_raw(struct transport *transport);
char *transport_anonymize_url(const char *url);
void transport_take_over(struct transport *transport,
struct child_process *child);
--
2.42.0
^ permalink raw reply related
* Re: Git in Outreachy? (December, 2023)
From: Kaartic Sivaraam @ 2023-10-29 18:23 UTC (permalink / raw)
To: Christian Couder, Taylor Blau; +Cc: git, Hariom verma, Victoria Dye
In-Reply-To: <CAP8UFD3i5ewCoaZOh09TtfuefEtWL4+z8P0_Z9DWdYCrShYheQ@mail.gmail.com>
Hi Christian and Taylor,
Excuse the huge delay in my response. I got too pre-occuppied with some
other commitments.
On 22/09/23 11:49, Christian Couder wrote:
>>
>> On Tue, Aug 29, 2023 at 10:38:45PM +0200, Christian Couder wrote:
>>> By the way, Kaartic, do you still want to be an org admin? And Taylor
>>> are you Ok with Kaartic being an org admin?
>>
>> Sorry that this dropped off of my queue. FWIW, no issues from me.
>
> Taylor, thanks for your answer!
>
> Kaartic, if you want to be org admin, just ask on the Outreachy web
> site and we will accept your request.
>
Thanks for the offer!
I think we're very close to the end of the contribution period of
Outreachy. I think the contribution period is the on that would've
needed significant participation of an Org Admin. I'm not sure if
there's anything I could contribute to much in coming days. If there's
anything I could help with, kindly let me know. I'll be glad to help :-)
--
Sivaraam
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-29 16:27 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <ZTucxlBR1VrJPuSK@tanuki>
Hi Patrick
On 27/10/2023 12:19, Patrick Steinhardt wrote:
> On Fri, Oct 27, 2023 at 11:19:04AM +0100, Phillip Wood wrote:
>> On 27/10/2023 10:25, Patrick Steinhardt wrote:
>>> + parallel:
>>> + matrix:
>>> + - jobname: linux-sha256
>>> + image: ubuntu:latest
>>> + CC: clang
>>> + - jobname: linux-gcc
>>> + image: ubuntu:20.04
>>> + CC: gcc
>>> + CC_PACKAGE: gcc-8
>>> + - jobname: linux-TEST-vars
>>> + image: ubuntu:20.04
>>> + CC: gcc
>>> + CC_PACKAGE: gcc-8
>>> + - jobname: linux-gcc-default
>>> + image: ubuntu:latest
>>> + CC: gcc
>>> + - jobname: linux-leaks
>>> + image: ubuntu:latest
>>> + CC: gcc
>>> + - jobname: linux-asan-ubsan
>>> + image: ubuntu:latest
>>> + CC: clang
>>> + - jobname: linux-musl
>>> + image: alpine:latest
I assume you've chosen the configurations from the existing GitHub
Actions that give the best coverage of the various options. One thing I
noticed is that the is no equivalent of the "pedantic" job that builds
git with "DEVELOPER=1 DEVOPTS=pedantic"
>>> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
>>> index d0bc19d3bb3..1cd92db1876 100755
>>> --- a/ci/install-docker-dependencies.sh
>>> +++ b/ci/install-docker-dependencies.sh
>>> @@ -16,9 +19,13 @@ linux32)
>>> '
>>> ;;
>>> linux-musl)
>>> - apk add --update build-base curl-dev openssl-dev expat-dev gettext \
>>> + apk add --update git shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
>>> pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
>>
>> It would be helpful to explain the new dependencies in the commit message. I
>> can see why you're adding sudo, but how were we getting away without
>> installing the other packages for GitHub Actions?
>
> True, that part is missing.
>
> - Both sudo and shadow are now required because of `useradd` that we use
> to set up the unprivileged build.
>
> - Git has been required all along, I think. `save_good_tree ()` is used
> in our CI scripts, and Toon (fellow GitLabber from my team) has
> noticed that the CI job warned about missing Git. The warning was
> mostly benign as it seems, but still, doesn't hurt to fix it while at
> it.
Oh I had a look at this and the docker based jobs on GitHub do not have
a git repository so installing git means we now get a repository not
found error from save_good_tree() instead. We should probably make
save_good_tree() and check_unignored_build_artifacts() return early if
there isn't a repository but that's orthogonal to this series.
Looking at the test output from the link in your cover letter we should
we should also install apache2[1] and gnupg[2]
[1] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1444
[2] https://gitlab.com/gitlab-org/git/-/jobs/5349205374#L1167
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-29 16:13 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <6ee43015-1abc-4269-b710-0647ce80d7a2@crinan.ddns.net>
Hi Patrick
On 27/10/2023 14:02, Phillip Wood wrote:
> On 27/10/2023 12:57, Patrick Steinhardt wrote:
>> Hum. After having a look at `ci/run-docker-build.sh` I don't feel like
>> it's sensible to update it. It's not even used anymore by our CI but
>> only by `ci/run-docker.sh`, which seems to be more of a developer-facing
>> script?
>>
>> As you said, this smells like rotting bits that might rather be removed.
>> But in any case, as they don't relate to our current CI infrastructure
>> except for being in "ci/" I'll leave them be for now.
>
> I was trying to suggest that we start using these scripts again.
Having taken a closer look I think we'd be better off adding something like
# Ensure the build and tests run as an unprivileged user
if test "$(id -u)" -eq 0
then
useradd --home-dir "$(pwd)" builder
chown -R builder .
exec sudo --preserve-env --set-home --user=builder "$0"
fi
To the beginning of ci/run-build-and-tests.sh.
Best Wishes
Phillip
^ permalink raw reply
* Re: [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Phillip Wood @ 2023-10-29 14:43 UTC (permalink / raw)
To: Isoken Ibizugbe, Christian Couder; +Cc: git
In-Reply-To: <CAJHH8bHCfx3vknPCGATbLZeTA7hYrVVtnYqfE1avWkiL1PvU1g@mail.gmail.com>
Hi Isoken
On 28/10/2023 15:07, Isoken Ibizugbe wrote:
> #include "test-lib.h"
> #include "ctype.h"
>
> static void t_digit_type(void)
> {
> int i;
>
> for (i = 0; i < 256; i++)
> {
> if (i < '0' || i > '9')
> check_int(isdigit(i), ==, 0);
> else
> check_int(isdigit(i), ==, 1);
> }
> }
I think this is correct but when you are writing tests it is important
to think about how easy they will be to debug if they fail. In this case
because there is a single test to check all the characters it will be
hard to tell which character caused the test to fail. If we restructure
the code to use a separate test for each character then we will be able
to see which characters are causing isdigit() to fail. To do that we
need a function that prints the character that we're testing. Because we
don't want to print raw control characters in the test name we need to
check if the character can be printed as is or if it needs to be printed
as an octal escape sequence. We can do that by writing a function like
static const char* char_name(int i)
{
static char buf[5];
if (i < ' ' || i >= 127)
xsnprintf(buf, sizeof(buf), "\\%03o", (unsigned int)i);
else
xsnprintf(buf, sizeof(buf), "%c", i);
return buf;
}
Then we can write a test function defines a separate test for each character
static void t_isdigit(void)
{
for (int i = 0; i < 256; i++) {
if (i < '0' || i > '9')
TEST(check(!isdigit(i)), "'%s' is not a digit",
char_name(i));
else
TEST(check(isdigit(i)), "'%s' is a digit",
char_name(i));
}
}
Note that as isdigit() returns a boolean we simplify things by using
check() rather than check_int().
Now we can easily see which character is being tested when a check fails
as the character being tested is in the test name. You would call this
function with
int cmd_main(int argc, const char** argv)
{
t_isdigit();
return test_done();
}
I think it would be helpful for you to try and build and run this test
by checking out the unit test branch from Junio's tree[1] and adding
this test. You could then try making the test fail to see what the
output for a failing test looks like.
Best Wishes
Phillip
[1] You can fetch that branch with
git fetch https://github.com/gitster/git.git
js/doc-unit-tests-with-cmake
and then create your branch with
git checkout -b isdigit-unit-tests FETCH_HEAD
^ permalink raw reply
* Re: [PATCH 1/1] merge-file: add an option to process object IDs
From: brian m. carlson @ 2023-10-29 14:18 UTC (permalink / raw)
To: Elijah Newren; +Cc: git, Junio C Hamano, Phillip Wood
In-Reply-To: <CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]
On 2023-10-29 at 06:17:09, Elijah Newren wrote:
> Hi,
>
> Overall, looks good. Just a couple questions...
>
> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > From: "brian m. carlson" <bk2204@github.com>
> >
> [...]
> > --- a/Documentation/git-merge-file.txt
> > +++ b/Documentation/git-merge-file.txt
> > @@ -12,6 +12,9 @@ SYNOPSIS
> > 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
> > [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
> > [--[no-]diff3] <current-file> <base-file> <other-file>
> > +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]
> > + [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]
> > + [--[no-]diff3] <current-oid> <base-oid> <other-oid>
>
> Why was the `[-p|--stdout]` option removed in the second synopsis?
> Elsewhere you explicitly call it out as a possibility to be used with
> --object-id.
Originally because it implied `-p`, but I changed that to write into the
object store. I'll restore it.
> Also, why the extra synopsis instead of just adding a `[--object-id]`
> option to the previous one?
Because there's a relevant difference: the former has <current-file>,
<base-file>, and <other-file>, and the latter has the -oid versions.
> Does "/dev/null" have any portability considerations? (I really don't
> know; just curious.)
We already use it elsewhere in the codebase, so I assume it works. We
also have a test for that case and it worked in CI, so it's probably
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 v5 00/14] Introduce new `git replay` command
From: Johannes Schindelin @ 2023-10-29 14:14 UTC (permalink / raw)
To: Elijah Newren
Cc: Christian Couder, git, Junio C Hamano, Patrick Steinhardt,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <CABPp-BGCzxL-kpOvZzWRTJcx2v18QHm5ev8bFv7bm0dyNqhKug@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]
Hi Elijah,
On Sat, 28 Oct 2023, Elijah Newren wrote:
> On Tue, Oct 10, 2023 at 5:39 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > * Patch 12/15 (replay: disallow revision specific options and
> > pathspecs) in version 4 has been removed, so there are now only 14
> > patches instead of 15 in the series. This follows a suggestion by
> > Dscho, and goes in the direction Elijah initially wanted before
> > Derrick Stolee argued for disallowing revision specific options and
> > pathspecs.
>
> [... snipping many parts that I agree with...]
>
> > Also instead of forcing reverse order we use the reverse order by
> > default but allow it to be changed using `--reverse`. Thanks to
> > Dscho.
>
> I can see why this might sometimes be useful for exclusively linear
> history, but it seems to open a can of worms and possibly unfixable
> corner cases for non-linear history. I'd rather not do this, or at
> least pull it out of this series and let us discuss it in some follow
> up series. There are some other alternatives that might handle such
> usecases better.
I find myself wishing for an easy way to reverse commits, if only to
switch around the latest two commits while stopped during a rebase.
So it would have been nice for me if there had been an easy, worktree-less
way to make that happen.
I guess this would be going in the direction of reordering commits,
though, something we deliberately left for later?
Ciao,
Johannes
^ 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