Date: Tue, 13 Jun 2023 12:00:01 +0900 (JST)
Message-Id: <20230613.120001.2091320520660271559.horikyota.ntt@gmail.com>
To: nathandbossart@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: add non-option reordering to in-tree getopt_long
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20230609232257.GA121461@nathanxps13>
References: <20230609232257.GA121461@nathanxps13>
User-Agent: Mew version 6.8 on Emacs 27.2
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230613.120001.2091320520660271559.horikyota.ntt%40gmail.com>
Precedence: bulk

At Fri, 9 Jun 2023 16:22:57 -0700, Nathan Bossart <nathandbossart@gmail.com> wrote in 
> While working on 2dcd157, I noticed cfbot failures for Windows due to tests
> with commands that had non-options specified before options.  For example,
> "createuser myrole -a myadmin" specified a non-option (myrole) before the
> option/argument pair (-a admin).  To get the tests passing for Windows,
> non-options must be placed at the end (e.g., "createuser -a myadmin
> myrole").  This same problem was encountered while working on 08951a7 [0],
> but it was again fortunately caught with cfbot.  Others have not been so
> lucky [1] [2] [3].

While I don't see it as reason to change the behavior, I do believe
the change could be beneficial from a user's perspective.

> The reason for this discrepancy is because Windows uses the in-tree
> implementation of getopt_long(), which differs from the other
> implementations I've found in that it doesn't reorder non-options to the
> end of argv by default.  Instead, it returns -1 as soon as the first
> non-option is found, even if there are other options listed afterwards.  By
> moving non-options to the end of argv, getopt_long() can parse all
> specified options and return -1 when only non-options remain.  The
> implementations I reviewed all reorder argv unless the POSIXLY_CORRECT
> environment variable is set or the "optstring" argument begins with '+'.
>
> The best reasons I can think of to keep the current behavior are 1)
> reordering involves writing to the original argv array, which could be
> risky (as noted by Tom [4]) and 2) any systems with a getopt_long()
> implementation that doesn't reorder non-options could begin failing tests
> that take advantage of this behavior.  However, this quirk in the in-tree
> getopt_long() is periodically missed by hackers, the only systems I'm aware
> of that use it are Windows and AIX, and every other implementation of
> getopt_long() I saw reorders non-options by default.  Furthermore, C99
> omits const decorations in main()'s signature, so modifying argv might not
> be too scary.

POSIXLY_CORRECT appears to be intended for debugging or feature
validation. If we know we can always rearrange argv on those
platforms, we don't need it.  I would suggest that we turn on the new
feature at the compile time on those platforms where we know this
rearrangement works, instead of switching at runtime.

> Thus, I propose introducing this non-option reordering behavior but
> allowing it to be disabled via the POSIXLY_CORRECT environment variable.
> The attached patch is my first attempt at implementing this proposal.  I
> don't think we need to bother with handling '+' at the beginning of
> "optstring" since it seems unlikely to be used in PostgreSQL, but it would
> probably be easy enough to add if folks want it.
> 
> I briefly looked at getopt() and concluded that we should probably retain
> its POSIX-compliant behavior for non-options, as reordering support seems
> much less universal than with getopt_long().  AFAICT all client utilities
> use getopt_long(), anyway.
> 
> Thoughts?

As far as I can see, getopt_long on Rocky9 does *not* rearrange argv
until it reaches the end of the array. But it won't matter much.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center