patch, 3 partsconfigure: respect --without-<package> flags

> diff --git a/configure.ac b/configure.ac
> @@ -592,6 +592,9 @@ fi
>  # Define NO_CURL if you do not have libcurl installed.  git-http-pull and
>  # git-http-push are not built, and you cannot use http:// and https://
>  # transports.
>
> +# Respect --without-curl
> +if test "x$NO_CURL" != "xYesPlease"; then
> ...
>  if test -z "$NO_CURL"; then

> On Wed, Mar 18, 2020 at 3:38 AM Đoàn Trần Công Danh
> <congdanhqx@gmail.com> wrote:
> > diff --git a/configure.ac b/configure.ac
> > @@ -592,6 +592,9 @@ fi
> >  # Define NO_CURL if you do not have libcurl installed.  git-http-pull and
> >  # git-http-push are not built, and you cannot use http:// and https://
> >  # transports.
> >
> > +# Respect --without-curl
> > +if test "x$NO_CURL" != "xYesPlease"; then
> > ...
> >  if test -z "$NO_CURL"; then
> 
> I realize that GIT_PARSE_WITH() will either clear NO_CURL or set it to
> literal "YesPlease", but the comment(s) in this file describing
> NO_CURL say only to _define_ it to build without curl support. So, I'm
> wondering if it would make more sense to take a looser view about the
> value of NO_CURL. The existing check of NO_CURL doesn't bother looking
> for a specific value, but instead just tests whether it has a value or
> not. Perhaps the new check can be modeled after that one.
> 
> Also, I think you can reduce the scope of this change quite a bit by
> merely wrapping the AC_CHECK_LIB() invocation. So, either:
> 
>     ...
>     if test -z "$NO_CURL"; then
>     GIT_STASH_FLAGS($CURLDIR)
> 
>     AC_CHECK_LIB([curl], [curl_global_init],
>     [NO_CURL=],
>     [NO_CURL=YesPlease])
> 
>     GIT_UNSTASH_FLAGS($CURLDIR)
>     fi
> 
>     GIT_CONF_SUBST([NO_CURL])
>     ...
> 
> or even:
> 
>     ...
>     if test -z "$NO_CURL"; then
>     AC_CHECK_LIB([curl], [curl_global_init],
>     [NO_CURL=],
>     [NO_CURL=YesPlease])
>     fi
>     ...
> 
> Same comments applies to the other patches, as well.

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  configure.ac | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 7468eb6bc8..20a11e1f32 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -544,9 +544,15 @@ GIT_STASH_FLAGS($OPENSSLDIR)
>  
>  AC_CHECK_LIB([crypto], [SHA1_Init],
>  [NEEDS_SSL_WITH_CRYPTO=],
> -[AC_CHECK_LIB([ssl], [SHA1_Init],
> - [NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],
> - [NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])])
> +[
> +if "x$NO_OPENSSL" = xYesPlease; then
> +	AC_MSG_ERROR([Disabled OpenSSL is required for SHA1])

> On 2020-03-18 at 07:38:02, Đoàn Trần Công Danh wrote:
> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> > ---
> >  configure.ac | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 7468eb6bc8..20a11e1f32 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -544,9 +544,15 @@ GIT_STASH_FLAGS($OPENSSLDIR)
> >  
> >  AC_CHECK_LIB([crypto], [SHA1_Init],
> >  [NEEDS_SSL_WITH_CRYPTO=],
> > -[AC_CHECK_LIB([ssl], [SHA1_Init],
> > - [NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],
> > - [NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])])
> > +[
> > +if "x$NO_OPENSSL" = xYesPlease; then
> > +	AC_MSG_ERROR([Disabled OpenSSL is required for SHA1])
> 
> I don't see this in the original.  Wouldn't we want to fall back to the
> default in this case, which would be SHA1DC?

> Current git's configure script allow --without-<package>,
> it rightfully set NO_<PACKAGE> in the beginning of the configure script,
> but overwrite them with autodetect later.
> 
> Respect them now.

> 
> Change from v1:
> - Simply check if NO_<PACKAGE> is empty or not instead of explicitly
>   check for YesPlease
> - --without-openssl will ignore libcrypto all-together and fallback
>   into DC_SHA1
> 
> Đoàn Trần Công Danh (3):
>   configure: respect --without-curl flag
>   configure: respect --without-expat flag
>   configure: respect --without-openssl flag
> 
>  configure.ac | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> Interdiff against v1:
> diff --git a/configure.ac b/configure.ac
> index 20a11e1f32..807bb8bc5a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -542,17 +542,18 @@ AC_MSG_NOTICE([CHECKS for libraries])
>  
>  GIT_STASH_FLAGS($OPENSSLDIR)
>  
> +if test -z "$NO_OPENSSL"; then
>  AC_CHECK_LIB([crypto], [SHA1_Init],
>  [NEEDS_SSL_WITH_CRYPTO=],
> -[
> -if "x$NO_OPENSSL" = xYesPlease; then
> -	AC_MSG_ERROR([Disabled OpenSSL is required for SHA1])
> +[AC_CHECK_LIB([ssl], [SHA1_Init],
> + [NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],
> + [NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])])
>  else
> -	AC_CHECK_LIB([ssl], [SHA1_Init],
> -	[NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],
> -	[NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])
> +	# Fallback to DC SHA1
> +	NEEDS_SSL_WITH_CRYPTO=
> +	DC_SHA1=YesPlease
> +	GIT_CONF_SUBST([DC_SHA1])
>  fi
> -])
>  
>  GIT_UNSTASH_FLAGS($OPENSSLDIR)
>  
> @@ -598,17 +599,19 @@ fi
>  # git-http-push are not built, and you cannot use http:// and https://
>  # transports.
>  
> -# Respect --without-curl
> -if test "x$NO_CURL" != "xYesPlease"; then
> -
>  GIT_STASH_FLAGS($CURLDIR)
>  
> +# Respect --without-curl
> +if test -z "$NO_CURL"; then
>  AC_CHECK_LIB([curl], [curl_global_init],
>  [NO_CURL=],
>  [NO_CURL=YesPlease])
> +fi
>  
>  GIT_UNSTASH_FLAGS($CURLDIR)
>  
> +GIT_CONF_SUBST([NO_CURL])
> +
>  if test -z "$NO_CURL"; then
>  
>  AC_CHECK_PROG([CURL_CONFIG], [curl-config],
> @@ -629,27 +632,22 @@ fi
>  
>  fi
>  
> -fi
> -
> -GIT_CONF_SUBST([NO_CURL])
>  
>  #
>  # Define NO_EXPAT if you do not have expat installed.  git-http-push is
>  # not built, and you cannot push using http:// and https:// transports.
>  
> -# Respect --without-expat
> -if test "x$NO_EXPAT" != xYesPlease; then
> -
>  GIT_STASH_FLAGS($EXPATDIR)
>  
> +# Respect --without-expat
> +if test -z "$NO_EXPAT"; then
>  AC_CHECK_LIB([expat], [XML_ParserCreate],
>  [NO_EXPAT=],
>  [NO_EXPAT=YesPlease])
> +fi
>  
>  GIT_UNSTASH_FLAGS($EXPATDIR)
>  
> -fi
> -
>  GIT_CONF_SUBST([NO_EXPAT])
>  
>  #
> -- 
> 2.26.0.rc2.234.g969ad452ca
> 

> On 2020-03-19 10:58:54+0700, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
>> Current git's configure script allow --without-<package>,
>> it rightfully set NO_<PACKAGE> in the beginning of the configure script,
>> but overwrite them with autodetect later.
>> 
>> Respect them now.
>
> Hi Junio,
>
> Can I get your opinion on this serie?