Is there a freeze on that I'm unaware of?  Is there so much traffic from
other developers that smaller output ones are being missed?  I'm
wondering about the initial revision of this series not getting any
feedback:
https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg01264.html


Due to the lack of news on the first thread, I've done some looking to
assess the feasibility.  The xl.cfg parser looks a bit jumbled.  Too many
things are visible to too many places.  In general some pieces have
really needed some TLC for a long time.

Note, some portions of this are semi-WIP.  The first 5 patches are simply
overdue maintenance.  Not particularly urgent, but should probably be
done.

Patch #06 is rather more urgent maintenance.  While it might not explode
soon, that is a landmine.

Patch #07 is the first big issue.  The roles of the various headers had
never been sorted out.  The underlying issue was more the contents of
"libxlu_cfg_i.h" needed to be merged into "libxlu_cfg_y.h".  Bison 2.3
may not have had the ability to embedded things into its header in 2010,
but the functionality appears to have been present in Bison 3.3.

There is an issue of what level of indentation should be used in
libxlu_cfg_y.y?  Normally the sections being added wouldn't be indented,
but normally they would be directly in headers.  I'm unsure of which
direction to go here.

Patch #08 seemed best to leave after #07 due to overlap and difficulty
with reordering.  I'm a bit worried about the interfacing with flex.


Then comes the issue of moving things out of libxlu_internal.h which
should never have been there.  The XLU_Config* values should have been
placed in libxlu_cfg_i.h instead.  Since I'm doing a thorough job,
they're instead moving to libxlu_cfg.c.

I'm unsure splitting libxlu_cfg.c is worthwhile.  The resultant reusable
libxlu_cfg.c is rather small.  Yet avoiding the need to reimplement the
small portion is handy.


Is the decision to keep in-tree copies of current Flex/Bison output still
valid?  I would be awful tempted to remove them from main/master, even
if copies are maintained on release branches.


Elliott Mitchell (22):
  tools/utils: cleanup formatting of libxlutil.h
  tools/utils: rename "n" arguments to "key"
  tools/utils: remove old declaration of xlu__cfg_yyparse()
  tools/utils: enable all Bison warnings
  tools/utils: update Bison parser directives
  tools/utils: remove libxlu_cfg_i.h from libxlu_disk.c
  tools/utils: merge contents of libxlu_cfg_i.h to libxlu_cfg_y.y
  tools/utils: Bison: switch from name-prefix to api.prefix
  tools/utils: move CfgParseContext to top of libxlu_cfg_y.y
  tools/utils: move XLU_ConfigSetting to libxl_cfg.c
  tools/utils: move XLU_ConfigValue to libxl_cfg.c
  tools/utils: remove YYLTYPE definition from libxlu_internal.h
  tools/utils: move XLU_ConfigList to libxl_cfg.c
  tools/utils: introduce xlu_cfg_printf() function
  tools/utils: move XLU_Config to libxlu_cfg.c
  tools/utils: move XLU_Operation to libxlu_cfg_y.h
  tools/utils: move setting free loop to xlu__cfg_set_free()
  tools/utils: spread xlu_cfg_printf() over libxlu_cfg.c
  tools/utils: add pointer to in-progress settings to CfgParseContext
  tools/utils: add wrapper for readfile()/readdata() functions
  tools/utils: add settings get function
  tools/utils: break flex/bison parser away from main config file

 tools/include/libxlutil.h                     |  31 +-
 tools/libs/util/Makefile                      |   3 +-
 tools/libs/util/libxlu_cfg.c                  | 679 ++----------------
 .../util/{libxlu_cfg.c => libxlu_cfg_i.c}     | 196 +++--
 tools/libs/util/libxlu_cfg_i.h                |  59 --
 tools/libs/util/libxlu_cfg_l.l                |   2 +-
 tools/libs/util/libxlu_cfg_y.y                |  58 +-
 tools/libs/util/libxlu_disk.c                 |  15 +-
 tools/libs/util/libxlu_internal.h             |  59 +-
 tools/libs/util/libxlu_pci.c                  |  11 +-
 tools/libs/util/libxlu_vif.c                  |   4 +-
 11 files changed, 223 insertions(+), 894 deletions(-)
 copy tools/libs/util/{libxlu_cfg.c => libxlu_cfg_i.c} (78%)
 delete mode 100644 tools/libs/util/libxlu_cfg_i.h

-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |       ehem+sigmsg@m5p.com      PGP 87145445       |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

On 02.08.2023 17:33, Elliott Mitchell wrote:
> Is there a freeze on that I'm unaware of?  Is there so much traffic from
> other developers that smaller output ones are being missed?  I'm
> wondering about the initial revision of this series not getting any
> feedback:
> https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg01264.html
> 
> 
> Due to the lack of news on the first thread, I've done some looking to
> assess the feasibility.  The xl.cfg parser looks a bit jumbled.  Too many
> things are visible to too many places.  In general some pieces have
> really needed some TLC for a long time.
> 
> Note, some portions of this are semi-WIP.  The first 5 patches are simply
> overdue maintenance.  Not particularly urgent, but should probably be
> done.
> 
> Patch #06 is rather more urgent maintenance.  While it might not explode
> soon, that is a landmine.
> 
> Patch #07 is the first big issue.  The roles of the various headers had
> never been sorted out.  The underlying issue was more the contents of
> "libxlu_cfg_i.h" needed to be merged into "libxlu_cfg_y.h".  Bison 2.3
> may not have had the ability to embedded things into its header in 2010,
> but the functionality appears to have been present in Bison 3.3.
> 
> There is an issue of what level of indentation should be used in
> libxlu_cfg_y.y?  Normally the sections being added wouldn't be indented,
> but normally they would be directly in headers.  I'm unsure of which
> direction to go here.
> 
> Patch #08 seemed best to leave after #07 due to overlap and difficulty
> with reordering.  I'm a bit worried about the interfacing with flex.
> 
> 
> Then comes the issue of moving things out of libxlu_internal.h which
> should never have been there.  The XLU_Config* values should have been
> placed in libxlu_cfg_i.h instead.  Since I'm doing a thorough job,
> they're instead moving to libxlu_cfg.c.
> 
> I'm unsure splitting libxlu_cfg.c is worthwhile.  The resultant reusable
> libxlu_cfg.c is rather small.  Yet avoiding the need to reimplement the
> small portion is handy.
> 
> 
> Is the decision to keep in-tree copies of current Flex/Bison output still
> valid?  I would be awful tempted to remove them from main/master, even
> if copies are maintained on release branches.
> 
> 
> Elliott Mitchell (22):
>   tools/utils: cleanup formatting of libxlutil.h
>   tools/utils: rename "n" arguments to "key"
>   tools/utils: remove old declaration of xlu__cfg_yyparse()
>   tools/utils: enable all Bison warnings
>   tools/utils: update Bison parser directives
>   tools/utils: remove libxlu_cfg_i.h from libxlu_disk.c
>   tools/utils: merge contents of libxlu_cfg_i.h to libxlu_cfg_y.y
>   tools/utils: Bison: switch from name-prefix to api.prefix
>   tools/utils: move CfgParseContext to top of libxlu_cfg_y.y
>   tools/utils: move XLU_ConfigSetting to libxl_cfg.c
>   tools/utils: move XLU_ConfigValue to libxl_cfg.c
>   tools/utils: remove YYLTYPE definition from libxlu_internal.h
>   tools/utils: move XLU_ConfigList to libxl_cfg.c
>   tools/utils: introduce xlu_cfg_printf() function
>   tools/utils: move XLU_Config to libxlu_cfg.c
>   tools/utils: move XLU_Operation to libxlu_cfg_y.h
>   tools/utils: move setting free loop to xlu__cfg_set_free()
>   tools/utils: spread xlu_cfg_printf() over libxlu_cfg.c
>   tools/utils: add pointer to in-progress settings to CfgParseContext
>   tools/utils: add wrapper for readfile()/readdata() functions
>   tools/utils: add settings get function
>   tools/utils: break flex/bison parser away from main config file

Some of the patches looks to have been posted previously as a 7-patch
series. It would have been nice if therefore this one was marked as
v2, indicating in a revision log what the differences are. It appears
as if at least one out of those 7 earlier patches was dropped (or
maybe assimilated into another one).

Jan

On Thu, Aug 03, 2023 at 10:35:53AM +0200, Jan Beulich wrote:
> 
> Some of the patches looks to have been posted previously as a 7-patch
> series. It would have been nice if therefore this one was marked as
> v2, indicating in a revision log what the differences are. It appears
> as if at least one out of those 7 earlier patches was dropped (or
> maybe assimilated into another one).

Indeed.  Problem is several tags could potentially have been used.
Should I have used all of them simultaneously?  Should I have used only
some of them?  Which subset?

Several were mildly adjusted, so it could have been marked "v2".

No one responded at all to the previous round, so this could have been
marked "RESEND".

Yet the refinements and general changes are large enough for the series
to be pretty distinct.

I didn't know which way to go, so with no idea which option to choose the
last one ended up winning out.  Perhaps that was wrong yet I've still no
feedback on the actual patches.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

On 03.08.2023 18:13, Elliott Mitchell wrote:
> On Thu, Aug 03, 2023 at 10:35:53AM +0200, Jan Beulich wrote:
>>
>> Some of the patches looks to have been posted previously as a 7-patch
>> series. It would have been nice if therefore this one was marked as
>> v2, indicating in a revision log what the differences are. It appears
>> as if at least one out of those 7 earlier patches was dropped (or
>> maybe assimilated into another one).
> 
> Indeed.  Problem is several tags could potentially have been used.
> Should I have used all of them simultaneously?  Should I have used only
> some of them?  Which subset?
> 
> Several were mildly adjusted, so it could have been marked "v2".
> 
> No one responded at all to the previous round, so this could have been
> marked "RESEND".
> 
> Yet the refinements and general changes are large enough for the series
> to be pretty distinct.
> 
> I didn't know which way to go, so with no idea which option to choose the
> last one ended up winning out.  Perhaps that was wrong yet I've still no
> feedback on the actual patches.

As, for this series, being just in the role of a committer, without clearly
identifying that some earlier patches can now be dropped from the list of
things which need monitoring, you're making my (in that regard) supposedly
purely mechanical job harder (and presumably every other committer's as
well). As to not getting feedback: Your posting (at least the cover letter)
dates back to July 19. That's not all that long compared to other series.
Plus if you were concerned, you could have pinged the respective
maintainers.

Jan

Hi Elliott,

On 03/08/2023 17:13, Elliott Mitchell wrote:
> On Thu, Aug 03, 2023 at 10:35:53AM +0200, Jan Beulich wrote:
>>
>> Some of the patches looks to have been posted previously as a 7-patch
>> series. It would have been nice if therefore this one was marked as
>> v2, indicating in a revision log what the differences are. It appears
>> as if at least one out of those 7 earlier patches was dropped (or
>> maybe assimilated into another one).
> 
> Indeed.  Problem is several tags could potentially have been used.
> Should I have used all of them simultaneously?  Should I have used only
> some of them?  Which subset?
> 
> Several were mildly adjusted, so it could have been marked "v2".
> 
> No one responded at all to the previous round, so this could have been
> marked "RESEND".
> 
> Yet the refinements and general changes are large enough for the series
> to be pretty distinct.
> 
> I didn't know which way to go, so with no idea which option to choose the
> last one ended up winning out.  Perhaps that was wrong yet I've still no
> feedback on the actual patches.

Not sure if this is related to the lack of answer. But I didn't receive 
any of your patches via xen-devel (I received your replies). Skimming 
through the bounce for the xenproject mail server, I noticed a lot of 
the following:

     host gmail-smtp-in.l.google.com [142.250.123.26]
     SMTP error from remote mail server after pipelined end of data:
     550-5.7.1 This message is not RFC 5322 compliant. There are 
multiple Cc headers.
     550-5.7.1 To reduce the amount of spam sent to Gmail, this message 
has been
     550-5.7.1 blocked. Please visit
     550 5.7.1 
https://support.google.com/mail/?p=RfcMessageNonCompliant 
t7-20020a81e447000000b005839e8b595dsi12027284ywl.554 - gsmtp

It might be possible that other mail server are not happy with your e-mails.

Cheers,

-- 
Julien Grall

On Thu, Aug 03, 2023 at 05:34:31PM +0100, Julien Grall wrote:
> 
> On 03/08/2023 17:13, Elliott Mitchell wrote:
> > 
> > I didn't know which way to go, so with no idea which option to choose the
> > last one ended up winning out.  Perhaps that was wrong yet I've still no
> > feedback on the actual patches.
> 
> Not sure if this is related to the lack of answer. But I didn't receive 
> any of your patches via xen-devel (I received your replies). Skimming 
> through the bounce for the xenproject mail server, I noticed a lot of 
> the following:
> 
>      host gmail-smtp-in.l.google.com [142.250.123.26]
>      SMTP error from remote mail server after pipelined end of data:
>      550-5.7.1 This message is not RFC 5322 compliant. There are 
> multiple Cc headers.
>      550-5.7.1 To reduce the amount of spam sent to Gmail, this message 
> has been
>      550-5.7.1 blocked. Please visit
>      550 5.7.1 
> https://support.google.com/mail/?p=RfcMessageNonCompliant 
> t7-20020a81e447000000b005839e8b595dsi12027284ywl.554 - gsmtp

That seems to be repotting a bug in `scripts/add_maintainers.pl`.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Hi,

On 03/08/2023 18:45, Elliott Mitchell wrote:
> On Thu, Aug 03, 2023 at 05:34:31PM +0100, Julien Grall wrote:
>>
>> On 03/08/2023 17:13, Elliott Mitchell wrote:
>>>
>>> I didn't know which way to go, so with no idea which option to choose the
>>> last one ended up winning out.  Perhaps that was wrong yet I've still no
>>> feedback on the actual patches.
>>
>> Not sure if this is related to the lack of answer. But I didn't receive
>> any of your patches via xen-devel (I received your replies). Skimming
>> through the bounce for the xenproject mail server, I noticed a lot of
>> the following:
>>
>>       host gmail-smtp-in.l.google.com [142.250.123.26]
>>       SMTP error from remote mail server after pipelined end of data:
>>       550-5.7.1 This message is not RFC 5322 compliant. There are
>> multiple Cc headers.
>>       550-5.7.1 To reduce the amount of spam sent to Gmail, this message
>> has been
>>       550-5.7.1 blocked. Please visit
>>       550 5.7.1
>> https://support.google.com/mail/?p=RfcMessageNonCompliant
>> t7-20020a81e447000000b005839e8b595dsi12027284ywl.554 - gsmtp
> 
> That seems to be repotting a bug in `scripts/add_maintainers.pl`.

I am curious to know why you think so?

I have been using scripts/add_maintainers.pl for quite a while and so 
far never seen any of my e-mail blocked.

My usual runes are:

42sh> scripts/add_maintainers.pl -d .
42sh> git-send-email *.patch

What's yours?

Cheers,

-- 
Julien Grall

On Thu, Aug 03, 2023 at 06:48:33PM +0100, Julien Grall wrote:
> Hi,
> 
> On 03/08/2023 18:45, Elliott Mitchell wrote:
> > On Thu, Aug 03, 2023 at 05:34:31PM +0100, Julien Grall wrote:
> >>
> >> Not sure if this is related to the lack of answer. But I didn't receive
> >> any of your patches via xen-devel (I received your replies). Skimming
> >> through the bounce for the xenproject mail server, I noticed a lot of
> >> the following:
> >>
> >>       host gmail-smtp-in.l.google.com [142.250.123.26]
> >>       SMTP error from remote mail server after pipelined end of data:
> >>       550-5.7.1 This message is not RFC 5322 compliant. There are
> >> multiple Cc headers.
> >>       550-5.7.1 To reduce the amount of spam sent to Gmail, this message
> >> has been
> >>       550-5.7.1 blocked. Please visit
> >>       550 5.7.1
> >> https://support.google.com/mail/?p=RfcMessageNonCompliant
> >> t7-20020a81e447000000b005839e8b595dsi12027284ywl.554 - gsmtp
> > 
> > That seems to be repotting a bug in `scripts/add_maintainers.pl`.
> 
> I am curious to know why you think so?
> 
> I have been using scripts/add_maintainers.pl for quite a while and so 
> far never seen any of my e-mail blocked.
> 
> My usual runes are:
> 
> 42sh> scripts/add_maintainers.pl -d .
> 42sh> git-send-email *.patch
> 
> What's yours?

Final steps are:

scp -r snd mailserver:
for f in snd/0*.patch
do sendmail -t < "$f"
	sleep 45
done

Issue with `git send-email` is it really needs all the setup of a MUA,
and I prefer to keep `git` on a distinct host.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445