* [PATCH v2 2/5] reftable/stack: refactor reloading to use file descriptor
From: Patrick Steinhardt @ 2024-01-11 10:06 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <cover.1704966670.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]
We're about to introduce a stat(3P)-based caching mechanism to reload
the list of stacks only when it has changed. In order to avoid race
conditions this requires us to have a file descriptor available that we
can use to call fstat(3P) on.
Prepare for this by converting the code to use `fd_read_lines()` so that
we have the file descriptor readily available.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index bf869a6772..b1ee247601 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -308,6 +308,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
struct timeval deadline;
int64_t delay = 0;
int tries = 0, err;
+ int fd = -1;
err = gettimeofday(&deadline, NULL);
if (err < 0)
@@ -329,9 +330,19 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
if (tries > 3 && tv_cmp(&now, &deadline) >= 0)
goto out;
- err = read_lines(st->list_file, &names);
- if (err < 0)
- goto out;
+ fd = open(st->list_file, O_RDONLY);
+ if (fd < 0) {
+ if (errno != ENOENT) {
+ err = REFTABLE_IO_ERROR;
+ goto out;
+ }
+
+ names = reftable_calloc(sizeof(char *));
+ } else {
+ err = fd_read_lines(fd, &names);
+ if (err < 0)
+ goto out;
+ }
err = reftable_stack_reload_once(st, names, reuse_open);
if (!err)
@@ -356,12 +367,16 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
names = NULL;
free_names(names_after);
names_after = NULL;
+ close(fd);
+ fd = -1;
delay = delay + (delay * rand()) / RAND_MAX + 1;
sleep_millisec(delay);
}
out:
+ if (fd >= 0)
+ close(fd);
free_names(names);
free_names(names_after);
return err;
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 3/5] reftable/stack: use stat info to avoid re-reading stack list
From: Patrick Steinhardt @ 2024-01-11 10:06 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <cover.1704966670.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5676 bytes --]
Whenever we call into the refs interfaces we potentially have to reload
refs in case they have been concurrently modified, either in-process or
externally. While this happens somewhat automatically for loose refs
because we simply try to re-read the files, the "packed" backend will
reload its snapshot of the packed-refs file in case its stat info has
changed since last reading it.
In the reftable backend we have a similar mechanism that is provided by
`reftable_stack_reload()`. This function will read the list of stacks
from "tables.list" and, if they have changed from the currently stored
list, reload the stacks. This is heavily inefficient though, as we have
to check whether the stack is up-to-date on basically every read and
thus keep on re-reading the file all the time even if it didn't change
at all.
We can do better and use the same stat(3P)-based mechanism that the
"packed" backend uses. Instead of reading the file, we will only open
the file descriptor, fstat(3P) it, and then compare the info against the
cached value from the last time we have updated the stack. This should
always work alright because "tables.list" is updated atomically via a
rename, so even if the ctime or mtime wasn't granular enough to identify
a change, at least the inode number or file size should have changed.
This change significantly speeds up operations where many refs are read,
like when using git-update-ref(1). The following benchmark creates N
refs in an otherwise-empty repository via `git update-ref --stdin`:
Benchmark 1: update-ref: create many refs (refcount = 1, revision = HEAD~)
Time (mean ± σ): 5.1 ms ± 0.2 ms [User: 2.4 ms, System: 2.6 ms]
Range (min … max): 4.8 ms … 7.2 ms 109 runs
Benchmark 2: update-ref: create many refs (refcount = 100, revision = HEAD~)
Time (mean ± σ): 19.1 ms ± 0.9 ms [User: 8.9 ms, System: 9.9 ms]
Range (min … max): 18.4 ms … 26.7 ms 72 runs
Benchmark 3: update-ref: create many refs (refcount = 10000, revision = HEAD~)
Time (mean ± σ): 1.336 s ± 0.018 s [User: 0.590 s, System: 0.724 s]
Range (min … max): 1.314 s … 1.373 s 10 runs
Benchmark 4: update-ref: create many refs (refcount = 1, revision = HEAD)
Time (mean ± σ): 5.1 ms ± 0.2 ms [User: 2.4 ms, System: 2.6 ms]
Range (min … max): 4.8 ms … 7.2 ms 109 runs
Benchmark 5: update-ref: create many refs (refcount = 100, revision = HEAD)
Time (mean ± σ): 14.8 ms ± 0.2 ms [User: 7.1 ms, System: 7.5 ms]
Range (min … max): 14.2 ms … 15.2 ms 82 runs
Benchmark 6: update-ref: create many refs (refcount = 10000, revision = HEAD)
Time (mean ± σ): 927.6 ms ± 5.3 ms [User: 437.8 ms, System: 489.5 ms]
Range (min … max): 919.4 ms … 936.4 ms 10 runs
Summary
update-ref: create many refs (refcount = 1, revision = HEAD) ran
1.00 ± 0.07 times faster than update-ref: create many refs (refcount = 1, revision = HEAD~)
2.89 ± 0.14 times faster than update-ref: create many refs (refcount = 100, revision = HEAD)
3.74 ± 0.25 times faster than update-ref: create many refs (refcount = 100, revision = HEAD~)
181.26 ± 8.30 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD)
261.01 ± 12.35 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD~)
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 12 +++++++++++-
reftable/stack.h | 1 +
reftable/system.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index b1ee247601..c28d82299d 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -175,6 +175,7 @@ void reftable_stack_destroy(struct reftable_stack *st)
st->readers_len = 0;
FREE_AND_NULL(st->readers);
}
+ stat_validity_clear(&st->list_validity);
FREE_AND_NULL(st->list_file);
FREE_AND_NULL(st->reftable_dir);
reftable_free(st);
@@ -374,7 +375,11 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
sleep_millisec(delay);
}
+ stat_validity_update(&st->list_validity, fd);
+
out:
+ if (err)
+ stat_validity_clear(&st->list_validity);
if (fd >= 0)
close(fd);
free_names(names);
@@ -388,8 +393,13 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
static int stack_uptodate(struct reftable_stack *st)
{
char **names = NULL;
- int err = read_lines(st->list_file, &names);
+ int err;
int i = 0;
+
+ if (stat_validity_check(&st->list_validity, st->list_file))
+ return 0;
+
+ err = read_lines(st->list_file, &names);
if (err < 0)
return err;
diff --git a/reftable/stack.h b/reftable/stack.h
index f57005846e..3f80cc598a 100644
--- a/reftable/stack.h
+++ b/reftable/stack.h
@@ -14,6 +14,7 @@ license that can be found in the LICENSE file or at
#include "reftable-stack.h"
struct reftable_stack {
+ struct stat_validity list_validity;
char *list_file;
char *reftable_dir;
int disable_auto_compact;
diff --git a/reftable/system.h b/reftable/system.h
index 6b74a81514..2cc7adf271 100644
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -12,6 +12,7 @@ license that can be found in the LICENSE file or at
/* This header glues the reftable library to the rest of Git */
#include "git-compat-util.h"
+#include "statinfo.h"
#include "strbuf.h"
#include "hash-ll.h" /* hash ID, sizes.*/
#include "dir.h" /* remove_dir_recursively, for tests.*/
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 4/5] reftable/blocksource: refactor code to match our coding style
From: Patrick Steinhardt @ 2024-01-11 10:06 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <cover.1704966670.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]
Refactor `reftable_block_source_from_file()` to match our coding style
better.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/blocksource.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index a1ea304429..1e2fb5e9fd 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -125,24 +125,23 @@ static struct reftable_block_source_vtable file_vtable = {
int reftable_block_source_from_file(struct reftable_block_source *bs,
const char *name)
{
- struct stat st = { 0 };
- int err = 0;
- int fd = open(name, O_RDONLY);
- struct file_block_source *p = NULL;
+ struct file_block_source *p;
+ struct stat st;
+ int fd;
+
+ fd = open(name, O_RDONLY);
if (fd < 0) {
- if (errno == ENOENT) {
+ if (errno == ENOENT)
return REFTABLE_NOT_EXIST_ERROR;
- }
return -1;
}
- err = fstat(fd, &st);
- if (err < 0) {
+ if (fstat(fd, &st) < 0) {
close(fd);
return REFTABLE_IO_ERROR;
}
- p = reftable_calloc(sizeof(struct file_block_source));
+ p = reftable_calloc(sizeof(*p));
p->size = st.st_size;
p->fd = fd;
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 5/5] reftable/blocksource: use mmap to read tables
From: Patrick Steinhardt @ 2024-01-11 10:06 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <cover.1704966670.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6238 bytes --]
The blocksource interface provides an interface to read blocks from a
reftable table. This interface is implemented using read(3P) calls on
the underlying file descriptor. While this works alright, this pattern
is very inefficient when repeatedly querying the reftable stack for one
or more refs. This inefficiency can mostly be attributed to the fact
that we often need to re-read the same blocks over and over again, and
every single time we need to call read(3P) again.
A natural fit in this context is to use mmap(3P) instead of read(3P),
which has a bunch of benefits:
- We do not need to come up with a caching strategy for some of the
blocks as this will be handled by the kernel already.
- We can avoid the overhead of having to call into the read(3P)
syscall repeatedly.
- We do not need to allocate returned blocks repeatedly, but can
instead hand out pointers into the mmapped region directly.
Using mmap comes with a significant drawback on Windows though, because
mmapped files cannot be deleted and neither is it possible to rename
files onto an mmapped file. But for one, the reftable library gracefully
handles the case where auto-compaction cannot delete a still-open stack
already and ignores any such errors. Also, `reftable_stack_clean()` will
prune stale tables which are not referenced by "tables.list" anymore so
that those files can eventually be pruned. And second, we never rewrite
already-written stacks, so it does not matter that we cannot rename a
file over an mmaped file, either.
Another unfortunate property of mmap is that it is not supported by all
systems. But given that the size of reftables should typically be rather
limited (megabytes at most in the vast majority of repositories), we can
use the fallback implementation provided by `git_mmap()` which reads the
whole file into memory instead. This is the same strategy that the
"packed" backend uses.
While this change doesn't significantly improve performance in the case
where we're seeking through stacks once (like e.g. git-for-each-ref(1)
would). But it does speed up usecases where there is lots of random
access to refs, e.g. when writing. The following benchmark demonstrates
these savings with git-update-ref(1) creating N refs in an otherwise
empty repository:
Benchmark 1: update-ref: create many refs (refcount = 1, revision = HEAD~)
Time (mean ± σ): 5.1 ms ± 0.2 ms [User: 2.5 ms, System: 2.5 ms]
Range (min … max): 4.8 ms … 7.1 ms 111 runs
Benchmark 2: update-ref: create many refs (refcount = 100, revision = HEAD~)
Time (mean ± σ): 14.8 ms ± 0.5 ms [User: 7.1 ms, System: 7.5 ms]
Range (min … max): 14.1 ms … 18.7 ms 84 runs
Benchmark 3: update-ref: create many refs (refcount = 10000, revision = HEAD~)
Time (mean ± σ): 926.4 ms ± 5.6 ms [User: 448.5 ms, System: 477.7 ms]
Range (min … max): 920.0 ms … 936.1 ms 10 runs
Benchmark 4: update-ref: create many refs (refcount = 1, revision = HEAD)
Time (mean ± σ): 5.0 ms ± 0.2 ms [User: 2.4 ms, System: 2.5 ms]
Range (min … max): 4.7 ms … 5.4 ms 111 runs
Benchmark 5: update-ref: create many refs (refcount = 100, revision = HEAD)
Time (mean ± σ): 10.5 ms ± 0.2 ms [User: 5.0 ms, System: 5.3 ms]
Range (min … max): 10.0 ms … 10.9 ms 93 runs
Benchmark 6: update-ref: create many refs (refcount = 10000, revision = HEAD)
Time (mean ± σ): 529.6 ms ± 9.1 ms [User: 268.0 ms, System: 261.4 ms]
Range (min … max): 522.4 ms … 547.1 ms 10 runs
Summary
update-ref: create many refs (refcount = 1, revision = HEAD) ran
1.01 ± 0.06 times faster than update-ref: create many refs (refcount = 1, revision = HEAD~)
2.08 ± 0.07 times faster than update-ref: create many refs (refcount = 100, revision = HEAD)
2.95 ± 0.14 times faster than update-ref: create many refs (refcount = 100, revision = HEAD~)
105.33 ± 3.76 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD)
184.24 ± 5.89 times faster than update-ref: create many refs (refcount = 10000, revision = HEAD~)
Theoretically, we could also replicate the strategy of the "packed"
backend where small tables are read into memory instead of using mmap.
Benchmarks did not confirm that this has a performance benefit though.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/blocksource.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 1e2fb5e9fd..8c41e3c70f 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -76,8 +76,8 @@ struct reftable_block_source malloc_block_source(void)
}
struct file_block_source {
- int fd;
uint64_t size;
+ unsigned char *data;
};
static uint64_t file_size(void *b)
@@ -87,19 +87,12 @@ static uint64_t file_size(void *b)
static void file_return_block(void *b, struct reftable_block *dest)
{
- if (dest->len)
- memset(dest->data, 0xff, dest->len);
- reftable_free(dest->data);
}
-static void file_close(void *b)
+static void file_close(void *v)
{
- int fd = ((struct file_block_source *)b)->fd;
- if (fd > 0) {
- close(fd);
- ((struct file_block_source *)b)->fd = 0;
- }
-
+ struct file_block_source *b = v;
+ munmap(b->data, b->size);
reftable_free(b);
}
@@ -108,9 +101,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
{
struct file_block_source *b = v;
assert(off + size <= b->size);
- dest->data = reftable_malloc(size);
- if (pread_in_full(b->fd, dest->data, size, off) != size)
- return -1;
+ dest->data = b->data + off;
dest->len = size;
return size;
}
@@ -143,7 +134,8 @@ int reftable_block_source_from_file(struct reftable_block_source *bs,
p = reftable_calloc(sizeof(*p));
p->size = st.st_size;
- p->fd = fd;
+ p->data = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ close(fd);
assert(!bs->ops);
bs->ops = &file_vtable;
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 0/2] completion: silence pseudo-ref existence check
From: Patrick Steinhardt @ 2024-01-11 10:41 UTC (permalink / raw)
To: git; +Cc: Stan Hu
[-- Attachment #1: Type: text/plain, Size: 536 bytes --]
Hi,
I recently noticed that the Bash completion script started to output
object IDs in repositories which use the "reftable" backend when
completing certain commands. This patch series fixes this issue.
Patrick
Patrick Steinhardt (2):
t9902: verify that completion does not print anything
completion: silence pseudoref existence check
contrib/completion/git-completion.bash | 2 +-
t/t9902-completion.sh | 14 ++++++++++++--
2 files changed, 13 insertions(+), 3 deletions(-)
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] t9902: verify that completion does not print anything
From: Patrick Steinhardt @ 2024-01-11 10:41 UTC (permalink / raw)
To: git; +Cc: Stan Hu
In-Reply-To: <cover.1704969119.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 908 bytes --]
The Bash completion script must not print anything to either stdout or
stderr. Instead, it is only expected to populate certain variables.
Tighten our `test_completion ()` test helper to verify this requirement.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t9902-completion.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..78cb93bea7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -87,9 +87,11 @@ test_completion ()
else
sed -e 's/Z$//' |sort >expected
fi &&
- run_completion "$1" &&
+ run_completion "$1" >"$TRASH_DIRECTORY"/output 2>&1 &&
sort out >out_sorted &&
- test_cmp expected out_sorted
+ test_cmp expected out_sorted &&
+ test_must_be_empty "$TRASH_DIRECTORY"/output &&
+ rm "$TRASH_DIRECTORY"/output
}
# Test __gitcomp.
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 2/2] completion: silence pseudoref existence check
From: Patrick Steinhardt @ 2024-01-11 10:41 UTC (permalink / raw)
To: git; +Cc: Stan Hu
In-Reply-To: <cover.1704969119.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]
In 44dbb3bf29 (completion: support pseudoref existence checks for
reftables, 2023-12-19), we have extended the Bash completion script to
support future ref backends better by using git-rev-parse(1) to check
for pseudo-ref existence. This conversion has introduced a bug, because
even though we pass `--quiet` to git-rev-parse(1) it would still output
the resolved object ID of the ref in question if it exists.
Fix this by redirecting its stdout to `/dev/null` and add a test that
catches this behaviour. Note that the test passes even without the fix
for the "files" backend because we parse pseudo refs via the filesystem
directly in that case. But the test will fail with the "reftable"
backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
contrib/completion/git-completion.bash | 2 +-
t/t9902-completion.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c40ade494..8872192e2b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -146,7 +146,7 @@ __git_pseudoref_exists ()
if __git_eread "$__git_repo_path/HEAD" head; then
b="${head#ref: }"
if [ "$b" == "refs/heads/.invalid" ]; then
- __git -C "$__git_repo_path" rev-parse --verify --quiet "$ref" 2>/dev/null
+ __git -C "$__git_repo_path" rev-parse --verify --quiet "$ref" >/dev/null 2>&1
return $?
fi
fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 78cb93bea7..b14ae4de14 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1927,6 +1927,14 @@ test_expect_success 'git checkout - --orphan with branch already provided comple
EOF
'
+test_expect_success 'git reset completes modified files' '
+ test_commit A a.file &&
+ echo B >a.file &&
+ test_completion "git restore a." <<-\EOF
+ a.file
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH 1/2] t1401: generalize reference locking
From: Patrick Steinhardt @ 2024-01-11 11:08 UTC (permalink / raw)
To: Jeff King; +Cc: Justin Tobler via GitGitGadget, git, Justin Tobler
In-Reply-To: <20240111071329.GC48154@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]
On Thu, Jan 11, 2024 at 02:13:29AM -0500, Jeff King wrote:
> On Wed, Jan 10, 2024 at 06:52:29PM +0000, Justin Tobler via GitGitGadget wrote:
>
> > From: Justin Tobler <jltobler@gmail.com>
> >
> > Some tests set up reference locks by directly creating the lockfile.
> > While this works for the files reference backend, reftable reference
> > locks operate differently and are incompatible with this approach.
> > Refactor the test to use git-update-ref(1) to lock refs instead so that
> > the test does not need to be aware of how the ref backend locks refs.
>
> It looks like you re-create this situation in a backend-agnostic way by
> having two simultaneous updates that conflict on the lock (but don't
> care how that lock is implemented).
>
> That works, but I think we could keep it simple. This test doesn't care
> about the exact error condition we create. The point was just to die in
> create_symref() and make sure the exit code was propagated. So something
> like this would work:
>
> $ git symbolic-ref refs/heads refs/heads/foo
> error: unable to write symref for refs/heads: Is a directory
>
> (note that you get a different error message if the refs are packed,
> since there we can notice the d/f conflict manually).
If all we care for is the exit code then this would work for the
reftable backend, too:
```
$ git init --ref-format=reftable repo
Initialized empty Git repository in /tmp/repo/.git/
$ cd repo/
$ git commit --allow-empty --message message
[main (root-commit) c2512d3] x
$ git symbolic-ref refs/heads refs/heads/foo
$ echo $?
1
```
A bit unfortunate that there is no proper error message in that case,
but that is a different topic.
Patrick
> There may be other ways to stimulate a failure. I thought "symbolic-ref
> HEAD refs/heads/.invalid" might work, but sadly the refname format check
> happens earlier.
>
> I think it is worth avoiding the fifo magic if we can. It's complicated,
> and it means that not all platforms run the test.
>
> -Peff
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [DISCUSS] Introducing Rust into the Git project
From: Sam James @ 2024-01-11 11:45 UTC (permalink / raw)
To: me; +Cc: git
In-Reply-To: <ZZ77NQkSuiRxRDwt@nand.local>
Something I'm a bit concerned about is that right now, neither
rustc_codegen_gcc nor gccrs are ready for use here.
We've had trouble getting things wired up for rustc_codegen_gcc
- which is not to speak against their wonderful efforts - because
the Rust community hasn't yet figured out how to handle things which
pure rustc supports yet. See
e.g. https://github.com/rust-lang/libc/pull/3032.
I think care should be taken in citing rustc_codegen_gcc and gccrs
as options for alternative platforms for now. They will hopefully
be great options in the future, but they aren't today, and they probably
won't be in the next 6 months at the least.
We also do use git heavily on platforms which rustc isn't supported
yet.
thanks,
sam
^ permalink raw reply
* [PATCH v2 0/3] branch: error description when deleting a not fully merged branch
From: Rubén Justo @ 2024-01-11 12:20 UTC (permalink / raw)
To: Git List, Junio C Hamano
In-Reply-To: <04c3556f-0242-4ac3-b94a-be824cd2004a@gmail.com>
The error message we show when the user tries to delete a not fully
merged branch describes the error and gives a hint to the user:
error: the branch 'foo' is not fully merged.
If you are sure you want to delete it, run 'git branch -D foo'.
Let's move the hint part so that it is displayed using the advice
machinery:
error: the branch 'foo' is not fully merged
hint: If you are sure you want to delete it, run 'git branch -D foo'
hint: Disable this message with "git config advice.forceDeleteBranch false"
Rubén Justo (3):
advice: sort the advice related lists
advice: fix an unexpected leading space
branch: make the advice to force-deleting a conditional one
Documentation/config/advice.txt | 157 ++++++++++++++++----------------
advice.c | 14 ++-
advice.h | 15 +--
builtin/branch.c | 8 +-
4 files changed, 99 insertions(+), 95 deletions(-)
--
2.42.0
^ permalink raw reply
* [PATCH v2 1/3] advice: sort the advice related lists
From: Rubén Justo @ 2024-01-11 12:40 UTC (permalink / raw)
To: Git List, Junio C Hamano
In-Reply-To: <4aedc15c-4b3f-4f5e-abea-581b501600f8@gmail.com>
Let's keep the advice related lists sorted to make them more
digestible.
A multi-line comment has also been changed; that produces the unexpected
'insertion != deletion' in this supposedly 'only sort lines' commit.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
Documentation/config/advice.txt | 154 ++++++++++++++++----------------
advice.c | 13 ++-
advice.h | 12 +--
3 files changed, 88 insertions(+), 91 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index 4d7e5d8759..e0deaf3144 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -4,27 +4,56 @@ advice.*::
can tell Git that you do not need help by setting these to 'false':
+
--
+ addEmbeddedRepo::
+ Advice on what to do when you've accidentally added one
+ git repo inside of another.
+ addEmptyPathspec::
+ Advice shown if a user runs the add command without providing
+ the pathspec parameter.
+ addIgnoredFile::
+ Advice shown if a user attempts to add an ignored file to
+ the index.
+ amWorkDir::
+ Advice that shows the location of the patch file when
+ linkgit:git-am[1] fails to apply it.
ambiguousFetchRefspec::
Advice shown when a fetch refspec for multiple remotes maps to
the same remote-tracking branch namespace and causes branch
tracking set-up to fail.
+ checkoutAmbiguousRemoteBranchName::
+ Advice shown when the argument to
+ linkgit:git-checkout[1] and linkgit:git-switch[1]
+ ambiguously resolves to a
+ remote tracking branch on more than one remote in
+ situations where an unambiguous argument would have
+ otherwise caused a remote-tracking branch to be
+ checked out. See the `checkout.defaultRemote`
+ configuration variable for how to set a given remote
+ to be used by default in some situations where this
+ advice would be printed.
+ commitBeforeMerge::
+ Advice shown when linkgit:git-merge[1] refuses to
+ merge to avoid overwriting local changes.
+ detachedHead::
+ Advice shown when you used
+ linkgit:git-switch[1] or linkgit:git-checkout[1]
+ to move to the detached HEAD state, to instruct how to
+ create a local branch after the fact.
+ diverging::
+ Advice shown when a fast-forward is not possible.
fetchShowForcedUpdates::
Advice shown when linkgit:git-fetch[1] takes a long time
to calculate forced updates after ref updates, or to warn
that the check is disabled.
- pushUpdateRejected::
- Set this variable to 'false' if you want to disable
- 'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',
- 'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'
- simultaneously.
- pushNonFFCurrent::
- Advice shown when linkgit:git-push[1] fails due to a
- non-fast-forward update to the current branch.
- pushNonFFMatching::
- Advice shown when you ran linkgit:git-push[1] and pushed
- 'matching refs' explicitly (i.e. you used ':', or
- specified a refspec that isn't your current branch) and
- it resulted in a non-fast-forward error.
+ ignoredHook::
+ Advice shown if a hook is ignored because the hook is not
+ set as executable.
+ implicitIdentity::
+ Advice on how to set your identity configuration when
+ your information is guessed from the system username and
+ domain name.
+ nestedTag::
+ Advice shown if a user attempts to recursively tag a tag object.
pushAlreadyExists::
Shown when linkgit:git-push[1] rejects an update that
does not qualify for fast-forwarding (e.g., a tag.)
@@ -37,6 +66,18 @@ advice.*::
tries to overwrite a remote ref that points at an
object that is not a commit-ish, or make the remote
ref point at an object that is not a commit-ish.
+ pushNonFFCurrent::
+ Advice shown when linkgit:git-push[1] fails due to a
+ non-fast-forward update to the current branch.
+ pushNonFFMatching::
+ Advice shown when you ran linkgit:git-push[1] and pushed
+ 'matching refs' explicitly (i.e. you used ':', or
+ specified a refspec that isn't your current branch) and
+ it resulted in a non-fast-forward error.
+ pushRefNeedsUpdate::
+ Shown when linkgit:git-push[1] rejects a forced update of
+ a branch when its remote-tracking ref has updates that we
+ do not have locally.
pushUnqualifiedRefname::
Shown when linkgit:git-push[1] gives up trying to
guess based on the source and destination refs what
@@ -44,10 +85,23 @@ advice.*::
we can still suggest that the user push to either
refs/heads/* or refs/tags/* based on the type of the
source object.
- pushRefNeedsUpdate::
- Shown when linkgit:git-push[1] rejects a forced update of
- a branch when its remote-tracking ref has updates that we
- do not have locally.
+ pushUpdateRejected::
+ Set this variable to 'false' if you want to disable
+ 'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists',
+ 'pushFetchFirst', 'pushNeedsForce', and 'pushRefNeedsUpdate'
+ simultaneously.
+ resetNoRefresh::
+ Advice to consider using the `--no-refresh` option to
+ linkgit:git-reset[1] when the command takes more than 2 seconds
+ to refresh the index after reset.
+ resolveConflict::
+ Advice shown by various commands when conflicts
+ prevent the operation from being performed.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
+ sequencerInUse::
+ Advice shown when a sequencer command is already in progress.
skippedCherryPicks::
Shown when linkgit:git-rebase[1] skips a commit that has already
been cherry-picked onto the upstream branch.
@@ -68,76 +122,22 @@ advice.*::
Advise to consider using the `-u` option to linkgit:git-status[1]
when the command takes more than 2 seconds to enumerate untracked
files.
- commitBeforeMerge::
- Advice shown when linkgit:git-merge[1] refuses to
- merge to avoid overwriting local changes.
- resetNoRefresh::
- Advice to consider using the `--no-refresh` option to
- linkgit:git-reset[1] when the command takes more than 2 seconds
- to refresh the index after reset.
- resolveConflict::
- Advice shown by various commands when conflicts
- prevent the operation from being performed.
- sequencerInUse::
- Advice shown when a sequencer command is already in progress.
- implicitIdentity::
- Advice on how to set your identity configuration when
- your information is guessed from the system username and
- domain name.
- detachedHead::
- Advice shown when you used
- linkgit:git-switch[1] or linkgit:git-checkout[1]
- to move to the detached HEAD state, to instruct how to
- create a local branch after the fact.
- suggestDetachingHead::
- Advice shown when linkgit:git-switch[1] refuses to detach HEAD
- without the explicit `--detach` option.
- checkoutAmbiguousRemoteBranchName::
- Advice shown when the argument to
- linkgit:git-checkout[1] and linkgit:git-switch[1]
- ambiguously resolves to a
- remote tracking branch on more than one remote in
- situations where an unambiguous argument would have
- otherwise caused a remote-tracking branch to be
- checked out. See the `checkout.defaultRemote`
- configuration variable for how to set a given remote
- to be used by default in some situations where this
- advice would be printed.
- amWorkDir::
- Advice that shows the location of the patch file when
- linkgit:git-am[1] fails to apply it.
- rmHints::
- In case of failure in the output of linkgit:git-rm[1],
- show directions on how to proceed from the current state.
- addEmbeddedRepo::
- Advice on what to do when you've accidentally added one
- git repo inside of another.
- ignoredHook::
- Advice shown if a hook is ignored because the hook is not
- set as executable.
- waitingForEditor::
- Print a message to the terminal whenever Git is waiting for
- editor input from the user.
- nestedTag::
- Advice shown if a user attempts to recursively tag a tag object.
submoduleAlternateErrorStrategyDie::
Advice shown when a submodule.alternateErrorStrategy option
configured to "die" causes a fatal error.
submodulesNotUpdated::
Advice shown when a user runs a submodule command that fails
because `git submodule update --init` was not run.
- addIgnoredFile::
- Advice shown if a user attempts to add an ignored file to
- the index.
- addEmptyPathspec::
- Advice shown if a user runs the add command without providing
- the pathspec parameter.
+ suggestDetachingHead::
+ Advice shown when linkgit:git-switch[1] refuses to detach HEAD
+ without the explicit `--detach` option.
updateSparsePath::
Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
is asked to update index entries outside the current sparse
checkout.
- diverging::
- Advice shown when a fast-forward is not possible.
+ waitingForEditor::
+ Print a message to the terminal whenever Git is waiting for
+ editor input from the user.
worktreeAddOrphan::
Advice shown when a user tries to create a worktree from an
invalid reference, to instruct how to create a new unborn
diff --git a/advice.c b/advice.c
index 50c79443ba..03322caba0 100644
--- a/advice.c
+++ b/advice.c
@@ -40,12 +40,11 @@ static struct {
[ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo", 1 },
[ADVICE_ADD_EMPTY_PATHSPEC] = { "addEmptyPathspec", 1 },
[ADVICE_ADD_IGNORED_FILE] = { "addIgnoredFile", 1 },
- [ADVICE_AM_WORK_DIR] = { "amWorkDir", 1 },
[ADVICE_AMBIGUOUS_FETCH_REFSPEC] = { "ambiguousFetchRefspec", 1 },
+ [ADVICE_AM_WORK_DIR] = { "amWorkDir", 1 },
[ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] = { "checkoutAmbiguousRemoteBranchName", 1 },
[ADVICE_COMMIT_BEFORE_MERGE] = { "commitBeforeMerge", 1 },
[ADVICE_DETACHED_HEAD] = { "detachedHead", 1 },
- [ADVICE_SUGGEST_DETACHING_HEAD] = { "suggestDetachingHead", 1 },
[ADVICE_DIVERGING] = { "diverging", 1 },
[ADVICE_FETCH_SHOW_FORCED_UPDATES] = { "fetchShowForcedUpdates", 1 },
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated", 1 },
@@ -56,15 +55,12 @@ static struct {
[ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists", 1 },
[ADVICE_PUSH_FETCH_FIRST] = { "pushFetchFirst", 1 },
[ADVICE_PUSH_NEEDS_FORCE] = { "pushNeedsForce", 1 },
- [ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate", 1 },
-
- /* make this an alias for backward compatibility */
- [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward", 1 },
-
[ADVICE_PUSH_NON_FF_CURRENT] = { "pushNonFFCurrent", 1 },
[ADVICE_PUSH_NON_FF_MATCHING] = { "pushNonFFMatching", 1 },
+ [ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate", 1 },
[ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName", 1 },
[ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected", 1 },
+ [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward", 1 }, /* backwards compatibility */
[ADVICE_RESET_NO_REFRESH_WARNING] = { "resetNoRefresh", 1 },
[ADVICE_RESOLVE_CONFLICT] = { "resolveConflict", 1 },
[ADVICE_RM_HINTS] = { "rmHints", 1 },
@@ -74,8 +70,9 @@ static struct {
[ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning", 1 },
[ADVICE_STATUS_HINTS] = { "statusHints", 1 },
[ADVICE_STATUS_U_OPTION] = { "statusUoption", 1 },
- [ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
[ADVICE_SUBMODULES_NOT_UPDATED] = { "submodulesNotUpdated", 1 },
+ [ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
+ [ADVICE_SUGGEST_DETACHING_HEAD] = { "suggestDetachingHead", 1 },
[ADVICE_UPDATE_SPARSE_PATH] = { "updateSparsePath", 1 },
[ADVICE_WAITING_FOR_EDITOR] = { "waitingForEditor", 1 },
[ADVICE_WORKTREE_ADD_ORPHAN] = { "worktreeAddOrphan", 1 },
diff --git a/advice.h b/advice.h
index 2affbe1426..9396bcdcf1 100644
--- a/advice.h
+++ b/advice.h
@@ -14,13 +14,12 @@ struct string_list;
ADVICE_ADD_EMBEDDED_REPO,
ADVICE_ADD_EMPTY_PATHSPEC,
ADVICE_ADD_IGNORED_FILE,
- ADVICE_AM_WORK_DIR,
ADVICE_AMBIGUOUS_FETCH_REFSPEC,
+ ADVICE_AM_WORK_DIR,
ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
ADVICE_COMMIT_BEFORE_MERGE,
ADVICE_DETACHED_HEAD,
ADVICE_DIVERGING,
- ADVICE_SUGGEST_DETACHING_HEAD,
ADVICE_FETCH_SHOW_FORCED_UPDATES,
ADVICE_GRAFT_FILE_DEPRECATED,
ADVICE_IGNORED_HOOK,
@@ -32,23 +31,24 @@ struct string_list;
ADVICE_PUSH_NEEDS_FORCE,
ADVICE_PUSH_NON_FF_CURRENT,
ADVICE_PUSH_NON_FF_MATCHING,
+ ADVICE_PUSH_REF_NEEDS_UPDATE,
ADVICE_PUSH_UNQUALIFIED_REF_NAME,
- ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
ADVICE_PUSH_UPDATE_REJECTED,
- ADVICE_PUSH_REF_NEEDS_UPDATE,
+ ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
ADVICE_RESET_NO_REFRESH_WARNING,
ADVICE_RESOLVE_CONFLICT,
ADVICE_RM_HINTS,
ADVICE_SEQUENCER_IN_USE,
ADVICE_SET_UPSTREAM_FAILURE,
+ ADVICE_SKIPPED_CHERRY_PICKS,
ADVICE_STATUS_AHEAD_BEHIND_WARNING,
ADVICE_STATUS_HINTS,
ADVICE_STATUS_U_OPTION,
- ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
ADVICE_SUBMODULES_NOT_UPDATED,
+ ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
+ ADVICE_SUGGEST_DETACHING_HEAD,
ADVICE_UPDATE_SPARSE_PATH,
ADVICE_WAITING_FOR_EDITOR,
- ADVICE_SKIPPED_CHERRY_PICKS,
ADVICE_WORKTREE_ADD_ORPHAN,
};
--
2.42.0
^ permalink raw reply related
* [PATCH v2 2/3] advice: fix an unexpected leading space
From: Rubén Justo @ 2024-01-11 12:40 UTC (permalink / raw)
To: Git List, Junio C Hamano
In-Reply-To: <4aedc15c-4b3f-4f5e-abea-581b501600f8@gmail.com>
This space was introduced, presumably unintentionally, in b3b18d1621
(advice: revamp advise API, 2020-03-02)
I notice this space due to confuse diff outputs while doing some
changes to enum advice_type.
As a reference, a recent change we have to that enum is:
$ git show 35f0383
[ ... ]
diff --git a/advice.h b/advice.h
index 0f584163f5..2affbe1426 100644
--- a/advice.h
+++ b/advice.h
@@ -49,6 +49,7 @@ struct string_list;
ADVICE_UPDATE_SPARSE_PATH,
ADVICE_WAITING_FOR_EDITOR,
ADVICE_SKIPPED_CHERRY_PICKS,
+ ADVICE_WORKTREE_ADD_ORPHAN,
};
Note the hunk header, instead of a much more expected:
@@ -49,6 +49,7 @@ enum advice_type
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
advice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/advice.h b/advice.h
index 9396bcdcf1..74d44d1156 100644
--- a/advice.h
+++ b/advice.h
@@ -10,7 +10,7 @@ struct string_list;
* Add the new config variable to Documentation/config/advice.txt.
* Call advise_if_enabled to print your advice.
*/
- enum advice_type {
+enum advice_type {
ADVICE_ADD_EMBEDDED_REPO,
ADVICE_ADD_EMPTY_PATHSPEC,
ADVICE_ADD_IGNORED_FILE,
--
2.42.0
^ permalink raw reply related
* [PATCH v2 3/3] branch: make the advice to force-deleting a conditional one
From: Rubén Justo @ 2024-01-11 12:40 UTC (permalink / raw)
To: Git List, Junio C Hamano
In-Reply-To: <4aedc15c-4b3f-4f5e-abea-581b501600f8@gmail.com>
The error message we show when the user tries to delete a not fully
merged branch describes the error and gives a hint to the user:
error: the branch 'foo' is not fully merged.
If you are sure you want to delete it, run 'git branch -D foo'.
Let's move the hint part so that it is displayed using the advice
machinery:
error: the branch 'foo' is not fully merged
hint: If you are sure you want to delete it, run 'git branch -D foo'
hint: Disable this message with "git config advice.forceDeleteBranch false"
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
Documentation/config/advice.txt | 3 +++
advice.c | 1 +
advice.h | 1 +
builtin/branch.c | 8 +++++---
4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index e0deaf3144..25c0917524 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -45,6 +45,9 @@ advice.*::
Advice shown when linkgit:git-fetch[1] takes a long time
to calculate forced updates after ref updates, or to warn
that the check is disabled.
+ forceDeleteBranch::
+ Advice shown when a user tries to delete a not fully merged
+ branch without the force option set.
ignoredHook::
Advice shown if a hook is ignored because the hook is not
set as executable.
diff --git a/advice.c b/advice.c
index 03322caba0..f6e4c2f302 100644
--- a/advice.c
+++ b/advice.c
@@ -47,6 +47,7 @@ static struct {
[ADVICE_DETACHED_HEAD] = { "detachedHead", 1 },
[ADVICE_DIVERGING] = { "diverging", 1 },
[ADVICE_FETCH_SHOW_FORCED_UPDATES] = { "fetchShowForcedUpdates", 1 },
+ [ADVICE_FORCE_DELETE_BRANCH] = { "forceDeleteBranch", 1 },
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated", 1 },
[ADVICE_IGNORED_HOOK] = { "ignoredHook", 1 },
[ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity", 1 },
diff --git a/advice.h b/advice.h
index 74d44d1156..9d4f49ae38 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ enum advice_type {
ADVICE_DETACHED_HEAD,
ADVICE_DIVERGING,
ADVICE_FETCH_SHOW_FORCED_UPDATES,
+ ADVICE_FORCE_DELETE_BRANCH,
ADVICE_GRAFT_FILE_DEPRECATED,
ADVICE_IGNORED_HOOK,
ADVICE_IMPLICIT_IDENTITY,
diff --git a/builtin/branch.c b/builtin/branch.c
index 0a32d1b6c8..cfb63cce5f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -24,6 +24,7 @@
#include "ref-filter.h"
#include "worktree.h"
#include "help.h"
+#include "advice.h"
#include "commit-reach.h"
static const char * const builtin_branch_usage[] = {
@@ -190,9 +191,10 @@ static int check_branch_commit(const char *branchname, const char *refname,
return -1;
}
if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("the branch '%s' is not fully merged.\n"
- "If you are sure you want to delete it, "
- "run 'git branch -D %s'"), branchname, branchname);
+ error(_("the branch '%s' is not fully merged"), branchname);
+ advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
+ _("If you are sure you want to delete it, "
+ "run 'git branch -D %s'"), branchname);
return -1;
}
return 0;
--
2.42.0
^ permalink raw reply related
* RE: [DISCUSS] Introducing Rust into the Git project
From: rsbecker @ 2024-01-11 13:07 UTC (permalink / raw)
To: 'Elijah Newren'
Cc: 'Taylor Blau', 'Junio C Hamano',
'Dragan Simic', git
In-Reply-To: <CABPp-BGmXw0NQ8yBaMiVXHiKr0-Y_jkZWmJB1CG_oc4UGxt_gA@mail.gmail.com>
On Thursday, January 11, 2024 12:06 AM, Elijah Newren wrote:
>On Wed, Jan 10, 2024 at 6:57 PM <rsbecker@nexbridge.com> wrote:
>>
>> On Wednesday, January 10, 2024 9:21 PM, Elijah Newren wrote:
>> >On Wed, Jan 10, 2024 at 5:44 PM <rsbecker@nexbridge.com> wrote:
>> >>
>> >> On Wednesday, January 10, 2024 7:59 PM, Elijah Newren wrote:
>> >[...]
>> >> >Would you be okay with the following alternative: requiring that
>> >> >all Rust code be optional for now?
>> >> >
>> >> >(In other words, allow you to build with USE_RUST=0, or something
>> >> >like that. And then we have both a Rust and a C implementation of
>> >> >anything that is required for backward compatibility, while any
>> >> >new Rust-only stuff would not be included in your build.)
>> >>
>> >> To address the immediate above, I assume this means that platform
>> >> maintainers will be responsible for developing non-portable
>> >> implementations that duplicate Rust functionality
>> >
>> >This doesn't at all sound like what I thought I said. The whole
>> >proposal was so that folks like NonStop could continue using Git with
>> >no more work than setting
>> >USE_RUST=0 at build time.
>> >
>> >Why do you feel you'd need to duplicate any functionality?
>>
>> I think I misunderstood. What I took from this is that all new functionality would
>be in Rust, which would require a custom implementation in C for platforms that did
>not have Rust available - if that is even practical. Did I get that wrong?
>
>I think you somehow missed the word optional?
>
>I did say that new functionality should be allowed to be Rust only (unlike existing
>functionality), but I'm not sure how you leaped to assuming that all new
>functionality would be in Rust. Further, I also don't understand why you jump to
>assuming that all new functionality needs to be supported on all platforms. The
>point of the word "optional" in my proposal is that it is not required. So, say, if git-
>replay is in Rust, well you've never had git-replay before in any release, so you
>haven't lost any functionality by it being implemented in Rust. And existing things
>(merge, cherry-pick, rebase, etc.) continue working with C-only code. But you may
>have one less optional addition.
>
>At least that was _my_ proposal -- that Rust be optional for now. It does differ from
>what I think Taylor was originally proposing, but that's why I brought it up as an
>alternative proposal.
Thank you for the clarification.
^ permalink raw reply
* Re: Storing private config files in .git directory?
From: Stefan Haller @ 2024-01-11 13:28 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git
In-Reply-To: <20240110110842.GD16674@coredump.intra.peff.net>
On 10.01.24 12:08, Jeff King wrote:
> On Mon, Jan 08, 2024 at 10:20:00AM -0800, Junio C Hamano wrote:
>
>> An obvious alternative is to have .lazygit directory next to .git directory
>> which would give you a bigger separation, which can cut both ways.
>
> Just to spell out one of those ways: unlike ".git", we will happily
> check out ".lazygit" from an untrusted remote repository. That may be a
> feature if you want to be able to share project-specific config, or it
> might be a terrible security vulnerability if lazygit config files can
> trigger arbitrary code execution.
Unless you don't version it and add it to .gitignore instead, which (I
suppose) is what most people do with their .vscode/settings.json, for
example.
-Stefan
^ permalink raw reply
* Re: [PATCH] branch: error description when deleting a not fully merged branch
From: Rubén Justo @ 2024-01-11 13:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqbk9tcc57.fsf@gitster.g>
On 10-ene-2024 09:48:52, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > The error message we show when the user tries to delete a not fully
> > merged branch describes the error and gives a hint to the user:
> >
> > error: the branch 'foo' is not fully merged.
> > If you are sure you want to delete it, run 'git branch -D foo'.
> >
> > Let's move the hint part so that it takes advantage of the advice
> > machinery:
> >
> > error: the branch 'foo' is not fully merged
> > hint: If you are sure you want to delete it, run 'git branch -D foo'
> > hint: Disable this message with "git config advice.forceDeleteBranch false"
>
> This is probably one sensible step forward, so let's queue it as-is.
Thanks.
> But with reservations for longer-term future direction. Stepping
> back a bit, when 'foo' is not fully merged and the user used "branch
> -d" on it, is it sensible for us to suggest use of "branch -D"
Maybe the user hits here because he's doing "branch -d" and so I would
find a more clear suggestion: "branch -d foo -f".
Or to be more generic, not suggesting a command line but a description
that explains how to use "the force" ... :) sorry for the joke
Anyway, I think you mean to suggest a less destructive approach. Which
is fine by me.
> Especially now this is a "hint" to help less experienced folks, it
> may be helpful to suggest how the user can answer "If you are sure
> you want to delete" part. As this knows what unique commits on the
> branch being deleted are about to be lost, one way to do so may be
> to tell the user about them ("you are about to lose 'branch: error
> description when deleting a not fully merged branch' and other 47
> commits that are not merged the target branch 'main'", for example).
That's an interesting idea. Maybe the hint becomes more informative
than a simple advice ... maybe a more-verbose error is needed ... just
thinking out loud ...
>
> Another possibility is to suggest merging the branch into the
> target, instead of suggesting a destructive "deletion", but I
> suspect that it goes too far second-guessing the end-user intention.
>
> Thanks.
Thank you.
^ permalink raw reply
* [PATCH] git-p4: stop reaching into the refdb
From: Patrick Steinhardt @ 2024-01-11 13:47 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]
The git-p4 tool creates a bunch of temporary branches that share a
common prefix "refs/git-p4-tmp/". These branches get cleaned up via
git-update-ref(1) after the import has finished. Once done, we try to
manually remove the now supposedly-empty ".git/refs/git-p4-tmp/"
directory.
This last step can fail in case there still are any temporary branches
around that we failed to delete because `os.rmdir()` refuses to delete a
non-empty directory. It can thus be seen as kind of a sanity check to
verify that we really did delete all temporary branches. Another failure
mode though is when the directory didn't exist in the first place, which
can be the case when using an alternate ref backend like the upcoming
"reftable" backend.
Convert the code to instead use git-for-each-ref(1) to verify that there
are no more temporary branches around. This works alright with alternate
ref backends while retaining the sanity check that we really did prune
all temporary branches.
This is a modification in behaviour for the "files" backend because the
empty directory does not get deleted anymore. But arguably we should not
care about such implementation details of the ref backend anyway, and
this should not cause any user-visible change in behaviour.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
git-p4.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 0eb3bb4c47..3ea1c405e5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -4251,7 +4251,8 @@ def run(self, args):
if self.tempBranches != []:
for branch in self.tempBranches:
read_pipe(["git", "update-ref", "-d", branch])
- os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
+ if len(read_pipe(["git", "for-each-ref", self.tempBranchLocation])) > 0:
+ die("There are unexpected temporary branches")
# Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
# a convenient shortcut refname "p4".
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Fwd: [GSOC][RFC] Add more builtin patterns for userdiff, as Mircroproject.
From: Sergius Nyah @ 2024-01-11 14:23 UTC (permalink / raw)
To: Christian Couder; +Cc: git, gitster, newren, l.s.r
In-Reply-To: <CAP8UFD3e=Zv2wkx5tswCz05Vwn3vD68Vw-TD6SoENWK+norYsw@mail.gmail.com>
---------- Forwarded message ---------
From: Christian Couder <christian.couder@gmail.com>
Date: Thu, Jan 11, 2024 at 11:51 AM
Subject: Re: [GSOC][RFC] Add more builtin patterns for userdiff, as
Mircroproject.
To: Sergius Nyah <sergiusnyah@gmail.com>
On Thu, Jan 11, 2024 at 11:48 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Jan 10, 2024 at 9:10 PM Sergius Nyah <sergiusnyah@gmail.com> wrote:
> > > Okay then. I will add support for shell, as it was primarily suggested
> > > in the idea itself.
>
> As Bash is a shell and is already supported, I am not sure which kind
> of shell you want to support. There are other shells or kinds of shell
> with different syntax that might be worth supporting, but then you
> might want to tell us which shell or which new kind of shell.
>
> Thanks!
I've definitely learned one more thing. Thank you!
Also, after taking another keen look at the Patterns defined in userdiff.c,
In accordance with what you've said, I realized that Kotlin isn't
supported yet.
So, I'd begin working on it.
Thanks!
By the way I realized that you replied privately to me instead of
keeping the mailing list in Cc: This made me reply privately to you.
Thank you for the correction Christian. Noted!
I think it would be better if the discussions were all public as this
way others could give you their opinion on this.
Definitely True.
^ permalink raw reply
* Re: [GSOC][RFC] Add more builtin patterns for userdiff, as Mircroproject.
From: Christian Couder @ 2024-01-11 15:11 UTC (permalink / raw)
To: Sergius Nyah; +Cc: git, gitster, newren, l.s.r
In-Reply-To: <CANAnif-hWovPnRmSy0pnEJnA274vwiSAkRexstGQdQJ4u-5+pw@mail.gmail.com>
On Thu, Jan 11, 2024 at 3:23 PM Sergius Nyah <sergiusnyah@gmail.com> wrote:
>
> ---------- Forwarded message ---------
> From: Christian Couder <christian.couder@gmail.com>
> Date: Thu, Jan 11, 2024 at 11:51 AM
> Subject: Re: [GSOC][RFC] Add more builtin patterns for userdiff, as
> Mircroproject.
> To: Sergius Nyah <sergiusnyah@gmail.com>
>
>
> On Thu, Jan 11, 2024 at 11:48 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> >
> > On Wed, Jan 10, 2024 at 9:10 PM Sergius Nyah <sergiusnyah@gmail.com> wrote:
>
> > > > Okay then. I will add support for shell, as it was primarily suggested
> > > > in the idea itself.
> >
> > As Bash is a shell and is already supported, I am not sure which kind
> > of shell you want to support. There are other shells or kinds of shell
> > with different syntax that might be worth supporting, but then you
> > might want to tell us which shell or which new kind of shell.
> >
> > Thanks!
>
> I've definitely learned one more thing. Thank you!
> Also, after taking another keen look at the Patterns defined in userdiff.c,
> In accordance with what you've said, I realized that Kotlin isn't
> supported yet.
It is actually already supported too:
$ grep -i -n kotlin userdiff.c
186:PATTERNS("kotlin",
To help you a bit, you can get the list of already supported languages using:
$ perl -ne 'print "$1\n" if m/PATTERNS\(\"(\w+)\"/ or
m/IPATTERN\(\"(\w+)\"/' <userdiff.c
ada
bash
bibtex
cpp
csharp
css
dts
elixir
fortran
fountain
golang
html
java
kotlin
markdown
matlab
objc
pascal
perl
php
python
ruby
rust
scheme
tex
^ permalink raw reply
* Re: [GSOC][RFC] Add more builtin patterns for userdiff, as Mircroproject.
From: Sergius Nyah @ 2024-01-11 15:42 UTC (permalink / raw)
To: Christian Couder; +Cc: git, gitster, newren, l.s.r
In-Reply-To: <CAP8UFD0YicYXrYgHdvcrWps+cMa=Dp9Ob7SfYLMXQKLp7-B+7w@mail.gmail.com>
On Thu, Jan 11, 2024 at 4:11 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Thu, Jan 11, 2024 at 3:23 PM Sergius Nyah <sergiusnyah@gmail.com> wrote:
> >
> > ---------- Forwarded message ---------
> > From: Christian Couder <christian.couder@gmail.com>
> > Date: Thu, Jan 11, 2024 at 11:51 AM
> > Subject: Re: [GSOC][RFC] Add more builtin patterns for userdiff, as
> > Mircroproject.
> > To: Sergius Nyah <sergiusnyah@gmail.com>
> >
> >
> > On Thu, Jan 11, 2024 at 11:48 AM Christian Couder
> > <christian.couder@gmail.com> wrote:
> > >
> > > On Wed, Jan 10, 2024 at 9:10 PM Sergius Nyah <sergiusnyah@gmail.com> wrote:
> >
> > > > > Okay then. I will add support for shell, as it was primarily suggested
> > > > > in the idea itself.
> > >
> > > As Bash is a shell and is already supported, I am not sure which kind
> > > of shell you want to support. There are other shells or kinds of shell
> > > with different syntax that might be worth supporting, but then you
> > > might want to tell us which shell or which new kind of shell.
> > >
> > > Thanks!
> >
> > I've definitely learned one more thing. Thank you!
> > Also, after taking another keen look at the Patterns defined in userdiff.c,
> > In accordance with what you've said, I realized that Kotlin isn't
> > supported yet.
>
> It is actually already supported too:
>
> $ grep -i -n kotlin userdiff.c
> 186:PATTERNS("kotlin",
>
> To help you a bit, you can get the list of already supported languages using:
>
> $ perl -ne 'print "$1\n" if m/PATTERNS\(\"(\w+)\"/ or
> m/IPATTERN\(\"(\w+)\"/' <userdiff.c
Thank you for these. $ perl -ne 'print "$1\n" if
m/IPATTERN\(\"(\w+)\"/' userdiff.c works too.
> ada
> bash
> bibtex
> cpp
> csharp
> css
> dts
> elixir
> fortran
> fountain
> golang
> html
> java
> kotlin
> markdown
> matlab
> objc
> pascal
> perl
> php
> python
> ruby
> rust
> scheme
> tex
Great! JavaScript, despite its widespread use, isn't implemented, so I
will take it.
Thank you!
^ permalink raw reply
* Re: [PATCH v3 2/2] t7501: add tests for --amend --signoff
From: phillip.wood123 @ 2024-01-11 16:30 UTC (permalink / raw)
To: Ghanshyam Thakkar, git; +Cc: christian.couder, gitster
In-Reply-To: <20240110163622.51182-6-shyamthakkar001@gmail.com>
On 10/01/2024 16:35, Ghanshyam Thakkar wrote:
> Add tests for amending the commit to add Signed-off-by trailer. And
> also to check if it does not add another trailer if one already exists.
>
> Currently, there are tests for --signoff separately in t7501, however,
> they are not tested with --amend.
>
> Therefore, these tests belong with other similar tests of --amend in
> t7501-commit-basic-functionality.
>
> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
This version looks good, thanks for re-rolling. I have left one comment
below but it is not worth re-rolling just for that.
> +test_expect_success 'amend commit to add signoff' '
> +
> + test_commit "msg" file content &&
> + git commit --amend --signoff &&
> + test_commit_message HEAD <<-EOF
> + msg
> +
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> + EOF
> +
> +'
If you do happen re-roll I think we could happily lose the empty line
before the closing "'" in this test and the next.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v3 1/2] t7501: add tests for --include and --only
From: phillip.wood123 @ 2024-01-11 16:33 UTC (permalink / raw)
To: Ghanshyam Thakkar, git; +Cc: christian.couder, gitster
In-Reply-To: <20240110163622.51182-4-shyamthakkar001@gmail.com>
On 10/01/2024 16:35, Ghanshyam Thakkar wrote:
I don't have much to add to what Junio said, I've just left one comment
below
> +test_expect_success '--only excludes staged changes' '
> + echo content >file &&
> + echo content >baz &&
> + git add baz &&
> + git commit --only -m "file" file &&
> +
> + git diff --name-only >actual &&
> + test_must_be_empty actual &&
> + git diff --name-only --staged >actual &&
> + test_grep "baz" actual
using test_grep feels a bit weak here, I think it would be better to use
test_cmp to ensure that there are no other staged changes.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 2/3] t7450: test submodule urls
From: Victoria Dye @ 2024-01-11 16:54 UTC (permalink / raw)
To: Jeff King, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <20240110103812.GB16674@coredump.intra.peff.net>
Jeff King wrote:
> On Tue, Jan 09, 2024 at 05:53:36PM +0000, Victoria Dye via GitGitGadget wrote:
>
>> +#define TEST_TOOL_CHECK_URL_USAGE \
>> + "test-tool submodule check-url <url>"
>
> I don't think this command-line "<url>" mode works at all. Your
> underlying function can handle either stdin or arguments:
...
> All of this is inherited from the existing check_name() code, which I
> think has all of the same bugs. The test scripts all just use the stdin
> mode, so they don't notice. It's not too hard to fix, but maybe it's
> worth just ripping out the unreachable code.
Thanks for pointing out those issues, I think removing the command line
input mode is the way to go. The description of the 'check_name()' mentions
that the stdin mode was "primarily intended for testing". But as 85321a346b5
(submodule--helper: move "check-name" to a test-tool, 2022-09-01) pointed
out, 'check_name()' was never used outside of tests anyway, so whatever use
case was imagined for the command line mode never seemed to have existed.
Combine that with the fact that the command line mode is so different from
the stdin mode (non-zero exit code for invalid names, prints nothing vs.
zero exit code, prints valid names), there don't seem to be any real
downsides to removing the unused code.
>
> -Peff
^ permalink raw reply
* Re: [PATCH] builtin/revert.c: refactor using an enum for cmd-action
From: Phillip Wood @ 2024-01-11 16:57 UTC (permalink / raw)
To: Michael Lohmann, git; +Cc: Wanja Henze
In-Reply-To: <20240111080417.59346-1-mi.al.lohmann@gmail.com>
Hi Michael
On 11/01/2024 08:04, Michael Lohmann wrote:
> This is done to avoid having to keep the char values in sync in
> different places and also to get compiler warnings on non-exhaustive
> switches.
I think this is a reasonable change, thanks for working on it.
> The newly introduced `revert_action`-enum aligns with the
> builtin/rebase.c `action`-enum though the name `revert_action` is chosen
> to better differentiate it from `replay_opts->action` with a different
> function. For rebase the equivalent of the latter is
> `rebase_options->type` and `rebase_options->action` corresponds to the
> `cmd` variable in revert.c.
>
> In the rebase `action` enum there is the enumeration constant
> `ACTION_NONE` which is not particularly descriptive, since it seems to
> imply that no action should be taken. Instead it signals a start of a
> revert/cherry-pick process, so here `REVERT_ACTION_START` was chosen.
I think ACTION_NONE was intended to covey that the user did not pass one
of the OPT_CMDMODE() options like "--continue" as there isn't a
"--start" option. I don't have a strong opinion between "_NONE" and
"_START".
> +enum revert_action {
> + REVERT_ACTION_START = 0,
> + REVERT_ACTION_CONTINUE,
> + REVERT_ACTION_SKIP,
> + REVERT_ACTION_ABORT,
> + REVERT_ACTION_QUIT,
> +};
The "REVERT_" prefix is a bit unfortunate as this is used by cherry-pick
as well but it does match the filename. As this enum is only used in
this file I'd be quite happy to drop the "REVERT_" prefix. (I don't
think we need to go messing with the "action" member of struct
replay_opts to do that)
> /* Check for incompatible command line arguments */
> - if (cmd) {
> - char *this_operation;
> - if (cmd == 'q')
> + {
> + char *this_operation = 0;
style note: we use "NULL" rather than "0" when initializing pointers.
Ideally this would be a "const char *" as we assign string literals but
that is not a new problem with this patch.
> + switch (cmd) {
> + case REVERT_ACTION_START:
> + break;
I can see the attraction of using an exhaustive switch() here but as we
don't want to do anything in the _START case it gets a bit messy with
the extra conditional below. I wonder if we'd be better to replace "if
(cmd) {" with "if (cmd != REVERT_ACTION_START) {". Alternatively if you
want to stick with the switch then declaring "this_operation" at the
beginning of the function would mean you can get rid of the new "{}" block.
> + case REVERT_ACTION_QUIT:
> this_operation = "--quit";
> - else if (cmd == 'c')
> + break;
> + case REVERT_ACTION_CONTINUE:
> this_operation = "--continue";
> - else if (cmd == 's')
> + break;
> + case REVERT_ACTION_SKIP:
> this_operation = "--skip";
> - else {
> - assert(cmd == 'a'); > + break;
> + case REVERT_ACTION_ABORT:
> this_operation = "--abort";
> + break;
> }
>
> - verify_opt_compatible(me, this_operation,
> - "--no-commit", opts->no_commit,
> - "--signoff", opts->signoff,
> - "--mainline", opts->mainline,
> - "--strategy", opts->strategy ? 1 : 0,
> - "--strategy-option", opts->xopts.nr ? 1 : 0,
> - "-x", opts->record_origin,
> - "--ff", opts->allow_ff,
> - "--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
> - "--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
> - NULL);
> + if (this_operation)
The extra indentation here is unfortunate as some of the lines are
rather long already. In the current code it is clear that we only call
verify_opt_compatible() when cmd is non-nul, I think it would be clearer
to use "if (cmd != REVERT_ACTION_START)" here.
> + verify_opt_compatible(me, this_operation,
> + "--no-commit", opts->no_commit,
> + "--signoff", opts->signoff,
> + "--mainline", opts->mainline,
> + "--strategy", opts->strategy ? 1 : 0,
> + "--strategy-option", opts->xopts.nr ? 1 : 0,
> + "-x", opts->record_origin,
> + "--ff", opts->allow_ff,
> + "--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
> + "--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
> + NULL);
> }
> [...]
> - if (cmd) {
> - opts->revs = NULL;
> - } else {
> + if (cmd == REVERT_ACTION_START) {
I was momentarily confused by this change but you're reversing the
conditional. I agree that the result is an improvement.
Best Wishes
Phillip
^ permalink raw reply
* Re: [DISCUSS] Introducing Rust into the Git project
From: Elijah Newren @ 2024-01-11 16:57 UTC (permalink / raw)
To: Dragan Simic; +Cc: Taylor Blau, git
In-Reply-To: <f5b9a57b6e2b513f1d79a93c6f0ccf45@manjaro.org>
Hi Dragan,
On Wed, Jan 10, 2024 at 9:39 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> On 2024-01-11 01:33, Elijah Newren wrote:
> > On Wed, Jan 10, 2024 at 1:57 PM Dragan Simic <dsimic@manjaro.org>
> > wrote:
> >>
> >> Thus, Git should probably follow the same approach of not converting
> >> the
> >> already existing code
> >
> > I disagree with this. I saw significant performance improvements
> > through converting some existing Git code to Rust. Granted, it was
> > only a small amount of code, but the performance benefits I saw
> > suggested we'd see more by also doing similar conversions elsewhere.
> > (Note that I kept the old C code and then conditionally compiled
> > either Rust or C versions of what I was converting.)
>
> Well, it's also possible that improving the old C code could also result
> in some performance improvements. Thus, quite frankly, I don't see that
> as a valid argument to rewrite some existing C code in Rust.
Yes, and I've made many performance improvements in the C code in git.
Sometimes I make some of the code 5% or 20% faster. Sometimes 1-3
orders of magnitude faster. Once over 60 orders of magnitude
faster.[1] Look around in git's history; I've done a fair amount of
performance stuff.
And I'm specifically arguing that I feel limited in some of the
performance work that can be done by remaining in C. Part of my
reason for interest in Rust is exactly because I think it can help us
improve performance in ways that are far more difficult to achieve in
C. And this isn't just guesswork, I've done some trials with it.
Further, I even took the time to document some of these reasons
elsewhere in this thread[2]. Arguing that some performance
improvements can be done in C is thus entirely missing the point.
If you want to dismiss the performance angle of argument for Rust, you
should take the time to address the actual reasons raised for why it
could make it easier to improve performance relative to continuing in
C.
Also, as a heads up since you seem to be relatively new to the list:
your position will probably carry more weight with others if you take
the time to understand, acknowledge, and/or address counterpoints of
the other party. It is certainly fine to simply express some concerns
without doing so (Randall and Patrick did a good job of this in this
thread), but when you simply assert that the benefits others point out
simply don't exist (e.g. your "Quite frankly, that would _only_
complicate things and cause fragmentation." (emphasis added) from your
first email in this thread[3], and which this latest email of yours
somewhat looks like as well), others may well start applying a
discount to any positions you state. Granted, it's totally up to you,
but I'm just giving a hint about how I think you might be able to be
more persuasive.
Hope that helps,
Elijah
[1] A couple examples: 6a5fb966720 ("Change default merge backend from
recursive to ort", 2021-08-04) and 8d92fb29270 ("dir: replace
exponential algorithm with a linear one", 2020-04-01)
[2] Footnote 6 of
https://lore.kernel.org/git/CABPp-BFOmwV-xBtjvtenb6RFz9wx2VWVpTeho0k=D8wsCCVwqQ@mail.gmail.com/
[3] https://lore.kernel.org/git/b2651b38a4f7edaf1c5ffee72af00e46@manjaro.org/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox