* Re: [PATCH 3/6] parse-options: factor out register_abbrev() and struct parsed_option
From: Junio C Hamano @ 2024-02-24 23:13 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <20240224212953.44026-4-l.s.r@web.de>
René Scharfe <l.s.r@web.de> writes:
> static enum parse_opt_result parse_long_opt(
> struct parse_opt_ctx_t *p, const char *arg,
> const struct option *options)
> {
> const char *arg_end = strchrnul(arg, '=');
> - const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
> - enum opt_parsed abbrev_flags = OPT_LONG, ambiguous_flags = OPT_LONG;
> - int allow_abbrev = !(p->flags & PARSE_OPT_KEEP_UNKNOWN_OPT);
> + struct parsed_option abbrev = { .option = NULL, .flags = OPT_LONG };
> + struct parsed_option ambiguous = { .option = NULL, .flags = OPT_LONG };
There is this "allow_abbrev" thing ...
> ...
> if (!skip_prefix(arg + 3, long_name, &rest)) {
> /* abbreviated and negated? */
> - if (allow_abbrev &&
> - !strncmp(long_name, arg + 3,
> + if (!strncmp(long_name, arg + 3,
> arg_end - arg - 3))
> - goto is_abbreviated;
> - else
> - continue;
> + register_abbrev(p, options,
> + flags ^ opt_flags,
> + &abbrev, &ambiguous);
> + continue;
> }
> }
... whose use goes away completely from the loop. We still call
register_abbrev(), but the helper function becomes no-op when
p->flags had KEEP_UNKNOWN_OPT bit set, so everything cancels out.
OK.
^ permalink raw reply
* Re: [PATCH] fetch: convert strncmp() with strlen() to starts_with()
From: Junio C Hamano @ 2024-02-24 23:45 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <cb94b938-03f9-4dd3-84c1-f5244ca81be3@web.de>
René Scharfe <l.s.r@web.de> writes:
> Using strncmp() and strlen() to check whether a string starts with
> another one requires repeating the prefix candidate. Use starts_with()
> instead, which reduces repetition and is more readable.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> builtin/fetch.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3aedfd1bb6..0a7a1a3476 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -448,9 +448,8 @@ static void filter_prefetch_refspec(struct refspec *rs)
> continue;
> if (!rs->items[i].dst ||
> (rs->items[i].src &&
> - !strncmp(rs->items[i].src,
> - ref_namespace[NAMESPACE_TAGS].ref,
> - strlen(ref_namespace[NAMESPACE_TAGS].ref)))) {
> + starts_with(rs->items[i].src,
> + ref_namespace[NAMESPACE_TAGS].ref))) {
The original tries to check that "namespace" fully matches the
initial part of .src string, which is exactly what starts_with()
does. Makes sense.
> int j;
>
> free(rs->items[i].src);
> --
> 2.44.0
^ permalink raw reply
* [PATCH v3] build: support z/OS (OS/390).
From: Haritha via GitGitGadget @ 2024-02-25 6:10 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Haritha, Haritha D
In-Reply-To: <pull.1663.v2.git.git.1708660111.gitgitgadget@gmail.com>
From: Haritha D <harithamma.d@ibm.com>
Since the z/OS linker does not support searching dynamic libraries,
and the current setting of CC_LD_DYNPATH results in a directory
to be supplied to the link step with no option as the suffix,
it causes a linker error because the z/OS LD linker
does not accept directories as input.
Therefore, -L option is added.
Also introduced z/OS (OS/390) as a platform in config.mak.uname
Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
This PR enables a successful git build on z/OS.
Since the z/OS linker does not support searching dynamic libraries, and
the current setting of CC_LD_DYNPATH results in a directory to be
supplied to the link step with no option as the suffix, it causes a
linker error because the z/OS LD linker does not accept directories as
input. Therefore, we workaround this by adding the -L option. And,
Introduced z/OS (OS/390) as a platform in config.mak.uname
Thanks for taking the time to contribute to Git! Please be advised that
the Git community does not use github.com for their contributions.
Instead, we use a mailing list (git@vger.kernel.org) for code
submissions, code reviews, and bug reports. Nevertheless, you can use
GitGitGadget (https://gitgitgadget.github.io/) to conveniently send your
Pull Requests commits to our mailing list.
Please read the "guidelines for contributing" linked above!
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v3
Pull-Request: https://github.com/git/git/pull/1663
Range-diff vs v2:
1: 53e211d7a65 ! 1: 2f1ad41bc14 build: support z/OS (OS/390).
@@ config.mak.uname: ifeq ($(uname_S),NONSTOP_KERNEL)
+ NO_MEMMEM = YesPlease
+ NO_GECOS_IN_PWENT = YesPlease
+ HAVE_STRINGS_H = YesPlease
-+ NEEDS_MODE_TRANSLATION = YesPlease
++ NEEDS_MODE_TRANSLATION = YesPlease
+endif
ifeq ($(uname_S),MINGW)
ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
$(error "Building with MSys is no longer supported")
-
- ## configure.ac ##
-@@ configure.ac: else
- CC_LD_DYNPATH=-Wl,+b,
- else
- CC_LD_DYNPATH=
-+ if test "$(uname -s)" = "OS/390"; then
-+ CC_LD_DYNPATH=-L
-+ fi
- AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
- fi
- fi
2: 05df5d7e2d5 < -: ----------- an improvement: removed configure.ac changes
config.mak.uname | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..03ee2b74525 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
SHELL_PATH = /usr/coreutils/bin/bash
endif
+ifeq ($(uname_S),OS/390)
+ NO_SYS_POLL_H = YesPlease
+ NO_STRCASESTR = YesPlease
+ NO_REGEX = YesPlease
+ NO_MMAP = YesPlease
+ NO_NSEC = YesPlease
+ NO_STRLCPY = YesPlease
+ NO_MEMMEM = YesPlease
+ NO_GECOS_IN_PWENT = YesPlease
+ HAVE_STRINGS_H = YesPlease
+ NEEDS_MODE_TRANSLATION = YesPlease
+endif
ifeq ($(uname_S),MINGW)
ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
$(error "Building with MSys is no longer supported")
base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11
--
gitgitgadget
^ permalink raw reply related
* Re: [ANNOUNCE] Git v2.44.0
From: Patrick Steinhardt @ 2024-02-25 7:09 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, git-packagers
In-Reply-To: <20240224195550.ignhzidmdy3ce6q4@glandium.org>
[-- Attachment #1: Type: text/plain, Size: 2618 bytes --]
On Sun, Feb 25, 2024 at 04:55:50AM +0900, Mike Hommey wrote:
> On Sat, Feb 24, 2024 at 07:36:21AM +0100, Patrick Steinhardt wrote:
> > I have to wonder whether we ever
> > really specified what the environment of a remote helper should look
> > like when used during cloning. Conceptually it doesn't feel _wrong_ to
> > have a not-yet-initialized repo during clone.
>
> How about this: it should look like what you'd get from
> `git init $repo`.
The problem is that it can't. With git-init(1) you already know how the
repo should look like as you specify parameters like SHA1 vs SHA256 via
parameters. But with git-clone(1) it's a different story, as you only
learn about how the repo should look like _after_ you have connected to
the remote. And thus, after you have executed the remote helper.
This has never been a problem with the old "files" backend because it
does not encode the object format in the refdb. But the "reftable"
backend does, and thus we can only creat the refdb after we have learned
about the object format. Future refdb implementations are likely to do
similar things.
> > Another idea would be to simply pre-create HEAD regardless of the ref
> > format, pointing to an invalid ref "refs/heads/.invalid". This is the
> > same trick we use for the reftable backend, and should likely address
> > your issue.
>
> The interesting thing is that `git init $repo` does give you an invalid
> HEAD (and that's what would happen during git clone too), with either
> ```
> ref: refs/heads/master
> ```
> or
> ```
> ref: refs/heads/main
> ```
> depending on configuration.
HEAD in your example is not invalid, it's unborn. That's a difference
because creating the branch would succeed just fine. In the case of
"refs/heads/.invalid", creating the branch will fail because the ref
name would be refused.
With the reftable backend, we are forced to create HEAD exactly due to
the problem you have just encountered: a repo would not be discovered if
it did not exist. But its value shouldn't ever be read, because it is in
part of the reftables in "reftable/".
So as a safety mechanism, we want to make sure that the value of HEAD
cannot be interpreted like a "proper" ref. Clients that do not
understand the reftable format should not see a HEAD ref pointing to
"refs/heads/main" and then happily create or access such a branch.
That's why we want it to be an invalid ref.
That's ultimately the reason why I don't want HEAD to look like a proper
ref. But doing the "refs/heads/.invalid" workaround shouldn't be too bad,
I guess.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] completion: fix __git_complete_worktree_paths
From: Rubén Justo @ 2024-02-25 8:16 UTC (permalink / raw)
To: Git List
Use __git to invoke "worktree list" in __git_complete_worktree_paths, to
respect any "-C" and "--git-dir" options present on the command line.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
I stumbled upon this in a situation like:
$ git init /tmp/foo && cd /tmp/foo
$ git worktree add --orphan foo_wt
$ git init /tmp/bar && cd /tmp/bar
$ git worktree add --orphan bar_wt
$ cd /tmp/foo
$ git -C /tmp/bar worktree remove <TAB>
... expecting /tmp/bar/wt, but ...
$ git -C /tmp/bar worktree remove /tmp/foo_wt
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 444b3efa63..86e55dc67f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3571,7 +3571,7 @@ __git_complete_worktree_paths ()
# Generate completion reply from worktree list skipping the first
# entry: it's the path of the main worktree, which can't be moved,
# removed, locked, etc.
- __gitcomp_nl "$(git worktree list --porcelain |
+ __gitcomp_nl "$(__git worktree list --porcelain |
sed -n -e '2,$ s/^worktree //p')"
}
--
2.44.0
^ permalink raw reply related
* [PATCH] send-email: implement SMTP bearer authentication
From: Julian Swagemakers @ 2024-02-25 10:34 UTC (permalink / raw)
To: git; +Cc: Julian Swagemakers
Manually send SMTP AUTH command for auth type OAUTHBEARER and XOAUTH2.
This is necessary since they are currently not supported by the Perls
Authen::SASL module.
The bearer token needs to be passed in as the password. This can be done
with git-credential-oauth[0] after minor modifications[1]. Which will
allow using git send-email with Gmail and oauth2 authentication:
```
[credential]
helper = cache --timeout 7200 # two hours
helper = oauth
[sendemail]
smtpEncryption = tls
smtpServer = smtp.gmail.com
smtpUser = example@gmail.com
smtpServerPort = 587
smtpauth = OAUTHBEARER
```
As well as Office 365 accounts:
```
[credential]
helper = cache --timeout 7200 # two hours
helper = oauth
[sendemail]
smtpEncryption = tls
smtpServer = smtp.office365.com
smtpUser = example@example.com
smtpServerPort = 587
smtpauth = XOAUTH2
```
[0] https://github.com/hickford/git-credential-oauth
[1] https://github.com/hickford/git-credential-oauth/issues/48
Signed-off-by: Julian Swagemakers <julian@swagemakers.org>
---
git-send-email.perl | 65 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 821b2b3a13..72d378f6fd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1359,6 +1359,63 @@ sub smtp_host_string {
}
}
+sub generate_oauthbearer_string {
+ # This will generate the oauthbearer string used for authentication.
+ #
+ # "n,a=" {User} ",^Ahost=" {Host} "^Aport=" {Port} "^Aauth=Bearer " {Access Token} "^A^A
+ #
+ # The first part `n,a=" {User} ",` is the gs2 header described in RFC5801.
+ # * gs2-cb-flag `n` -> client does not support CB
+ # * gs2-authzid `a=" {User} "`
+ #
+ # The second part are key value pairs containing host, port and auth as
+ # described in RFC7628.
+ #
+ # https://datatracker.ietf.org/doc/html/rfc5801
+ # https://datatracker.ietf.org/doc/html/rfc7628
+ my $username = shift;
+ my $token = shift;
+ return "n,a=$username,\001port=$smtp_server_port\001auth=Bearer $token\001\001";
+}
+
+sub generate_xoauth2_string {
+ # "user=" {User} "^Aauth=Bearer " {Access Token} "^A^A"
+ # https://developers.google.com/gmail/imap/xoauth2-protocol#initial_client_response
+ my $username = shift;
+ my $token = shift;
+ return "user=$username\001auth=Bearer $token\001\001";
+}
+
+sub smtp_bearer_auth {
+ my $username = shift;
+ my $token = shift;
+ my $auth_string;
+ if ($smtp_encryption ne "tls") {
+ # As described in RFC7628 TLS is required and will be will
+ # be enforced at this point.
+ #
+ # https://datatracker.ietf.org/doc/html/rfc7628#section-3
+ die __("For $smtp_auth TLS is required.")
+ }
+ if ($smtp_auth eq "OAUTHBEARER") {
+ $auth_string = generate_oauthbearer_string($username, $token);
+ } elsif ($smtp_auth eq "XOAUTH2") {
+ $auth_string = generate_xoauth2_string($username, $token);
+ }
+ my $encoded_auth_string = MIME::Base64::encode($auth_string, "");
+ $smtp->command("AUTH $smtp_auth $encoded_auth_string\r\n");
+ use Net::Cmd qw(CMD_OK);
+ if ($smtp->response() == CMD_OK){
+ return 1;
+ } else {
+ # Send dummy request on authentication failure according to rfc7628.
+ # https://datatracker.ietf.org/doc/html/rfc7628#section-3.2.3
+ $smtp->command(MIME::Base64::encode("\001"));
+ $smtp->response();
+ return 0;
+ }
+}
+
# Returns 1 if authentication succeeded or was not necessary
# (smtp_user was not specified), and 0 otherwise.
@@ -1392,8 +1449,12 @@ sub smtp_auth_maybe {
'password' => $smtp_authpass
}, sub {
my $cred = shift;
-
- if ($smtp_auth) {
+ if ($smtp_auth eq "OAUTHBEARER" or $smtp_auth eq "XOAUTH2") {
+ # Since Authen:SASL does not support XOAUTH2 nor OAUTHBEARER we will
+ # manuall authenticate for tese types. The password field should
+ # contain the auth token at this point.
+ return smtp_bearer_auth($cred->{'username'}, $cred->{'password'});
+ } elsif ($smtp_auth) {
my $sasl = Authen::SASL->new(
mechanism => $smtp_auth,
callback => {
--
2.43.2
^ permalink raw reply related
* [PATCH 0/3] t-ctype: simplify unit test definitions
From: René Scharfe @ 2024-02-25 11:27 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
Simplify the ctype unit tests to allow combining specification strings
in any order and no longer require repeating class names. Patch 3 gets
rid of aggregation by class and implements the tests in arguably the
most basic and straight-forward way for the unit test framework, at the
cost of producing raw and lengthy output.
t-ctype: allow NUL anywhere in the specification string
t-ctype: avoid duplicating class names
t-ctype: do one test per class and char
t/unit-tests/t-ctype.c | 73 ++++++++++++------------------------------
1 file changed, 20 insertions(+), 53 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH 1/3] t-ctype: allow NUL anywhere in the specification string
From: René Scharfe @ 2024-02-25 11:27 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
In-Reply-To: <20240225112722.89221-1-l.s.r@web.de>
Replace the custom function is_in() for looking up a character in the
specification string with memchr(3) and sizeof. This is shorter,
simpler and allows NUL anywhere in the string, which may come in handy
if we ever want to support more character classes that contain it.
Getting the string size using sizeof only works in a macro and with a
string constant, but that's exactly what we have and I don't see it
changing anytime soon.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
t/unit-tests/t-ctype.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index f315489984..64d7186258 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -1,23 +1,11 @@
#include "test-lib.h"
-static int is_in(const char *s, int ch)
-{
- /*
- * We can't find NUL using strchr. Accept it as the first
- * character in the spec -- there are no empty classes.
- */
- if (ch == '\0')
- return ch == *s;
- if (*s == '\0')
- s++;
- return !!strchr(s, ch);
-}
-
/* Macro to test a character type */
#define TEST_CTYPE_FUNC(func, string) \
static void test_ctype_##func(void) { \
for (int i = 0; i < 256; i++) { \
- if (!check_int(func(i), ==, is_in(string, i))) \
+ int expect = !!memchr(string, i, sizeof(string) - 1); \
+ if (!check_int(func(i), ==, expect)) \
test_msg(" i: 0x%02x", i); \
} \
if (!check(!func(EOF))) \
--
2.44.0
^ permalink raw reply related
* [PATCH 2/3] t-ctype: avoid duplicating class names
From: René Scharfe @ 2024-02-25 11:27 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
In-Reply-To: <20240225112722.89221-1-l.s.r@web.de>
TEST_CTYPE_FUNC defines a function for testing a character classifier,
TEST_CHAR_CLASS calls it, causing the class name to be mentioned twice.
Avoid the need to define a class-specific function by letting
TEST_CHAR_CLASS do all the work. This is done by using the internal
functions test__run_begin() and test__run_end(), but they do exist to be
used in test macros after all.
Alternatively we could unroll the loop to provide a very long expression
that tests all 256 characters and EOF and hand that to TEST, but that
seems awkward and hard to read.
No change of behavior or output intended.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
t/unit-tests/t-ctype.c | 68 ++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 42 deletions(-)
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index 64d7186258..56dfefb68e 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -1,18 +1,18 @@
#include "test-lib.h"
-/* Macro to test a character type */
-#define TEST_CTYPE_FUNC(func, string) \
-static void test_ctype_##func(void) { \
- for (int i = 0; i < 256; i++) { \
- int expect = !!memchr(string, i, sizeof(string) - 1); \
- if (!check_int(func(i), ==, expect)) \
- test_msg(" i: 0x%02x", i); \
- } \
- if (!check(!func(EOF))) \
+#define TEST_CHAR_CLASS(class, string) do { \
+ int skip = test__run_begin(); \
+ if (!skip) { \
+ for (int i = 0; i < 256; i++) { \
+ int expect = !!memchr(string, i, sizeof(string) - 1); \
+ if (!check_int(class(i), ==, expect)) \
+ test_msg(" i: 0x%02x", i); \
+ } \
+ if (!check(!class(EOF))) \
test_msg(" i: 0x%02x (EOF)", EOF); \
-}
-
-#define TEST_CHAR_CLASS(class) TEST(test_ctype_##class(), #class " works")
+ } \
+ test__run_end(!skip, TEST_LOCATION(), #class " works"); \
+} while (0)
#define DIGIT "0123456789"
#define LOWER "abcdefghijklmnopqrstuvwxyz"
@@ -32,37 +32,21 @@ static void test_ctype_##func(void) { \
"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
"\x7f"
-TEST_CTYPE_FUNC(isdigit, DIGIT)
-TEST_CTYPE_FUNC(isspace, " \n\r\t")
-TEST_CTYPE_FUNC(isalpha, LOWER UPPER)
-TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT)
-TEST_CTYPE_FUNC(is_glob_special, "*?[\\")
-TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|")
-TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~")
-TEST_CTYPE_FUNC(isascii, ASCII)
-TEST_CTYPE_FUNC(islower, LOWER)
-TEST_CTYPE_FUNC(isupper, UPPER)
-TEST_CTYPE_FUNC(iscntrl, CNTRL)
-TEST_CTYPE_FUNC(ispunct, PUNCT)
-TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF")
-TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
-
int cmd_main(int argc, const char **argv) {
- /* Run all character type tests */
- TEST_CHAR_CLASS(isspace);
- TEST_CHAR_CLASS(isdigit);
- TEST_CHAR_CLASS(isalpha);
- TEST_CHAR_CLASS(isalnum);
- TEST_CHAR_CLASS(is_glob_special);
- TEST_CHAR_CLASS(is_regex_special);
- TEST_CHAR_CLASS(is_pathspec_magic);
- TEST_CHAR_CLASS(isascii);
- TEST_CHAR_CLASS(islower);
- TEST_CHAR_CLASS(isupper);
- TEST_CHAR_CLASS(iscntrl);
- TEST_CHAR_CLASS(ispunct);
- TEST_CHAR_CLASS(isxdigit);
- TEST_CHAR_CLASS(isprint);
+ TEST_CHAR_CLASS(isspace, " \n\r\t");
+ TEST_CHAR_CLASS(isdigit, DIGIT);
+ TEST_CHAR_CLASS(isalpha, LOWER UPPER);
+ TEST_CHAR_CLASS(isalnum, LOWER UPPER DIGIT);
+ TEST_CHAR_CLASS(is_glob_special, "*?[\\");
+ TEST_CHAR_CLASS(is_regex_special, "$()*+.?[\\^{|");
+ TEST_CHAR_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~");
+ TEST_CHAR_CLASS(isascii, ASCII);
+ TEST_CHAR_CLASS(islower, LOWER);
+ TEST_CHAR_CLASS(isupper, UPPER);
+ TEST_CHAR_CLASS(iscntrl, CNTRL);
+ TEST_CHAR_CLASS(ispunct, PUNCT);
+ TEST_CHAR_CLASS(isxdigit, DIGIT "abcdefABCDEF");
+ TEST_CHAR_CLASS(isprint, LOWER UPPER DIGIT PUNCT " ");
return test_done();
}
--
2.44.0
^ permalink raw reply related
* [PATCH 3/3] t-ctype: do one test per class and char
From: René Scharfe @ 2024-02-25 11:27 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
In-Reply-To: <20240225112722.89221-1-l.s.r@web.de>
Simplify TEST_CHAR_CLASS by using TEST for each character separately.
This increases the number of tests to 3598, but avoids the need for
using internal functions and test_msg() for custom messages. The
resulting macro has minimal test setup overhead.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
t/unit-tests/t-ctype.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
index 56dfefb68e..aa728175a6 100644
--- a/t/unit-tests/t-ctype.c
+++ b/t/unit-tests/t-ctype.c
@@ -1,17 +1,12 @@
#include "test-lib.h"
#define TEST_CHAR_CLASS(class, string) do { \
- int skip = test__run_begin(); \
- if (!skip) { \
- for (int i = 0; i < 256; i++) { \
- int expect = !!memchr(string, i, sizeof(string) - 1); \
- if (!check_int(class(i), ==, expect)) \
- test_msg(" i: 0x%02x", i); \
- } \
- if (!check(!class(EOF))) \
- test_msg(" i: 0x%02x (EOF)", EOF); \
+ for (int i = 0; i < 256; i++) { \
+ int expect = !!memchr(string, i, sizeof(string) - 1); \
+ TEST(check_int(class(i), ==, expect), \
+ #class "(0x%02x) works", i); \
} \
- test__run_end(!skip, TEST_LOCATION(), #class " works"); \
+ TEST(check(!class(EOF)), #class "(EOF) works"); \
} while (0)
#define DIGIT "0123456789"
--
2.44.0
^ permalink raw reply related
* [PATCH 0/2] name-rev: use memory pool for name strings
From: René Scharfe @ 2024-02-25 11:39 UTC (permalink / raw)
To: git
Avoid awkward string buffer pre-sizing by adding and using an efficient
mem-pool-backed string format function.
mem-pool: add mem_pool_strfmt()
name-rev: use mem_pool_strfmt()
builtin/name-rev.c | 39 ++++++++++++++++++++-------------------
mem-pool.c | 39 +++++++++++++++++++++++++++++++++++++++
mem-pool.h | 5 +++++
3 files changed, 64 insertions(+), 19 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH 2/2] name-rev: use mem_pool_strfmt()
From: René Scharfe @ 2024-02-25 11:39 UTC (permalink / raw)
To: git
In-Reply-To: <20240225113947.89357-1-l.s.r@web.de>
1c56fc2084 (name-rev: pre-size buffer in get_parent_name(), 2020-02-04)
got a big performance boost in an unusual repository by calculating the
name length in advance. This is a bit awkward, as it references the
name components twice.
Use a memory pool to store the strings for the struct rev_name member
tip_name. Using mem_pool_strfmt() allows efficient allocation without
explicit size calculation. This simplifies the formatting part of the
code without giving up performance:
Benchmark 1: ./git_2.44.0 -C ../chromium/src name-rev --all
Time (mean ± σ): 1.231 s ± 0.013 s [User: 1.082 s, System: 0.136 s]
Range (min … max): 1.214 s … 1.252 s 10 runs
Benchmark 2: ./git -C ../chromium/src name-rev --all
Time (mean ± σ): 1.220 s ± 0.020 s [User: 1.083 s, System: 0.130 s]
Range (min … max): 1.197 s … 1.254 s 10 runs
Don't bother discarding the memory pool just before exiting. The effort
for that would be very low, but actually measurable in the above
example, with no benefit to users. At least UNLEAK it to calm down leak
checkers. This addresses the leaks that 45a14f578e (Revert "name-rev:
release unused name strings", 2022-04-22) brought back.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
This doesn't make any test script leak-free, though.
builtin/name-rev.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 2dd1807c4e..ad9930c831 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -15,6 +15,7 @@
#include "commit-slab.h"
#include "commit-graph.h"
#include "wildmatch.h"
+#include "mem-pool.h"
/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -155,30 +156,25 @@ static struct rev_name *create_or_update_name(struct commit *commit,
return name;
}
-static char *get_parent_name(const struct rev_name *name, int parent_number)
+static char *get_parent_name(const struct rev_name *name, int parent_number,
+ struct mem_pool *string_pool)
{
- struct strbuf sb = STRBUF_INIT;
size_t len;
strip_suffix(name->tip_name, "^0", &len);
if (name->generation > 0) {
- strbuf_grow(&sb, len +
- 1 + decimal_width(name->generation) +
- 1 + decimal_width(parent_number));
- strbuf_addf(&sb, "%.*s~%d^%d", (int)len, name->tip_name,
- name->generation, parent_number);
+ return mem_pool_strfmt(string_pool, "%.*s~%d^%d",
+ (int)len, name->tip_name,
+ name->generation, parent_number);
} else {
- strbuf_grow(&sb, len +
- 1 + decimal_width(parent_number));
- strbuf_addf(&sb, "%.*s^%d", (int)len, name->tip_name,
- parent_number);
+ return mem_pool_strfmt(string_pool, "%.*s^%d",
+ (int)len, name->tip_name, parent_number);
}
- return strbuf_detach(&sb, NULL);
}
static void name_rev(struct commit *start_commit,
const char *tip_name, timestamp_t taggerdate,
- int from_tag, int deref)
+ int from_tag, int deref, struct mem_pool *string_pool)
{
struct prio_queue queue;
struct commit *commit;
@@ -195,9 +191,10 @@ static void name_rev(struct commit *start_commit,
if (!start_name)
return;
if (deref)
- start_name->tip_name = xstrfmt("%s^0", tip_name);
+ start_name->tip_name = mem_pool_strfmt(string_pool, "%s^0",
+ tip_name);
else
- start_name->tip_name = xstrdup(tip_name);
+ start_name->tip_name = mem_pool_strdup(string_pool, tip_name);
memset(&queue, 0, sizeof(queue)); /* Use the prio_queue as LIFO */
prio_queue_put(&queue, start_commit);
@@ -235,7 +232,8 @@ static void name_rev(struct commit *start_commit,
if (parent_number > 1)
parent_name->tip_name =
get_parent_name(name,
- parent_number);
+ parent_number,
+ string_pool);
else
parent_name->tip_name = name->tip_name;
ALLOC_GROW(parents_to_queue,
@@ -415,7 +413,7 @@ static int name_ref(const char *path, const struct object_id *oid,
return 0;
}
-static void name_tips(void)
+static void name_tips(struct mem_pool *string_pool)
{
int i;
@@ -428,7 +426,7 @@ static void name_tips(void)
struct tip_table_entry *e = &tip_table.table[i];
if (e->commit) {
name_rev(e->commit, e->refname, e->taggerdate,
- e->from_tag, e->deref);
+ e->from_tag, e->deref, string_pool);
}
}
}
@@ -561,6 +559,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)
int cmd_name_rev(int argc, const char **argv, const char *prefix)
{
+ struct mem_pool string_pool;
struct object_array revs = OBJECT_ARRAY_INIT;
int all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
@@ -587,6 +586,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+ mem_pool_init(&string_pool, 0);
init_commit_rev_name(&rev_names);
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
@@ -648,7 +648,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
adjust_cutoff_timestamp_for_slop();
for_each_ref(name_ref, &data);
- name_tips();
+ name_tips(&string_pool);
if (annotate_stdin) {
struct strbuf sb = STRBUF_INIT;
@@ -676,6 +676,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
always, allow_undefined, data.name_only);
}
+ UNLEAK(string_pool);
UNLEAK(revs);
return 0;
}
--
2.44.0
^ permalink raw reply related
* [PATCH 1/2] mem-pool: add mem_pool_strfmt()
From: René Scharfe @ 2024-02-25 11:39 UTC (permalink / raw)
To: git
In-Reply-To: <20240225113947.89357-1-l.s.r@web.de>
Add a function for building a string, printf style, using a memory pool.
It uses the free space in the current block in the first attempt. If
that suffices then the result can already be used without copying or
reformatting.
For strings that are significantly shorter on average than the block
size (ca. 1 MiB by default) this is the case most of the time, leading
to a better perfomance than a solution that doesn't access mem-pool
internals.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
mem-pool.c | 39 +++++++++++++++++++++++++++++++++++++++
mem-pool.h | 5 +++++
2 files changed, 44 insertions(+)
diff --git a/mem-pool.c b/mem-pool.c
index c7d6256020..2078c22b09 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -107,6 +107,45 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len)
return r;
}
+static char *mem_pool_strvfmt(struct mem_pool *pool, const char *fmt,
+ va_list ap)
+{
+ struct mp_block *block = pool->mp_block;
+ char *next_free = block ? block->next_free : NULL;
+ size_t available = block ? block->end - block->next_free : 0;
+ va_list cp;
+ int len, len2;
+ char *ret;
+
+ va_copy(cp, ap);
+ len = vsnprintf(next_free, available, fmt, cp);
+ va_end(cp);
+ if (len < 0)
+ BUG("your vsnprintf is broken (returned %d)", len);
+
+ ret = mem_pool_alloc(pool, len + 1); /* 1 for NUL */
+
+ /* Shortcut; relies on mem_pool_alloc() not touching buffer contents. */
+ if (ret == next_free)
+ return ret;
+
+ len2 = vsnprintf(ret, len + 1, fmt, ap);
+ if (len2 != len)
+ BUG("your vsnprintf is broken (returns inconsistent lengths)");
+ return ret;
+}
+
+char *mem_pool_strfmt(struct mem_pool *pool, const char *fmt, ...)
+{
+ va_list ap;
+ char *ret;
+
+ va_start(ap, fmt);
+ ret = mem_pool_strvfmt(pool, fmt, ap);
+ va_end(ap);
+ return ret;
+}
+
void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size)
{
size_t len = st_mult(count, size);
diff --git a/mem-pool.h b/mem-pool.h
index fe7507f022..d1c66413ec 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -47,6 +47,11 @@ void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);
char *mem_pool_strdup(struct mem_pool *pool, const char *str);
char *mem_pool_strndup(struct mem_pool *pool, const char *str, size_t len);
+/*
+ * Allocate memory from the memory pool and format a string into it.
+ */
+char *mem_pool_strfmt(struct mem_pool *pool, const char *fmt, ...);
+
/*
* Move the memory associated with the 'src' pool to the 'dst' pool. The 'src'
* pool will be empty and not contain any memory. It still needs to be free'd
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2 06/16] fsmonitor: refactor refresh callback for non-directory events
From: Torsten Bögershausen @ 2024-02-25 12:30 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <7ee6ca1aefd34a37d749300e317df10d80ef2b29.1708658300.git.gitgitgadget@gmail.com>
On Fri, Feb 23, 2024 at 03:18:10AM +0000, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Move the code handle unqualified FSEvents (without a trailing slash)
> into a helper function.
>
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> fsmonitor.c | 67 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 29cce32d81c..364198d258f 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -183,6 +183,43 @@ static int query_fsmonitor_hook(struct repository *r,
> return result;
> }
>
> +static void handle_path_without_trailing_slash(
> + struct index_state *istate, const char *name, int pos)
> +{
> + int i;
> +
> + if (pos >= 0) {
> + /*
> + * We have an exact match for this path and can just
> + * invalidate it.
> + */
> + istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
> + } else {
> + /*
> + * The path is not a tracked file -or- it is a
> + * directory event on a platform that cannot
> + * distinguish between file and directory events in
> + * the event handler, such as Windows.
> + *
> + * Scan as if it is a directory and invalidate the
> + * cone under it. (But remember to ignore items
> + * between "name" and "name/", such as "name-" and
> + * "name.".
> + */
> + int len = strlen(name);
should this be
size_t len = strlen(name);
> + pos = -pos - 1;
> +
> + for (i = pos; i < istate->cache_nr; i++) {
> + if (!starts_with(istate->cache[i]->name, name))
> + break;
> + if ((unsigned char)istate->cache[i]->name[len] > '/')
> + break;
Hm, this covers all digits, letters, :;<=>?
but not e.g. !+-. (and others). What do i miss ?
> + if (istate->cache[i]->name[len] == '/')
> + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> + }
> + }
> +}
> +
> /*
> * The daemon can decorate directory events, such as a move or rename,
> * by adding a trailing slash to the observed name. Use this to
> @@ -225,7 +262,7 @@ static void handle_path_with_trailing_slash(
>
> static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> {
> - int i, len = strlen(name);
> + int len = strlen(name);
Same here: size_t len = strlen() ?
> int pos = index_name_pos(istate, name, len);
>
> trace_printf_key(&trace_fsmonitor,
> @@ -240,34 +277,8 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> * for the untracked cache.
> */
> name[len - 1] = '\0';
> - } else if (pos >= 0) {
> - /*
> - * We have an exact match for this path and can just
> - * invalidate it.
> - */
> - istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
> } else {
> - /*
> - * The path is not a tracked file -or- it is a
> - * directory event on a platform that cannot
> - * distinguish between file and directory events in
> - * the event handler, such as Windows.
> - *
> - * Scan as if it is a directory and invalidate the
> - * cone under it. (But remember to ignore items
> - * between "name" and "name/", such as "name-" and
> - * "name.".
> - */
> - pos = -pos - 1;
> -
> - for (i = pos; i < istate->cache_nr; i++) {
> - if (!starts_with(istate->cache[i]->name, name))
> - break;
> - if ((unsigned char)istate->cache[i]->name[len] > '/')
> - break;
> - if (istate->cache[i]->name[len] == '/')
> - istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> - }
> + handle_path_without_trailing_slash(istate, name, pos);
> }
>
> /*
> --
> gitgitgadget
>
>
^ permalink raw reply
* Re: [PATCH v2 07/16] dir: create untracked_cache_invalidate_trimmed_path()
From: Torsten Bögershausen @ 2024-02-25 12:35 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <99c0d3e0742c1a7e0f7608707402a772ec112716.1708658300.git.gitgitgadget@gmail.com>
On Fri, Feb 23, 2024 at 03:18:11AM +0000, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Create a wrapper function for untracked_cache_invalidate_path()
> that silently trims a trailing slash, if present, before calling
> the wrapped function.
>
> The untracked cache expects to be called with a pathname that
> does not contain a trailing slash. This can make it inconvenient
> for callers that have a directory path. Lets hide this complexity.
>
> This will be used by a later commit in the FSMonitor code which
> may receive directory pathnames from an FSEvent.
>
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> dir.c | 20 ++++++++++++++++++++
> dir.h | 7 +++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/dir.c b/dir.c
> index ac699542302..1157f3e43fa 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -3918,6 +3918,26 @@ void untracked_cache_invalidate_path(struct index_state *istate,
> path, strlen(path));
> }
>
> +void untracked_cache_invalidate_trimmed_path(struct index_state *istate,
> + const char *path,
> + int safe_path)
> +{
> + size_t len = strlen(path);
> +
> + if (!len)
> + return; /* should not happen */
Should a BUG() be used ? Or bug(), for the record:
Please see Documentation/technical/api-error-handling.txt
> +
> + if (path[len - 1] != '/') {
> + untracked_cache_invalidate_path(istate, path, safe_path);
> + } else {
> + struct strbuf tmp = STRBUF_INIT;
> +
> + strbuf_add(&tmp, path, len - 1);
> + untracked_cache_invalidate_path(istate, tmp.buf, safe_path);
> + strbuf_release(&tmp);
> + }
> +}
> +
> void untracked_cache_remove_from_index(struct index_state *istate,
> const char *path)
> {
> diff --git a/dir.h b/dir.h
> index 98aa85fcc0e..45a7b9ec5f2 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -576,6 +576,13 @@ int cmp_dir_entry(const void *p1, const void *p2);
> int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);
>
> void untracked_cache_invalidate_path(struct index_state *, const char *, int safe_path);
> +/*
> + * Invalidate the untracked-cache for this path, but first strip
> + * off a trailing slash, if present.
> + */
> +void untracked_cache_invalidate_trimmed_path(struct index_state *,
> + const char *path,
> + int safe_path);
> void untracked_cache_remove_from_index(struct index_state *, const char *);
> void untracked_cache_add_to_index(struct index_state *, const char *);
>
> --
> gitgitgadget
>
>
^ permalink raw reply
* Re: [PATCH v2 14/16] fsmonitor: support case-insensitive events
From: Torsten Bögershausen @ 2024-02-25 13:10 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <288f3f4e54e98a68d72e97125b1520605c138c3c.1708658300.git.gitgitgadget@gmail.com>
On Fri, Feb 23, 2024 at 03:18:18AM +0000, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Teach fsmonitor_refresh_callback() to handle case-insensitive
> lookups if case-sensitive lookups fail on case-insensitive systems.
> This can cause 'git status' to report stale status for files if there
> are case issues/errors in the worktree.
>
> The FSMonitor daemon sends FSEvents using the observed spelling
> of each pathname. On case-insensitive file systems this may be
> different than the expected case spelling.
>
> The existing code uses index_name_pos() to find the cache-entry for
> the pathname in the FSEvent and clear the CE_FSMONITOR_VALID bit so
> that the worktree scan/index refresh will revisit and revalidate the
> path.
>
> On a case-insensitive file system, the exact match lookup may fail
> to find the associated cache-entry. This causes status to think that
> the cached CE flags are correct and skip over the file.
>
> Update event handling to optionally use the name-hash and dir-name-hash
> if necessary.
>
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> fsmonitor.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 110 insertions(+)
>
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 739ddbf7aca..ac638a61c00 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -5,6 +5,7 @@
> #include "ewah/ewok.h"
> #include "fsmonitor.h"
> #include "fsmonitor-ipc.h"
> +#include "name-hash.h"
> #include "run-command.h"
> #include "strbuf.h"
> #include "trace2.h"
> @@ -186,6 +187,102 @@ static int query_fsmonitor_hook(struct repository *r,
> static size_t handle_path_with_trailing_slash(
> struct index_state *istate, const char *name, int pos);
>
> +/*
> + * Use the name-hash to do a case-insensitive cache-entry lookup with
> + * the pathname and invalidate the cache-entry.
> + *
> + * Returns the number of cache-entries that we invalidated.
> + */
> +static size_t handle_using_name_hash_icase(
> + struct index_state *istate, const char *name)
> +{
> + struct cache_entry *ce = NULL;
> +
> + ce = index_file_exists(istate, name, strlen(name), 1);
> + if (!ce)
> + return 0;
> +
> + /*
> + * A case-insensitive search in the name-hash using the
> + * observed pathname found a cache-entry, so the observed path
> + * is case-incorrect. Invalidate the cache-entry and use the
> + * correct spelling from the cache-entry to invalidate the
> + * untracked-cache. Since we now have sparse-directories in
> + * the index, the observed pathname may represent a regular
> + * file or a sparse-index directory.
> + *
> + * Note that we should not have seen FSEvents for a
> + * sparse-index directory, but we handle it just in case.
> + *
> + * Either way, we know that there are not any cache-entries for
> + * children inside the cone of the directory, so we don't need to
> + * do the usual scan.
> + */
> + trace_printf_key(&trace_fsmonitor,
> + "fsmonitor_refresh_callback MAP: '%s' '%s'",
> + name, ce->name);
> +
> + untracked_cache_invalidate_trimmed_path(istate, ce->name, 0);
> +
> + ce->ce_flags &= ~CE_FSMONITOR_VALID;
> + return 1;
> +}
> +
> +/*
> + * Use the dir-name-hash to find the correct-case spelling of the
> + * directory. Use the canonical spelling to invalidate all of the
> + * cache-entries within the matching cone.
> + *
> + * Returns the number of cache-entries that we invalidated.
> + */
> +static size_t handle_using_dir_name_hash_icase(
> + struct index_state *istate, const char *name)
> +{
> + struct strbuf canonical_path = STRBUF_INIT;
> + int pos;
> + size_t len = strlen(name);
> + size_t nr_in_cone;
> +
> + if (name[len - 1] == '/')
> + len--;
> +
> + if (!index_dir_find(istate, name, len, &canonical_path))
> + return 0; /* name is untracked */
> +
> + if (!memcmp(name, canonical_path.buf, canonical_path.len)) {
> + strbuf_release(&canonical_path);
> + /*
> + * NEEDSWORK: Our caller already tried an exact match
> + * and failed to find one. They called us to do an
> + * ICASE match, so we should never get an exact match,
> + * so we could promote this to a BUG() here if we
> + * wanted to. It doesn't hurt anything to just return
> + * 0 and go on becaus we should never get here. Or we
> + * could just get rid of the memcmp() and this "if"
> + * clause completely.
> + */
> + return 0; /* should not happen */
In very very theory, there may be a race-condition,
when a directory is renamed very fast, more than once.
I don't think, that the "it did not match exactly, but
now it matches" is a problem.
Question: Does it make sense to just remove this ?
And, may be, find out that the "corrected spelling (tm)"
of "DIR1" is not "dir1", neither "Dir1", but, exactly, "DIR1" ?
Would that be a problem ?
> + }
> +
> + trace_printf_key(&trace_fsmonitor,
> + "fsmonitor_refresh_callback MAP: '%s' '%s'",
> + name, canonical_path.buf);
> +
> + /*
> + * The dir-name-hash only tells us the corrected spelling of
> + * the prefix. We have to use this canonical path to do a
> + * lookup in the cache-entry array so that we repeat the
> + * original search using the case-corrected spelling.
> + */
> + strbuf_addch(&canonical_path, '/');
> + pos = index_name_pos(istate, canonical_path.buf,
> + canonical_path.len);
> + nr_in_cone = handle_path_with_trailing_slash(
> + istate, canonical_path.buf, pos);
> + strbuf_release(&canonical_path);
> + return nr_in_cone;
> +}
> +
> /*
> * The daemon sent an observed pathname without a trailing slash.
> * (This is the normal case.) We do not know if it is a tracked or
> @@ -319,6 +416,19 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> else
> nr_in_cone = handle_path_without_trailing_slash(istate, name, pos);
>
> + /*
> + * If we did not find an exact match for this pathname or any
> + * cache-entries with this directory prefix and we're on a
> + * case-insensitive file system, try again using the name-hash
> + * and dir-name-hash.
> + */
> + if (!nr_in_cone && ignore_case) {
> + nr_in_cone = handle_using_name_hash_icase(istate, name);
> + if (!nr_in_cone)
> + nr_in_cone = handle_using_dir_name_hash_icase(
> + istate, name);
> + }
> +
> if (nr_in_cone)
> trace_printf_key(&trace_fsmonitor,
> "fsmonitor_refresh_callback CNT: %d",
> --
> gitgitgadget
>
>
^ permalink raw reply
* Re: [PATCH v2 4/8] sequencer: treat error reading HEAD as unborn branch
From: phillip.wood123 @ 2024-02-25 16:57 UTC (permalink / raw)
To: Brian Lyles, phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <17b666ca6c4b7561.70b1dd9aae081c6e.203dcd72f6563036@zivdesk>
Hi Brian
On 23/02/2024 05:28, Brian Lyles wrote:
> On Thu, Feb 22, 2024 at 10:34 AM <phillip.wood123@gmail.com> wrote:
>>> +test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
>>> + git checkout main &&
>>
>> I'm a bit confused by this - are we already on the branch "unborn" and
>> so need to move away from it to delete it?
>
> Yes, the previous test leaves us on that branch. In v3, I will update
> this to instead just use `git checkout --detach`, as that does seem a
> little less confusing than switching to some other branch that is only
> relevant because it's not `unborn`. If there is a cleaner way to do
> this, I'd be happy to switch to it.
I think "git checkout --detach" is probably the best we can do. It would
be nice to be able to do "git switch -C --orphan unborn" but "-C" does
not work with "--orphan"
>>> + git branch -D unborn &&
>>> + git checkout --orphan unborn &&
>>> + git rm --cached -r . &&
>>> + rm -rf * &&
>>
>> "git switch --orphan" leaves us with an empty index and working copy
>> without having to remove the files ourselves.
>
> Thanks for pointing this out. Using git-switch(1) here instead of
> git-checkout(1) allows us to drop the `rm -rf *` call form both the
> existing 'cherry-pick on unborn branch' test as well as my new test. It
> appears that the `git rm --cached -r .` call is still necessary in the
> existing test.
It looks like the previous test 'revert forbidden on dirty working tree'
fails to clean up properly and so "git switch" is carrying the
uncommitted changes across to the new orphan branch. I think that "git
switch --discard-changes --orphan unborn" ought to clean the worktree
and index but it doesn't seem to work.
>>> + git cherry-pick initial --allow-empty &&
>>> + git diff --quiet initial &&
>>
>> I'd drop "--quiet" here as it makes debugging easier if we can see the
>> diff if the test fails.
>
> This makes sense. In v3, I will update this new test as well as the
> existing test to not use `--quiet`.
>
> Combining the above suggestions, here's the version of the existing and
> new tests that I intend to use in v3. Let me know if this isn't what you
> had in mind!
>
> test_expect_success 'cherry-pick on unborn branch' '
> git switch --orphan unborn &&
> git rm --cached -r . &&
> git cherry-pick initial &&
> git diff initial &&
> test_cmp_rev ! initial HEAD
> '
>
> test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
> git checkout --detach &&
> git branch -D unborn &&
> git switch --orphan unborn &&
> git cherry-pick initial --allow-empty &&
> git diff initial &&
> test_cmp_rev ! initial HEAD
> '
These look good
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v2 8/8] cherry-pick: add `--empty` for more robust redundant commit handling
From: phillip.wood123 @ 2024-02-25 16:57 UTC (permalink / raw)
To: Brian Lyles, phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <17b66baf4d8c4255.70b1dd9aae081c6e.203dcd72f6563036@zivdesk>
Hi Brian
On 23/02/2024 06:58, Brian Lyles wrote:
>
> On Thu, Feb 22, 2024 at 10:36 AM <phillip.wood123@gmail.com> wrote:
>
>> I'm leaning towards leaving `--keep-redundant-commits` alone. That
>> introduces an inconsistency between `--keep-redundant-commits` and
>> `--empty=keep` as the latter does not imply `--allow-empty` but it does
>> avoid breaking existing users. We could document
>> `--keep-redundant-commits` as predating `--empty` and behaving like
>> `--empty=keep --allow-empty`. The documentation and implementation of
>> the new option look good modulo the typo that has already been pointed
>> out and a couple of small comments below.
>
> I think I'm on board with leaving `--keep-redundant-commits` alone. I'm
> on the fence about having `--empty=keep` imply `--allow-empty` after
> seeing Junio's concerns. I laid out the options that I see in a reply to
> patch 6/8[1] and would appreciate input there. I'll adjust the details
> of this commit in v3 based on what we decide there.
I'll take a look at that in the next couple of days
> [1]: https://lore.kernel.org/git/17b666ca6c4b7561.70b1dd9aae081c6e.203dcd72f6563036@zivdesk/
>>
>>> +enum empty_action {
>>> + EMPTY_COMMIT_UNSPECIFIED = 0,
>>
>> We tend to use -1 for unspecified options
>
> Thanks, I'll update this in v3.
>
>>> +test_expect_success 'cherry-pick persists --empty=stop correctly' '
>>> + pristine_detach initial &&
>>> + # to make sure that the session to cherry-pick a sequence
>>> + # gets interrupted, use a high-enough number that is larger
>>> + # than the number of parents of any commit we have created
>>> + mainline=4 &&
>>> + test_expect_code 128 git cherry-pick -s -m $mainline --empty=stop initial..anotherpick &&
>>> + test_path_is_file .git/sequencer/opts &&
>>> + test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits &&
>>> + test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits
>>> +'
>>
>> Thanks for adding these tests to check that the --empty option persists.
>> Usually for tests like this we prefer to check the user visible behavior
>> rather than the implementation details (I suspect we have some older
>> tests that do the latter). To check the behavior we usually arrange for
>> a merge conflict but using -m is a creative alternative, then we'd run
>> "git cherry-pick --continue" and check that the commits that become
>> empty have been preserved or dropped or that the cherry-pick stops.
>
> Indeed, I was modelling these new tests after other existing tests in
> this file. While I agree with you in theory, I am hesitant to make these
> new tests drastically different from the existing tests that are testing
> the same mechanisms (and appear to be very intentionally testing that
> the options are persisted in that config file). I'm also hesitant to
> update the existing tests as part of this series (primarily due to a
> lack of familiarity, and partially to avoid scope creep of the series).
I certainly don't think it should be up to you to update the existing
tests. I'm not sure adding more tests in the same pattern is a good idea
though. Apart from the fact that they are testing an implementation
detail rather than the user facing behavior they don't actually check
that the option is respected by "git cherry-pick --continue", only that
we save it when stopping for a conflict resolution.
> How concerned are you about the current implementation? Does it make
> sense to you to defer this suggestion to a future series that cleans up
> the tests to do more user-oriented checks?
I think adding tests that follow a pattern we want to change is just
storing up work for the future and makes it less likely we'll improve
things because it will be more work to do so.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 7/8] cherry-pick: enforce `--keep-redundant-commits` incompatibility
From: phillip.wood123 @ 2024-02-25 16:58 UTC (permalink / raw)
To: Brian Lyles, phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <17b669c4bfe6602f.70b1dd9aae081c6e.203dcd72f6563036@zivdesk>
Hi Brian
On 23/02/2024 06:23, Brian Lyles wrote:
> On Thu, Feb 22, 2024 at 10:35 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
>> Well spotted, do we really need a new test file just for this though? I
>> wonder if the new test would be better off living in
>> t3505-cherry-pick-empty.sh or t3507-cherry-pick-conflict.sh
>
> I was modelling this off of 't3422-rebase-incompatible-options.sh'.
The rebase case is more complicated due to different options being
supported by the two different backends. Thankfully here we only have to
worry about options that are incompatible with "--continue/--abort" and
so adding "--continue rejects --foo" into the file that tests option
"--foo" keeps everything together.
> Additionally, I do feel like these tests are only tangentially related
> to the tests that actually exercise the features themselves. Notably,
> the setup requirements are drastically different (simpler) since the
> test should fail long before any setup actually matters. For that
> reason, I think a separate file where other future tests for
> incompatible options can also live does make sense.
>
> Is there any particular downside to the new file that I am unaware of?
The main downside is that it spreads the tests for a particular option
over several test files. There is also an overhead in setting up the
repository at the start of each test file.
Best Wishes
Phillip
^ permalink raw reply
* Re: [ANNOUNCE] Git v2.44.0
From: Junio C Hamano @ 2024-02-25 17:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Mike Hommey, git, git-packagers
In-Reply-To: <ZdrnvKRl8oGPJxOD@framework>
Patrick Steinhardt <ps@pks.im> writes:
> That's ultimately the reason why I don't want HEAD to look like a proper
> ref. But doing the "refs/heads/.invalid" workaround shouldn't be too bad,
> I guess.
Isn't the reason why reftable backend initializes refs/heads to be a
regular file exactly because we want to reject an attempt to create
such a file on the filesystem, though?
^ permalink raw reply
* Re: [PATCH v2 06/16] fsmonitor: refactor refresh callback for non-directory events
From: Junio C Hamano @ 2024-02-25 17:24 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Jeff Hostetler via GitGitGadget, git, Patrick Steinhardt,
Jeff Hostetler, Jeff Hostetler
In-Reply-To: <20240225123050.GA1952@tb-raspi4>
Torsten Bögershausen <tboegi@web.de> writes:
>> + pos = -pos - 1;
>> +
>> + for (i = pos; i < istate->cache_nr; i++) {
>> + if (!starts_with(istate->cache[i]->name, name))
>> + break;
>> + if ((unsigned char)istate->cache[i]->name[len] > '/')
>> + break;
>
> Hm, this covers all digits, letters, :;<=>?
> but not e.g. !+-. (and others). What do i miss ?
This is scanning an in-core array of cache entries, which is sorted
by name in lexicographic order, and the loop knows that files under
the directory "foo", whose pathnames all share prefix "foo/", would
sort between "foo.h" and "foo00", because "." sorts before "/" and
"0" sorts after "/".
It is trying to find where in the array a hypothetical directory
would appear, if any of the files in it existed in the array, and
exiting early, taking advantage of the fact that after seeing
something that sorts after a '/', it will never see an entry that
shares cache[i]->name[] as a prefix.
It is not a new code in the patch, of course; merely got moved from
elsewhere below.
>> - /*
>> - * The path is not a tracked file -or- it is a
>> - * directory event on a platform that cannot
>> - * distinguish between file and directory events in
>> - * the event handler, such as Windows.
>> - *
>> - * Scan as if it is a directory and invalidate the
>> - * cone under it. (But remember to ignore items
>> - * between "name" and "name/", such as "name-" and
>> - * "name.".
>> - */
>> - pos = -pos - 1;
>> -
>> - for (i = pos; i < istate->cache_nr; i++) {
>> - if (!starts_with(istate->cache[i]->name, name))
>> - break;
>> - if ((unsigned char)istate->cache[i]->name[len] > '/')
>> - break;
>> - if (istate->cache[i]->name[len] == '/')
>> - istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
>> - }
>> + handle_path_without_trailing_slash(istate, name, pos);
>> }
>>
>> /*
>> --
>> gitgitgadget
>>
>>
^ permalink raw reply
* [PATCH] userdiff: better method/property matching for C#
From: Steven Jeuris via GitGitGadget @ 2024-02-25 17:33 UTC (permalink / raw)
To: git; +Cc: Steven Jeuris, Steven Jeuris
From: Steven Jeuris <steven.jeuris@3shape.com>
- Support multi-line methods by not requiring closing parenthesis.
- Support multiple generics (comma was missing before).
- Add missing `foreach`, `from`, `lock` and `fixed` keywords to skip over.
- Remove `instanceof` keyword, which isn't C#.
- Also detect non-method keywords not positioned at the start of a line.
- Added tests; none existed before.
The overall strategy is to focus more on what isn't expected for
method/property definitions, instead of what is, but is fully optional.
Signed-off-by: Steven Jeuris <steven.jeuris@gmail.com>
---
userdiff: better method/property matching for C#
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1682%2FWhathecode%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1682/Whathecode/master-v1
Pull-Request: https://github.com/git/git/pull/1682
t/t4018/csharp-method | 10 +++
t/t4018/csharp-method-explicit | 12 +++
t/t4018/csharp-method-generics | 11 +++
t/t4018/csharp-method-modifiers | 13 ++++
t/t4018/csharp-method-multiline | 10 +++
t/t4018/csharp-method-params | 10 +++
t/t4018/csharp-method-skip-body | 115 ++++++++++++++++++++++++++++
t/t4018/csharp-method-special-chars | 11 +++
t/t4018/csharp-method-with-spacing | 10 +++
t/t4018/csharp-property | 11 +++
userdiff.c | 16 ++--
11 files changed, 224 insertions(+), 5 deletions(-)
create mode 100644 t/t4018/csharp-method
create mode 100644 t/t4018/csharp-method-explicit
create mode 100644 t/t4018/csharp-method-generics
create mode 100644 t/t4018/csharp-method-modifiers
create mode 100644 t/t4018/csharp-method-multiline
create mode 100644 t/t4018/csharp-method-params
create mode 100644 t/t4018/csharp-method-skip-body
create mode 100644 t/t4018/csharp-method-special-chars
create mode 100644 t/t4018/csharp-method-with-spacing
create mode 100644 t/t4018/csharp-property
diff --git a/t/t4018/csharp-method b/t/t4018/csharp-method
new file mode 100644
index 00000000000..85ff0cb8b5b
--- /dev/null
+++ b/t/t4018/csharp-method
@@ -0,0 +1,10 @@
+class Example
+{
+ string Method(int RIGHT)
+ {
+ // Filler
+ // Filler
+
+ return "ChangeMe";
+ }
+}
diff --git a/t/t4018/csharp-method-explicit b/t/t4018/csharp-method-explicit
new file mode 100644
index 00000000000..083aa094ce2
--- /dev/null
+++ b/t/t4018/csharp-method-explicit
@@ -0,0 +1,12 @@
+using System;
+
+class Example : IDisposable
+{
+ void IDisposable.Dispose() // RIGHT
+ {
+ // Filler
+ // Filler
+
+ // ChangeMe
+ }
+}
diff --git a/t/t4018/csharp-method-generics b/t/t4018/csharp-method-generics
new file mode 100644
index 00000000000..c472d4a18df
--- /dev/null
+++ b/t/t4018/csharp-method-generics
@@ -0,0 +1,11 @@
+class Example<T1, T2>
+{
+ Example<int, string> Method<TA, TB>(TA RIGHT, TB b)
+ {
+ // Filler
+ // Filler
+
+ // ChangeMe
+ return null;
+ }
+}
diff --git a/t/t4018/csharp-method-modifiers b/t/t4018/csharp-method-modifiers
new file mode 100644
index 00000000000..f1c008a4749
--- /dev/null
+++ b/t/t4018/csharp-method-modifiers
@@ -0,0 +1,13 @@
+using System.Threading.Tasks;
+
+class Example
+{
+ static internal async Task Method(int RIGHT)
+ {
+ // Filler
+ // Filler
+
+ // ChangeMe
+ await Task.Delay(1);
+ }
+}
diff --git a/t/t4018/csharp-method-multiline b/t/t4018/csharp-method-multiline
new file mode 100644
index 00000000000..0a20b0cb49c
--- /dev/null
+++ b/t/t4018/csharp-method-multiline
@@ -0,0 +1,10 @@
+class Example
+{
+ string Method_RIGHT(
+ int a,
+ int b,
+ int c)
+ {
+ return "ChangeMe";
+ }
+}
diff --git a/t/t4018/csharp-method-params b/t/t4018/csharp-method-params
new file mode 100644
index 00000000000..18598449008
--- /dev/null
+++ b/t/t4018/csharp-method-params
@@ -0,0 +1,10 @@
+class Example
+{
+ string Method(int RIGHT, int b, int c = 42)
+ {
+ // Filler
+ // Filler
+
+ return "ChangeMe";
+ }
+}
diff --git a/t/t4018/csharp-method-skip-body b/t/t4018/csharp-method-skip-body
new file mode 100644
index 00000000000..95e93ea995c
--- /dev/null
+++ b/t/t4018/csharp-method-skip-body
@@ -0,0 +1,115 @@
+using System.Linq;
+using System;
+
+class Example : IDisposable
+{
+ string Method(int RIGHT)
+ {
+ // Method calls
+ MethodCall();
+ MethodCall(1, 2);
+ MethodCall(
+ 1, 2);
+
+ // Assignments
+ var constantAssignment = "test";
+ var methodAssignment = MethodCall();
+ var multiLineMethodAssignment = MethodCall(
+ );
+
+ // Initializations/disposal
+ new Example();
+ new Example(
+ );
+ new Example { };
+ using (this)
+ {
+ }
+ var def =
+ this is default(
+ Example);
+
+ // Iteration statements
+ do { } while (true);
+ do MethodCall(
+ ); while (true);
+ while (true);
+ while (true) {
+ break;
+ }
+ for (int i = 0; i < 10; ++i)
+ {
+ }
+ foreach (int i in Enumerable.Range(0, 10))
+ {
+ }
+ int[] numbers = [5, 4, 1, 3, 9, 8, 6, 7, 2, 0];
+ var test =
+ from num in Numbers(
+ ) select num;
+
+ // Control
+ if (false)
+ {
+ return "out";
+ }
+ else { }
+ if (true) MethodCall(
+ );
+ else MethodCall(
+ );
+ switch ("test")
+ {
+ case "one":
+ return MethodCall(
+ );
+ case "two":
+ break;
+ }
+ (int, int) tuple = (1, 4);
+ switch (tuple)
+ {
+ case (1, 4):
+ MethodCall();
+ }
+
+ // Exceptions
+ try
+ {
+ throw new Exception("fail");
+ }
+ catch (Exception)
+ {
+ }
+ finally
+ {
+ }
+ try { } catch (Exception) {}
+ try
+ {
+ throw GetException(
+ );
+ }
+ catch (Exception) { }
+
+ // Others
+ lock (this)
+ {
+ }
+ unsafe
+ {
+ byte[] bytes = [1, 2, 3];
+ fixed (byte* pointerToFirst = bytes)
+ {
+ }
+ }
+
+ return "ChangeMe";
+ }
+
+ public void Dispose() {}
+
+ string MethodCall(int a = 0, int b = 0) => "test";
+ Exception GetException() => new Exception("fail");
+ int[] Numbers() => [0, 1];
+}
diff --git a/t/t4018/csharp-method-special-chars b/t/t4018/csharp-method-special-chars
new file mode 100644
index 00000000000..ec3565fd000
--- /dev/null
+++ b/t/t4018/csharp-method-special-chars
@@ -0,0 +1,11 @@
+class @Some_Type
+{
+ @Some_Type @Method_With_Underscore(int RIGHT)
+ {
+ // Filler
+ // Filler
+
+ // ChangeMe
+ return new @Some_Type();
+ }
+}
diff --git a/t/t4018/csharp-method-with-spacing b/t/t4018/csharp-method-with-spacing
new file mode 100644
index 00000000000..4143929a711
--- /dev/null
+++ b/t/t4018/csharp-method-with-spacing
@@ -0,0 +1,10 @@
+class Example
+{
+ string Method ( int RIGHT )
+ {
+ // Filler
+ // Filler
+
+ return "ChangeMe";
+ }
+}
diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property
new file mode 100644
index 00000000000..1792117f964
--- /dev/null
+++ b/t/t4018/csharp-property
@@ -0,0 +1,11 @@
+class Example
+{
+ public bool RIGHT
+ {
+ get { return true; }
+ set
+ {
+ // ChangeMe
+ }
+ }
+}
diff --git a/userdiff.c b/userdiff.c
index e399543823b..a6a26a97e4c 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -89,12 +89,18 @@ PATTERNS("cpp",
"|\\.[0-9][0-9]*([Ee][-+]?[0-9]+)?[fFlL]?"
"|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->\\*?|\\.\\*|<=>"),
PATTERNS("csharp",
- /* Keywords */
- "!^[ \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n"
- /* Methods and constructors */
- "^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe|async)[ \t]+)*[][<>@.~_[:alnum:]]+[ \t]+[<>@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n"
+ /*
+ * Jump over keywords not used by methods which can be followed by parentheses without special characters in between,
+ * making them look like methods.
+ */
+ "!(^|[ \t]+)(do|while|for|foreach|from|if|else|new|default|return|switch|case|throw|catch|using|lock|fixed)([ \t(]+|$)\n"
+ /* Methods/constructors:
+ * the strategy is to identify a minimum of two groups (any combination of keywords/type/name),
+ * without intermediate or final characters which can't be part of method definitions before the opening parenthesis.
+ */
+ "^[ \t]*(([][[:alnum:]@_<>.,]*[^=:{ \t][ \t]+[][[:alnum:]@_<>.,]*)+\\([^;]*)$\n"
/* Properties */
- "^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][<>@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n"
+ "^[ \t]*((([][[:alnum:]@_<>.,]+)[ \t]+[][[:alnum:]@_]*)+[^=:;,()]*)$\n"
/* Type definitions */
"^[ \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \t]+)*(class|enum|interface|struct|record)[ \t]+.*)$\n"
/* Namespace */
base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 1/3] t-ctype: allow NUL anywhere in the specification string
From: Eric Sunshine @ 2024-02-25 18:05 UTC (permalink / raw)
To: René Scharfe
Cc: git, Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
In-Reply-To: <20240225112722.89221-2-l.s.r@web.de>
On Sun, Feb 25, 2024 at 6:27 AM René Scharfe <l.s.r@web.de> wrote:
> Replace the custom function is_in() for looking up a character in the
> specification string with memchr(3) and sizeof. This is shorter,
> simpler and allows NUL anywhere in the string, which may come in handy
> if we ever want to support more character classes that contain it.
>
> Getting the string size using sizeof only works in a macro and with a
> string constant, but that's exactly what we have and I don't see it
> changing anytime soon.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
> @@ -1,23 +1,11 @@
> /* Macro to test a character type */
> #define TEST_CTYPE_FUNC(func, string) \
Taking into consideration the commit message warning about string
constants, would it make sense to update the comment to mention that
limitation?
/* Test a character type. (Only use with string constants.) */
#define TEST_CTYPE_FUNC(func, string) \
> static void test_ctype_##func(void) { \
> for (int i = 0; i < 256; i++) { \
> - if (!check_int(func(i), ==, is_in(string, i))) \
> + int expect = !!memchr(string, i, sizeof(string) - 1); \
> + if (!check_int(func(i), ==, expect)) \
> test_msg(" i: 0x%02x", i); \
> } \
> if (!check(!func(EOF))) \
^ permalink raw reply
* [PATCH] submodule: use strvec_pushf() for --submodule-prefix
From: René Scharfe @ 2024-02-25 18:24 UTC (permalink / raw)
To: Git List
Add the option --submodule-prefix and its argument directly using
strvec_pushf() instead of via a detour through a strbuf. This is
shorter, easier to read and doesn't require any explicit cleanup
afterwards.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
submodule.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/submodule.c b/submodule.c
index 213da79f66..40f13a3685 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1687,8 +1687,6 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
task = get_fetch_task_from_changed(spf, err);
if (task) {
- struct strbuf submodule_prefix = STRBUF_INIT;
-
child_process_init(cp);
cp->dir = task->repo->gitdir;
prepare_submodule_repo_env_in_gitdir(&cp->env);
@@ -1698,15 +1696,11 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
strvec_pushv(&cp->args, task->git_args.v);
strvec_pushv(&cp->args, spf->args.v);
strvec_push(&cp->args, task->default_argv);
- strvec_push(&cp->args, "--submodule-prefix");
+ strvec_pushf(&cp->args, "--submodule-prefix=%s%s/",
+ spf->prefix, task->sub->path);
- strbuf_addf(&submodule_prefix, "%s%s/",
- spf->prefix,
- task->sub->path);
- strvec_push(&cp->args, submodule_prefix.buf);
*task_cb = task;
- strbuf_release(&submodule_prefix);
string_list_insert(&spf->seen_submodule_names, task->sub->name);
return 1;
}
@@ -1714,12 +1708,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
if (spf->oid_fetch_tasks_nr) {
struct fetch_task *task =
spf->oid_fetch_tasks[spf->oid_fetch_tasks_nr - 1];
- struct strbuf submodule_prefix = STRBUF_INIT;
spf->oid_fetch_tasks_nr--;
- strbuf_addf(&submodule_prefix, "%s%s/",
- spf->prefix, task->sub->path);
-
child_process_init(cp);
prepare_submodule_repo_env_in_gitdir(&cp->env);
cp->git_cmd = 1;
@@ -1728,8 +1718,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
strvec_init(&cp->args);
strvec_pushv(&cp->args, spf->args.v);
strvec_push(&cp->args, "on-demand");
- strvec_push(&cp->args, "--submodule-prefix");
- strvec_push(&cp->args, submodule_prefix.buf);
+ strvec_pushf(&cp->args, "--submodule-prefix=%s%s/",
+ spf->prefix, task->sub->path);
/* NEEDSWORK: have get_default_remote from submodule--helper */
strvec_push(&cp->args, "origin");
@@ -1737,7 +1727,6 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
append_oid_to_argv, &cp->args);
*task_cb = task;
- strbuf_release(&submodule_prefix);
return 1;
}
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 1/3] t-ctype: allow NUL anywhere in the specification string
From: René Scharfe @ 2024-02-25 18:28 UTC (permalink / raw)
To: Eric Sunshine
Cc: git, Phillip Wood, Josh Steadmon, Achu Luma, Christian Couder
In-Reply-To: <CAPig+cTvs7Ckg=k1T+N3N=yT840xM2DYf3LV9NZhii8+c1nkbw@mail.gmail.com>
Am 25.02.24 um 19:05 schrieb Eric Sunshine:
> On Sun, Feb 25, 2024 at 6:27 AM René Scharfe <l.s.r@web.de> wrote:
>> Replace the custom function is_in() for looking up a character in the
>> specification string with memchr(3) and sizeof. This is shorter,
>> simpler and allows NUL anywhere in the string, which may come in handy
>> if we ever want to support more character classes that contain it.
>>
>> Getting the string size using sizeof only works in a macro and with a
>> string constant, but that's exactly what we have and I don't see it
>> changing anytime soon.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
>> @@ -1,23 +1,11 @@
>> /* Macro to test a character type */
>> #define TEST_CTYPE_FUNC(func, string) \
>
> Taking into consideration the commit message warning about string
> constants, would it make sense to update the comment to mention that
> limitation?
I think the temptation to pass a string pointer is low -- if only
because there aren't any in this file. But adding such a warning
can't hurt, so yeah, why not?
>
> /* Test a character type. (Only use with string constants.) */
> #define TEST_CTYPE_FUNC(func, string) \
>
>> static void test_ctype_##func(void) { \
>> for (int i = 0; i < 256; i++) { \
>> - if (!check_int(func(i), ==, is_in(string, i))) \
>> + int expect = !!memchr(string, i, sizeof(string) - 1); \
>> + if (!check_int(func(i), ==, expect)) \
>> test_msg(" i: 0x%02x", i); \
>> } \
>> if (!check(!func(EOF))) \
^ 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