diff mbox series

[1/3] configure: respect --without-curl flags

Message ID c1c007190683d7ab49e854a66a4832b5ace72b51.1584516715.git.congdanhqx@gmail.com (mailing list archive)
State New, archived
Headers show
Series configure: respect --without-<package> flags | expand

Commit Message

Đoàn Trần Công Danh March 18, 2020, 7:38 a.m. UTC 
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 configure.ac | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Eric Sunshine March 18, 2020, 3:51 p.m. UTC | #1 
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.
Đoàn Trần Công Danh March 19, 2020, 3:19 a.m. UTC | #2 
On 2020-03-18 11:51:34-0400, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 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.

I've re-checked the configure.ac code.

We've already use:

	test -z "$NO_ICONV"

I think rewrite like your suggestion would be better choice, and
it'll be consistence with the current code.

I'll send a reroll after checking what should be done with NO_OPENSSL.
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 66aedb9288..f4742878c0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -592,6 +592,9 @@  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)
 
 AC_CHECK_LIB([curl], [curl_global_init],
@@ -600,8 +603,6 @@  AC_CHECK_LIB([curl], [curl_global_init],
 
 GIT_UNSTASH_FLAGS($CURLDIR)
 
-GIT_CONF_SUBST([NO_CURL])
-
 if test -z "$NO_CURL"; then
 
 AC_CHECK_PROG([CURL_CONFIG], [curl-config],
@@ -622,6 +623,9 @@  fi
 
 fi
 
+fi
+
+GIT_CONF_SUBST([NO_CURL])
 
 #
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is