git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] grep: print the pcre2_jit_on value
@ 2019-07-22 18:19 Beat Bolli
  2019-07-23 17:34 ` Junio C Hamano
  2019-07-23 19:19 ` Johannes Schindelin
  0 siblings, 2 replies; 6+ messages in thread
From: Beat Bolli @ 2019-07-22 18:19 UTC (permalink / raw)
  To: git; +Cc: gitster, Beat Bolli

When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value
of pcre1_jit_on.

Print the value of pcre2_jit_on instead.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grep.c b/grep.c
index f7c3a5803e..cd952ef5d3 100644
--- a/grep.c
+++ b/grep.c
@@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
 	} else if (p->pcre2_jit_on != 0) {
 		BUG("The pcre2_jit_on variable should be 0 or 1, not %d",
-		    p->pcre1_jit_on);
+		    p->pcre2_jit_on);
 	}
 }
 
-- 
2.21.0.1020.gf2820cf01a


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] grep: print the pcre2_jit_on value
  2019-07-22 18:19 [PATCH] grep: print the pcre2_jit_on value Beat Bolli
@ 2019-07-23 17:34 ` Junio C Hamano
  2019-07-23 19:19 ` Johannes Schindelin
  1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2019-07-23 17:34 UTC (permalink / raw)
  To: Beat Bolli; +Cc: git

Beat Bolli <dev+git@drbeat.li> writes:

> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value
> of pcre1_jit_on.
>
> Print the value of pcre2_jit_on instead.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---

Thanks.


>  grep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grep.c b/grep.c
> index f7c3a5803e..cd952ef5d3 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>  		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
>  	} else if (p->pcre2_jit_on != 0) {
>  		BUG("The pcre2_jit_on variable should be 0 or 1, not %d",
> -		    p->pcre1_jit_on);
> +		    p->pcre2_jit_on);
>  	}
>  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] grep: print the pcre2_jit_on value
  2019-07-22 18:19 [PATCH] grep: print the pcre2_jit_on value Beat Bolli
  2019-07-23 17:34 ` Junio C Hamano
@ 2019-07-23 19:19 ` Johannes Schindelin
  2019-07-24 21:25   ` Beat Bolli
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2019-07-23 19:19 UTC (permalink / raw)
  To: Beat Bolli; +Cc: git, gitster

Hi Beat,

On Mon, 22 Jul 2019, Beat Bolli wrote:

> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value
> of pcre1_jit_on.
>
> Print the value of pcre2_jit_on instead.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>  grep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grep.c b/grep.c
> index f7c3a5803e..cd952ef5d3 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>  		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
>  	} else if (p->pcre2_jit_on != 0) {
>  		BUG("The pcre2_jit_on variable should be 0 or 1, not %d",
> -		    p->pcre1_jit_on);
> +		    p->pcre2_jit_on);

Seems obviously good.

Maybe while you're in the vicinity, you can add that information to the
`--debug` output?

Thanks,
Dscho

>  	}
>  }
>
> --
> 2.21.0.1020.gf2820cf01a
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] grep: print the pcre2_jit_on value
  2019-07-23 19:19 ` Johannes Schindelin
@ 2019-07-24 21:25   ` Beat Bolli
  2019-07-25  9:13     ` Johannes Schindelin
  0 siblings, 1 reply; 6+ messages in thread
From: Beat Bolli @ 2019-07-24 21:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Ævar Arnfjörð Bjarmason

Hi Dscho,

On 23.07.19 21:19, Johannes Schindelin wrote:
> Hi Beat,
> 
> On Mon, 22 Jul 2019, Beat Bolli wrote:
> 
>> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value
>> of pcre1_jit_on.
>>
>> Print the value of pcre2_jit_on instead.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>  grep.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/grep.c b/grep.c
>> index f7c3a5803e..cd952ef5d3 100644
>> --- a/grep.c
>> +++ b/grep.c
>> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>>  		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
>>  	} else if (p->pcre2_jit_on != 0) {
>>  		BUG("The pcre2_jit_on variable should be 0 or 1, not %d",
>> -		    p->pcre1_jit_on);
>> +		    p->pcre2_jit_on);
> 
> Seems obviously good.
> 
> Maybe while you're in the vicinity, you can add that information to the
> `--debug` output?

Do you mean something like this?

diff --git a/grep.c b/grep.c
index cd952ef5d3..8a57ba998f 100644
--- a/grep.c
+++ b/grep.c
@@ -406,6 +406,8 @@ static void compile_pcre1_regexp(struct grep_pat *p,
const struct grep_opt *opt)

 #ifdef GIT_PCRE1_USE_JIT
        pcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);
+       if (opt->debug)
+               fprintf(stderr, "pcre1_jit_on=%d\n", p->pcre1_jit_on);
        if (p->pcre1_jit_on == 1) {
                p->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);
                if (!p->pcre1_jit_stack)
@@ -522,6 +524,8 @@ static void compile_pcre2_pattern(struct grep_pat
*p, const struct grep_opt *opt
        }

        pcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);
+       if (opt->debug)
+               fprintf(stderr, "pcre2_jit_on=%d\n", p->pcre2_jit_on);
        if (p->pcre2_jit_on == 1) {
                jitret = pcre2_jit_compile(p->pcre2_pattern,
PCRE2_JIT_COMPLETE);
                if (jitret)

If so, I'll wait a bit until it's clear whether Ævar's series [1] or my
patch is going to be applied.

If this is missing your intentions, I need more guidance ;-)

[1] https://public-inbox.org/git/20190724151415.3698-1-avarab@gmail.com/


> 
> Thanks,
> Dscho
> 
>>  	}
>>  }
>>

Cheers,
Beat

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] grep: print the pcre2_jit_on value
  2019-07-24 21:25   ` Beat Bolli
@ 2019-07-25  9:13     ` Johannes Schindelin
  2019-07-29  8:43       ` Carlo Arenas
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2019-07-25  9:13 UTC (permalink / raw)
  To: Beat Bolli; +Cc: git, Ævar Arnfjörð Bjarmason

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

Hi Beat,

On Wed, 24 Jul 2019, Beat Bolli wrote:

> On 23.07.19 21:19, Johannes Schindelin wrote:
> >
> > On Mon, 22 Jul 2019, Beat Bolli wrote:
> >
> >> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value
> >> of pcre1_jit_on.
> >>
> >> Print the value of pcre2_jit_on instead.
> >>
> >> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> >> ---
> >>  grep.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/grep.c b/grep.c
> >> index f7c3a5803e..cd952ef5d3 100644
> >> --- a/grep.c
> >> +++ b/grep.c
> >> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
> >>  		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
> >>  	} else if (p->pcre2_jit_on != 0) {
> >>  		BUG("The pcre2_jit_on variable should be 0 or 1, not %d",
> >> -		    p->pcre1_jit_on);
> >> +		    p->pcre2_jit_on);
> >
> > Seems obviously good.
> >
> > Maybe while you're in the vicinity, you can add that information to the
> > `--debug` output?
>
> Do you mean something like this?

Yes!

> diff --git a/grep.c b/grep.c
> index cd952ef5d3..8a57ba998f 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -406,6 +406,8 @@ static void compile_pcre1_regexp(struct grep_pat *p,
> const struct grep_opt *opt)
>
>  #ifdef GIT_PCRE1_USE_JIT
>         pcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);
> +       if (opt->debug)
> +               fprintf(stderr, "pcre1_jit_on=%d\n", p->pcre1_jit_on);
>         if (p->pcre1_jit_on == 1) {
>                 p->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);
>                 if (!p->pcre1_jit_stack)
> @@ -522,6 +524,8 @@ static void compile_pcre2_pattern(struct grep_pat
> *p, const struct grep_opt *opt
>         }
>
>         pcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);
> +       if (opt->debug)
> +               fprintf(stderr, "pcre2_jit_on=%d\n", p->pcre2_jit_on);
>         if (p->pcre2_jit_on == 1) {
>                 jitret = pcre2_jit_compile(p->pcre2_pattern,
> PCRE2_JIT_COMPLETE);
>                 if (jitret)
>
> If so, I'll wait a bit until it's clear whether Ævar's series [1] or my
> patch is going to be applied.

Sounds fair.

However, my reading of those patches is that they still keep support for
JIT'ed and non-JIT'ed PCRE2 invocations (preferring the former if
available, without any UI to change Git's behavior in that respect), so
I would be surprised if this patch wasn't applicable after Ævar's
patches.

Thanks,
Dscho

>
> If this is missing your intentions, I need more guidance ;-)
>
> [1] https://public-inbox.org/git/20190724151415.3698-1-avarab@gmail.com/
>
>
> >
> > Thanks,
> > Dscho
> >
> >>  	}
> >>  }
> >>
>
> Cheers,
> Beat
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] grep: print the pcre2_jit_on value
  2019-07-25  9:13     ` Johannes Schindelin
@ 2019-07-29  8:43       ` Carlo Arenas
  0 siblings, 0 replies; 6+ messages in thread
From: Carlo Arenas @ 2019-07-29  8:43 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Beat Bolli, git, Ævar Arnfjörð Bjarmason

On Thu, Jul 25, 2019 at 5:50 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Wed, 24 Jul 2019, Beat Bolli wrote:
> > Do you mean something like this?
>
> Yes!

Ideally, though, you want to print those values closing to the match
function (pcre_exec for PCRE1
or pcre2_[jit_]match for PCRE2 as they might change after they are
extracted from the PCRE configuration
with pcre[2]-config()

> > If so, I'll wait a bit until it's clear whether Ævar's series [1] or my
> > patch is going to be applied.
>
> Sounds fair

FWIW they are both applied (yours all the way to next), but Ævar's
changes now in pu (but likely to be rerolled soon) will most likely
remove your code (as explained on Junio's emails)

> However, my reading of those patches is that they still keep support for
> JIT'ed and non-JIT'ed PCRE2 invocations (preferring the former if
> available, without any UI to change Git's behavior in that respect), so
> I would be surprised if this patch wasn't applicable after Ævar's
> patches.

the PCRE1 changes are significant enough that would break the current
check and might
need a more convoluted check to figure out if JIT was actually used by
pcre_exec.

more details in the configuration[1]

Carlo

[1] https://www.pcre.org/original/doc/html/pcrejit.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-29  8:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 18:19 [PATCH] grep: print the pcre2_jit_on value Beat Bolli
2019-07-23 17:34 ` Junio C Hamano
2019-07-23 19:19 ` Johannes Schindelin
2019-07-24 21:25   ` Beat Bolli
2019-07-25  9:13     ` Johannes Schindelin
2019-07-29  8:43       ` Carlo Arenas

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).