GNOME Bugzilla – Bug 723160
GOption: add strict posix mode
Last modified: 2014-10-20 12:36:11 UTC
Add a "posixly correct" mode to GOption to stop parsing arguments as soon as the first non-option argument is encountered. We determine the default value on the basis of duplicating the behaviour of the system getopt() implementation (which we directly check the behaviour of at runtime). On GNU systems this allows the user to modify our behaviour using POSIXLY_CORRECT. The user can change the value by g_option_context_set_posixly_correct(), which might be useful for some usecases of GOptionContext (as mentioned in the doc string of this new function).
Created attachment 267396 [details] [review] GOption: add strict posix mode
Review of attachment 267396 [details] [review]: I like the idea ::: glib/goption.c @@ +375,3 @@ + * no arguments then we are in strict POSIX mode. + */ + context->strict_posix = getopt (3, (char **) argv, "a") != 'a'; Wouldn't it be more straightforward to check POSIXLY_CORRECT directly ? @@ +508,3 @@ + * @context: a #GoptionContext + * + * Sets strict posix mode. We should be consistent in capitalizing POSIX. @@ +538,3 @@ +void +g_option_context_set_posixly_correct (GOptionContext *context, + gboolean posixly_correct) I like the idea of introducing this mode, but I don't like the name very much. Could we call it 'strict' or 'incremental' or some other more descriptive name ?
(In reply to comment #2) > Wouldn't it be more straightforward to check POSIXLY_CORRECT directly ? Although this would give the correct behaviour on Linux systems, I think we may want to copy the strict POSIX behaviour that getopt() has on BSD systems. Matching the behaviour of the system getopt seems like it will get us a sane default in all cases. > We should be consistent in capitalizing POSIX. Sure. > I like the idea of introducing this mode, but I don't like the name very much. > Could we call it 'strict' or 'incremental' or some other more descriptive name I think I agree -- I only used this name on the API because it matches the variable. Internally I call it "strict posix" so how about _set_strict_posix()?
(In reply to comment #3) > (In reply to comment #2) > > Wouldn't it be more straightforward to check POSIXLY_CORRECT directly ? > > Although this would give the correct behaviour on Linux systems, I think we may > want to copy the strict POSIX behaviour that getopt() has on BSD systems. > > Matching the behaviour of the system getopt seems like it will get us a sane > default in all cases. ok > > I like the idea of introducing this mode, but I don't like the name very much. > > Could we call it 'strict' or 'incremental' or some other more descriptive name > > I think I agree -- I only used this name on the API because it matches the > variable. Internally I call it "strict posix" so how about > _set_strict_posix()? Fine with me
Created attachment 288614 [details] [review] GOption: add strict posix mode I like the idea as well and fixed a couple of things in the patch: - make it compile (wrong function signature) - rename the functions to _strict_posix and capitalized all occurences of POSIX, as discussed - reset optind to 1 before calling getopt(), otherwise every call after the first one will fail (option-context tests failed, for example) - update AVAILABLE_IN - add a test case
Review of attachment 288614 [details] [review]: looks good to me, otherwise ::: glib/goption.c @@ +524,3 @@ + * parsing). + * + * Since: 2.40 needs to be 2.44 now (already ?!)
Attachment 288614 [details] pushed as ae52ab3 - GOption: add strict posix mode
This commit breaks g-ir-compiler on FreeBSD. The getopt check causes GOption to work in strict posix mode by default, so the command g-ir-compiler --includedir=. --includedir=./gir --includedir=. --includedir=. --includedir=. gir/DBus-1.0.gir -o gir/DBus-1.0.typelib used to generate .typelib file when building gobject-introspection will cause the compiled file to be written to stdout instead of the specified file.
I found it also breaks other programs, including glib-compile-resources and valac.
I guess this isn't too surprising. We have two real options here: 1) directly inspect POSIXLY_CORRECT for ourselves, irrespective of the platform (also solves bug 738659) 2) never automatically enable strict-posix mode and the unrealistic option: 3) fix the world
I think we can choose 1). This behavior is expected by existing GNU users.
Option (1) won't fix the problem. The build would still be broken on platforms (or for users) that have POSIXLY_CORRECT defined. I think we should just go for (2).
Created attachment 288929 [details] [review] GOption: stop calling getopt() We called getopt() to try to find out of the platform on which we are running defaults to strict POSIX-style argument handling (ie: flags following the first filename are considered as further filenames rather than flags). This is the default case on BSDs, for example. It is also the case on GNU systems with the POSIXLY_CORRECT environment variable set. Unfortunately many of our tools rely on being able to accept commandline arguments in the non-strict ordering and the code for making these calls is spread widely (for example in MAkefile fragments invoking some of our build tools). For this reason we need to revert the getopt() check and only enable strict POSIX mode in the case that the application explicitly opts into it using the _set_strict_posix() API. This also fixs a failure to build on Windows due to missing getopt().
Review of attachment 288929 [details] [review]: Thanks. Please also update the doc string which still says that the default is system-dependant.
Created attachment 288930 [details] [review] GOption: stop calling getopt() Updated with a couple of quick fixes after a review from larsu.
Comment on attachment 288930 [details] [review] GOption: stop calling getopt() Attachment 288930 [details] pushed as 817f82d - GOption: stop calling getopt()
*** Bug 738659 has been marked as a duplicate of this bug. ***