Re: [PATCH] gas: Add --force-compress-debug-sections

[Jan Beulich <jbeulich@suse.com>]

On 27.02.2023 14:44, Pedro Alves wrote:
> On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote:
>> On 24.02.2023 15:57, Tom de Vries wrote:
>>>
>>> Is this more how you want it?
>>
>> I have to admit that I'm still puzzled by the presence of
>> finalize_parse_compress_debug_optarg() as well as you needing both a new
>> static variable and a new global one. But I guess whether that's really
>> needed first of all depends on the semantics we want e.g.
>>
>> --nocompress-debug-sections --compress-debug-sections=force
>>
>> to have (which, with how you have it presently, could also be expressed
>> as
>>
>> --compress-debug-sections=none+force
>>
>> or
>>
>> --compress-debug-sections=none --compress-debug-sections=force
>>
>> afaict). I view the present meaning as one sensible one, but I could
>> also see "none" (or equivalent) simply zapping the compression type
>> (and hence rendering "force" meaningless) as another sensible one. A
>> change in meaning may then also result in the three option combinations
>> above possibly not all doing the same.
> 
> 
> ISTM that these confusions (which no doubt users will have too) would go away if
> we did not try to put orthogonal settings into one option.
> 
> Witness how the implementation uses two different enums:
> 
> This new one:
> 
>  +enum compress_debug_action
>  +{
>  +  cda_default,
>  +  cda_none,
>  +  cda_force,
>  +  cda_yes,
>  +};
> 
> And this preexisting one:
> 
>  /* Types of compressed DWARF debug sections.  */
>  enum compressed_debug_section_type
>  {
>    COMPRESS_DEBUG_NONE = 0,
>    COMPRESS_DEBUG_GNU_ZLIB = 1 << 1,
>    COMPRESS_DEBUG_GABI_ZLIB = 1 << 2,
>    COMPRESS_DEBUG_ZSTD = 1 << 3,
>    COMPRESS_UNKNOWN = 1 << 4
>  };
> 
> Imagine we started over from scratch, and had these two orthogonal options,
> matching internal enums (enum compress_debug_action would be slightly different):
> 
>  --compressed-debug-sections-format=zlib|zstd|...|none
> 
>    # Iff we're compressing, what format shall we use?
> 
>  --compress-debug-sections=no|yes|sizewin
> 
>    # Are we compressing debug sections?  When?
>    #
>    #  - "no" - never, we're not compressing.
>    #
>    #  - "yes" - always, we're compressing, using the format specified
>    #     by --compressed-debug-sections-format (and if that is "none", well, 
>    #     you still get what you asked for).
>    #
>    #  - "sizewin" - compress if there's a size win.  Like "yes", if the format is
>    #    "none", well, this becomes a nop.
> 
> ... then the semantics of mixing these options, or what happens when you repeat them
> would be much more obvious:
> 
>   - You can specify '--compressed-debug-sections-format' multiple times.  The last wins.
> 
>   - You can specify '--compress-debug-sections' multiple times.  The last wins.
> 
>   - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'.
> 
>   - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format'
> 
> 
> Now, we can't use those option names with that meaning, though, because
> '--compress-debug-sections' today already has the meaning of selecting
> the compression format.  But that just means we would need to pick different
> names, like for example:
> 
>  --compress-debug-sections=zlib|zstd|none
> 
>    # Iff we're compressing, what format shall we use?
> 
>  --nocompress-debug-sections
> 
>    # Shorthand for --compress-debug-sections=none
> 
>  --compress-debug-sections-when=never|always|sizewin
> 
>    # Are we compressing debug sections?  When?
>    #
>    #  - "never" - never, we're not compressing.
>    #
>    #  - "always" - always compress, using the format specified by
>    #     by --compress-debug-sections (and if that is "none", well, 
>    #     you still get what you asked for).
>    #
>    #  - "sizewin", compress if there's a size win.  Like "always", respects
>    #    the format specified by --compress-debug-sections.
> 
> 
> The semantics of mixing these options, or what happens when you repeat them
> would be obvious in the same way in the other options naming earlier:
> 
>   - You can specify '--compress-debug-sections' multiple times.  The last wins.
> 
>   - You can specify '--compress-debug-sections-when' multiple times.  The last wins.
> 
>   - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'.
> 
>   - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections'
> 
> You end up with two different ways to disable compressing debug sections,
> but that seems OK to me.

That would be okay with me as well, but (in part to avoid ...

> All that would be left would be bikeshed on the new option name.

... this aspect) I fail to see why a single option won't do:

--compress-debug-sections=zlib|zstd|no|yes|sizewin

Since the sub-options are distinct sets (alongside "no" one could add "none"
if wanted, to separate "method: no compression" from "don't compress", which
I think would then also eliminate the ambiguity referred to earlier on
[still visible in context above]), within each set your "last wins" would
apply equally. And in fact that was the main reason for my original response
back to Tom: I view the proliferation of command line options (not just
here, but in general) as a scalability issue. Therefore my rule of thumb is
that things which belong together are best grouped under a single top-level
option.

Jan