configure.ac: revert three bad nghttp2 library "fixes"

[bagder]

This reverts commit b4b34db, 6737533 and 29c7cf7.

The logic is now back to assuming that the nghttp2 lib is called nghttp2 and
nothing else.

Reported-by: Rui Pinheiro
Reported-by: Alex Crichton
Fixes #7514

/cc @jay @t-artikov

[jay]

Considering the name was only nghttp2_static for a short period of time this is probably ok. However, the suffix is optional but I don't know how common it is to use that. Anyway, I don't see how it was a bad fix, can you explain?

[bagder]

29c7cf7 didn't handle white space
6737533 didn't work when multiple libs were provided in pkg-config
b4b34db generated /usr/bin/ld: cannot find -l-lnghttp2 (see comment)

[jay]

b4b34db generated /usr/bin/ld: cannot find -l-lnghttp2 (see comment)

I forgot to escape the brackets

diff --git a/configure.ac b/configure.ac
index c92ab9e..6feaeb4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2541,7 +2541,7 @@ if test X"$want_h2" != Xno; then
     LDFLAGS="$LDFLAGS $LD_H2"
     CPPFLAGS="$CPPFLAGS $CPP_H2"
     LIBS="$LIB_H2 $LIBS"
-    LIB_H2_NAME=`echo $LIB_H2 | $SED -ne 's/.*-l *\(nghttp2[^ ]*\).*/\1/p'`
+    LIB_H2_NAME=`echo $LIB_H2 | $SED -ne 's/.*-l *\(nghttp2[[^ ]]*\).*/\1/p'`
 
     # use nghttp2_session_set_local_window_size to require nghttp2
     # >= 1.12.0

https://github.com/curl/curl/compare/master...jay:pr_7515_alternate?expand=1

[bagder]

the suffix is optional but I don't know how common it is to use that

All other libraries we check for in configure we check for the lib's "real" name so I would claim that there's a long history of sticking to the same name. Especially when pkg-config can't even tell us the "raw" name.

I personally prefer to go back to using the fixed library name, even for static builds. Like we do for all other ~30 libs we can build with...

[rruipinheiro]

I still can't build with nghtt2p with these fixes... I don't know if this is a problem or not but my lib name is libnghttp2, not nghttp2

[t-artikov]

Hi. I'm sorry my change led to the problem. It is needed to build curl with nghttp2 using conan (conan-io/conan-center-index#6241). Perhaps there is another solution - do not add the "_static" suffix to nghttp2 (https://github.com/conan-io/conan-center-index/blob/master/recipes/libnghttp2/all/conanfile.py#L103), but I don't know what consequences it can lead to.

[bagder]

@t-artikov I take it you then would prefer @jay's fix in #7515 (comment) ?

[t-artikov]

@bagder
Yes, it works for my case.

[jay]

I still can't build with nghtt2p with these fixes... I don't know if this is a problem or not but my lib name is libnghttp2, not nghttp2

Library filenames are prefixed with lib, libname.a or in Windows you might have libname.dll.a. You'd specify just the name like -lname. I notice a later comment in your issue says this PR works for you. Does the alternate fix I submitted work for you?

[rruipinheiro]

@jay Yes, both worked for me!

[vszakats]

The history of this is that I added in nghttp2 1.32.0 (2018-05-18) the ability to build static lib in an opt-in fashion (via ENABLE_STATIC_LIB=1, off by default), then later an nghttp2 commit broke compatibility and added hard-coded _static suffix in 1.40.0 (2019-11-15). This also broke curl-for-win (and any dependents using its static libs), so committed a fix that replaced the hard-coded value with the envvar STATIC_LIB_SUFFIX, which is empty by default. This was released in 1.41.0 (2020-06-02).

Conan gets the timeline and history slightly wrong (can't blame them!) and then explicitly sets STATIC_LIB_SUFFIX to _static for nghttp2 1.42.0+ versions, thus reverting to the incompatible naming scheme used in 1.40.0 only. Unless Conan wants to (~) offer both shared and static libs at the same time and make the shared libs the default, this change seems unnecessary and apparently breaks other projects, like curl.

I feel the best course of action is to keep using nghttp2 in curl, which is the default name used by nghttp2. Then fix Conan to not force the incompatible name (that only existed in nghttp2 1.40.0) on its environment. The full blown solution would be to allow in curl to manually override the nghttp2 lib name to anything, even though that's overkill for the majority of environments.

The reason for all this is the lack of Windows convention on how to name and choose libs wrt shared/static.

[jay]

Either way is fine with me

[bagder]

Then I prefer we go back to the original state with just 'nghttp2', for simplicity.

[ghost]

Considering the name was only nghttp2_static for a short period of time this is probably ok.

This is not quite true. Currently nghttp2_static is still used in case of static library: https://github.com/nghttp2/nghttp2/blob/master/lib/CMakeLists.txt#L62

[ghost]

I still wonder if 29c7cf7 was bad due to not handling whitespace. Can this really be the case? I mean, I personally haven't seen a library name containing space, but of course my experience is much more modest.

Is there a way to improve that particular change in order to keep spaces in mind? Will simply wrapping the variable with quotes do the trick?

[ghost]

Sorry, I read this comment and I take it back.

[vszakats]

@mkoviazin: Are you sure this line sets the output name (as written to disk) and not the CMake name of the static lib for internal purposes?

Few lines later the output name is set to the non-hardcoded suffix.

[ghost]

@vszakats you're right, I overlooked this and your comment from earlier. Thanks! :)