On Mon, 7 Mar 2022 13:57:42 GMT, Erik Joelsson <er...@openjdk.org> wrote:
>> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Handle remaining options and change critical options to error when >> --without is passed > > make/autoconf/jdk-version.m4 line 113: > >> 111: AC_MSG_ERROR([--with-vendor-name must have a value]) >> 112: elif test "x$with_vendor_name" = xno; then >> 113: AC_MSG_WARN([--without-vendor-name is the same as not passing >> --with-vendor-name to begin with]) > > I don't think this warrants a warning. The user is just trying to be > explicit. It's good to handle the "no" case though. The entire semantics of the `--without-X` flags in configure is a bit unclear. If someone where to actually give `--without-vendor-name`, what would they suppose should happen? If it is left unspecified, the default value `N/A` is taken from branding.conf. But when setting `--without-vendor-name`, do you mean that you explicitly want the `N/A` value? Or do you mean that you want the build to not use a vendor name at all? In the latter case, that is not possible. And if we make that interpretation, this should not be a warning, but an error. To me it feel strange to set `--without-vendor-name` and assume that means "use the default value". I'd rather go with the interpretation that this is a misunderstanding of how the option works, and emit an error. Do you want to > make/autoconf/jdk-version.m4 line 129: > >> 127: AC_MSG_ERROR([--with-vendor-url must have a value]) >> 128: elif test "x$with_vendor_url" = xno; then >> 129: AC_MSG_WARN([--without-vendor-url is the same as not passing >> --with-vendor-url to begin with]) > > Same as with vendor. I think this, even more, makes it clear that `--without-vendor-url` can´t possible be meant to be interpreted as "use https://openjdk.java.net/";. Basically, I think what I'm arguing for is that we can fold this check into the "yes" check -- both `--with-vendor-url` (with no given value) and `--without-vendor-url` are invalid. So something like this: if test "x$with_vendor_url" = xyes || test "x$with_vendor_url" = xno; then AC_MSG_ERROR([--with-vendor-url must have a value]) elif... ------------- PR: https://git.openjdk.java.net/jdk/pull/7713
