Git development
 help / color / mirror / Atom feed
* Re: facing issue in git in a perticuler directory
From: brian m. carlson @ 2023-10-31 22:46 UTC (permalink / raw)
  To: Injamul Hasan; +Cc: git
In-Reply-To: <CAG4aqRxyY+xeWVc+StqsE3AVp6O2ghFhtW9iHBUFfq+hCiTWEQ@mail.gmail.com>

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

On 2023-10-31 at 20:21:08, Injamul Hasan wrote:
> hello i'm facing this error in my e drive but when i try anything in other
> drive it works properly .do you have any solution?

What kind of disk is your E drive?  Is it built-in or external, is it
local or remote, is it NTFS, FAT, FATX, or something else, and is it
synced in any way with a cloud syncing service (e.g., DropBox,
OneDrive)?

If you are syncing it with a cloud syncing service, you should stop
doing that, since it can cause problems like this and it very frequently
causes corruption.  The recommendation is to use a regular file system
with normal OS semantics.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

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

^ permalink raw reply

* Re: [PATCH v2 1/1] merge-file: add an option to process object IDs
From: brian m. carlson @ 2023-10-31 22:31 UTC (permalink / raw)
  To: Martin Ågren
  Cc: git, Junio C Hamano, Elijah Newren, Phillip Wood, Eric Sunshine,
	Taylor Blau
In-Reply-To: <20231031214859.25293-1-martin.agren@gmail.com>

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

On 2023-10-31 at 21:48:57, Martin Ågren wrote:
> 
> On Mon, 30 Oct 2023 at 17:37, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> >
> > Since we obviously won't be writing the data to the first argument,
> > imply the -p option so we write to standard output.
> 
> This paragraph changed from v1, but this doesn't match the actual
> behavior, from what I can tell: `-p` is not implied.

Yes, apparently that commit message snuck back in after it having been
edited out.

> -- >8 --
> Subject: [PATCH] git-merge-file doc: drop "-file" from argument placeholders
> 
> `git merge-file` takes three positional arguments. Each of them is
> documented as `<foo-file>`. In preparation for teaching this command to
> alternatively take three object IDs, make these placeholders a bit more
> generic by dropping the "-file" parts. Instead, clarify early that the
> three arguments are filenames. Even after the next commit, we can afford
> to present this file-centric view up front and in the general
> discussion, since it will remain the default one.

This seems reasonable.  Junio, do you want to sneak this in and fix the
commit message above, or do you want me to do a v3?
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

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

^ permalink raw reply

* [PATCH v2 4/4] strbuf: move env-using functions to environment.c
From: Jonathan Tan @ 2023-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, Dragan Simic, Phillip Wood, Junio C Hamano
In-Reply-To: <cover.1698791220.git.jonathantanmy@google.com>

This eliminates the dependency from strbuf to environment.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 environment.c | 32 ++++++++++++++++++++++++++++++++
 environment.h | 14 ++++++++++++++
 strbuf.c      | 32 --------------------------------
 strbuf.h      | 15 ---------------
 4 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/environment.c b/environment.c
index bb3c2a96a3..942c5b8dd3 100644
--- a/environment.c
+++ b/environment.c
@@ -18,6 +18,7 @@
 #include "refs.h"
 #include "fmt-merge-msg.h"
 #include "commit.h"
+#include "strbuf.h"
 #include "strvec.h"
 #include "object-file.h"
 #include "object-store-ll.h"
@@ -416,3 +417,34 @@ int print_sha1_ellipsis(void)
 	}
 	return cached_result;
 }
+
+void strbuf_commented_addf(struct strbuf *sb,
+			   const char *fmt, ...)
+{
+	va_list params;
+	struct strbuf buf = STRBUF_INIT;
+	int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
+
+	va_start(params, fmt);
+	strbuf_vaddf(&buf, fmt, params);
+	va_end(params);
+
+	strbuf_add_commented_lines(sb, buf.buf, buf.len);
+	if (incomplete_line)
+		sb->buf[--sb->len] = '\0';
+
+	strbuf_release(&buf);
+}
+
+void strbuf_add_commented_lines(struct strbuf *out,
+				const char *buf, size_t size)
+{
+	static char prefix1[3];
+	static char prefix2[2];
+
+	if (prefix1[0] != comment_line_char) {
+		xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
+		xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
+	}
+	strbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);
+}
diff --git a/environment.h b/environment.h
index e5351c9dd9..f801dbe36e 100644
--- a/environment.h
+++ b/environment.h
@@ -229,4 +229,18 @@ extern const char *excludes_file;
  */
 int print_sha1_ellipsis(void);
 
+/**
+ * Add a formatted string prepended by a comment character and a
+ * blank to the buffer.
+ */
+__attribute__((format (printf, 2, 3)))
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
+
+/**
+ * Add a NUL-terminated string to the buffer. Each line will be prepended
+ * by a comment character and a blank.
+ */
+void strbuf_add_commented_lines(struct strbuf *out,
+				const char *buf, size_t size);
+
 #endif
diff --git a/strbuf.c b/strbuf.c
index d5ee8874f8..f6c1978ecf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "environment.h"
 #include "gettext.h"
 #include "hex-ll.h"
 #include "strbuf.h"
@@ -360,37 +359,6 @@ void strbuf_add_lines_varied_prefix(struct strbuf *sb,
 	strbuf_complete_line(sb);
 }
 
-void strbuf_add_commented_lines(struct strbuf *out,
-				const char *buf, size_t size)
-{
-	static char prefix1[3];
-	static char prefix2[2];
-
-	if (prefix1[0] != comment_line_char) {
-		xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
-		xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
-	}
-	strbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);
-}
-
-void strbuf_commented_addf(struct strbuf *sb,
-			   const char *fmt, ...)
-{
-	va_list params;
-	struct strbuf buf = STRBUF_INIT;
-	int incomplete_line = sb->len && sb->buf[sb->len - 1] != '\n';
-
-	va_start(params, fmt);
-	strbuf_vaddf(&buf, fmt, params);
-	va_end(params);
-
-	strbuf_add_commented_lines(sb, buf.buf, buf.len);
-	if (incomplete_line)
-		sb->buf[--sb->len] = '\0';
-
-	strbuf_release(&buf);
-}
-
 void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
 {
 	int len;
diff --git a/strbuf.h b/strbuf.h
index a9333ac1ad..d5f0d4c579 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -282,14 +282,6 @@ void strbuf_remove(struct strbuf *sb, size_t pos, size_t len);
 void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
 		   const void *data, size_t data_len);
 
-/**
- * Add a NUL-terminated string to the buffer. Each line will be prepended
- * by a comment character and a blank.
- */
-void strbuf_add_commented_lines(struct strbuf *out,
-				const char *buf, size_t size);
-
-
 /**
  * Add data of given length to the buffer.
  */
@@ -373,13 +365,6 @@ void strbuf_humanise_rate(struct strbuf *buf, off_t bytes);
 __attribute__((format (printf,2,3)))
 void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
-/**
- * Add a formatted string prepended by a comment character and a
- * blank to the buffer.
- */
-__attribute__((format (printf, 2, 3)))
-void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
-
 __attribute__((format (printf,2,0)))
 void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
 
-- 
2.42.0.820.g83a721a137-goog


^ permalink raw reply related

* [PATCH v2 3/4] strbuf: make add_lines() public
From: Jonathan Tan @ 2023-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, Dragan Simic, Phillip Wood, Junio C Hamano
In-Reply-To: <cover.1698791220.git.jonathantanmy@google.com>

A subsequent patch will require the ability to add different
prefixes to different lines (depending on their contents), so make
this functionality available from outside strbuf.c. The function
name is chosen to avoid a conflict with the existing function named
strbuf_add_lines().

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 strbuf.c | 22 +++++++++++-----------
 strbuf.h |  4 ++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 2088f7800a..d5ee8874f8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -340,24 +340,24 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	va_end(ap);
 }
 
-static void add_lines(struct strbuf *out,
-			const char *prefix1,
-			const char *prefix2,
-			const char *buf, size_t size)
+void strbuf_add_lines_varied_prefix(struct strbuf *sb,
+				    const char *default_prefix,
+				    const char *tab_nl_prefix,
+				    const char *buf, size_t size)
 {
 	while (size) {
 		const char *prefix;
 		const char *next = memchr(buf, '\n', size);
 		next = next ? (next + 1) : (buf + size);
 
-		prefix = ((prefix2 && (buf[0] == '\n' || buf[0] == '\t'))
-			  ? prefix2 : prefix1);
-		strbuf_addstr(out, prefix);
-		strbuf_add(out, buf, next - buf);
+		prefix = (buf[0] == '\n' || buf[0] == '\t')
+			  ? tab_nl_prefix : default_prefix;
+		strbuf_addstr(sb, prefix);
+		strbuf_add(sb, buf, next - buf);
 		size -= next - buf;
 		buf = next;
 	}
-	strbuf_complete_line(out);
+	strbuf_complete_line(sb);
 }
 
 void strbuf_add_commented_lines(struct strbuf *out,
@@ -370,7 +370,7 @@ void strbuf_add_commented_lines(struct strbuf *out,
 		xsnprintf(prefix1, sizeof(prefix1), "%c ", comment_line_char);
 		xsnprintf(prefix2, sizeof(prefix2), "%c", comment_line_char);
 	}
-	add_lines(out, prefix1, prefix2, buf, size);
+	strbuf_add_lines_varied_prefix(out, prefix1, prefix2, buf, size);
 }
 
 void strbuf_commented_addf(struct strbuf *sb,
@@ -751,7 +751,7 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
 void strbuf_add_lines(struct strbuf *out, const char *prefix,
 		      const char *buf, size_t size)
 {
-	add_lines(out, prefix, NULL, buf, size);
+	strbuf_add_lines_varied_prefix(out, prefix, prefix, buf, size);
 }
 
 void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
diff --git a/strbuf.h b/strbuf.h
index 4547efa62e..a9333ac1ad 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -601,6 +601,10 @@ void strbuf_add_lines(struct strbuf *sb,
 		      const char *prefix,
 		      const char *buf,
 		      size_t size);
+void strbuf_add_lines_varied_prefix(struct strbuf *sb,
+				    const char *default_prefix,
+				    const char *tab_nl_prefix,
+				    const char *buf, size_t size);
 
 /**
  * Append s to sb, with the characters '<', '>', '&' and '"' converted
-- 
2.42.0.820.g83a721a137-goog


^ permalink raw reply related

* [PATCH v2 2/4] strbuf_add_commented_lines(): drop the comment_line_char parameter
From: Jonathan Tan @ 2023-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Dragan Simic, Phillip Wood, Jonathan Tan
In-Reply-To: <cover.1698791220.git.jonathantanmy@google.com>

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

All the callers of this function supply the global variable
comment_line_char as an argument to its last parameter.  Remove the
parameter to allow us in the future to change the reference to the
global variable with something else, like a function call.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/notes.c      |  9 ++++-----
 builtin/stripspace.c |  2 +-
 fmt-merge-msg.c      |  9 +++------
 rebase-interactive.c |  6 +++---
 sequencer.c          | 10 ++++------
 strbuf.c             |  6 +++---
 strbuf.h             |  3 +--
 wt-status.c          |  4 ++--
 8 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 9f38863dd5..355ecce07a 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -181,7 +181,7 @@ static void write_commented_object(int fd, const struct object_id *object)
 
 	if (strbuf_read(&buf, show.out, 0) < 0)
 		die_errno(_("could not read 'show' output"));
-	strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_char);
+	strbuf_add_commented_lines(&cbuf, buf.buf, buf.len);
 	write_or_die(fd, cbuf.buf, cbuf.len);
 
 	strbuf_release(&cbuf);
@@ -209,10 +209,9 @@ static void prepare_note_data(const struct object_id *object, struct note_data *
 			copy_obj_to_fd(fd, old_note);
 
 		strbuf_addch(&buf, '\n');
-		strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_char);
-		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)),
-					   comment_line_char);
-		strbuf_add_commented_lines(&buf, "\n", strlen("\n"), comment_line_char);
+		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
+		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
+		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
 		write_or_die(fd, buf.buf, buf.len);
 
 		write_commented_object(fd, object);
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 7b700a9fb1..11e475760c 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -13,7 +13,7 @@ static void comment_lines(struct strbuf *buf)
 	size_t len;
 
 	msg = strbuf_detach(buf, &len);
-	strbuf_add_commented_lines(buf, msg, len, comment_line_char);
+	strbuf_add_commented_lines(buf, msg, len);
 	free(msg);
 }
 
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 66e47449a0..adc85d2a72 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -509,8 +509,7 @@ static void fmt_tag_signature(struct strbuf *tagbuf,
 	strbuf_complete_line(tagbuf);
 	if (sig->len) {
 		strbuf_addch(tagbuf, '\n');
-		strbuf_add_commented_lines(tagbuf, sig->buf, sig->len,
-					   comment_line_char);
+		strbuf_add_commented_lines(tagbuf, sig->buf, sig->len);
 	}
 }
 
@@ -556,8 +555,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 				strbuf_addch(&tagline, '\n');
 				strbuf_add_commented_lines(&tagline,
 						origins.items[first_tag].string,
-						strlen(origins.items[first_tag].string),
-						comment_line_char);
+						strlen(origins.items[first_tag].string));
 				strbuf_insert(&tagbuf, 0, tagline.buf,
 					      tagline.len);
 				strbuf_release(&tagline);
@@ -565,8 +563,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 			strbuf_addch(&tagbuf, '\n');
 			strbuf_add_commented_lines(&tagbuf,
 					origins.items[i].string,
-					strlen(origins.items[i].string),
-					comment_line_char);
+					strlen(origins.items[i].string));
 			fmt_tag_signature(&tagbuf, &sig, buf, len);
 		}
 		strbuf_release(&payload);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 3f33da7f03..1138bd37ba 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -78,7 +78,7 @@ void append_todo_help(int command_count,
 				      shortrevisions, shortonto, command_count);
 	}
 
-	strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);
+	strbuf_add_commented_lines(buf, msg, strlen(msg));
 
 	if (get_missing_commit_check_level() == MISSING_COMMIT_CHECK_ERROR)
 		msg = _("\nDo not remove any line. Use 'drop' "
@@ -87,7 +87,7 @@ void append_todo_help(int command_count,
 		msg = _("\nIf you remove a line here "
 			 "THAT COMMIT WILL BE LOST.\n");
 
-	strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);
+	strbuf_add_commented_lines(buf, msg, strlen(msg));
 
 	if (edit_todo)
 		msg = _("\nYou are editing the todo file "
@@ -98,7 +98,7 @@ void append_todo_help(int command_count,
 		msg = _("\nHowever, if you remove everything, "
 			"the rebase will be aborted.\n\n");
 
-	strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_char);
+	strbuf_add_commented_lines(buf, msg, strlen(msg));
 }
 
 int edit_todo_list(struct repository *r, struct todo_list *todo_list,
diff --git a/sequencer.c b/sequencer.c
index 5d348a3f12..29c8b5e32b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1859,7 +1859,7 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len)
 		s += count;
 		len -= count;
 	}
-	strbuf_add_commented_lines(buf, s, len, comment_line_char);
+	strbuf_add_commented_lines(buf, s, len);
 }
 
 /* Does the current fixup chain contain a squash command? */
@@ -1958,7 +1958,7 @@ static int append_squash_message(struct strbuf *buf, const char *body,
 	strbuf_addf(buf, _(nth_commit_msg_fmt),
 		    ++opts->current_fixup_count + 1);
 	strbuf_addstr(buf, "\n\n");
-	strbuf_add_commented_lines(buf, body, commented_len, comment_line_char);
+	strbuf_add_commented_lines(buf, body, commented_len);
 	/* buf->buf may be reallocated so store an offset into the buffer */
 	fixup_off = buf->len;
 	strbuf_addstr(buf, body + commented_len);
@@ -2048,8 +2048,7 @@ static int update_squash_messages(struct repository *r,
 			      _(first_commit_msg_str));
 		strbuf_addstr(&buf, "\n\n");
 		if (is_fixup_flag(command, flag))
-			strbuf_add_commented_lines(&buf, body, strlen(body),
-						   comment_line_char);
+			strbuf_add_commented_lines(&buf, body, strlen(body));
 		else
 			strbuf_addstr(&buf, body);
 
@@ -2068,8 +2067,7 @@ static int update_squash_messages(struct repository *r,
 		strbuf_addf(&buf, _(skip_nth_commit_msg_fmt),
 			    ++opts->current_fixup_count + 1);
 		strbuf_addstr(&buf, "\n\n");
-		strbuf_add_commented_lines(&buf, body, strlen(body),
-					   comment_line_char);
+		strbuf_add_commented_lines(&buf, body, strlen(body));
 	} else
 		return error(_("unknown command: %d"), command);
 	repo_unuse_commit_buffer(r, commit, message);
diff --git a/strbuf.c b/strbuf.c
index 15550b2619..2088f7800a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -360,8 +360,8 @@ static void add_lines(struct strbuf *out,
 	strbuf_complete_line(out);
 }
 
-void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
-				size_t size, char comment_line_char)
+void strbuf_add_commented_lines(struct strbuf *out,
+				const char *buf, size_t size)
 {
 	static char prefix1[3];
 	static char prefix2[2];
@@ -384,7 +384,7 @@ void strbuf_commented_addf(struct strbuf *sb,
 	strbuf_vaddf(&buf, fmt, params);
 	va_end(params);
 
-	strbuf_add_commented_lines(sb, buf.buf, buf.len, comment_line_char);
+	strbuf_add_commented_lines(sb, buf.buf, buf.len);
 	if (incomplete_line)
 		sb->buf[--sb->len] = '\0';
 
diff --git a/strbuf.h b/strbuf.h
index 981617dc77..4547efa62e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -287,8 +287,7 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
  * by a comment character and a blank.
  */
 void strbuf_add_commented_lines(struct strbuf *out,
-				const char *buf, size_t size,
-				char comment_line_char);
+				const char *buf, size_t size);
 
 
 /**
diff --git a/wt-status.c b/wt-status.c
index 54b2775730..b390c77334 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1027,7 +1027,7 @@ static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncom
 	if (s->display_comment_prefix) {
 		size_t len;
 		summary_content = strbuf_detach(&summary, &len);
-		strbuf_add_commented_lines(&summary, summary_content, len, comment_line_char);
+		strbuf_add_commented_lines(&summary, summary_content, len);
 		free(summary_content);
 	}
 
@@ -1103,7 +1103,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
 	const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
 
 	strbuf_commented_addf(buf, "%s", cut_line);
-	strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);
+	strbuf_add_commented_lines(buf, explanation, strlen(explanation));
 }
 
 void wt_status_add_cut_line(FILE *fp)
-- 
2.42.0.820.g83a721a137-goog


^ permalink raw reply related

* [PATCH v2 1/4] strbuf_commented_addf(): drop the comment_line_char parameter
From: Jonathan Tan @ 2023-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Dragan Simic, Phillip Wood, Jonathan Tan
In-Reply-To: <cover.1698791220.git.jonathantanmy@google.com>

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

All the callers of this function supply the global variable
comment_line_char as an argument to its second parameter.  Remove
the parameter to allow us in the future to change the reference to
the global variable with something else, like a function call.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 add-patch.c          | 8 ++++----
 builtin/branch.c     | 2 +-
 builtin/merge.c      | 8 ++++----
 builtin/tag.c        | 4 ++--
 rebase-interactive.c | 2 +-
 sequencer.c          | 4 ++--
 strbuf.c             | 3 ++-
 strbuf.h             | 4 ++--
 wt-status.c          | 2 +-
 9 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index bfe19876cd..471a0037be 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1106,11 +1106,11 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 	size_t i;
 
 	strbuf_reset(&s->buf);
-	strbuf_commented_addf(&s->buf, comment_line_char,
+	strbuf_commented_addf(&s->buf,
 			      _("Manual hunk edit mode -- see bottom for "
 				"a quick guide.\n"));
 	render_hunk(s, hunk, 0, 0, &s->buf);
-	strbuf_commented_addf(&s->buf, comment_line_char,
+	strbuf_commented_addf(&s->buf,
 			      _("---\n"
 				"To remove '%c' lines, make them ' ' lines "
 				"(context).\n"
@@ -1119,13 +1119,13 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 			      s->mode->is_reverse ? '+' : '-',
 			      s->mode->is_reverse ? '-' : '+',
 			      comment_line_char);
-	strbuf_commented_addf(&s->buf, comment_line_char, "%s",
+	strbuf_commented_addf(&s->buf, "%s",
 			      _(s->mode->edit_hunk_hint));
 	/*
 	 * TRANSLATORS: 'it' refers to the patch mentioned in the previous
 	 * messages.
 	 */
-	strbuf_commented_addf(&s->buf, comment_line_char,
+	strbuf_commented_addf(&s->buf,
 			      _("If it does not apply cleanly, you will be "
 				"given an opportunity to\n"
 				"edit again.  If all lines of the hunk are "
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ec190b14a..b2f171e10b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -668,7 +668,7 @@ static int edit_branch_description(const char *branch_name)
 	exists = !read_branch_desc(&buf, branch_name);
 	if (!buf.len || buf.buf[buf.len-1] != '\n')
 		strbuf_addch(&buf, '\n');
-	strbuf_commented_addf(&buf, comment_line_char,
+	strbuf_commented_addf(&buf,
 		    _("Please edit the description for the branch\n"
 		      "  %s\n"
 		      "Lines starting with '%c' will be stripped.\n"),
diff --git a/builtin/merge.c b/builtin/merge.c
index d748d46e13..8f0e8be7c3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -857,15 +857,15 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 		strbuf_addch(&msg, '\n');
 		if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
 			wt_status_append_cut_line(&msg);
-			strbuf_commented_addf(&msg, comment_line_char, "\n");
+			strbuf_commented_addf(&msg, "\n");
 		}
-		strbuf_commented_addf(&msg, comment_line_char,
+		strbuf_commented_addf(&msg,
 				      _(merge_editor_comment));
 		if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
-			strbuf_commented_addf(&msg, comment_line_char,
+			strbuf_commented_addf(&msg,
 					      _(scissors_editor_comment));
 		else
-			strbuf_commented_addf(&msg, comment_line_char,
+			strbuf_commented_addf(&msg,
 				_(no_scissors_editor_comment), comment_line_char);
 	}
 	if (signoff)
diff --git a/builtin/tag.c b/builtin/tag.c
index 3918eacbb5..a85a0d8def 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -314,10 +314,10 @@ static void create_tag(const struct object_id *object, const char *object_ref,
 			struct strbuf buf = STRBUF_INIT;
 			strbuf_addch(&buf, '\n');
 			if (opt->cleanup_mode == CLEANUP_ALL)
-				strbuf_commented_addf(&buf, comment_line_char,
+				strbuf_commented_addf(&buf,
 				      _(tag_template), tag, comment_line_char);
 			else
-				strbuf_commented_addf(&buf, comment_line_char,
+				strbuf_commented_addf(&buf,
 				      _(tag_template_nocleanup), tag, comment_line_char);
 			write_or_die(fd, buf.buf, buf.len);
 			strbuf_release(&buf);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index d9718409b3..3f33da7f03 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -71,7 +71,7 @@ void append_todo_help(int command_count,
 
 	if (!edit_todo) {
 		strbuf_addch(buf, '\n');
-		strbuf_commented_addf(buf, comment_line_char,
+		strbuf_commented_addf(buf,
 				      Q_("Rebase %s onto %s (%d command)",
 					 "Rebase %s onto %s (%d commands)",
 					 command_count),
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed..5d348a3f12 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -675,11 +675,11 @@ void append_conflicts_hint(struct index_state *istate,
 	}
 
 	strbuf_addch(msgbuf, '\n');
-	strbuf_commented_addf(msgbuf, comment_line_char, "Conflicts:\n");
+	strbuf_commented_addf(msgbuf, "Conflicts:\n");
 	for (i = 0; i < istate->cache_nr;) {
 		const struct cache_entry *ce = istate->cache[i++];
 		if (ce_stage(ce)) {
-			strbuf_commented_addf(msgbuf, comment_line_char,
+			strbuf_commented_addf(msgbuf,
 					      "\t%s\n", ce->name);
 			while (i < istate->cache_nr &&
 			       !strcmp(ce->name, istate->cache[i]->name))
diff --git a/strbuf.c b/strbuf.c
index 7827178d8e..15550b2619 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,4 +1,5 @@
 #include "git-compat-util.h"
+#include "environment.h"
 #include "gettext.h"
 #include "hex-ll.h"
 #include "strbuf.h"
@@ -372,7 +373,7 @@ void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
 	add_lines(out, prefix1, prefix2, buf, size);
 }
 
-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char,
+void strbuf_commented_addf(struct strbuf *sb,
 			   const char *fmt, ...)
 {
 	va_list params;
diff --git a/strbuf.h b/strbuf.h
index e959caca87..981617dc77 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -378,8 +378,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
  * Add a formatted string prepended by a comment character and a
  * blank to the buffer.
  */
-__attribute__((format (printf, 3, 4)))
-void strbuf_commented_addf(struct strbuf *sb, char comment_line_char, const char *fmt, ...);
+__attribute__((format (printf, 2, 3)))
+void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
 
 __attribute__((format (printf,2,0)))
 void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..54b2775730 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1102,7 +1102,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
 {
 	const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
 
-	strbuf_commented_addf(buf, comment_line_char, "%s", cut_line);
+	strbuf_commented_addf(buf, "%s", cut_line);
 	strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);
 }
 
-- 
2.42.0.820.g83a721a137-goog


^ permalink raw reply related

* [PATCH v2 0/4] Avoid passing global comment_line_char repeatedly
From: Jonathan Tan @ 2023-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, Dragan Simic, Phillip Wood, Junio C Hamano
In-Reply-To: <db6702ba-11a7-44c1-af2a-95b080aaeb77@gmail.com>

Here's an updated patchset. The first 2 are the exact same as what Junio
has sent.

Jonathan Tan (2):
  strbuf: make add_lines() public
  strbuf: move env-using functions to environment.c

Junio C Hamano (2):
  strbuf_commented_addf(): drop the comment_line_char parameter
  strbuf_add_commented_lines(): drop the comment_line_char parameter

 add-patch.c          |  8 +++----
 builtin/branch.c     |  2 +-
 builtin/merge.c      |  8 +++----
 builtin/notes.c      |  9 ++++----
 builtin/stripspace.c |  2 +-
 builtin/tag.c        |  4 ++--
 environment.c        | 32 +++++++++++++++++++++++++++
 environment.h        | 14 ++++++++++++
 fmt-merge-msg.c      |  9 +++-----
 rebase-interactive.c |  8 +++----
 sequencer.c          | 14 ++++++------
 strbuf.c             | 51 +++++++++-----------------------------------
 strbuf.h             | 20 ++++-------------
 wt-status.c          |  6 +++---
 14 files changed, 92 insertions(+), 95 deletions(-)

-- 
2.42.0.820.g83a721a137-goog


^ permalink raw reply

* Re: [RFC PATCH 2/3] strbuf_commented_addf(): drop the comment_line_char parameter
From: Jonathan Tan @ 2023-10-31 22:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Tan, git, Phillip Wood, Dragan Simic
In-Reply-To: <xmqqh6m74bdo.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:
> This moving of the helper function does not belong to the "fix
> commented_addf() not to take the comment_line_char" step.
> 
> The series should be restructured to have the two patches from me
> first, and then your moving some stuff to environment.c, probably.

This means that #include "environment.h" will be added and then removed
in the same series, but I don't feel too strongly about that. I'll send
an updated set of patches.

^ permalink raw reply

* Re: [PATCH v2 1/1] merge-file: add an option to process object IDs
From: Martin Ågren @ 2023-10-31 21:48 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Junio C Hamano, Elijah Newren, Phillip Wood, Eric Sunshine,
	Taylor Blau
In-Reply-To: <20231030162658.567523-2-sandals@crustytoothpaste.net>


On Mon, 30 Oct 2023 at 17:37, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>
> Since we obviously won't be writing the data to the first argument,
> imply the -p option so we write to standard output.

This paragraph changed from v1, but this doesn't match the actual
behavior, from what I can tell: `-p` is not implied.

>  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
>         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
>         [--[no-]diff3] <current-file> <base-file> <other-file>
> +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]
> +       [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
> +       [--[no-]diff3] <current-oid> <base-oid> <other-oid>

I see this duplicated synopsis was discussed on v1, and that the
difference here is "file" vs "oid". It seems we could avoid this
redundancy and risk of going out of sync with no downside that I can see
by instead dropping all these "-file". See below for a patch that could
go in as a preparatory step.

> +If `--object-id` is specified, exactly the same behavior occurs, except that
> +instead of specifying what to merge as files, it is specified as a list of
> +object IDs referring to blobs.

Makes sense.

> +If the `-p` option is specified, the merged file (including conflicts, if any)
> +goes to standard output as normal; otherwise, the merged file is written to the
> +object store and the object ID of its blob is written to standard output.

(Here, `-p` is not implied.)

> +test_expect_success 'merge without conflict with --object-id' '
> +       git add orig.txt new2.txt &&
> +       git merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&
> +       git rev-parse :new2.txt >expected &&
> +       test_cmp actual expected
> +'

(Here, `-p` is not implied.)

Martin

-- >8 --
Subject: [PATCH] git-merge-file doc: drop "-file" from argument placeholders

`git merge-file` takes three positional arguments. Each of them is
documented as `<foo-file>`. In preparation for teaching this command to
alternatively take three object IDs, make these placeholders a bit more
generic by dropping the "-file" parts. Instead, clarify early that the
three arguments are filenames. Even after the next commit, we can afford
to present this file-centric view up front and in the general
discussion, since it will remain the default one.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/git-merge-file.txt | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 7e9093fab6..bf0a18cf02 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -11,19 +11,20 @@ SYNOPSIS
 [verse]
 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
 	[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
-	[--[no-]diff3] <current-file> <base-file> <other-file>
+	[--[no-]diff3] <current> <base> <other>
 
 
 DESCRIPTION
 -----------
-'git merge-file' incorporates all changes that lead from the `<base-file>`
-to `<other-file>` into `<current-file>`. The result ordinarily goes into
-`<current-file>`. 'git merge-file' is useful for combining separate changes
-to an original. Suppose `<base-file>` is the original, and both
-`<current-file>` and `<other-file>` are modifications of `<base-file>`,
+Given three files `<current>`, `<base>` and `<other>`,
+'git merge-file' incorporates all changes that lead from `<base>`
+to `<other>` into `<current>`. The result ordinarily goes into
+`<current>`. 'git merge-file' is useful for combining separate changes
+to an original. Suppose `<base>` is the original, and both
+`<current>` and `<other>` are modifications of `<base>`,
 then 'git merge-file' combines both changes.
 
-A conflict occurs if both `<current-file>` and `<other-file>` have changes
+A conflict occurs if both `<current>` and `<other>` have changes
 in a common segment of lines. If a conflict is found, 'git merge-file'
 normally outputs a warning and brackets the conflict with lines containing
 <<<<<<< and >>>>>>> markers. A typical conflict will look like this:
@@ -36,8 +37,8 @@ normally outputs a warning and brackets the conflict with lines containing
 
 If there are conflicts, the user should edit the result and delete one of
 the alternatives.  When `--ours`, `--theirs`, or `--union` option is in effect,
-however, these conflicts are resolved favouring lines from `<current-file>`,
-lines from `<other-file>`, or lines from both respectively.  The length of the
+however, these conflicts are resolved favouring lines from `<current>`,
+lines from `<other>`, or lines from both respectively.  The length of the
 conflict markers can be given with the `--marker-size` option.
 
 The exit value of this program is negative on error, and the number of
@@ -62,7 +63,7 @@ OPTIONS
 
 -p::
 	Send results to standard output instead of overwriting
-	`<current-file>`.
+	`<current>`.
 
 -q::
 	Quiet; do not warn about conflicts.
-- 
2.42.0.899.gfd14d11e2b


^ permalink raw reply related

* facing issue in git in a perticuler directory
From: Injamul Hasan @ 2023-10-31 20:21 UTC (permalink / raw)
  To: git


[-- Attachment #1.1: Type: text/plain, Size: 126 bytes --]

hello i'm facing this error in my e drive but when i try anything in other
drive it works properly .do you have any solution?

[-- Attachment #1.2: Type: text/html, Size: 151 bytes --]

[-- Attachment #2: Screenshot 2023-11-01 021641.png --]
[-- Type: image/png, Size: 13425 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/2] Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
From: Jeff King @ 2023-10-31 20:00 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Junio C Hamano
In-Reply-To: <c149be35a1da66c5e1bbc1dd82839e32a52ace36.1698780244.git.me@ttaylorr.com>

On Tue, Oct 31, 2023 at 03:24:11PM -0400, Taylor Blau wrote:

> Back in 32f3c541e3 (multi-pack-index: write pack names in chunk,
> 2018-07-12) the MIDX's "Packfile Names" (or "PNAM", for short) chunk was
> described as containing an array of string entries. e0d1bcf825 notes
> that this is the only chunk in the MIDX format's specification that is
> not guaranteed to be 4-byte aligned, and so should be placed last.
> 
> This isn't quite accurate: the entries within the PNAM chunk are not
> guaranteed to be 4-byte aligned since they are arbitrary strings, but
> the chunk itself is 4-byte aligned since the ending is padded with NUL
> bytes.

We also don't place it last! :) So the alignment is very important, as I
found out in the recent chunk-corruption series.

> So these have always been externally aligned. Correct the corresponding
> part of our documentation to reflect that.

Both this and the previous patch look good to me.

-Peff

^ permalink raw reply

* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Jeff King @ 2023-10-31 19:36 UTC (permalink / raw)
  To: phillip.wood; +Cc: Oswald Buddenhagen, Patrick Steinhardt, git
In-Reply-To: <d00b02e9-fb05-44bc-90ee-1851ef98dd26@gmail.com>

On Fri, Oct 27, 2023 at 02:17:02PM +0100, Phillip Wood wrote:

> On 27/10/2023 12:01, Oswald Buddenhagen wrote:
> > On Fri, Oct 27, 2023 at 11:25:41AM +0200, Patrick Steinhardt wrote:
> > > +    export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
> > > +    export GIT_TEST_OPTS="--verbose-log -x"
> > > 
> > fwiw (as this is again only copied), export with assignment is a
> > bash-ism
> 
> Not according to https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
> 
>     SYNOPSIS
> 
>         export name[=word]...
> 
>     DESCRIPTION
> 
>         The shell shall give the export attribute to the variables
>         corresponding to the specified names, which shall cause them
>         to be in the environment of subsequently executed commands. If
>         the name of a variable is followed by = word, then the value
>         of that variable shall be set to word.
> 
> It is true that in our test suite we separate a variable assignment when
> exporting. Presumably that is because someone reported that their shell did
> not support the "export name=WORD" syntax in the past. As we're already
> using this syntax with the same docker images in Github Actions I think we
> can assume it is safe here.

I've wondered about the origin of this myself, and tried to do some
digging. All of the commits I found removing "export var=val" vaguely
say "unportable" or "some shells can't handle", etc.

The oldest mention I found on the mailing list was this thread:

  https://lore.kernel.org/git/7vfyb0wexo.fsf@assigned-by-dhcp.cox.net/

which is even more explicit about its vagueness.

I wouldn't be surprised if SunOS/Solaris /bin/sh had problems with it,
as that has been a common headache shell in the past. But I think we
finally declared it unusable for other reasons (and they ship a more
capable shell in /usr/xpg6, if anybody even still wants to build on
those operating systems these days).

So it's possible that avoiding "export var=val" is mostly superstition,
and we could loosen our rules these days. But some things to consider:

  1. Some people may prefer reading the separated form (Junio indicates
     such in the thread linked above, but I don't know how strong a
     preference that is).

  2. We won't really know if there is a odd-ball shell that rejects it
     unless we make a change and wait for a while to see if anybody
     screams. The existing ones in ci/ show that it is not a problem for
     the platforms where we run CI, but I suspect the scripts in t/ see
     a wider audience.

I don't think this has any real bearing on the patches being proposed,
but I have been curious about this for our other scripts for a while
now.

-Peff

^ permalink raw reply

* [PATCH v2 2/2] Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
From: Taylor Blau @ 2023-10-31 19:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1698780244.git.me@ttaylorr.com>

Back in 32f3c541e3 (multi-pack-index: write pack names in chunk,
2018-07-12) the MIDX's "Packfile Names" (or "PNAM", for short) chunk was
described as containing an array of string entries. e0d1bcf825 notes
that this is the only chunk in the MIDX format's specification that is
not guaranteed to be 4-byte aligned, and so should be placed last.

This isn't quite accurate: the entries within the PNAM chunk are not
guaranteed to be 4-byte aligned since they are arbitrary strings, but
the chunk itself is 4-byte aligned since the ending is padded with NUL
bytes.

That padding has always been there since 32f3c541e3 via
midx.c::write_midx_pack_names(), which ended with:

    i = MIDX_CHUNK_ALIGNMENT - (written % MIDX_CHUNK_ALIGNMENT)
    if (i < MIDX_CHUNK_ALIGNMENT) {
      unsigned char padding[MIDX_CHUNK_ALIGNMENT];
      memset(padding, 0, sizeof(padding))
      hashwrite(f, padding, i);
      written += i;
    }

In fact, 32f3c541e3's log message itself describes the chunk in its
first paragraph with:

    Since filenames are not well structured, add padding to keep good
    alignment in later chunks.

So these have always been externally aligned. Correct the corresponding
part of our documentation to reflect that.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/gitformat-pack.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/gitformat-pack.txt b/Documentation/gitformat-pack.txt
index c4eb09d52a..9fcb29a9c8 100644
--- a/Documentation/gitformat-pack.txt
+++ b/Documentation/gitformat-pack.txt
@@ -390,10 +390,11 @@ CHUNK LOOKUP:
 CHUNK DATA:
 
 	Packfile Names (ID: {'P', 'N', 'A', 'M'})
-	    Stores the packfile names as concatenated, NUL-terminated strings.
-	    Packfiles must be listed in lexicographic order for fast lookups by
-	    name. This is the only chunk not guaranteed to be a multiple of four
-	    bytes in length, so should be the last chunk for alignment reasons.
+	    Store the names of packfiles as a sequence of NUL-terminated
+	    strings. There is no extra padding between the filenames,
+	    and they are listed in lexicographic order. The chunk itself
+	    is padded at the end with between 0 and 3 NUL bytes to make the
+	    chunk size a multiple of 4 bytes.
 
 	OID Fanout (ID: {'O', 'I', 'D', 'F'})
 	    The ith entry, F[i], stores the number of OIDs with first
-- 
2.42.0.527.ge89c67d052

^ permalink raw reply related

* [PATCH v2 1/2] Documentation/gitformat-pack.txt: fix typo
From: Taylor Blau @ 2023-10-31 19:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1698780244.git.me@ttaylorr.com>

e0d1bcf825 (multi-pack-index: add format details, 2018-07-12) describes
the MIDX's "PNAM" chunk as having entries which are "null-terminated
strings".

This is a typo, as strings are terminated with a NUL character, which is
a distinct concept from "NULL" or "null", which we typically reserve for
the void pointer to address 0.

Correct the documentation accordingly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/gitformat-pack.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitformat-pack.txt b/Documentation/gitformat-pack.txt
index 4a4d87e7db..c4eb09d52a 100644
--- a/Documentation/gitformat-pack.txt
+++ b/Documentation/gitformat-pack.txt
@@ -390,7 +390,7 @@ CHUNK LOOKUP:
 CHUNK DATA:
 
 	Packfile Names (ID: {'P', 'N', 'A', 'M'})
-	    Stores the packfile names as concatenated, null-terminated strings.
+	    Stores the packfile names as concatenated, NUL-terminated strings.
 	    Packfiles must be listed in lexicographic order for fast lookups by
 	    name. This is the only chunk not guaranteed to be a multiple of four
 	    bytes in length, so should be the last chunk for alignment reasons.
-- 
2.42.0.527.ge89c67d052


^ permalink raw reply related

* [PATCH v2 0/2] Documentation/gitformat-pack.txt: correct a few issues/typos
From: Taylor Blau @ 2023-10-31 19:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1697144959.git.me@ttaylorr.com>

A minor reroll to adjust the text of the second patch to read more
clearly, thanks to input from Junio.

This has been rebased onto 692be87cbb (Merge branch
'jm/bisect-run-synopsis-fix', 2023-10-31). Thanks in advance for your
review!

Taylor Blau (2):
  Documentation/gitformat-pack.txt: fix typo
  Documentation/gitformat-pack.txt: fix incorrect MIDX documentation

 Documentation/gitformat-pack.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Range-diff against v1:
1:  8c5fa1ff4f = 1:  92e9bee4ad Documentation/gitformat-pack.txt: fix typo
2:  af2742e05d ! 2:  c149be35a1 Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
    @@ Metadata
      ## Commit message ##
         Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
     
    -    Back in 32f3c541e3 (multi-pack-index: write pack names in chunk, 2018-07-12)
    -    the MIDX's "Packfile Names" (or "PNAM", for short) chunk was described
    -    as containing an array of string entries. e0d1bcf825 notes that this is
    -    the only chunk in the MIDX format's specification that is not guaranteed
    -    to be 4-byte aligned, and so should be placed last.
    +    Back in 32f3c541e3 (multi-pack-index: write pack names in chunk,
    +    2018-07-12) the MIDX's "Packfile Names" (or "PNAM", for short) chunk was
    +    described as containing an array of string entries. e0d1bcf825 notes
    +    that this is the only chunk in the MIDX format's specification that is
    +    not guaranteed to be 4-byte aligned, and so should be placed last.
     
         This isn't quite accurate: the entries within the PNAM chunk are not
    -    guaranteed to be aligned since they are arbitrary strings, but the
    -    chunk itself is aligned since the ending is padded with NUL bytes.
    +    guaranteed to be 4-byte aligned since they are arbitrary strings, but
    +    the chunk itself is 4-byte aligned since the ending is padded with NUL
    +    bytes.
     
    -    That external padding has always been there since 32f3c541e3 via
    +    That padding has always been there since 32f3c541e3 via
         midx.c::write_midx_pack_names(), which ended with:
     
             i = MIDX_CHUNK_ALIGNMENT - (written % MIDX_CHUNK_ALIGNMENT)
    @@ Commit message
         So these have always been externally aligned. Correct the corresponding
         part of our documentation to reflect that.
     
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
      ## Documentation/gitformat-pack.txt ##
    -@@ Documentation/gitformat-pack.txt: CHUNK DATA:
    +@@ Documentation/gitformat-pack.txt: CHUNK LOOKUP:
    + CHUNK DATA:
    + 
      	Packfile Names (ID: {'P', 'N', 'A', 'M'})
    - 	    Stores the packfile names as concatenated, NUL-terminated strings.
    - 	    Packfiles must be listed in lexicographic order for fast lookups by
    +-	    Stores the packfile names as concatenated, NUL-terminated strings.
    +-	    Packfiles must be listed in lexicographic order for fast lookups by
     -	    name. This is the only chunk not guaranteed to be a multiple of four
     -	    bytes in length, so should be the last chunk for alignment reasons.
    -+	    name. Individual entries in this chunk are not guarenteed to be
    -+	    aligned. The chunk is externally padded with zeros to align
    -+	    remaining chunks.
    ++	    Store the names of packfiles as a sequence of NUL-terminated
    ++	    strings. There is no extra padding between the filenames,
    ++	    and they are listed in lexicographic order. The chunk itself
    ++	    is padded at the end with between 0 and 3 NUL bytes to make the
    ++	    chunk size a multiple of 4 bytes.
      
      	OID Fanout (ID: {'O', 'I', 'D', 'F'})
      	    The ith entry, F[i], stores the number of OIDs with first

base-commit: 692be87cbba55e8488f805d236f2ad50483bd7d5
-- 
2.42.0.527.ge89c67d052

^ permalink raw reply

* Re: [PATCH v3 0/2] commit-graph: detect commits missing in ODB
From: Taylor Blau @ 2023-10-31 19:16 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Junio C Hamano, Jeff King
In-Reply-To: <cover.1698736363.git.ps@pks.im>

On Tue, Oct 31, 2023 at 08:16:09AM +0100, Patrick Steinhardt wrote:
> Patrick Steinhardt (2):
>   commit-graph: introduce envvar to disable commit existence checks
>   commit: detect commits that exist in commit-graph but not in the ODB
>
>  Documentation/git.txt   | 10 +++++++++
>  commit-graph.c          |  6 +++++-
>  commit-graph.h          |  6 ++++++
>  commit.c                | 16 +++++++++++++-
>  t/t5318-commit-graph.sh | 48 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 84 insertions(+), 2 deletions(-)
>
> Range-diff against v2:

Thanks, the range-diff here looks exactly as expected. Thanks for
working on this, this version LGTM.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH 0/5] ci: add GitLab CI definition
From: Taylor Blau @ 2023-10-31 19:12 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ZUCw1B6oQaDWKx3O@tanuki>

On Tue, Oct 31, 2023 at 08:46:28AM +0100, Patrick Steinhardt wrote:
> On Mon, Oct 30, 2023 at 11:46:44AM -0400, Taylor Blau wrote:
> > On Thu, Oct 26, 2023 at 09:59:59AM +0200, Patrick Steinhardt wrote:
> > > And this is exactly what this patch series does: it adds GitLab-specific
> > > knowledge to our CI scripts and adds a CI definition that builds on top
> > > of those scripts. This is rather straight forward, as the scripts
> > > already know to discern Azure Pipelines and GitHub Actions, and adding
> > > a third item to this list feels quite natural. And by building on top of
> > > the preexisting infra, the actual ".gitlab-ci.yml" is really quite
> > > small.
> > >
> > > I acknowledge that the Git project may not be willing to fully support
> > > GitLab CI, and that's fine with me. If we want to further stress that
> > > point then I'd also be perfectly happy to move the definitions into the
> > > "contrib/" directory -- it would still be a huge win for our workflow.
> > > In any case, I'm happy to keep on maintaining the intgeration with
> > > GitLab CI, and if things break I'll do my best to fix them fast.
> >
> > I don't have any strong opinions here, but my preference would probably
> > be to keep any GitLab-specific CI configuration limited to "contrib", if
> > it lands in the tree at all.
>
> As mentioned, I would not mind at all if we wanted to instead carry this
> as part of "contrib/".

I think my concern with having it in Junio's tree at all is that it
gives the impression that this is being maintained indefinitely. There
is definitely some cruft in contrib that we should probably get rid of.

But since this is coming from an organization that sponsors work on the
Git project, I think that my concerns there are somewhat more relaxed.
I'm not worried about this going unmaintained, so I have no objection to
having it live in contrib.

> Yup, that's a valid concern. As mentioned, this patch series does not
> have the intent to make GitLab CI a second authoritative CI platform.
> GitHub Actions should remain the source of truth of whether a pipeline
> passes or not. Most importantly, I do not want to require the maintainer
> to now watch both pipelines on GitHub and GitLab. This might be another
> indicator that the pipeline should rather be in "contrib/", so that
> people don't start to treat it as authoritative.

Yeah, I agree with everything you said here.

> > My other concern is that we're doubling the cost of any new changes to
> > our CI definition. Perhaps this is more of an academic concern, but I
> > think my fear would be that one or the other would fall behind on in
> > implementation leading to further divergence between the two.
> >
> > I think having the new CI definition live in "contrib" somewhat
> > addresses the "which CI is authoritative?" problem, but that it doesn't
> > address the "we have two of these" problem.
>
> I do see that this requires us to be a bit more careful with future
> changes to our CI definitions. But I think the additional work that this
> creates is really very limited. Except for the `.gitlab-ci.yml`, there
> are only 54 lines specific to GitLab in our CI scripts now, which I
> think should be rather manageable.

Agreed.

> I also think that it is sensible to ensure that our CI scripts are as
> agnostic to the CI platform as possible, as it ensures that we continue
> to be agile here in the future if we are ever forced to switch due to
> whatever reason. In the best case, our CI scripts would allow a user to
> also easily run the tests locally via e.g. Docker. We're not there yet,
> but this patch series is a good step into that direction already.

Agreed there as well.

> Last but not least, I actually think that having multiple supported CI
> platforms also has the benefit that people can more readily set it up
> for themselves. In theory, this has the potential to broaden the set of
> people willing to contribute to our `ci/` scripts, which would in the
> end also benefit GitHub Actions.
>
> In my opinion, this benefit is demonstrated by this patch series
> already: besides all the changes that aim to prepare for GitLab CI,
> there are also patches that deduplicate code and improve test coverage
> for Alpine Linux. These changes likely wouldn't have happened if it
> wasn't for the GitLab CI.

Yeah, I appreciate your careful and patient explanation here. I think
that my concerns here are resolved.

Thanks,
Taylor

^ permalink raw reply

* Re: [bug] 2.39.0: error in help for ls-remote
From: Jeff King @ 2023-10-31 19:09 UTC (permalink / raw)
  To: Jeremy Hetzler; +Cc: git
In-Reply-To: <CAOh4nmm9fTm8fa=1Hyi8t3R-VMnf30-0xe0vcst57dvY-FrL+w@mail.gmail.com>

On Tue, Oct 31, 2023 at 02:11:23PM -0400, Jeremy Hetzler wrote:

> The short help for ls-remote advertises that '-h' is short for '--heads':
> [...]
> However, 'git ls-remote -h' instead prints the help. So perhaps the
> help message should be revised.

It does work as documented with an argument, like:

  git ls-remote -h <remote>

Yes, this is somewhat weird, but is a balance between consistency and
backwards compatibility. See:

  https://lore.kernel.org/git/YU4QxcORBBR01iV8@coredump.intra.peff.net/

as a starting point for past discussions.

The manpage (or "--help") describes the behavior correctly; it may be
that the "-h" output could do so as well, but it's sometimes hard to
communicate such subtleties in such a terse format. So there may be room
for a patch to make things more clear there, but I think it may be
difficult to do well.

-Peff

^ permalink raw reply

* Re: [PATCH v2] clang-format: fix typo in comment
From: Taylor Blau @ 2023-10-31 19:09 UTC (permalink / raw)
  To: Aditya Neelamraju via GitGitGadget; +Cc: git, Aditya Neelamraju
In-Reply-To: <pull.1602.v2.git.git.1698759629166.gitgitgadget@gmail.com>

On Tue, Oct 31, 2023 at 01:40:28PM +0000, Aditya Neelamraju via GitGitGadget wrote:
> From: Aditya Neelamraju <adityanv97@gmail.com>
>
> Signed-off-by: Aditya Neelamraju <adityanv97@gmail.com>
> ---

Thanks, this version looks great to me.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH] chore: fix typo in .clang-format comment
From: Taylor Blau @ 2023-10-31 19:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Aditya Neelamraju via GitGitGadget, git, Aditya Neelamraju
In-Reply-To: <xmqqpm0v5vtg.fsf@gitster.g>

On Tue, Oct 31, 2023 at 12:12:43PM +0900, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Sun, Oct 29, 2023 at 08:23:07PM +0000, Aditya Neelamraju via GitGitGadget wrote:
> >> From: Aditya Neelamraju <adityanv97@gmail.com>
> >
> > We typically prefix commit messages with the subject area they're
> > working in, not with "chore", or "feat" like some Git workflows
> > recommend.
> > ...
> > That said, the contents of this patch look obviously correct to me.
> > Thanks for noticing and fixing!
>
> As a comment for a new contributor, it is a bit unhelpful not to
> suggest what the "subject area" string we would use if we were
> working on this patch, I think.

Good suggestion. I would have suggested "clang-format", which is
exactly Aditya ended up choosing, anyway. Thanks, Aditya!

> I also suspected that valuve may be a valid word in some language,
> as the indentation in the example looked as if the six-letter word
> was meant, not typoed.  https://www.gasolineravaluve.com/ was one
> of the first hits I saw in my search ;-)

;-)

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH v3 00/12] builtin/show-ref: introduce mode to check for ref existence
From: Taylor Blau @ 2023-10-31 19:06 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>

On Tue, Oct 31, 2023 at 09:16:08AM +0100, Patrick Steinhardt wrote:
> Hi,
>
> this is the third version of my patch series that introduces a new `git
> show-ref --exists` mode to check for reference existence.
>
> Changes compared to v2:
>
>     - Patch 5: Document why we need `exclude_existing_options.enabled`,
>       which isn't exactly obvious.
>
>     - Patch 6: Fix a grammar issue in the commit message.
>
>     - Patch 9: Switch to `test_cmp` instead of grep(1).
>
> Thanks!

Thanks for the updated round. I took a look at the range-diff and didn't
see anything surprising. This version looks great to me, thanks for
working on this!

Thanks,
Taylor

^ permalink raw reply

* Re: Method for Calculating Statistics of Developer Contribution to a Specified Branch.
From: Taylor Blau @ 2023-10-31 19:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Hongyi Zhao, brian m. carlson, Git List
In-Reply-To: <xmqqsf5r2tr3.fsf@gitster.g>

On Tue, Oct 31, 2023 at 03:25:36PM +0900, Junio C Hamano wrote:
> Hongyi Zhao <hongyi.zhao@gmail.com> writes:
>
> >> But I think that there is a slightly cleaner way to compute the result
> >> you're after, like so:
> >> ...
> > So, your method and my original one give exactly the same result.
> > Therefore, I can't see what their fundamental difference is.
>
> I think Taylor offered a "slightly cleaner way", and not a
> "different way that computes better result".  So it is not
> surprising, at least to me who is watching from the sideline, that
> you cannot see any fundamental difference.

Indeed.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-31 18:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Oswald Buddenhagen, git, Phillip Wood, Taylor Blau,
	Christian Couder, Charvi Mendiratta
In-Reply-To: <xmqqh6mbod1b.fsf@gitster.g>


On 2023-10-27 19:34, Junio C Hamano wrote:
> 
> Thanks for a good review.  I guess the patch is very near the finish
> line?

Yes.  In my mind, all that's needed is to remove the part about "should 
not be relied upon".

		M.

^ permalink raw reply

* Re: [PATCH v4 0/8] ci: add GitLab CI definition
From: Victoria Dye @ 2023-10-31 18:22 UTC (permalink / raw)
  To: Patrick Steinhardt, git
  Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>

Patrick Steinhardt wrote:
> Hi,
> 
> this is the fourth version of my patch series that introduces support
> for GitLab CI.
> 
> Changes compared to v3:
> 
>     - Stopped using nproc(1) to figure out the number of builds jobs for
>       GitHub Actions and Azure Pipelines. Instead, we now continue to
>       use the hardcoded 10 jobs there, whereas on GitLab CI we now use
>       nproc. We can adapt GitHub/Azure at a later point as required, but
>       I don't feel comfortable doing changes there.
> 
>     - Improved the linux-musl job. Namely, we now also install all
>       required Apache modules, which makes the Apache-based test setup
>       work. There is a packaging issue with the WebDAV module though, so
>       we now skip tests that depend on it on Alpine Linux.
> 
> I still didn't move `.gitlab-ci.yml` to `contrib/`. As Taylor argued
> (and I don't disagree), moving it to `contrib/` would convey the spirit
> that this is _not_ an authoritative CI pipeline setup. But I wanted to
> hear other opinions first before moving it into `contrib/`.

I've read through some of the earlier discussion on this (as well as your
original cover letter [1]), so I'll throw in my 2c.

The majority of the changes in this patch series aren't conditioned on
anything that says "gitlab", they just improve the flexibility of our CI
scripts. I personally didn't notice anything too cumbersome added in the
series, so I'm happy with all of that (essentially, patches 1-7 & parts of
8).

As for adding the GitLab-specific stuff, I'm not opposed to having it in the
main tree. For one, there doesn't seem to be a clean way to "move it into
`contrib/`" - '.gitlab-ci.yml' must be at the root of the project [2], and
moving the $GITLAB_CI conditions out of the 'ci/*.sh' files into dedicated
scripts would likely result in a lot of duplicated code (which doesn't solve
the maintenance burden issue this series intends to address).

More generally, there are lots of open source projects that include CI
configurations across different forges, _especially_ those that are
officially mirrored across a bunch of them. As long as there are
contributors with a vested interest in keeping the GitLab CI definition
stable (and your cover letter indicates that there are), and the GitLab
stuff doesn't negatively impact any other CI configurations, I think it
warrants the same treatment as e.g. GitHub CI.

[1] https://lore.kernel.org/git/cover.1698305961.git.ps@pks.im/
[2] https://docs.gitlab.com/ee/ci/index.html#the-gitlab-ciyml-file

> 
> Patrick
> 

^ permalink raw reply

* [bug] 2.39.0: error in help for ls-remote
From: Jeremy Hetzler @ 2023-10-31 18:11 UTC (permalink / raw)
  To: git
In-Reply-To: <CAOh4nmk2KZBTuW9qn_ZgDY3yLRZ6NgGOWuBMLRRm1sU=pdmRoQ@mail.gmail.com>

All,

The short help for ls-remote advertises that '-h' is short for '--heads':

> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]
>                      [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
>                      [--symref] [<repository> [<refs>...]]
>
>
>     -q, --quiet           do not print remote URL
>     --upload-pack <exec>  path of git-upload-pack on the remote host
>     -t, --tags            limit to tags
>     -h, --heads           limit to heads
>     --refs                do not show peeled tags
>     --get-url             take url.<base>.insteadOf into account
>     --sort <key>          field name to sort on
>     --exit-code           exit with exit code 2 if no matching refs are found
>     --symref              show underlying ref in addition to the object pointed by it
>     -o, --server-option <server-specific>
>                           option to transmit

However, 'git ls-remote -h' instead prints the help. So perhaps the
help message should be revised.

git version 2.39.0

Thanks,
Jeremy

^ permalink raw reply


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