* [RFC PATCH v2 1/6] status: add noob format from status.noob config
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
Makefile | 1 +
builtin/commit.c | 7 +++
table.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++
table.h | 6 +++
wt-status.c | 72 +++++++++++++++++++----------
wt-status.h | 1 +
6 files changed, 179 insertions(+), 25 deletions(-)
create mode 100644 table.c
create mode 100644 table.h
diff --git a/Makefile b/Makefile
index 9c6a2f125f..a7399ca8f0 100644
--- a/Makefile
+++ b/Makefile
@@ -1155,6 +1155,7 @@ LIB_OBJS += submodule-config.o
LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
+LIB_OBJS += table.o
LIB_OBJS += tempfile.o
LIB_OBJS += thread-utils.o
LIB_OBJS += tmp-objdir.o
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f92448..880c42f5b7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1430,6 +1430,13 @@ static int git_status_config(const char *k, const char *v,
status_deferred_config.status_format = STATUS_FORMAT_NONE;
return 0;
}
+ if (!strcmp(k, "status.noob")) {
+ if (git_config_bool(k, v))
+ status_deferred_config.status_format = STATUS_FORMAT_NOOB;
+ else
+ status_deferred_config.status_format = STATUS_FORMAT_NONE;
+ return 0;
+ }
if (!strcmp(k, "status.branch")) {
status_deferred_config.show_branch = git_config_bool(k, v);
return 0;
diff --git a/table.c b/table.c
new file mode 100644
index 0000000000..15600e117f
--- /dev/null
+++ b/table.c
@@ -0,0 +1,117 @@
+#define USE_THE_INDEX_VARIABLE
+#include "builtin.h"
+#include "gettext.h"
+#include "strbuf.h"
+#include "wt-status.h"
+#include "config.h"
+#include "string-list.h"
+#include "sys/ioctl.h"
+
+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;
+}
+
+static void build_table_border(struct strbuf *buf, int cols)
+{
+ strbuf_reset(buf);
+ strbuf_addchars(buf, '-', cols);
+}
+
+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);
+}
+
+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"));
+}
+
+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;
+
+ /* Ensure table is divisible into 3 even columns */
+ while (((cols - 1) % 3) > 0 || !(cols % 2)) {
+ cols -= 1;
+ }
+
+ build_table_border(&table_border, cols);
+ 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);
+
+ /* Draw table header */
+ printf(_("%s\n"), table_border.buf);
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ printf(_("%s\n"), table_border.buf);
+
+ /* Draw table body */
+ for_each_string_list_item(item, &s->untracked) {
+ build_table_entry(&table_col_entry_1, item->string, cols);
+ build_table_entry(&table_col_entry_2, "", cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ }
+
+ for_each_string_list_item(item, &s->change) {
+ struct wt_status_change_data *d = item->util;
+ if (d->worktree_status && d->index_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, item->string, cols);
+ build_table_entry(&table_col_entry_3, item->string, cols);
+ } else if (d->worktree_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, item->string, cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+ } else if (d->index_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, "", cols);
+ build_table_entry(&table_col_entry_3, item->string, cols);
+ }
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ }
+
+ if (!s->untracked.nr && !s->change.nr) {
+ build_table_entry(&table_col_entry_1, "-", cols);
+ build_table_entry(&table_col_entry_2, "-", cols);
+ build_table_entry(&table_col_entry_3, "-", cols);
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ }
+
+ printf(_("%s\n"), table_border.buf);
+ strbuf_release(&table_border);
+ strbuf_release(&table_col_entry_1);
+ strbuf_release(&table_col_entry_2);
+ strbuf_release(&table_col_entry_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 */
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..712807aa8f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -31,6 +31,7 @@
#include "lockfile.h"
#include "sequencer.h"
#include "fsmonitor-settings.h"
+#include "table.h"
#define AB_DELAY_WARNING_IN_MS (2 * 1000)
#define UF_DELAY_WARNING_IN_MS (2 * 1000)
@@ -1833,39 +1834,46 @@ static void wt_longstatus_print_state(struct wt_status *s)
show_sparse_checkout_in_use(s, state_color);
}
-static void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print_onwhat(struct wt_status *s, const char *branch_name)
{
+ const char *on_what = _("On branch ");
const char *branch_color = color(WT_STATUS_ONBRANCH, s);
const char *branch_status_color = color(WT_STATUS_HEADER, s);
+
+ if (!strcmp(branch_name, "HEAD")) {
+ branch_status_color = color(WT_STATUS_NOBRANCH, s);
+ if (s->state.rebase_in_progress ||
+ s->state.rebase_interactive_in_progress) {
+ if (s->state.rebase_interactive_in_progress)
+ on_what = _("interactive rebase in progress; onto ");
+ else
+ on_what = _("rebase in progress; onto ");
+ branch_name = s->state.onto;
+ } else if (s->state.detached_from) {
+ branch_name = s->state.detached_from;
+ if (s->state.detached_at)
+ on_what = _("HEAD detached at ");
+ else
+ on_what = _("HEAD detached from ");
+ } else {
+ branch_name = "";
+ on_what = _("Not currently on any branch.");
+ }
+ } else
+ skip_prefix(branch_name, "refs/heads/", &branch_name);
+
+ status_printf_more(s, branch_status_color, "%s", on_what);
+ status_printf_more(s, branch_color, "%s\n", branch_name);
+}
+
+static void wt_longstatus_print(struct wt_status *s)
+{
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
if (s->branch) {
- const char *on_what = _("On branch ");
const char *branch_name = s->branch;
- if (!strcmp(branch_name, "HEAD")) {
- branch_status_color = color(WT_STATUS_NOBRANCH, s);
- if (s->state.rebase_in_progress ||
- s->state.rebase_interactive_in_progress) {
- if (s->state.rebase_interactive_in_progress)
- on_what = _("interactive rebase in progress; onto ");
- else
- on_what = _("rebase in progress; onto ");
- branch_name = s->state.onto;
- } else if (s->state.detached_from) {
- branch_name = s->state.detached_from;
- if (s->state.detached_at)
- on_what = _("HEAD detached at ");
- else
- on_what = _("HEAD detached from ");
- } else {
- branch_name = "";
- on_what = _("Not currently on any branch.");
- }
- } else
- skip_prefix(branch_name, "refs/heads/", &branch_name);
status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
- status_printf_more(s, branch_status_color, "%s", on_what);
- status_printf_more(s, branch_color, "%s\n", branch_name);
+ wt_longstatus_print_onwhat(s, branch_name);
if (!s->is_initial)
wt_longstatus_print_tracking(s);
}
@@ -2133,6 +2141,17 @@ static void wt_shortstatus_print(struct wt_status *s)
wt_shortstatus_other(it, s, "!!");
}
+static void wt_noobstatus_print(struct wt_status *s)
+{
+ if (s->show_branch) {
+ const char *branch_name = s->branch;
+ wt_longstatus_print_onwhat(s, branch_name);
+ wt_longstatus_print_tracking(s);
+ }
+
+ print_noob_status(s);
+}
+
static void wt_porcelain_print(struct wt_status *s)
{
s->use_color = 0;
@@ -2560,6 +2579,9 @@ void wt_status_print(struct wt_status *s)
case STATUS_FORMAT_LONG:
wt_longstatus_print(s);
break;
+ case STATUS_FORMAT_NOOB:
+ wt_noobstatus_print(s);
+ break;
}
trace2_region_leave("status", "print", s->repo);
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..3f08f0d72b 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -73,6 +73,7 @@ enum wt_status_format {
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN,
STATUS_FORMAT_PORCELAIN_V2,
+ STATUS_FORMAT_NOOB,
STATUS_FORMAT_UNSPECIFIED
};
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [RFC PATCH v2 2/6] status: handle long paths in noob format
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
table.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
table.h | 2 +-
wt-status.c | 2 +-
3 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/table.c b/table.c
index 15600e117f..d085f2a098 100644
--- a/table.c
+++ b/table.c
@@ -25,15 +25,44 @@ static void build_table_border(struct strbuf *buf, int cols)
static void build_table_entry(struct strbuf *buf, char *entry, int cols)
{
+ int len = strlen(entry);
+ size_t col_width = (cols / 3) - 5; /* subtract for padding */
+
strbuf_reset(buf);
- strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+
+ /* Trim equally from both sides if it doesn't fit in column */
+ if (len > col_width) {
+ struct strbuf start_buf = STRBUF_INIT;
+ struct strbuf end_buf = STRBUF_INIT;
+ struct strbuf entry_buf = STRBUF_INIT;
+
+ strbuf_addstr(&start_buf, entry);
+ strbuf_addstr(&end_buf, entry);
+
+ strbuf_remove(&start_buf, col_width / 2, len - col_width / 2);
+ strbuf_remove(&end_buf, 0, len - col_width / 2);
+
+ strbuf_addstr(&entry_buf, start_buf.buf);
+ strbuf_addstr(&entry_buf, "...");
+ strbuf_addstr(&entry_buf, end_buf.buf);
+
+ entry = strbuf_detach(&entry_buf, &col_width);
+ len = strlen(entry);
+
+ strbuf_release(&start_buf);
+ strbuf_release(&end_buf);
+ strbuf_release(&entry_buf);
+ }
+
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2); /* left padding */
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);
+ /* right padding */
+ if (!(len % 2))
+ /* Bump right padding if entry length is odd */
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2 + 1);
else
- strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
@@ -47,7 +76,7 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
printf(_("|\n"));
}
-void print_noob_status(struct wt_status *s)
+void print_noob_status(struct wt_status *s, int add_advice)
{
struct winsize w;
int cols;
@@ -66,14 +95,29 @@ void print_noob_status(struct wt_status *s)
cols -= 1;
}
+ /* Draw table header */
build_table_border(&table_border, cols);
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);
- /* Draw table header */
printf(_("%s\n"), table_border.buf);
printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+ if (add_advice) {
+ build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
+ build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
+ build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
+
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ }
+
printf(_("%s\n"), table_border.buf);
/* Draw table body */
diff --git a/table.h b/table.h
index c9e8c386de..5dff7162a4 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
#ifndef TABLE_H
#define TABLE_H
-void print_noob_status(struct wt_status *s);
+void print_noob_status(struct wt_status *s, int i);
#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 712807aa8f..b5899dcc98 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2149,7 +2149,7 @@ static void wt_noobstatus_print(struct wt_status *s)
wt_longstatus_print_tracking(s);
}
- print_noob_status(s);
+ print_noob_status(s, 0);
}
static void wt_porcelain_print(struct wt_status *s)
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [RFC PATCH v2 4/6] add: set unique color for noob mode arrows
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
table.c | 64 +++++++++++++++++++++++++++++++----------------------
wt-status.c | 1 +
wt-status.h | 1 +
3 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/table.c b/table.c
index 527e38c07d..d29b311440 100644
--- a/table.c
+++ b/table.c
@@ -5,6 +5,7 @@
#include "wt-status.h"
#include "config.h"
#include "string-list.h"
+#include "color.h"
#include "sys/ioctl.h"
static const char *color(int slot, struct wt_status *s)
@@ -26,7 +27,7 @@ static void build_table_border(struct strbuf *buf, int cols)
static void build_table_entry(struct strbuf *buf, char *entry, int cols)
{
int len = strlen(entry);
- size_t col_width = (cols / 3) - 5; /* subtract for padding */
+ size_t col_width = (cols / 3) - 9; /* subtract for padding */
strbuf_reset(buf);
@@ -65,52 +66,51 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
-static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
{
struct strbuf empty = STRBUF_INIT;
struct strbuf trimmed = STRBUF_INIT;
- struct strbuf holder = STRBUF_INIT;
int len = strlen(buf->buf);
+ strbuf_reset(arrow);
strbuf_addstr(&trimmed, buf->buf);
strbuf_trim(&trimmed);
if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
strbuf_reset(buf);
- strbuf_addchars(buf, '-', len + 1);
+ strbuf_addchars(arrow, '-', len + 1);
} else if (add_after_entry) {
strbuf_rtrim(buf);
- strbuf_addchars(buf, ' ', 1);
- strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+ strbuf_addchars(arrow, ' ', 1);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
} else if (!add_after_entry) {
strbuf_ltrim(buf);
- strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
- strbuf_addchars(&holder, '>', 1);
- strbuf_addchars(&holder, ' ', 1);
- strbuf_addstr(&holder, buf->buf);
- strbuf_reset(buf);
- strbuf_addstr(buf, holder.buf);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+ strbuf_addchars(arrow, '>', 1);
+ strbuf_addchars(arrow, ' ', 1);
}
}
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
{
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+ if (strlen(arrow1->buf) > 0)
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow1->buf);
if (hide_pipe != 1 && hide_pipe != 3)
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+ if (strlen(arrow2->buf) > 0)
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow2->buf);
if (hide_pipe != 2 && hide_pipe != 3)
printf(_("|"));
+ if (strlen(arrow3->buf) > 0) {
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow3->buf);
+ }
color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
printf(_("|\n"));
}
-static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
-{
- print_table_body_line(buf1, buf2, buf3, s, 0);
-}
-
void print_noob_status(struct wt_status *s, int advice)
{
struct winsize w;
@@ -119,6 +119,9 @@ void print_noob_status(struct wt_status *s, int advice)
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 strbuf arrow_1 = STRBUF_INIT;
+ struct strbuf arrow_2 = STRBUF_INIT;
+ struct strbuf arrow_3 = STRBUF_INIT;
struct string_list_item *item, *item2;
/* Get terminal width */
@@ -170,17 +173,21 @@ void print_noob_status(struct wt_status *s, int advice)
strbuf_addstr(&buf_2, item2->string);
if (!strbuf_cmp(&buf_1, &buf_2)) {
build_table_entry(&table_col_entry_3, buf_1.buf, cols);
- add_arrow_to_entry(&table_col_entry_1, 1);
- add_arrow_to_entry(&table_col_entry_2, 0);
- add_arrow_to_entry(&table_col_entry_3, 0);
+ build_arrow(&table_col_entry_1, &arrow_1, 1);
+ build_arrow(&table_col_entry_2, &arrow_2, 0);
+ build_arrow(&table_col_entry_3, &arrow_3, 0);
is_arrow = 1;
}
}
if (!is_arrow)
- print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
else
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 3);
+
+ strbuf_reset(&arrow_1);
+ strbuf_reset(&arrow_2);
+ strbuf_reset(&arrow_3);
}
for_each_string_list_item(item, &s->change) {
@@ -203,8 +210,8 @@ void print_noob_status(struct wt_status *s, int advice)
strbuf_addstr(&buf_2, item2->string);
if (!strbuf_cmp(&buf_1, &buf_2)) {
build_table_entry(&table_col_entry_3, buf_1.buf, cols);
- add_arrow_to_entry(&table_col_entry_2, 1);
- add_arrow_to_entry(&table_col_entry_3, 0);
+ build_arrow(&table_col_entry_2, &arrow_2, 1);
+ build_arrow(&table_col_entry_3, &arrow_3, 0);
is_arrow = 1;
}
}
@@ -215,9 +222,12 @@ void print_noob_status(struct wt_status *s, int advice)
}
if (!is_arrow)
- print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
else
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 2);
+ strbuf_reset(&arrow_1);
+ strbuf_reset(&arrow_2);
+ strbuf_reset(&arrow_3);
}
if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 969f79f441..1332d07dba 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -49,6 +49,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_GREEN, /* WT_STATUS_LOCAL_BRANCH */
GIT_COLOR_RED, /* WT_STATUS_REMOTE_BRANCH */
GIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */
+ GIT_COLOR_CYAN, /* WT_STATUS_ARROW */
};
static const char *color(int slot, struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 64551f3a75..7b883fd476 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -19,6 +19,7 @@ enum color_wt_status {
WT_STATUS_LOCAL_BRANCH,
WT_STATUS_REMOTE_BRANCH,
WT_STATUS_ONBRANCH,
+ WT_STATUS_ARROW,
WT_STATUS_MAXSLOT
};
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [RFC PATCH v2 3/6] add: implement noob mode
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
Makefile | 1 +
builtin/add.c | 47 ++++++++---
builtin/commit.c | 163 +-------------------------------------
commit.c | 2 +
noob.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++
noob.h | 21 +++++
read-cache-ll.h | 9 ++-
read-cache.c | 32 ++++++--
table.c | 92 +++++++++++++++++++---
wt-status.c | 1 +
wt-status.h | 1 +
11 files changed, 381 insertions(+), 186 deletions(-)
create mode 100644 noob.c
create mode 100644 noob.h
diff --git a/Makefile b/Makefile
index a7399ca8f0..78acfaf14d 100644
--- a/Makefile
+++ b/Makefile
@@ -1070,6 +1070,7 @@ LIB_OBJS += name-hash.o
LIB_OBJS += negotiator/default.o
LIB_OBJS += negotiator/noop.o
LIB_OBJS += negotiator/skipping.o
+LIB_OBJS += noob.o
LIB_OBJS += notes-cache.o
LIB_OBJS += notes-merge.o
LIB_OBJS += notes-utils.o
diff --git a/builtin/add.c b/builtin/add.c
index c27254a5cd..dbb99d179e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -27,6 +27,9 @@
#include "strvec.h"
#include "submodule.h"
#include "add-interactive.h"
+#include "wt-status.h"
+#include "commit.h"
+#include "noob.h"
static const char * const builtin_add_usage[] = {
N_("git add [<options>] [--] <pathspec>..."),
@@ -322,7 +325,7 @@ static void check_embedded_repo(const char *path)
strbuf_release(&name);
}
-static int add_files(struct dir_struct *dir, int flags)
+static int add_files(struct dir_struct *dir, int flags, struct wt_status *status)
{
int i, exit_status = 0;
struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;
@@ -345,7 +348,7 @@ static int add_files(struct dir_struct *dir, int flags)
dir->entries[i]->name);
continue;
}
- if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
+ if (add_file_to_index_with_status(&the_index, dir->entries[i]->name, flags, status)) {
if (!ignore_add_errors)
die(_("adding files failed"));
exit_status = 1;
@@ -374,8 +377,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int require_pathspec;
char *seen = NULL;
struct lock_file lock_file = LOCK_INIT;
-
+ struct wt_status status;
+ unsigned int progress_flag = 0;
+
+ wt_status_prepare(the_repository, &status);
git_config(add_config, NULL);
+ git_config(git_status_config, &status);
+ finalize_deferred_config(&status);
+ status.status_format = status_format;
argc = parse_options(argc, argv, prefix, builtin_add_options,
builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
@@ -459,7 +468,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
(intent_to_add ? ADD_CACHE_INTENT : 0) |
(ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
(!(addremove || take_worktree_changes)
- ? ADD_CACHE_IGNORE_REMOVAL : 0));
+ ? ADD_CACHE_IGNORE_REMOVAL : 0) |
+ (status.status_format == STATUS_FORMAT_NOOB ? ADD_CACHE_FORMAT_NOOB : 0));
if (repo_read_index_preload(the_repository, &pathspec, 0) < 0)
die(_("index file corrupt"));
@@ -551,15 +561,32 @@ int cmd_add(int argc, const char **argv, const char *prefix)
begin_odb_transaction();
- if (add_renormalize)
+ if (status.status_format == STATUS_FORMAT_NOOB) {
+ /* Read index and populate status */
+ repo_read_index(the_repository);
+ refresh_index(&the_index,
+ REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+ &status.pathspec, NULL, NULL);
+ status.show_branch = 0;
+ wt_status_collect(&status);
+ }
+
+ if (add_renormalize) {
exit_status |= renormalize_tracked_files(&pathspec, flags);
- else
- exit_status |= add_files_to_cache(the_repository, prefix,
+ } else {
+ exit_status |= add_files_to_cache_with_status(the_repository, prefix,
&pathspec, include_sparse,
- flags);
+ flags, &status);
+ }
- if (add_new_files)
- exit_status |= add_files(&dir, flags);
+ if (add_new_files) {
+ exit_status |= add_files(&dir, flags, &status);
+ }
+
+ if (status.status_format == STATUS_FORMAT_NOOB) {
+ wt_status_print(&status);
+ wt_status_collect_free_buffers(&status);
+ }
if (chmod_arg && pathspec.nr)
exit_status |= chmod_pathspec(&pathspec, chmod_arg[0], show_only);
diff --git a/builtin/commit.c b/builtin/commit.c
index 880c42f5b7..3f816c117d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -46,6 +46,7 @@
#include "commit-reach.h"
#include "commit-graph.h"
#include "pretty.h"
+#include "noob.h"
static const char * const builtin_commit_usage[] = {
N_("git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n"
@@ -148,8 +149,6 @@ static int use_editor = 1, include_status = 1;
static int have_option_m;
static struct strbuf message = STRBUF_INIT;
-static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
-
static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
{
BUG_ON_OPT_NEG(unset);
@@ -310,7 +309,7 @@ static void add_remove_files(struct string_list *list)
continue;
if (!lstat(p->string, &st)) {
- if (add_to_index(&the_index, p->string, &st, 0))
+ if (add_file_to_index(&the_index, p->string, 0))
die(_("updating files failed"));
} else
remove_file_from_index(&the_index, p->string);
@@ -1196,59 +1195,6 @@ static const char *read_commit_message(const char *name)
return repo_logmsg_reencode(the_repository, commit, NULL, out_enc);
}
-/*
- * Enumerate what needs to be propagated when --porcelain
- * is not in effect here.
- */
-static struct status_deferred_config {
- enum wt_status_format status_format;
- int show_branch;
- enum ahead_behind_flags ahead_behind;
-} status_deferred_config = {
- STATUS_FORMAT_UNSPECIFIED,
- -1, /* unspecified */
- AHEAD_BEHIND_UNSPECIFIED,
-};
-
-static void finalize_deferred_config(struct wt_status *s)
-{
- int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
- status_format != STATUS_FORMAT_PORCELAIN_V2 &&
- !s->null_termination);
-
- if (s->null_termination) {
- if (status_format == STATUS_FORMAT_NONE ||
- status_format == STATUS_FORMAT_UNSPECIFIED)
- status_format = STATUS_FORMAT_PORCELAIN;
- else if (status_format == STATUS_FORMAT_LONG)
- die(_("options '%s' and '%s' cannot be used together"), "--long", "-z");
- }
-
- if (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED)
- status_format = status_deferred_config.status_format;
- if (status_format == STATUS_FORMAT_UNSPECIFIED)
- status_format = STATUS_FORMAT_NONE;
-
- if (use_deferred_config && s->show_branch < 0)
- s->show_branch = status_deferred_config.show_branch;
- if (s->show_branch < 0)
- s->show_branch = 0;
-
- /*
- * If the user did not give a "--[no]-ahead-behind" command
- * line argument *AND* we will print in a human-readable format
- * (short, long etc.) then we inherit from the status.aheadbehind
- * config setting. In all other cases (and porcelain V[12] formats
- * in particular), we inherit _FULL for backwards compatibility.
- */
- if (use_deferred_config &&
- s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
- s->ahead_behind_flags = status_deferred_config.ahead_behind;
-
- if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
- s->ahead_behind_flags = AHEAD_BEHIND_FULL;
-}
-
static void check_fixup_reword_options(int argc, const char *argv[]) {
if (whence != FROM_COMMIT) {
if (whence == FROM_MERGE)
@@ -1399,111 +1345,6 @@ static int dry_run_commit(const char **argv, const char *prefix,
define_list_config_array_extra(color_status_slots, {"added"});
-static int parse_status_slot(const char *slot)
-{
- if (!strcasecmp(slot, "added"))
- return WT_STATUS_UPDATED;
-
- return LOOKUP_CONFIG(color_status_slots, slot);
-}
-
-static int git_status_config(const char *k, const char *v,
- const struct config_context *ctx, void *cb)
-{
- struct wt_status *s = cb;
- const char *slot_name;
-
- if (starts_with(k, "column."))
- return git_column_config(k, v, "status", &s->colopts);
- if (!strcmp(k, "status.submodulesummary")) {
- int is_bool;
- s->submodule_summary = git_config_bool_or_int(k, v, ctx->kvi,
- &is_bool);
- if (is_bool && s->submodule_summary)
- s->submodule_summary = -1;
- return 0;
- }
- if (!strcmp(k, "status.short")) {
- if (git_config_bool(k, v))
- status_deferred_config.status_format = STATUS_FORMAT_SHORT;
- else
- status_deferred_config.status_format = STATUS_FORMAT_NONE;
- return 0;
- }
- if (!strcmp(k, "status.noob")) {
- if (git_config_bool(k, v))
- status_deferred_config.status_format = STATUS_FORMAT_NOOB;
- else
- status_deferred_config.status_format = STATUS_FORMAT_NONE;
- return 0;
- }
- if (!strcmp(k, "status.branch")) {
- status_deferred_config.show_branch = git_config_bool(k, v);
- return 0;
- }
- if (!strcmp(k, "status.aheadbehind")) {
- status_deferred_config.ahead_behind = git_config_bool(k, v);
- return 0;
- }
- if (!strcmp(k, "status.showstash")) {
- s->show_stash = git_config_bool(k, v);
- return 0;
- }
- if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
- s->use_color = git_config_colorbool(k, v);
- return 0;
- }
- if (!strcmp(k, "status.displaycommentprefix")) {
- s->display_comment_prefix = git_config_bool(k, v);
- return 0;
- }
- if (skip_prefix(k, "status.color.", &slot_name) ||
- skip_prefix(k, "color.status.", &slot_name)) {
- int slot = parse_status_slot(slot_name);
- if (slot < 0)
- return 0;
- if (!v)
- return config_error_nonbool(k);
- return color_parse(v, s->color_palette[slot]);
- }
- if (!strcmp(k, "status.relativepaths")) {
- s->relative_paths = git_config_bool(k, v);
- return 0;
- }
- if (!strcmp(k, "status.showuntrackedfiles")) {
- if (!v)
- return config_error_nonbool(k);
- else if (!strcmp(v, "no"))
- s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
- else if (!strcmp(v, "normal"))
- s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
- else if (!strcmp(v, "all"))
- s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
- else
- return error(_("Invalid untracked files mode '%s'"), v);
- return 0;
- }
- if (!strcmp(k, "diff.renamelimit")) {
- if (s->rename_limit == -1)
- s->rename_limit = git_config_int(k, v, ctx->kvi);
- return 0;
- }
- if (!strcmp(k, "status.renamelimit")) {
- s->rename_limit = git_config_int(k, v, ctx->kvi);
- return 0;
- }
- if (!strcmp(k, "diff.renames")) {
- if (s->detect_rename == -1)
- s->detect_rename = git_config_rename(k, v);
- return 0;
- }
- if (!strcmp(k, "status.renames")) {
- s->detect_rename = git_config_rename(k, v);
- return 0;
- }
- return git_diff_ui_config(k, v, ctx, NULL);
-}
-
int cmd_status(int argc, const char **argv, const char *prefix)
{
static int no_renames = -1;
diff --git a/commit.c b/commit.c
index b3223478bc..c08faf48fd 100644
--- a/commit.c
+++ b/commit.c
@@ -28,6 +28,8 @@
#include "shallow.h"
#include "tree.h"
#include "hook.h"
+#include "column.h"
+#include "config.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
diff --git a/noob.c b/noob.c
new file mode 100644
index 0000000000..680d461698
--- /dev/null
+++ b/noob.c
@@ -0,0 +1,198 @@
+#include "git-compat-util.h"
+#include "tag.h"
+#include "commit.h"
+#include "commit-graph.h"
+#include "environment.h"
+#include "gettext.h"
+#include "hex.h"
+#include "repository.h"
+#include "object-name.h"
+#include "object-store-ll.h"
+#include "pkt-line.h"
+#include "utf8.h"
+#include "diff.h"
+#include "revision.h"
+#include "notes.h"
+#include "alloc.h"
+#include "gpg-interface.h"
+#include "mergesort.h"
+#include "commit-slab.h"
+#include "prio-queue.h"
+#include "hash-lookup.h"
+#include "wt-status.h"
+#include "advice.h"
+#include "refs.h"
+#include "commit-reach.h"
+#include "run-command.h"
+#include "setup.h"
+#include "shallow.h"
+#include "tree.h"
+#include "hook.h"
+#include "column.h"
+#include "config.h"
+#include "noob.h"
+
+static const char *color_status_slots[] = {
+ [WT_STATUS_HEADER] = "header",
+ [WT_STATUS_UPDATED] = "updated",
+ [WT_STATUS_CHANGED] = "changed",
+ [WT_STATUS_UNTRACKED] = "untracked",
+ [WT_STATUS_NOBRANCH] = "noBranch",
+ [WT_STATUS_UNMERGED] = "unmerged",
+ [WT_STATUS_LOCAL_BRANCH] = "localBranch",
+ [WT_STATUS_REMOTE_BRANCH] = "remoteBranch",
+ [WT_STATUS_ONBRANCH] = "branch",
+};
+
+enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
+
+struct status_deferred_config status_deferred_config = {
+ STATUS_FORMAT_UNSPECIFIED,
+ -1, /* unspecified */
+ AHEAD_BEHIND_UNSPECIFIED,
+};
+
+int parse_status_slot(const char *slot)
+{
+ if (!strcasecmp(slot, "added"))
+ return WT_STATUS_UPDATED;
+
+ return LOOKUP_CONFIG(color_status_slots, slot);
+}
+
+int git_status_config(const char *k, const char *v,
+ const struct config_context *ctx, void *cb)
+{
+ struct wt_status *s = cb;
+ const char *slot_name;
+
+ if (starts_with(k, "column."))
+ return git_column_config(k, v, "status", &s->colopts);
+ if (!strcmp(k, "status.submodulesummary")) {
+ int is_bool;
+ s->submodule_summary = git_config_bool_or_int(k, v, ctx->kvi,
+ &is_bool);
+ if (is_bool && s->submodule_summary)
+ s->submodule_summary = -1;
+ return 0;
+ }
+ if (!strcmp(k, "status.short")) {
+ if (git_config_bool(k, v))
+ status_deferred_config.status_format = STATUS_FORMAT_SHORT;
+ else
+ status_deferred_config.status_format = STATUS_FORMAT_NONE;
+ return 0;
+ }
+ if (!strcmp(k, "status.noob")) {
+ if (git_config_bool(k, v))
+ status_deferred_config.status_format = STATUS_FORMAT_NOOB;
+ else
+ status_deferred_config.status_format = STATUS_FORMAT_NONE;
+ return 0;
+ }
+ if (!strcmp(k, "status.branch")) {
+ status_deferred_config.show_branch = git_config_bool(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.aheadbehind")) {
+ status_deferred_config.ahead_behind = git_config_bool(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.showstash")) {
+ s->show_stash = git_config_bool(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
+ s->use_color = git_config_colorbool(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.displaycommentprefix")) {
+ s->display_comment_prefix = git_config_bool(k, v);
+ return 0;
+ }
+ if (skip_prefix(k, "status.color.", &slot_name) ||
+ skip_prefix(k, "color.status.", &slot_name)) {
+ int slot = parse_status_slot(slot_name);
+ if (slot < 0)
+ return 0;
+ if (!v)
+ return config_error_nonbool(k);
+ return color_parse(v, s->color_palette[slot]);
+ }
+ if (!strcmp(k, "status.relativepaths")) {
+ s->relative_paths = git_config_bool(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.showuntrackedfiles")) {
+ if (!v)
+ return config_error_nonbool(k);
+ else if (!strcmp(v, "no"))
+ s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
+ else if (!strcmp(v, "normal"))
+ s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
+ else if (!strcmp(v, "all"))
+ s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
+ else
+ return error(_("Invalid untracked files mode '%s'"), v);
+ return 0;
+ }
+ if (!strcmp(k, "diff.renamelimit")) {
+ if (s->rename_limit == -1)
+ s->rename_limit = git_config_int(k, v, ctx->kvi);
+ return 0;
+ }
+ if (!strcmp(k, "status.renamelimit")) {
+ s->rename_limit = git_config_int(k, v, ctx->kvi);
+ return 0;
+ }
+ if (!strcmp(k, "diff.renames")) {
+ if (s->detect_rename == -1)
+ s->detect_rename = git_config_rename(k, v);
+ return 0;
+ }
+ if (!strcmp(k, "status.renames")) {
+ s->detect_rename = git_config_rename(k, v);
+ return 0;
+ }
+ return git_diff_ui_config(k, v, ctx, NULL);
+}
+
+void finalize_deferred_config(struct wt_status *s)
+{
+ int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+ status_format != STATUS_FORMAT_PORCELAIN_V2 &&
+ !s->null_termination);
+
+ if (s->null_termination) {
+ if (status_format == STATUS_FORMAT_NONE ||
+ status_format == STATUS_FORMAT_UNSPECIFIED)
+ status_format = STATUS_FORMAT_PORCELAIN;
+ else if (status_format == STATUS_FORMAT_LONG)
+ die(_("options '%s' and '%s' cannot be used together"), "--long", "-z");
+ }
+
+ if (use_deferred_config && status_format == STATUS_FORMAT_UNSPECIFIED) {
+ status_format = status_deferred_config.status_format;
+ }
+ if (status_format == STATUS_FORMAT_UNSPECIFIED)
+ status_format = STATUS_FORMAT_NONE;
+
+ if (use_deferred_config && s->show_branch < 0)
+ s->show_branch = status_deferred_config.show_branch;
+ if (s->show_branch < 0)
+ s->show_branch = 0;
+
+ /*
+ * If the user did not give a "--[no]-ahead-behind" command
+ * line argument *AND* we will print in a human-readable format
+ * (short, long etc.) then we inherit from the status.aheadbehind
+ * config setting. In all other cases (and porcelain V[12] formats
+ * in particular), we inherit _FULL for backwards compatibility.
+ */
+ if (use_deferred_config &&
+ s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+ s->ahead_behind_flags = status_deferred_config.ahead_behind;
+
+ if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+ s->ahead_behind_flags = AHEAD_BEHIND_FULL;
+}
diff --git a/noob.h b/noob.h
new file mode 100644
index 0000000000..d5bc073594
--- /dev/null
+++ b/noob.h
@@ -0,0 +1,21 @@
+#ifndef NOOB_H
+#define NOOB_H
+
+struct status_deferred_config {
+ enum wt_status_format status_format;
+ int show_branch;
+ enum ahead_behind_flags ahead_behind;
+};
+
+extern enum wt_status_format status_format;
+
+extern struct status_deferred_config status_deferred_config;
+
+int git_status_config(const char *k, const char *v,
+ const struct config_context *ctx, void *cb);
+
+int parse_status_slot(const char *slot);
+
+void finalize_deferred_config(struct wt_status *s);
+
+#endif /* NOOB_H */
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 9a1a7edc5a..302a075714 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -4,6 +4,7 @@
#include "hash-ll.h"
#include "hashmap.h"
#include "statinfo.h"
+#include "wt-status.h"
/*
* Basic data structures for the directory cache
@@ -395,6 +396,7 @@ int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_IGNORE_ERRORS 4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
+#define ADD_CACHE_FORMAT_NOOB 32
/*
* These two are used to add the contents of the file at path
* to the index, marking the working tree up-to-date by storing
@@ -404,7 +406,8 @@ int remove_file_from_index(struct index_state *, const char *path);
* the latter will do necessary lstat(2) internally before
* calling the former.
*/
-int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+int add_to_index(struct index_state *, const char *path, struct stat *, int flags, struct wt_status *status);
+int add_file_to_index_with_status(struct index_state *, const char *path, int flags, struct wt_status *status);
int add_file_to_index(struct index_state *, const char *path, int flags);
int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
@@ -475,6 +478,10 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
const struct pathspec *pathspec, int include_sparse,
int flags);
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+ const struct pathspec *pathspec, int include_sparse,
+ int flags, struct wt_status *status);
+
void overlay_tree_on_index(struct index_state *istate,
const char *tree_name, const char *prefix);
diff --git a/read-cache.c b/read-cache.c
index 080bd39713..319415430a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -45,6 +45,8 @@
#include "csum-file.h"
#include "promisor-remote.h"
#include "hook.h"
+#include "wt-status.h"
+#include "string-list.h"
/* Mask for the name length in ce_flags in the on-disk index */
@@ -664,7 +666,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
oidcpy(&ce->oid, &oid);
}
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, struct wt_status *status)
{
int namelen, was_same;
mode_t st_mode = st->st_mode;
@@ -672,6 +674,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE|CE_MATCH_RACY_IS_DIRTY;
int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
int pretend = flags & ADD_CACHE_PRETEND;
+ int noob = flags & ADD_CACHE_FORMAT_NOOB;
int intent_only = flags & ADD_CACHE_INTENT;
int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
(intent_only ? ADD_CACHE_NEW_ONLY : 0));
@@ -760,17 +763,26 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
discard_cache_entry(ce);
return error(_("unable to add '%s' to index"), path);
}
- if (verbose && !was_same)
+ if (verbose && !was_same && !noob)
printf("add '%s'\n", path);
+ if (noob && !was_same) {
+ string_list_insert(&status->added, path);
+ }
return 0;
}
-int add_file_to_index(struct index_state *istate, const char *path, int flags)
+int add_file_to_index_with_status(struct index_state *istate, const char *path, int flags, struct wt_status *status)
{
struct stat st;
if (lstat(path, &st))
die_errno(_("unable to stat '%s'"), path);
- return add_to_index(istate, path, &st, flags);
+ return add_to_index(istate, path, &st, flags, status);
+}
+
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
+{
+ struct wt_status status;
+ return add_file_to_index_with_status(istate, path, flags, &status);
}
struct cache_entry *make_empty_cache_entry(struct index_state *istate, size_t len)
@@ -3872,6 +3884,7 @@ struct update_callback_data {
int include_sparse;
int flags;
int add_errors;
+ struct wt_status *status;
};
static int fix_unmerged_status(struct diff_filepair *p,
@@ -3914,7 +3927,7 @@ static void update_callback(struct diff_queue_struct *q,
die(_("unexpected diff status %c"), p->status);
case DIFF_STATUS_MODIFIED:
case DIFF_STATUS_TYPE_CHANGED:
- if (add_file_to_index(data->index, path, data->flags)) {
+ if (add_file_to_index_with_status(data->index, path, data->flags, data->status)) {
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
die(_("updating files failed"));
data->add_errors++;
@@ -3935,6 +3948,14 @@ static void update_callback(struct diff_queue_struct *q,
int add_files_to_cache(struct repository *repo, const char *prefix,
const struct pathspec *pathspec, int include_sparse,
int flags)
+{
+ struct wt_status status;
+ return add_files_to_cache_with_status(repo, prefix, pathspec, include_sparse, flags, &status);
+}
+
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+ const struct pathspec *pathspec, int include_sparse,
+ int flags, struct wt_status *status)
{
struct update_callback_data data;
struct rev_info rev;
@@ -3943,6 +3964,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
data.index = repo->index;
data.include_sparse = include_sparse;
data.flags = flags;
+ data.status = status;
repo_init_revisions(repo, &rev, prefix);
setup_revisions(0, NULL, &rev, NULL);
diff --git a/table.c b/table.c
index d085f2a098..527e38c07d 100644
--- a/table.c
+++ b/table.c
@@ -65,18 +65,53 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+{
+ struct strbuf empty = STRBUF_INIT;
+ struct strbuf trimmed = STRBUF_INIT;
+ struct strbuf holder = STRBUF_INIT;
+ int len = strlen(buf->buf);
+
+ strbuf_addstr(&trimmed, buf->buf);
+ strbuf_trim(&trimmed);
+
+ if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
+ strbuf_reset(buf);
+ strbuf_addchars(buf, '-', len + 1);
+ } else if (add_after_entry) {
+ strbuf_rtrim(buf);
+ strbuf_addchars(buf, ' ', 1);
+ strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+ } else if (!add_after_entry) {
+ strbuf_ltrim(buf);
+ strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
+ strbuf_addchars(&holder, '>', 1);
+ strbuf_addchars(&holder, ' ', 1);
+ strbuf_addstr(&holder, buf->buf);
+ strbuf_reset(buf);
+ strbuf_addstr(buf, holder.buf);
+ }
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
{
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
- printf(_("|"));
+ if (hide_pipe != 1 && hide_pipe != 3)
+ printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
- printf(_("|"));
+ if (hide_pipe != 2 && hide_pipe != 3)
+ printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
printf(_("|\n"));
}
-void print_noob_status(struct wt_status *s, int add_advice)
+static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+ print_table_body_line(buf1, buf2, buf3, s, 0);
+}
+
+void print_noob_status(struct wt_status *s, int advice)
{
struct winsize w;
int cols;
@@ -84,7 +119,7 @@ void print_noob_status(struct wt_status *s, int add_advice)
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;
+ struct string_list_item *item, *item2;
/* Get terminal width */
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
@@ -104,7 +139,7 @@ void print_noob_status(struct wt_status *s, int add_advice)
printf(_("%s\n"), table_border.buf);
printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
- if (add_advice) {
+ if (advice) {
build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
@@ -122,14 +157,38 @@ void print_noob_status(struct wt_status *s, int add_advice)
/* Draw table body */
for_each_string_list_item(item, &s->untracked) {
- build_table_entry(&table_col_entry_1, item->string, cols);
+ struct strbuf buf_1 = STRBUF_INIT;
+ struct strbuf buf_2 = STRBUF_INIT;
+ int is_arrow = 0;
+ strbuf_addstr(&buf_1, item->string);
+ build_table_entry(&table_col_entry_1, buf_1.buf, cols);
build_table_entry(&table_col_entry_2, "", cols);
build_table_entry(&table_col_entry_3, "", cols);
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+ for_each_string_list_item(item2, &s->added) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ add_arrow_to_entry(&table_col_entry_1, 1);
+ add_arrow_to_entry(&table_col_entry_2, 0);
+ add_arrow_to_entry(&table_col_entry_3, 0);
+ is_arrow = 1;
+ }
+ }
+
+ if (!is_arrow)
+ print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ else
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
}
for_each_string_list_item(item, &s->change) {
struct wt_status_change_data *d = item->util;
+ struct strbuf buf_1 = STRBUF_INIT;
+ struct strbuf buf_2 = STRBUF_INIT;
+ int is_arrow = 0;
+ strbuf_addstr(&buf_1, item->string);
if (d->worktree_status && d->index_status) {
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, item->string, cols);
@@ -138,12 +197,27 @@ void print_noob_status(struct wt_status *s, int add_advice)
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, item->string, cols);
build_table_entry(&table_col_entry_3, "", cols);
+
+ for_each_string_list_item(item2, &s->added) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ add_arrow_to_entry(&table_col_entry_2, 1);
+ add_arrow_to_entry(&table_col_entry_3, 0);
+ is_arrow = 1;
+ }
+ }
} else if (d->index_status) {
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, "", cols);
build_table_entry(&table_col_entry_3, item->string, cols);
}
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+ if (!is_arrow)
+ print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ else
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
}
if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index b5899dcc98..969f79f441 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -153,6 +153,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
s->change.strdup_strings = 1;
s->untracked.strdup_strings = 1;
s->ignored.strdup_strings = 1;
+ s->added.strdup_strings = 1;
s->show_branch = -1; /* unspecified */
s->show_stash = 0;
s->ahead_behind_flags = AHEAD_BEHIND_UNSPECIFIED;
diff --git a/wt-status.h b/wt-status.h
index 3f08f0d72b..64551f3a75 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -142,6 +142,7 @@ struct wt_status {
struct string_list change;
struct string_list untracked;
struct string_list ignored;
+ struct string_list added;
uint32_t untracked_in_ms;
};
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [RFC PATCH v2 5/6] restore: implement noob mode
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
builtin/checkout.c | 46 ++++++++++++++++++++++++++++-------
read-cache-ll.h | 1 +
read-cache.c | 9 ++++++-
table.c | 60 +++++++++++++++++++++++++++++++++++++++-------
wt-status.h | 1 +
5 files changed, 100 insertions(+), 17 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f02434bc15..afc414b0b1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -41,6 +41,7 @@
#include "entry.h"
#include "parallel-checkout.h"
#include "add-interactive.h"
+#include "noob.h"
static const char * const checkout_usage[] = {
N_("git checkout [<options>] <branch>"),
@@ -456,7 +457,8 @@ static int checkout_worktree(const struct checkout_opts *opts,
}
static int checkout_paths(const struct checkout_opts *opts,
- const struct branch_info *new_branch_info)
+ const struct branch_info *new_branch_info,
+ struct wt_status *status)
{
int pos;
static char *ps_matched;
@@ -598,8 +600,10 @@ static int checkout_paths(const struct checkout_opts *opts,
for (pos = 0; pos < the_index.cache_nr; pos++) {
const struct cache_entry *ce = the_index.cache[pos];
if (ce->ce_flags & CE_MATCHED) {
- if (!ce_stage(ce))
+ if (!ce_stage(ce)) {
+ string_list_insert(&status->restored, ce->name);
continue;
+ }
if (opts->ignore_unmerged) {
if (!opts->quiet)
warning(_("path '%s' is unmerged"), ce->name);
@@ -621,7 +625,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (opts->checkout_worktree)
errs |= checkout_worktree(opts, new_branch_info);
else
- remove_marked_cache_entries(&the_index, 1);
+ remove_marked_cache_entries_with_status(&the_index, 1, status);
/*
* Allow updating the index when checking out from the index.
@@ -1668,7 +1672,8 @@ static char cb_option = 'b';
static int checkout_main(int argc, const char **argv, const char *prefix,
struct checkout_opts *opts, struct option *options,
const char * const usagestr[],
- struct branch_info *new_branch_info)
+ struct branch_info *new_branch_info,
+ struct wt_status *status)
{
int parseopt_flags = 0;
@@ -1865,7 +1870,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
}
if (opts->patch_mode || opts->pathspec.nr)
- return checkout_paths(opts, new_branch_info);
+ return checkout_paths(opts, new_branch_info, status);
else
return checkout_branch(opts, new_branch_info);
}
@@ -1887,6 +1892,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
};
int ret;
struct branch_info new_branch_info = { 0 };
+ struct wt_status status;
memset(&opts, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
@@ -1917,7 +1923,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
options = add_checkout_path_options(&opts, options);
ret = checkout_main(argc, argv, prefix, &opts,
- options, checkout_usage, &new_branch_info);
+ options, checkout_usage,
+ &new_branch_info, &status);
branch_info_release(&new_branch_info);
clear_pathspec(&opts.pathspec);
free(opts.pathspec_from_file);
@@ -1942,6 +1949,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
};
int ret;
struct branch_info new_branch_info = { 0 };
+ struct wt_status status;
memset(&opts, 0, sizeof(opts));
opts.dwim_new_local_branch = 1;
@@ -1961,7 +1969,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
cb_option = 'c';
ret = checkout_main(argc, argv, prefix, &opts,
- options, switch_branch_usage, &new_branch_info);
+ options, switch_branch_usage,
+ &new_branch_info, &status);
branch_info_release(&new_branch_info);
FREE_AND_NULL(options);
return ret;
@@ -1985,6 +1994,13 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
};
int ret;
struct branch_info new_branch_info = { 0 };
+ struct wt_status status;
+ unsigned int progress_flag = 0;
+
+ wt_status_prepare(the_repository, &status);
+ git_config(git_status_config, &status);
+ finalize_deferred_config(&status);
+ status.status_format = status_format;
memset(&opts, 0, sizeof(opts));
opts.accept_ref = 0;
@@ -2000,7 +2016,21 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
options = add_checkout_path_options(&opts, options);
ret = checkout_main(argc, argv, prefix, &opts,
- options, restore_usage, &new_branch_info);
+ options, restore_usage,
+ &new_branch_info, &status);
+
+ if (status.status_format == STATUS_FORMAT_NOOB) {
+ /* Read index and populate status */
+ repo_read_index(the_repository);
+ refresh_index(&the_index,
+ REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+ &status.pathspec, NULL, NULL);
+ status.show_branch = 0;
+ wt_status_collect(&status);
+ wt_status_print(&status);
+ wt_status_collect_free_buffers(&status);
+ }
+
branch_info_release(&new_branch_info);
FREE_AND_NULL(options);
return ret;
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 302a075714..8bdc157196 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -389,6 +389,7 @@ void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
/* Remove entry, return true if there are more entries to go. */
int remove_index_entry_at(struct index_state *, int pos);
+void remove_marked_cache_entries_with_status(struct index_state *istate, int invalidate, struct wt_status *status);
void remove_marked_cache_entries(struct index_state *istate, int invalidate);
int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 319415430a..1c1a3290c0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -558,7 +558,7 @@ int remove_index_entry_at(struct index_state *istate, int pos)
* CE_REMOVE is set in ce_flags. This is much more effective than
* calling remove_index_entry_at() for each entry to be removed.
*/
-void remove_marked_cache_entries(struct index_state *istate, int invalidate)
+void remove_marked_cache_entries_with_status(struct index_state *istate, int invalidate, struct wt_status *status)
{
struct cache_entry **ce_array = istate->cache;
unsigned int i, j;
@@ -570,6 +570,7 @@ void remove_marked_cache_entries(struct index_state *istate, int invalidate)
ce_array[i]->name);
untracked_cache_remove_from_index(istate,
ce_array[i]->name);
+ string_list_insert(&status->restored, ce_array[i]->name);
}
remove_name_hash(istate, ce_array[i]);
save_or_free_index_entry(istate, ce_array[i]);
@@ -583,6 +584,12 @@ void remove_marked_cache_entries(struct index_state *istate, int invalidate)
istate->cache_nr = j;
}
+void remove_marked_cache_entries(struct index_state *istate, int invalidate)
+{
+ struct wt_status status;
+ remove_marked_cache_entries_with_status(istate, invalidate, &status);
+}
+
int remove_file_from_index(struct index_state *istate, const char *path)
{
int pos = index_name_pos(istate, path, strlen(path));
diff --git a/table.c b/table.c
index d29b311440..3602def17a 100644
--- a/table.c
+++ b/table.c
@@ -66,7 +66,7 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
-static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
+static void build_arrow_(struct strbuf *buf, struct strbuf* arrow, int add_after_entry, int reversed)
{
struct strbuf empty = STRBUF_INIT;
struct strbuf trimmed = STRBUF_INIT;
@@ -80,17 +80,38 @@ static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_
strbuf_reset(buf);
strbuf_addchars(arrow, '-', len + 1);
} else if (add_after_entry) {
- strbuf_rtrim(buf);
- strbuf_addchars(arrow, ' ', 1);
- strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+ if (!reversed) {
+ strbuf_rtrim(buf);
+ strbuf_addchars(arrow, ' ', 1);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+ } else {
+ strbuf_rtrim(buf);
+ strbuf_addchars(arrow, ' ', 1);
+ strbuf_addchars(arrow, '<', 1);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+ }
} else if (!add_after_entry) {
- strbuf_ltrim(buf);
- strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
- strbuf_addchars(arrow, '>', 1);
- strbuf_addchars(arrow, ' ', 1);
+ if (!reversed) {
+ strbuf_ltrim(buf);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+ strbuf_addchars(arrow, '>', 1);
+ strbuf_addchars(arrow, ' ', 1);
+ } else {
+ strbuf_ltrim(buf);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
+ strbuf_addchars(arrow, ' ', 1);
+ }
}
}
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry) {
+ build_arrow_(buf, arrow, add_after_entry, 0);
+}
+
+static void build_reversed_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry) {
+ build_arrow_(buf, arrow, add_after_entry, 1);
+}
+
static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
{
printf(_("|"));
@@ -180,6 +201,18 @@ void print_noob_status(struct wt_status *s, int advice)
}
}
+ for_each_string_list_item(item2, &s->restored) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ build_reversed_arrow(&table_col_entry_1, &arrow_1, 1);
+ build_reversed_arrow(&table_col_entry_2, &arrow_2, 0);
+ build_reversed_arrow(&table_col_entry_3, &arrow_3, 0);
+ is_arrow = 1;
+ }
+ }
+
if (!is_arrow)
print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
else
@@ -215,6 +248,17 @@ void print_noob_status(struct wt_status *s, int advice)
is_arrow = 1;
}
}
+
+ for_each_string_list_item(item2, &s->restored) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ build_reversed_arrow(&table_col_entry_2, &arrow_2, 1);
+ build_reversed_arrow(&table_col_entry_3, &arrow_3, 0);
+ is_arrow = 1;
+ }
+ }
} else if (d->index_status) {
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, "", cols);
diff --git a/wt-status.h b/wt-status.h
index 7b883fd476..c6bce8f74a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -144,6 +144,7 @@ struct wt_status {
struct string_list untracked;
struct string_list ignored;
struct string_list added;
+ struct string_list restored;
uint32_t untracked_in_ms;
};
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [RFC PATCH v2 6/6] status: add advice status hints as table footer
From: Jacob Stopak @ 2023-10-26 22:46 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
builtin/commit.c | 1 +
table.c | 42 +++++++++++++++++++++++++++---------------
table.h | 2 +-
wt-status.c | 3 ++-
wt-status.h | 2 ++
5 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 3f816c117d..b97943e642 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1434,6 +1434,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
s.ignore_submodule_arg = ignore_submodule_arg;
s.status_format = status_format;
s.verbose = verbose;
+ s.is_cmd_status = 1;
if (no_renames != -1)
s.detect_rename = !no_renames;
if ((intptr_t)rename_score_arg != -1) {
diff --git a/table.c b/table.c
index 3602def17a..36719e3d09 100644
--- a/table.c
+++ b/table.c
@@ -6,6 +6,7 @@
#include "config.h"
#include "string-list.h"
#include "color.h"
+#include "advice.h"
#include "sys/ioctl.h"
static const char *color(int slot, struct wt_status *s)
@@ -132,7 +133,18 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
printf(_("|\n"));
}
-void print_noob_status(struct wt_status *s, int advice)
+static void print_table_hint_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf1->buf);
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf2->buf);
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_HINT, s), "%s", buf3->buf);
+ printf(_("|\n"));
+}
+
+void print_noob_status(struct wt_status *s)
{
struct winsize w;
int cols;
@@ -163,20 +175,6 @@ void print_noob_status(struct wt_status *s, int advice)
printf(_("%s\n"), table_border.buf);
printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
- if (advice) {
- build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
- build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
- build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
-
- printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
-
- build_table_entry(&table_col_entry_1, "", cols);
- build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
- build_table_entry(&table_col_entry_3, "", cols);
-
- printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
- }
-
printf(_("%s\n"), table_border.buf);
/* Draw table body */
@@ -282,6 +280,20 @@ void print_noob_status(struct wt_status *s, int advice)
}
printf(_("%s\n"), table_border.buf);
+
+ if (s->is_cmd_status && advice_enabled(ADVICE_STATUS_HINTS)) {
+ build_table_entry(&table_col_entry_1, "stage: git add ...", cols);
+ build_table_entry(&table_col_entry_2, "stage: git add ...", cols);
+ build_table_entry(&table_col_entry_3, "unstage: git restore --staged ...", cols);
+ print_table_hint_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, "discard: git restore --staged ...", cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+ print_table_hint_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ printf(_("%s\n"), table_border.buf);
+ }
+
strbuf_release(&table_border);
strbuf_release(&table_col_entry_1);
strbuf_release(&table_col_entry_2);
diff --git a/table.h b/table.h
index 5dff7162a4..c9e8c386de 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
#ifndef TABLE_H
#define TABLE_H
-void print_noob_status(struct wt_status *s, int i);
+void print_noob_status(struct wt_status *s);
#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 1332d07dba..288817dcf7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -50,6 +50,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RED, /* WT_STATUS_REMOTE_BRANCH */
GIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */
GIT_COLOR_CYAN, /* WT_STATUS_ARROW */
+ GIT_COLOR_YELLOW, /* WT_STATUS_HINT */
};
static const char *color(int slot, struct wt_status *s)
@@ -2151,7 +2152,7 @@ static void wt_noobstatus_print(struct wt_status *s)
wt_longstatus_print_tracking(s);
}
- print_noob_status(s, 0);
+ print_noob_status(s);
}
static void wt_porcelain_print(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index c6bce8f74a..0a14b4b064 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -20,6 +20,7 @@ enum color_wt_status {
WT_STATUS_REMOTE_BRANCH,
WT_STATUS_ONBRANCH,
WT_STATUS_ARROW,
+ WT_STATUS_HINT,
WT_STATUS_MAXSLOT
};
@@ -146,6 +147,7 @@ struct wt_status {
struct string_list added;
struct string_list restored;
uint32_t untracked_in_ms;
+ int is_cmd_status;
};
size_t wt_status_locate_end(const char *s, size_t len);
--
2.42.0.404.g2bcc23f3db
^ permalink raw reply related
* [PATCH 0/2] pretty: add %aA to show domain-part of email addresses
From: Liam Beguin @ 2023-10-26 23:16 UTC (permalink / raw)
To: git; +Cc: Liam Beguin
Many reports use the email domain to keep track of organizations
contributing to projects.
Add support for formatting the domain-part of a contributor's address so
that this can be done using git itself, with something like:
git shortlog -sn --group=format:%aA v2.41.0..v2.42.0
---
Liam Beguin (2):
doc: pretty-formats: add missing word
pretty: add '%aA' to show domain-part of email addresses
Documentation/pretty-formats.txt | 10 ++++++++--
pretty.c | 13 ++++++++++++-
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
t/t6006-rev-list-format.sh | 6 ++++--
4 files changed, 52 insertions(+), 5 deletions(-)
---
base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14
change-id: 20231025-pretty-email-domain-2eb2ae23f416
Best regards,
--
Liam Beguin <liambeguin@gmail.com>
^ permalink raw reply
* [PATCH 1/2] doc: pretty-formats: add missing word
From: Liam Beguin @ 2023-10-26 23:16 UTC (permalink / raw)
To: git; +Cc: Liam Beguin
In-Reply-To: <20231026-pretty-email-domain-v1-0-5d6bfa6615c0@gmail.com>
Follow %al and %cl and make sure to mention it's the 'email' local-part.
Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
Documentation/pretty-formats.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d38b4ab5666c..a22f6fceecdd 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -193,7 +193,7 @@ The placeholders are:
'%aE':: author email (respecting .mailmap, see linkgit:git-shortlog[1]
or linkgit:git-blame[1])
'%al':: author email local-part (the part before the '@' sign)
-'%aL':: author local-part (see '%al') respecting .mailmap, see
+'%aL':: author email local-part (see '%al') respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%ad':: author date (format respects --date= option)
'%aD':: author date, RFC2822 style
@@ -211,7 +211,7 @@ The placeholders are:
'%cE':: committer email (respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%cl':: committer email local-part (the part before the '@' sign)
-'%cL':: committer local-part (see '%cl') respecting .mailmap, see
+'%cL':: committer email local-part (see '%cl') respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%cd':: committer date (format respects --date= option)
'%cD':: committer date, RFC2822 style
--
2.39.0
^ permalink raw reply related
* [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Liam Beguin @ 2023-10-26 23:16 UTC (permalink / raw)
To: git; +Cc: Liam Beguin
In-Reply-To: <20231026-pretty-email-domain-v1-0-5d6bfa6615c0@gmail.com>
Many reports use the email domain to keep track of organizations
contributing to projects.
Add support for formatting the domain-part of a contributor's address so
that this can be done using git itself, with something like:
git shortlog -sn --group=format:%aA v2.41.0..v2.42.0
Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
Documentation/pretty-formats.txt | 6 ++++++
pretty.c | 13 ++++++++++++-
t/t4203-mailmap.sh | 28 ++++++++++++++++++++++++++++
t/t6006-rev-list-format.sh | 6 ++++--
4 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a22f6fceecdd..72102a681c3a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -195,6 +195,9 @@ The placeholders are:
'%al':: author email local-part (the part before the '@' sign)
'%aL':: author email local-part (see '%al') respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%aa':: author email domain-part (the part after the '@' sign)
+'%aA':: author email domain-part (see '%al') respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%ad':: author date (format respects --date= option)
'%aD':: author date, RFC2822 style
'%ar':: author date, relative
@@ -213,6 +216,9 @@ The placeholders are:
'%cl':: committer email local-part (the part before the '@' sign)
'%cL':: committer email local-part (see '%cl') respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%ca':: committer email domain-part (the part before the '@' sign)
+'%cA':: committer email domain-part (see '%cl') respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%cd':: committer date (format respects --date= option)
'%cD':: committer date, RFC2822 style
'%cr':: committer date, relative
diff --git a/pretty.c b/pretty.c
index cf964b060cd1..4f5d081589ea 100644
--- a/pretty.c
+++ b/pretty.c
@@ -791,7 +791,7 @@ static size_t format_person_part(struct strbuf *sb, char part,
mail = s.mail_begin;
maillen = s.mail_end - s.mail_begin;
- if (part == 'N' || part == 'E' || part == 'L') /* mailmap lookup */
+ if (part == 'N' || part == 'E' || part == 'L' || part == 'A') /* mailmap lookup */
mailmap_name(&mail, &maillen, &name, &namelen);
if (part == 'n' || part == 'N') { /* name */
strbuf_add(sb, name, namelen);
@@ -808,6 +808,17 @@ static size_t format_person_part(struct strbuf *sb, char part,
strbuf_add(sb, mail, maillen);
return placeholder_len;
}
+ if (part == 'a' || part == 'A') { /* domain-part */
+ const char *at = memchr(mail, '@', maillen);
+ if (at) {
+ at += 1;
+ maillen -= at - mail;
+ strbuf_add(sb, at, maillen);
+ } else {
+ strbuf_add(sb, mail, maillen);
+ }
+ return placeholder_len;
+ }
if (!s.date_begin)
goto skip;
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 2016132f5161..35bf7bb05bea 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -624,6 +624,34 @@ test_expect_success 'Log output (local-part email address)' '
test_cmp expect actual
'
+test_expect_success 'Log output (domain-part email address)' '
+ cat >expect <<-EOF &&
+ Author email cto@coompany.xx has domain-part coompany.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email me@company.xx has domain-part company.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email me@company.xx has domain-part company.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email nick2@company.xx has domain-part company.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email bugs@company.xx has domain-part company.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email bugs@company.xx has domain-part company.xx
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+
+ Author email author@example.com has domain-part example.com
+ Committer email $GIT_COMMITTER_EMAIL has domain-part $TEST_COMMITTER_DOMAIN
+ EOF
+
+ git log --pretty=format:"Author email %ae has domain-part %aa%nCommitter email %ce has domain-part %ca%n" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'Log output with --use-mailmap' '
test_config mailmap.file complex.map &&
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 573eb97a0f7f..34c686becf2d 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -163,11 +163,12 @@ commit $head1
EOF
# we don't test relative here
-test_format author %an%n%ae%n%al%n%ad%n%aD%n%at <<EOF
+test_format author %an%n%ae%n%al%aa%n%ad%n%aD%n%at <<EOF
commit $head2
$GIT_AUTHOR_NAME
$GIT_AUTHOR_EMAIL
$TEST_AUTHOR_LOCALNAME
+$TEST_AUTHOR_DOMAIN
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
@@ -180,11 +181,12 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format committer %cn%n%ce%n%cl%n%cd%n%cD%n%ct <<EOF
+test_format committer %cn%n%ce%n%cl%ca%n%cd%n%cD%n%ct <<EOF
commit $head2
$GIT_COMMITTER_NAME
$GIT_COMMITTER_EMAIL
$TEST_COMMITTER_LOCALNAME
+$TEST_COMMITTER_DOMAIN
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
--
2.39.0
^ permalink raw reply related
* Re: ls-remote bug
From: Bagas Sanjaya @ 2023-10-27 1:08 UTC (permalink / raw)
To: Lior Zeltzer, Git Mailing List
Cc: Andrzej Hunt, Ævar Arnfjörð Bjarmason,
Junio C Hamano
In-Reply-To: <BL0PR18MB2130A3CA5DEF0DD7199F2979BADFA@BL0PR18MB2130.namprd18.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]
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
>
>
>
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/9] t1450: convert tests to remove worktrees via git-worktree(1)
From: Eric Sunshine @ 2023-10-27 2:42 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys, Taylor Blau, Junio C Hamano
In-Reply-To: <089565a358eb28544f0ad6b83b8c47e1edf2db6f.1698156169.git.ps@pks.im>
On Tue, Oct 24, 2023 at 10:05 AM Patrick Steinhardt <ps@pks.im> wrote:
> Some of our tests in t1450 create worktrees and then corrupt them.
> As it is impossible to delete such worktrees via a normal call to `git
> worktree remove`, we instead opt to remove them manually by calling
> rm(1) instead.
>
> This is ultimately unnecessary though as we can use the `-f` switch to
> remove the worktree. Let's convert the tests to do so such that we don't
> have to reach into internal implementation details of worktrees.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> @@ -141,7 +141,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
> test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
> test_when_finished "git update-ref HEAD $orig_head" &&
> - test_when_finished "rm -rf .git/worktrees wt" &&
> + test_when_finished "git worktree remove -f wt" &&
> git worktree add wt &&
> echo $ZERO_OID >.git/HEAD &&
Technically, this is a change of behavior since the original code
removed the entire .git/worktrees directory, which deleted
administrative metainformation for _all_ worktrees, whereas the new
code only deletes administrative metadata for the mentioned worktree.
However, since there are no other existing worktrees at this point in
any of these tests, the result is functionally the same, so the change
of behavior is immaterial. Moreover, the revised code has a smaller
blast-radius, which may be a desirable property since there has been
some movement toward making tests more self-contained so that they can
be run individually more easily.
^ permalink raw reply
* Re: [PATCH v5 3/3] rev-list: add commit object support in `--missing` option
From: Patrick Steinhardt @ 2023-10-27 6:25 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, gitster
In-Reply-To: <20231026101109.43110-4-karthik.188@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8377 bytes --]
On Thu, Oct 26, 2023 at 12:11:09PM +0200, Karthik Nayak wrote:
> The `--missing` object option in rev-list currently works only with
> missing blobs/trees. For missing commits the revision walker fails with
> a fatal error.
>
> Let's extend the functionality of `--missing` option to also support
> commit objects. This is done by adding a `missing_objects` field to
> `rev_info`. This field is an `oidset` to which we'll add the missing
> commits as we encounter them. The revision walker will now continue the
> traversal and call `show_commit()` even for missing commits. In rev-list
> we can then check if the commit is a missing commit and call the
> existing code for parsing `--missing` objects.
>
> A scenario where this option would be used is to find the boundary
> objects between different object directories. Consider a repository with
> a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
> object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
> repository, using the `--missing=print` option while disabling the
> alternate object directory allows us to find the boundary objects
> between the main and alternate object directory.
>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> builtin/rev-list.c | 6 +++
> list-objects.c | 3 ++
> revision.c | 17 ++++++++-
> revision.h | 4 ++
> t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 102 insertions(+), 2 deletions(-)
> create mode 100755 t/t6022-rev-list-missing.sh
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 98542e8b3c..181353dcf5 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -149,6 +149,12 @@ static void show_commit(struct commit *commit, void *data)
>
> display_progress(progress, ++progress_counter);
>
> + if (revs->do_not_die_on_missing_objects &&
> + oidset_contains(&revs->missing_commits, &commit->object.oid)) {
> + finish_object__ma(&commit->object);
> + return;
> + }
> +
> if (show_disk_usage)
> total_disk_usage += get_object_disk_usage(&commit->object);
>
> diff --git a/list-objects.c b/list-objects.c
> index 47296dff2f..f4e1104b56 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -389,6 +389,9 @@ static void do_traverse(struct traversal_context *ctx)
> */
> if (!ctx->revs->tree_objects)
> ; /* do not bother loading tree */
> + else if (ctx->revs->do_not_die_on_missing_objects &&
> + oidset_contains(&ctx->revs->missing_commits, &commit->object.oid))
> + ;
> else if (repo_get_commit_tree(the_repository, commit)) {
> struct tree *tree = repo_get_commit_tree(the_repository,
> commit);
> diff --git a/revision.c b/revision.c
> index 219dc76716..738bacad08 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -6,6 +6,7 @@
> #include "object-name.h"
> #include "object-file.h"
> #include "object-store-ll.h"
> +#include "oidset.h"
> #include "tag.h"
> #include "blob.h"
> #include "tree.h"
> @@ -1112,6 +1113,9 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
>
> if (commit->object.flags & ADDED)
> return 0;
> + if (revs->do_not_die_on_missing_objects &&
> + oidset_contains(&revs->missing_commits, &commit->object.oid))
> + return 0;
> commit->object.flags |= ADDED;
>
> if (revs->include_check &&
> @@ -1168,7 +1172,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
> for (parent = commit->parents; parent; parent = parent->next) {
> struct commit *p = parent->item;
> int gently = revs->ignore_missing_links ||
> - revs->exclude_promisor_objects;
> + revs->exclude_promisor_objects ||
> + revs->do_not_die_on_missing_objects;
> if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
> if (revs->exclude_promisor_objects &&
> is_promisor_object(&p->object.oid)) {
> @@ -1176,7 +1181,11 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
> break;
> continue;
> }
> - return -1;
> +
> + if (revs->do_not_die_on_missing_objects)
> + oidset_insert(&revs->missing_commits, &p->object.oid);
> + else
> + return -1; /* corrupt repository */
> }
> if (revs->sources) {
> char **slot = revision_sources_at(revs->sources, p);
> @@ -3109,6 +3118,7 @@ void release_revisions(struct rev_info *revs)
> clear_decoration(&revs->merge_simplification, free);
> clear_decoration(&revs->treesame, free);
> line_log_free(revs);
> + oidset_clear(&revs->missing_commits);
> }
>
> static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
> @@ -3800,6 +3810,9 @@ int prepare_revision_walk(struct rev_info *revs)
> FOR_EACH_OBJECT_PROMISOR_ONLY);
> }
>
> + if (revs->do_not_die_on_missing_objects)
> + oidset_init(&revs->missing_commits, 0);
> +
We unconditionally clear the oidset now, so shouldn't we also
unconditionally initialize it?
Patrick
> if (!revs->reflog_info)
> prepare_to_use_bloom_filter(revs);
> if (!revs->unsorted_input)
> diff --git a/revision.h b/revision.h
> index c73c92ef40..94c43138bc 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -4,6 +4,7 @@
> #include "commit.h"
> #include "grep.h"
> #include "notes.h"
> +#include "oidset.h"
> #include "pretty.h"
> #include "diff.h"
> #include "commit-slab-decl.h"
> @@ -373,6 +374,9 @@ struct rev_info {
>
> /* Location where temporary objects for remerge-diff are written. */
> struct tmp_objdir *remerge_objdir;
> +
> + /* Missing commits to be tracked without failing traversal. */
> + struct oidset missing_commits;
> };
>
> /**
> diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
> new file mode 100755
> index 0000000000..40265a4f66
> --- /dev/null
> +++ b/t/t6022-rev-list-missing.sh
> @@ -0,0 +1,74 @@
> +#!/bin/sh
> +
> +test_description='handling of missing objects in rev-list'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +# We setup the repository with two commits, this way HEAD is always
> +# available and we can hide commit 1.
> +test_expect_success 'create repository and alternate directory' '
> + test_commit 1 &&
> + test_commit 2 &&
> + test_commit 3
> +'
> +
> +for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
> +do
> + test_expect_success "rev-list --missing=error fails with missing object $obj" '
> + oid="$(git rev-parse $obj)" &&
> + path=".git/objects/$(test_oid_to_path $oid)" &&
> +
> + mv "$path" "$path.hidden" &&
> + test_when_finished "mv $path.hidden $path" &&
> +
> + test_must_fail git rev-list --missing=error --objects \
> + --no-object-names HEAD
> + '
> +done
> +
> +for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
> +do
> + for action in "allow-any" "print"
> + do
> + test_expect_success "rev-list --missing=$action with missing $obj" '
> + oid="$(git rev-parse $obj)" &&
> + path=".git/objects/$(test_oid_to_path $oid)" &&
> +
> + # Before the object is made missing, we use rev-list to
> + # get the expected oids.
> + git rev-list --objects --no-object-names \
> + HEAD ^$obj >expect.raw &&
> +
> + # Blobs are shared by all commits, so evethough a commit/tree
> + # might be skipped, its blob must be accounted for.
> + if [ $obj != "HEAD:1.t" ]; then
> + echo $(git rev-parse HEAD:1.t) >>expect.raw &&
> + echo $(git rev-parse HEAD:2.t) >>expect.raw
> + fi &&
> +
> + mv "$path" "$path.hidden" &&
> + test_when_finished "mv $path.hidden $path" &&
> +
> + git rev-list --missing=$action --objects --no-object-names \
> + HEAD >actual.raw &&
> +
> + # When the action is to print, we should also add the missing
> + # oid to the expect list.
> + case $action in
> + allow-any)
> + ;;
> + print)
> + grep ?$oid actual.raw &&
> + echo ?$oid >>expect.raw
> + ;;
> + esac &&
> +
> + sort actual.raw >actual &&
> + sort expect.raw >expect &&
> + test_cmp expect actual
> + '
> + done
> +done
> +
> +test_done
> --
> 2.42.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/1] bugreport: include +i in outfile suffix as needed
From: Jacob Stopak @ 2023-10-27 6:34 UTC (permalink / raw)
To: Emily Shaffer; +Cc: git
In-Reply-To: <ZTrX4PMYtbVT-tUu@google.com>
> > +static void build_path(struct strbuf *buf, const char *dir_path,
> > + const char *prefix, const char *suffix,
> > + time_t t, int *i, const char *ext)
> > +{
> > + struct tm tm;
> > +
> > + strbuf_reset(buf);
> > + strbuf_addstr(buf, dir_path);
> > + strbuf_complete(buf, '/');
> > +
> > + strbuf_addstr(buf, prefix);
> > + strbuf_addftime(buf, suffix, localtime_r(&t, &tm), 0, 0);
> > +
> > + if (*i > 0)
> > + strbuf_addf(buf, "+%d", *i);
> > +
> > + strbuf_addstr(buf, ext);
> > +
> > + (*i)++;
> > +}
>
> I commented on the weirdness of having to decrement i for --diagnose
> below, but I think I generally just wish that instead of build_path()
> this function did create_file_with_optional_suffix() and returned the
> final modified option_suffix(). Better still would be if this function
> created (or at least tested) all the necessary output paths so you don't
> end up succeeding in creating a bugreport.txt but failing in creating
> the diagnostics.zip in some edge case, something like....
So the funny thing is that the existing behavior of the diagnostics file
just overwrites any existing diagnostics file with the same name, which
is at odds with how the bugreport throws an error in the case of a
conflict. I wasn't sure whether it made sense to touch that here since it
seemed possibly out of the scope of this change, given that there is
a separate "git diagnose" command to take into account.
But if we keep this behavior it basically means there's no need to test
the necessary diagnostics output paths when determining the path of the
bugreport itself, since whatever we pick for the bugreport will just
overwrite anything for the diagnotics zip if it already exists. The one
caveat is that any future files created should behave the same way...
But I get that this is perhaps inconsistent and it might be worth it to
make "git diagnose" work the same way as bugreport so it makes more sense
to do the kind of validations you mentioned.
> build_suffix(..., &option_suffix) {
> ... build timestamp ...
> while (...)
> for (final_path in eventual_paths) {
> err = select(final_path);
> if (err)
> final_path = strcat(most_of_path, i)
> else
> break
> (Yeah, that's very handwavey, but I hope you see what I'm getting at.)
>
> Really, though, I mostly don't think I like leaving the control variable i raw
> to the calling scope and making it be manipulated later. Fancy
> pre-guessing-path-availability aside, I think you could achieve a more
> pleasant solution even by letting build_path() become
> create_file_with_optional_suffix() (that reports the optional suffix
> eventually settled on).
Hmm... so when you say "create_file_with_optional_suffix", do you mean
a function that tests a set of paths to find the minimum incremented
suffix that will work for all the paths? Would it use the current method
the patch uses of trying to create the files and if creation fails we
know the file exists -> increment -> try again? Repeat until all the
files are able to be created and make sure to clean up any extras?
(And maybe just clean up everything since the actual files with content
will be created later on, now that the suffix is known).
An alternative could be to wrap `build_path()` in a function that does
this work, locally scope `i` in it, call build_path on each needed path,
and test creation until all paths are valid, clean up all the paths,
then return the suffix.
> > + if (!strlen(option_suffix))
> > + option_suffix = "%Y-%m-%d-%H%M";
> > + else
> > + option_suffix_is_from_user = 1;
>
> Looking at where this is used, it looks like you're saying "if the user
> specified the suffix manually and has this problem, then that sucks for
> them, they put their own foot in it".
>
> But I don't know if I necessarily
> follow that logic - I'd just as soon drop the exception, append an int
> to the user-provided suffix, and document that we'll do that in the
> manpage.
Haha - it's interesting how we each interpreted the user's experience here,
and I was a bit torn about this too. But here are my thoughts on it:
If the user specifies their own suffix, it seems more natural and even
expected for them to just get an error if a file with that name already
exists. To me it's not that they put their foot in anything, it's that
they asked for a specific thing that we can't deliver since it's already
taken, so just let them know so they can either delete the existing one
or alter the custom suffix. Incremeting the filename without telling them
in this case might not be what they actually want, we're kindof guessing.
However, in the default case where no suffix is specified, the user likely
has no assumption or awareness about the suffix at all, which is why it
felt odd to throw an error out of the blue if the default suffix happens
to conflict with an existing file that was also created by default. The
user didn't ask for anything specific in this case, so would likely be
confused as to why they are getting an error when it just worked a second
ago. So I was thinking we can just handle it gracefully and give them the
incremented report.
> (This isn't something I feel strongly about, except that I think it
> makes the code harder to follow for not very notable user benefit. I
> also didn't look through the reviews up until now, so if this was
> already hashed back and forth, just go ahead and ignore me.)
Setting up the default variables like this was discussed, but not the
difference in behavior based on whether the user specifies a suffix.
Altho I do agree that we sacrifice some consistency here and it does add
an extra pathway to the code, imo there is a good enough reason to do it
as described above - to me it makes things more intuitive to the user.
> > + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
> > + report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
> > + if (report < 0 && errno == EEXIST && !option_suffix_is_from_user) {
> > + build_path(&report_path, prefixed_filename,
> > + "git-bugreport-", option_suffix, now, &i,
> > + ".txt");
> > + goto again;
> > + } else if (report < 0) {
> Nit, but the double-checking of (report < 0) bothers me a little. Is it
> nicer if it's nested?
>
> if (report < 0) {
> if (errno == EEXIST) {
> build_path(...);
> goto again;
> }
>
> die_errno(_(...));
> }
>
> I like it a little more, but that's up to taste, I suppose.
I like yours better too :)
> > + die_errno(_("unable to open '%s'"), report_path.buf);
> > + }
> >
> > if (write_in_full(report, buffer.buf, buffer.len) < 0)
> > die_errno(_("unable to write to %s"), report_path.buf);
> >
> > close(report);
> >
> > + /* Prepare diagnostics, if requested */
> > + if (diagnose != DIAGNOSE_NONE) {
> > + struct strbuf zip_path = STRBUF_INIT;
> > + i--; /* Undo last increment to match zipfile suffix to bugreport */
> I understand why you're doing this, but I'd rather see it decremented
> (or more care taken in the increment logic elsewhere) closer to where it
> is being increment-and-checked. If someone wants to add another
> associated file besides the report and the diagnostics, then the logic
> for the decrement becomes complicated (what happens if I run `git
> bugreport --diagnostics --desktop_screencap`? what if I run only `git
> bugreport --desktop_screencap`?). Even without that potential pain,
> reading this comment here means I have to say "oh wait, what did I read
> above? hold on, let me page it back in".
>
Yeah these are great points and I completely agree! Even if we stick with
this method of doing things, the `i` decrement should be moved out of the
conditional and up closer to where the value of `i` is set.
> > + build_path(&zip_path, prefixed_filename, "git-diagnostics-",
> > + option_suffix, now, &i, ".zip");
> > +
> > + if (create_diagnostics_archive(&zip_path, diagnose))
> > + die_errno(_("unable to create diagnostics archive %s"),
> > + zip_path.buf);
> > +
> > + strbuf_release(&zip_path);
> > + }
> > +
> > /*
> > * We want to print the path relative to the user, but we still need the
> > * path relative to us to give to the editor.
> > --
> > 2.42.0.297.g36452639b8
>
> Last thing: it probably makes sense to mention this new behavior in the
> manpage, especially if you'll apply that behavior to user-provided
> suffixes too.
Sure I can add some docs to the manpage, altho as described above I
prefer throwing an error instead of adding the increment to the
user-provided suffixes :D
> Thanks for your effort on the patch so far and again, sorry for the late
> reply.
> - Emily
No worries at all and thanks a lot for your review! I'll start working
on a v3 - it would be great to get your thoughts on the unresolved items.
^ permalink raw reply
* GFW fails with ST3 on Windows 11
From: John @ 2023-10-27 7:31 UTC (permalink / raw)
To: git
I have been using Sublime Text 3 as the editor on Git for Windows for years, on Windows 10. I recently purchased a Windows 11 machine. On that machine, when I give GFW the following command, I get the response shown:
$ git commit -a
[…]
hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
Please supply the message using either -m or -F option.
Does anyone know of a fix? To repeat, it’s only a problem on Windows 11.
^ permalink raw reply
* Re: [PATCH v5 3/3] rev-list: add commit object support in `--missing` option
From: Karthik Nayak @ 2023-10-27 7:54 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster
In-Reply-To: <ZTtXzg4NGJZzAqfS@tanuki>
On Fri, Oct 27, 2023 at 8:25 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> We unconditionally clear the oidset now, so shouldn't we also
> unconditionally initialize it?
>
You're right. I should have added that in.
Junio, I'm replying to this email with the amended commit for v5. If
you want me to reroll
v6, I could do that too. Thanks!
^ permalink raw reply
* [PATCH v5 3/3] rev-list: add commit object support in `--missing` option
From: Karthik Nayak @ 2023-10-27 7:59 UTC (permalink / raw)
To: karthik.188; +Cc: git, gitster, ps
In-Reply-To: <ZTtXzg4NGJZzAqfS@tanuki>
The `--missing` object option in rev-list currently works only with
missing blobs/trees. For missing commits the revision walker fails with
a fatal error.
Let's extend the functionality of `--missing` option to also support
commit objects. This is done by adding a `missing_objects` field to
`rev_info`. This field is an `oidset` to which we'll add the missing
commits as we encounter them. The revision walker will now continue the
traversal and call `show_commit()` even for missing commits. In rev-list
we can then check if the commit is a missing commit and call the
existing code for parsing `--missing` objects.
A scenario where this option would be used is to find the boundary
objects between different object directories. Consider a repository with
a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate
object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a
repository, using the `--missing=print` option while disabling the
alternate object directory allows us to find the boundary objects
between the main and alternate object directory.
Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
builtin/rev-list.c | 6 +++
list-objects.c | 3 ++
revision.c | 16 +++++++-
revision.h | 4 ++
t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++++++++++
5 files changed, 101 insertions(+), 2 deletions(-)
create mode 100755 t/t6022-rev-list-missing.sh
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 98542e8b3c..181353dcf5 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -149,6 +149,12 @@ static void show_commit(struct commit *commit, void *data)
display_progress(progress, ++progress_counter);
+ if (revs->do_not_die_on_missing_objects &&
+ oidset_contains(&revs->missing_commits, &commit->object.oid)) {
+ finish_object__ma(&commit->object);
+ return;
+ }
+
if (show_disk_usage)
total_disk_usage += get_object_disk_usage(&commit->object);
diff --git a/list-objects.c b/list-objects.c
index 47296dff2f..f4e1104b56 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -389,6 +389,9 @@ static void do_traverse(struct traversal_context *ctx)
*/
if (!ctx->revs->tree_objects)
; /* do not bother loading tree */
+ else if (ctx->revs->do_not_die_on_missing_objects &&
+ oidset_contains(&ctx->revs->missing_commits, &commit->object.oid))
+ ;
else if (repo_get_commit_tree(the_repository, commit)) {
struct tree *tree = repo_get_commit_tree(the_repository,
commit);
diff --git a/revision.c b/revision.c
index 219dc76716..00d5c29bfc 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
#include "object-name.h"
#include "object-file.h"
#include "object-store-ll.h"
+#include "oidset.h"
#include "tag.h"
#include "blob.h"
#include "tree.h"
@@ -1112,6 +1113,9 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
if (commit->object.flags & ADDED)
return 0;
+ if (revs->do_not_die_on_missing_objects &&
+ oidset_contains(&revs->missing_commits, &commit->object.oid))
+ return 0;
commit->object.flags |= ADDED;
if (revs->include_check &&
@@ -1168,7 +1172,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
for (parent = commit->parents; parent; parent = parent->next) {
struct commit *p = parent->item;
int gently = revs->ignore_missing_links ||
- revs->exclude_promisor_objects;
+ revs->exclude_promisor_objects ||
+ revs->do_not_die_on_missing_objects;
if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
if (revs->exclude_promisor_objects &&
is_promisor_object(&p->object.oid)) {
@@ -1176,7 +1181,11 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
break;
continue;
}
- return -1;
+
+ if (revs->do_not_die_on_missing_objects)
+ oidset_insert(&revs->missing_commits, &p->object.oid);
+ else
+ return -1; /* corrupt repository */
}
if (revs->sources) {
char **slot = revision_sources_at(revs->sources, p);
@@ -3109,6 +3118,7 @@ void release_revisions(struct rev_info *revs)
clear_decoration(&revs->merge_simplification, free);
clear_decoration(&revs->treesame, free);
line_log_free(revs);
+ oidset_clear(&revs->missing_commits);
}
static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
@@ -3800,6 +3810,8 @@ int prepare_revision_walk(struct rev_info *revs)
FOR_EACH_OBJECT_PROMISOR_ONLY);
}
+ oidset_init(&revs->missing_commits, 0);
+
if (!revs->reflog_info)
prepare_to_use_bloom_filter(revs);
if (!revs->unsorted_input)
diff --git a/revision.h b/revision.h
index c73c92ef40..94c43138bc 100644
--- a/revision.h
+++ b/revision.h
@@ -4,6 +4,7 @@
#include "commit.h"
#include "grep.h"
#include "notes.h"
+#include "oidset.h"
#include "pretty.h"
#include "diff.h"
#include "commit-slab-decl.h"
@@ -373,6 +374,9 @@ struct rev_info {
/* Location where temporary objects for remerge-diff are written. */
struct tmp_objdir *remerge_objdir;
+
+ /* Missing commits to be tracked without failing traversal. */
+ struct oidset missing_commits;
};
/**
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
new file mode 100755
index 0000000000..40265a4f66
--- /dev/null
+++ b/t/t6022-rev-list-missing.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='handling of missing objects in rev-list'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# We setup the repository with two commits, this way HEAD is always
+# available and we can hide commit 1.
+test_expect_success 'create repository and alternate directory' '
+ test_commit 1 &&
+ test_commit 2 &&
+ test_commit 3
+'
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ test_expect_success "rev-list --missing=error fails with missing object $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ test_must_fail git rev-list --missing=error --objects \
+ --no-object-names HEAD
+ '
+done
+
+for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
+do
+ for action in "allow-any" "print"
+ do
+ test_expect_success "rev-list --missing=$action with missing $obj" '
+ oid="$(git rev-parse $obj)" &&
+ path=".git/objects/$(test_oid_to_path $oid)" &&
+
+ # Before the object is made missing, we use rev-list to
+ # get the expected oids.
+ git rev-list --objects --no-object-names \
+ HEAD ^$obj >expect.raw &&
+
+ # Blobs are shared by all commits, so evethough a commit/tree
+ # might be skipped, its blob must be accounted for.
+ if [ $obj != "HEAD:1.t" ]; then
+ echo $(git rev-parse HEAD:1.t) >>expect.raw &&
+ echo $(git rev-parse HEAD:2.t) >>expect.raw
+ fi &&
+
+ mv "$path" "$path.hidden" &&
+ test_when_finished "mv $path.hidden $path" &&
+
+ git rev-list --missing=$action --objects --no-object-names \
+ HEAD >actual.raw &&
+
+ # When the action is to print, we should also add the missing
+ # oid to the expect list.
+ case $action in
+ allow-any)
+ ;;
+ print)
+ grep ?$oid actual.raw &&
+ echo ?$oid >>expect.raw
+ ;;
+ esac &&
+
+ sort actual.raw >actual &&
+ sort expect.raw >expect &&
+ test_cmp expect actual
+ '
+ done
+done
+
+test_done
--
2.42.0
^ permalink raw reply related
* Re: [PATCH 1/5] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZToin/fQiZrmUJTS@ugly>
[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]
On Thu, Oct 26, 2023 at 10:26:07AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:03AM +0200, Patrick Steinhardt wrote:
> > [...]
> > _not_ being GitLab Actions, where we define the non-stub logic in the
> >
> you meant GitHub here.
>
> > else branch.
> >
> > Reorder the definitions such that we explicitly handle GitHub Actions.
> >
> i'd say something like "the conditional branches". imo that makes it clearer
> that you're actually talking about code, not some markup or whatever.
> for that matter, this is my overall impression of the commit message - it
> seems way too detached from the near-trivial fact that you're just slightly
> adjusting the code structure to make it easier to implement a cascade (aka a
> switch).
>
> regards
The change is trivial indeed, but even a trivial change needs a reason
why it should be done. Maybe it's too long, maybe it isn't... I'm happy
to take suggestions.
But anyway, I've adopted both of the other two suggestions you made,
thanks.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTokgo1mQ7ZVH7GU@ugly>
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Thu, Oct 26, 2023 at 10:34:10AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:12AM +0200, Patrick Steinhardt wrote:
> > Pull in "lib.sh" into "install-docker-dependencies.sh" such that we can
> > set up proper groups for those dependencise. This allows the reader to
> ^^ !!!
> > collapse sections in the CI output on GitHub Actions (and later on on
> > GitLab CI).
> >
> the structure of the text is kind of backwards - the fact that you need to
> pull in a lib is just a consequence, not the intent, which imo should come
> first. tough it mostly doesn't matter, as it's just one short paragraph.
>
> regards
Good point indeed. Will rewrite.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 8:17 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZTosPCkpx/FMTDH5@ugly>
[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]
On Thu, Oct 26, 2023 at 11:07:08AM +0200, Oswald Buddenhagen wrote:
> On Thu, Oct 26, 2023 at 10:00:20AM +0200, Patrick Steinhardt wrote:
> > project has. More esoteric jobs like "linux-TEST-vars" that also sets a
> ^ -s
>
> > couple of environment variables do not exist in GitLab's custom CI
> > setup, and maintaining them to keep up with what Git does feels like
> > wasted time. The result is that we regularly send patch series upstream
> > that would otherwise fail to compile or pass tests in GitHub Workflows.
> ^^^^^^^^^^^^^^^
> that inverts the meaning
>
> > [...]
> > project to help us ensure to send better patch series upstream and thus
> ^^
> "we"
> > reduce overhead for the maintainer.
>
> > --- 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
> > ;;
> > +linux-*)
> >
> you should probably choose a less generic name for the jobs, at least
> debian-*.
The names are all preexisting, so I cannot change them. And the CI infra
does indeed rely on the exact names to choose what to do.
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 33005854520..102e9d04a1f 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -15,6 +15,42 @@ then
> > echo '::endgroup::' >&2
> > }
> >
> > + group () {
> > [...]
> > + }
> >
> this counter-intutive patch structure reveals that the function is
> duplicated between github and gitlab. you may want to factor it out and
> alias it. or change the structure entirely (circling back to patch 1/5).
I don't quite know what you mean by counter-intuitive patch structure.
But regarding the duplication for the `group ()` function I agree, it's
a bit unfortunate. My first version did de-duplicate it indeed, but it
resulted in some weirdness in the stubbed case.
I'll revisit this.
> > + CI_BRANCH="$CI_COMMIT_REF_NAME"
> > + CI_COMMIT="$CI_COMMIT_SHA"
> >
> assignments need no quoting to prevent word splitting.
> repeats below.
>
> > + case "$CI_JOB_IMAGE" in
> >
> ... as does the selector in case statements.
True, but I'm simply matching the coding style in this script.
> > --- a/ci/print-test-failures.sh
> > +++ b/ci/print-test-failures.sh
> > @@ -51,6 +51,12 @@ do
> > tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> > continue
> > ;;
> > + gitlab-ci)
> > + mkdir -p failed-test-artifacts
> > + cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
> > + tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> >
> you're just following the precedent, but imo it's more legible to quote the
> entire string, not just the variable expansion. the code doesn't even agree
> with itself, as the line directly above apparently agrees with me.
>
> regards
Yeah, as you say, this is another case where I follow precedent. I
honestly don't quite care which way we go in this case.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 1/5] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
We define a set of grouping functions that are used to group together
output in our CI, where these groups then end up as collapsible sections
in the respective pipeline platform. The way these functions are defined
is not easily extensible though as we have an up front check for the CI
_not_ being GitHub Actions, where we define the non-stub logic in the
else branch.
Reorder the conditional branches such that we explicitly handle GitHub
Actions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index 6fbb5bade12..eb384f4e952 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -1,16 +1,7 @@
# Library of functions shared by all CI scripts
-if test true != "$GITHUB_ACTIONS"
+if test true = "$GITHUB_ACTIONS"
then
- begin_group () { :; }
- end_group () { :; }
-
- group () {
- shift
- "$@"
- }
- set -x
-else
begin_group () {
need_to_end_group=t
echo "::group::$1" >&2
@@ -42,6 +33,15 @@ else
}
begin_group "CI setup"
+else
+ begin_group () { :; }
+ end_group () { :; }
+
+ group () {
+ shift
+ "$@"
+ }
+ set -x
fi
# Set 'exit on error' for all CI scripts to let the caller know that
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 0/5] ci: add GitLab CI definition
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698305961.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9177 bytes --]
Hi,
this is the second version of this patch series that adds GitLab CI
definitions to the Git project. Please refer to the cover letter for v1
of this series [1] for the motivation and intent -- I won't repeat it
here as it's a bit on the longer side.
Changes compared to v1:
- Improved commit messages.
- The `group ()` function is now generic across all the different CI
solutions to reduce duplication.
This should hopefully address the feedback from Oswald -- thanks again!
Patrick
[1]: <cover.1698305961.git.ps@pks.im>
Patrick Steinhardt (5):
ci: reorder definitions for grouping functions
ci: make grouping setup more generic
ci: group installation of Docker dependencies
ci: split out logic to set up failed test artifacts
ci: add support for GitLab CI
.gitlab-ci.yml | 51 +++++++++++
ci/install-docker-dependencies.sh | 15 +++-
ci/lib.sh | 139 ++++++++++++++++++++----------
ci/print-test-failures.sh | 6 ++
4 files changed, 166 insertions(+), 45 deletions(-)
create mode 100644 .gitlab-ci.yml
Range-diff against v1:
1: 586a8d1003b ! 1: 4eb9cfc816b ci: reorder definitions for grouping functions
@@ Commit message
output in our CI, where these groups then end up as collapsible sections
in the respective pipeline platform. The way these functions are defined
is not easily extensible though as we have an up front check for the CI
- _not_ being GitLab Actions, where we define the non-stub logic in the
+ _not_ being GitHub Actions, where we define the non-stub logic in the
else branch.
- Reorder the definitions such that we explicitly handle GitHub Actions.
+ Reorder the conditional branches such that we explicitly handle GitHub
+ Actions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
2: ec390354f15 ! 2: 85617ef8577 ci: make grouping setup more generic
@@ Commit message
or not. This ensures we can more readily add support for additional CI
platforms.
- Second, this commit changes `end_group ()` to also accept a parameter
- that indicates _which_ group should end. This will be required by a
- later commit that introduces support for GitLab CI.
+ Furthermore, the `group ()` function is made generic so that it is the
+ same for both GitHub Actions and for other platforms. There is a
+ semantic conflict here though: GitHub Actions used to call `set +x` in
+ `group ()` whereas the non-GitHub case unconditionally uses `set -x`.
+ The latter would get overriden if we kept the `set +x` in the generic
+ version of `group ()`. To resolve this conflict, we simply drop the `set
+ +x` in the generic variant of this function. As `begin_group ()` calls
+ `set -x` anyway this is not much of a change though, as the only
+ commands that aren't printed anymore now are the ones between the
+ beginning of `group ()` and the end of `begin_group ()`.
+
+ Last, this commit changes `end_group ()` to also accept a parameter that
+ indicates _which_ group should end. This will be required by a later
+ commit that introduces support for GitLab CI.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
@@ ci/lib.sh: then
echo '::endgroup::' >&2
}
- trap end_group EXIT
-
- group () {
- set +x
+-
+- group () {
+- set +x
- begin_group "$1"
-+
-+ group="$1"
- shift
-+ begin_group "$group"
-+
- # work around `dash` not supporting `set -o pipefail`
- (
- "$@" 2>&1
-@@ ci/lib.sh: then
- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
- res=$(cat exit.status)
- rm exit.status
+- shift
+- # work around `dash` not supporting `set -o pipefail`
+- (
+- "$@" 2>&1
+- echo $? >exit.status
+- ) |
+- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
+- res=$(cat exit.status)
+- rm exit.status
- end_group
-+
-+ end_group "$group"
- return $res
- }
+- return $res
+- }
-
- begin_group "CI setup"
else
begin_group () { :; }
end_group () { :; }
-@@ ci/lib.sh: else
+
+- group () {
+- shift
+- "$@"
+- }
set -x
fi
++group () {
++ group="$1"
++ shift
++ begin_group "$group"
++
++ # work around `dash` not supporting `set -o pipefail`
++ (
++ "$@" 2>&1
++ echo $? >exit.status
++ ) |
++ sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
++ res=$(cat exit.status)
++ rm exit.status
++
++ end_group "$group"
++ return $res
++}
++
+begin_group "CI setup"
+trap "end_group 'CI setup'" EXIT
+
3: a65d235dd3c ! 3: 57bbc50e3dc ci: group installation of Docker dependencies
@@ Metadata
## Commit message ##
ci: group installation of Docker dependencies
- Pull in "lib.sh" into "install-docker-dependencies.sh" such that we can
- set up proper groups for those dependencise. This allows the reader to
- collapse sections in the CI output on GitHub Actions (and later on on
- GitLab CI).
+ The output of CI jobs tends to be quite long-winded and hard to digest.
+ To help with this, many CI systems provide the ability to group output
+ into collapsible sections, and we're also doing this in some of our
+ scripts.
+
+ One notable omission is the script to install Docker dependencies.
+ Address it to bring more structure to the output for Docker-based jobs.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
4: 4a864a1d174 ! 4: 5ab11d5236d ci: split out logic to set up failed test artifacts
@@ Commit message
ci: split out logic to set up failed test artifacts
We have some logic in place to create a directory with the output from
- failed tests, which will then subsequently be uploaded as CI artifact.
+ failed tests, which will then subsequently be uploaded as CI artifacts.
We're about to add support for GitLab CI, which will want to reuse the
logic.
5: 35b07e5378d ! 5: 37a507e9b25 ci: add support for GitLab CI
@@ Commit message
Part of a problem we hit at GitLab rather frequently is that our own,
custom CI setup we have is so different to the setup that the Git
- project has. More esoteric jobs like "linux-TEST-vars" that also sets a
+ project has. More esoteric jobs like "linux-TEST-vars" that also set a
couple of environment variables do not exist in GitLab's custom CI
setup, and maintaining them to keep up with what Git does feels like
wasted time. The result is that we regularly send patch series upstream
- that would otherwise fail to compile or pass tests in GitHub Workflows.
- We would thus like to integrate the GitLab CI configuration into the Git
- project to help us ensure to send better patch series upstream and thus
- reduce overhead for the maintainer.
+ that fail to compile or pass tests in GitHub Workflows. We would thus
+ like to integrate the GitLab CI configuration into the Git project to
+ help us send better patch series upstream and thus reduce overhead for
+ the maintainer.
The integration does not necessarily have to be a first-class citizen,
which would in practice only add to the fallout that pipeline failures
@@ ci/install-docker-dependencies.sh: linux32)
## ci/lib.sh ##
@@ ci/lib.sh: then
+ need_to_end_group=
echo '::endgroup::' >&2
}
-
-+ group () {
-+ set +x
-+
-+ group="$1"
-+ shift
-+ begin_group "$group"
-+
-+ # work around `dash` not supporting `set -o pipefail`
-+ (
-+ "$@" 2>&1
-+ echo $? >exit.status
-+ ) |
-+ sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
-+ res=$(cat exit.status)
-+ rm exit.status
-+
-+ end_group "$group"
-+ return $res
-+ }
+elif test true = "$GITLAB_CI"
+then
+ begin_group () {
@@ ci/lib.sh: then
+ echo -e "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K"
+ trap - EXIT
+ }
-+
- group () {
- set +x
-
+ else
+ begin_group () { :; }
+ end_group () { :; }
@@ ci/lib.sh: then
MAKEFLAGS="$MAKEFLAGS --jobs=10"
test windows != "$CI_OS_NAME" ||
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 2/5] ci: make grouping setup more generic
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
Make the grouping setup more generic by always calling `begin_group ()`
and `end_group ()` regardless of whether we have stubbed those functions
or not. This ensures we can more readily add support for additional CI
platforms.
Furthermore, the `group ()` function is made generic so that it is the
same for both GitHub Actions and for other platforms. There is a
semantic conflict here though: GitHub Actions used to call `set +x` in
`group ()` whereas the non-GitHub case unconditionally uses `set -x`.
The latter would get overriden if we kept the `set +x` in the generic
version of `group ()`. To resolve this conflict, we simply drop the `set
+x` in the generic variant of this function. As `begin_group ()` calls
`set -x` anyway this is not much of a change though, as the only
commands that aren't printed anymore now are the ones between the
beginning of `group ()` and the end of `begin_group ()`.
Last, this commit changes `end_group ()` to also accept a parameter that
indicates _which_ group should end. This will be required by a later
commit that introduces support for GitLab CI.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index eb384f4e952..b3411afae8e 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,36 +14,34 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
- trap end_group EXIT
-
- group () {
- set +x
- begin_group "$1"
- shift
- # work around `dash` not supporting `set -o pipefail`
- (
- "$@" 2>&1
- echo $? >exit.status
- ) |
- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
- res=$(cat exit.status)
- rm exit.status
- end_group
- return $res
- }
-
- begin_group "CI setup"
else
begin_group () { :; }
end_group () { :; }
- group () {
- shift
- "$@"
- }
set -x
fi
+group () {
+ group="$1"
+ shift
+ begin_group "$group"
+
+ # work around `dash` not supporting `set -o pipefail`
+ (
+ "$@" 2>&1
+ echo $? >exit.status
+ ) |
+ sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
+ res=$(cat exit.status)
+ rm exit.status
+
+ end_group "$group"
+ return $res
+}
+
+begin_group "CI setup"
+trap "end_group 'CI setup'" EXIT
+
# Set 'exit on error' for all CI scripts to let the caller know that
# something went wrong.
#
@@ -287,5 +285,5 @@ esac
MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
-end_group
+end_group "CI setup"
set -x
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 3/5] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
The output of CI jobs tends to be quite long-winded and hard to digest.
To help with this, many CI systems provide the ability to group output
into collapsible sections, and we're also doing this in some of our
scripts.
One notable omission is the script to install Docker dependencies.
Address it to bring more structure to the output for Docker-based jobs.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/install-docker-dependencies.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 78b7e326da6..d0bc19d3bb3 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -3,6 +3,10 @@
# Install dependencies required to build and test Git inside container
#
+. ${0%/*}/lib.sh
+
+begin_group "Install dependencies"
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -20,3 +24,5 @@ pedantic)
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
;;
esac
+
+end_group "Install dependencies"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 4/5] ci: split out logic to set up failed test artifacts
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
We have some logic in place to create a directory with the output from
failed tests, which will then subsequently be uploaded as CI artifacts.
We're about to add support for GitLab CI, which will want to reuse the
logic.
Split the logic into a separate function so that it is reusable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index b3411afae8e..9ffdf743903 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -131,6 +131,27 @@ handle_failed_tests () {
return 1
}
+create_failed_test_artifacts () {
+ mkdir -p t/failed-test-artifacts
+
+ for test_exit in t/test-results/*.exit
+ do
+ test 0 != "$(cat "$test_exit")" || continue
+
+ test_name="${test_exit%.exit}"
+ test_name="${test_name##*/}"
+ printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
+ echo "The full logs are in the 'print test failures' step below."
+ echo "See also the 'failed-tests-*' artifacts attached to this run."
+ cat "t/test-results/$test_name.markup"
+
+ trash_dir="t/trash directory.$test_name"
+ cp "t/test-results/$test_name.out" t/failed-test-artifacts/
+ tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+ done
+ return 1
+}
+
# GitHub Action doesn't set TERM, which is required by tput
export TERM=${TERM:-dumb}
@@ -171,25 +192,8 @@ then
CC="${CC_PACKAGE:-${CC:-gcc}}"
DONT_SKIP_TAGS=t
handle_failed_tests () {
- mkdir -p t/failed-test-artifacts
echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
-
- for test_exit in t/test-results/*.exit
- do
- test 0 != "$(cat "$test_exit")" || continue
-
- test_name="${test_exit%.exit}"
- test_name="${test_name##*/}"
- printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
- echo "The full logs are in the 'print test failures' step below."
- echo "See also the 'failed-tests-*' artifacts attached to this run."
- cat "t/test-results/$test_name.markup"
-
- trash_dir="t/trash directory.$test_name"
- cp "t/test-results/$test_name.out" t/failed-test-artifacts/
- tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
- done
- return 1
+ create_failed_test_artifacts
}
cache_dir="$HOME/none"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 5/5] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-27 9:25 UTC (permalink / raw)
To: git; +Cc: Oswald Buddenhagen
In-Reply-To: <cover.1698398590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6955 bytes --]
We already support Azure Pipelines and GitHub Workflows in the Git
project, but until now we do not have support for GitLab CI. While it is
arguably not in the interest of the Git project to maintain a ton of
different CI platforms, GitLab has recently ramped up its efforts and
tries to contribute to the Git project more regularly.
Part of a problem we hit at GitLab rather frequently is that our own,
custom CI setup we have is so different to the setup that the Git
project has. More esoteric jobs like "linux-TEST-vars" that also set a
couple of environment variables do not exist in GitLab's custom CI
setup, and maintaining them to keep up with what Git does feels like
wasted time. The result is that we regularly send patch series upstream
that fail to compile or pass tests in GitHub Workflows. We would thus
like to integrate the GitLab CI configuration into the Git project to
help us send better patch series upstream and thus reduce overhead for
the maintainer.
The integration does not necessarily have to be a first-class citizen,
which would in practice only add to the fallout that pipeline failures
have for the maintainer. That being said, we are happy to maintain this
alternative CI setup for the Git project and will make test results
available as part of our own mirror of the Git project at [1].
This commit introduces the integration into our regular CI scripts so
that most of the setup continues to be shared across all of the CI
solutions.
[1]: https://gitlab.com/gitlab-org/git
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.gitlab-ci.yml | 51 +++++++++++++++++++++++++++++++
ci/install-docker-dependencies.sh | 9 +++++-
ci/lib.sh | 49 +++++++++++++++++++++++++++++
ci/print-test-failures.sh | 6 ++++
4 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 .gitlab-ci.yml
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 00000000000..43d3a961fa0
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,51 @@
+default:
+ timeout: 2h
+
+workflow:
+ rules:
+ - if: $CI_PIPELINE_SOURCE == "merge_request_event"
+ - if: $CI_COMMIT_TAG
+ - if: $CI_COMMIT_REF_PROTECTED == "true"
+
+test:
+ image: $image
+ before_script:
+ - ./ci/install-docker-dependencies.sh
+ script:
+ - useradd builder --home-dir "${CI_PROJECT_DIR}"
+ - chown -R builder "${CI_PROJECT_DIR}"
+ - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
+ after_script:
+ - |
+ if test "$CI_JOB_STATUS" != 'success'
+ then
+ sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
+ fi
+ 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
+ artifacts:
+ paths:
+ - t/failed-test-artifacts
+ when: on_failure
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
@@ -7,6 +7,9 @@
begin_group "Install dependencies"
+# Required so that apt doesn't wait for user input on certain packages.
+export DEBIAN_FRONTEND=noninteractive
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -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
;;
+linux-*)
+ apt update -q &&
+ apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}}
+ ;;
pedantic)
dnf -yq update >/dev/null &&
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
diff --git a/ci/lib.sh b/ci/lib.sh
index 9ffdf743903..f518df7e2cb 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,6 +14,22 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
+elif test true = "$GITLAB_CI"
+then
+ begin_group () {
+ need_to_end_group=t
+ echo -e "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1"
+ trap "end_group '$1'" EXIT
+ set -x
+ }
+
+ end_group () {
+ test -n "$need_to_end_group" || return 0
+ set +x
+ need_to_end_group=
+ echo -e "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K"
+ trap - EXIT
+ }
else
begin_group () { :; }
end_group () { :; }
@@ -203,6 +219,39 @@ then
MAKEFLAGS="$MAKEFLAGS --jobs=10"
test windows != "$CI_OS_NAME" ||
GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+elif test true = "$GITLAB_CI"
+then
+ CI_TYPE=gitlab-ci
+ CI_BRANCH="$CI_COMMIT_REF_NAME"
+ CI_COMMIT="$CI_COMMIT_SHA"
+ case "$CI_JOB_IMAGE" in
+ macos-*)
+ CI_OS_NAME=osx;;
+ alpine:*|ubuntu:*)
+ CI_OS_NAME=linux;;
+ *)
+ echo "Could not identify OS image" >&2
+ env >&2
+ exit 1
+ ;;
+ esac
+ CI_REPO_SLUG="$CI_PROJECT_PATH"
+ CI_JOB_ID="$CI_JOB_ID"
+ CC="${CC_PACKAGE:-${CC:-gcc}}"
+ DONT_SKIP_TAGS=t
+ handle_failed_tests () {
+ create_failed_test_artifacts
+ }
+
+ cache_dir="$HOME/none"
+
+ runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+
+ export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
+ export GIT_TEST_OPTS="--verbose-log -x"
+ MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
+ test windows != "$CI_OS_NAME" ||
+ GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
else
echo "Could not identify CI type" >&2
env >&2
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index 57277eefcd0..c33ad4e3a22 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -51,6 +51,12 @@ do
tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
continue
;;
+ gitlab-ci)
+ mkdir -p failed-test-artifacts
+ cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
+ tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+ continue
+ ;;
*)
echo "Unhandled CI type: $CI_TYPE" >&2
exit 1
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox