libllhttp.pc.in is missing in release/v6.0.7

[sunpoet]

The build log is as follows:

-- The C compiler identification is Clang 13.0.0
-- The CXX compiler identification is Clang 13.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/local/libexec/ccache/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/local/libexec/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error: File /usr/ports/works/usr/ports/www/llhttp/work/llhttp-release-v6.0.7/libllhttp.pc.in does not exist.
CMake Error at CMakeLists.txt:41 (configure_file):
  configure_file Problem configuring file


--
--
-- Project configure summary:
--
--   CMake build type .................: RELEASE
--   Install prefix ...................: /usr/local
--   Build shared library .............: ON
--   Build static library .............: ON
--
-- Configuring incomplete, errors occurred!
See also "/usr/ports/works/usr/ports/www/llhttp/work/.build/CMakeFiles/CMakeOutput.log".
*** Error code 1

Stop.
make: stopped in /usr/ports/www/llhttp

[fduncanh]

This is probably fixed by the recent re-release of v6.0.7 (originally the main branch was accidentally used for the release instead of he release branch)

[sgallagher]

No, the source code link for the release still points to https://github.com/nodejs/llhttp/archive/refs/tags/v6.0.7.tar.gz which is missing the pkg-config file.

[fduncanh]

The 6.0.7 release has definitely changed (due to my intervention) Previously it was a 6 MB incorrect file based the main branch,

now it is replaced by a correct 40Kb file based on release branch, with the same but updated contents as 6.0.6.

I dont think either should have a pkg-config file

llhttp-release-v6.0.6:
total 24
-rw-r--r-- 1 duncan users 5315 Dec  4  2021 README.md
-rw-r--r-- 1 duncan users  290 Dec  4  2021 llhttp.gyp
-rw-r--r-- 1 duncan users 1105 Dec  4  2021 LICENSE-MIT
-rw-r--r-- 1 duncan users 1157 Dec  4  2021 common.gypi
-rw-r--r-- 1 duncan users 1139 Dec  4  2021 CMakeLists.txt

llhttp-release-v6.0.6/src:
total 456
-rw-r--r-- 1 duncan users 444714 Dec  4  2021 llhttp.c
-rw-r--r-- 1 duncan users   4474 Dec  4  2021 http.c
-rw-r--r-- 1 duncan users   9441 Dec  4  2021 api.c

llhttp-release-v6.0.6/include:
total 16
-rw-r--r-- 1 duncan users 15786 Dec  4  2021 llhttp.h

llhttp-6.0.7:
total 24
-rw-r--r-- 1 duncan users 6701 Jul  6 08:25 README.md
-rw-r--r-- 1 duncan users  290 Jul  6 08:25 llhttp.gyp
-rw-r--r-- 1 duncan users 1105 Jul  6 08:25 LICENSE-MIT
-rw-r--r-- 1 duncan users 1157 Jul  6 08:25 common.gypi
-rw-r--r-- 1 duncan users 3239 Jul  6 08:25 CMakeLists.txt

llhttp-6.0.7/src:
total 468
-rw-r--r-- 1 duncan users 455468 Jul  6 08:25 llhttp.c
-rw-r--r-- 1 duncan users   4542 Jul  6 08:25 http.c
-rw-r--r-- 1 duncan users   9664 Jul  6 08:25 api.c

llhttp-6.0.7/include:
total 20
-rw-r--r-- 1 duncan users 16390 Jul  6 08:25 llhttp.h

[sgallagher]

Without the pkg-config file, building the package from the CmakeLists.txt does not work:

$ tar xf v6.0.7.tar.gz && cd llhttp-6.0.7/

$ cmake -S . -B builddir
-- The C compiler identification is GNU 12.1.1
-- The CXX compiler identification is GNU 12.1.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error: File /var/home/sgallagh/workspace/fedora/packagereview/llhttp/llhttp-6.0.7/libllhttp.pc.in does not exist.
CMake Error at CMakeLists.txt:41 (configure_file):
  configure_file Problem configuring file


-- 
-- 
-- Project configure summary:
-- 
--   CMake build type .................: RELWITHDEBINFO
--   Install prefix ...................: /usr/local
--   Build shared library .............: ON
--   Build static library .............: OFF
-- 
-- Configuring incomplete, errors occurred!
See also "/var/home/sgallagh/workspace/fedora/packagereview/llhttp/llhttp-6.0.7/builddir/CMakeFiles/CMakeOutput.log".

This tarball is not functional as-is for packaging purposes.

Additionally, the CMakeLists.txt still has project(llhttp VERSION 6.0.5) in it, rather than 6.0.7.

[fduncanh]

attn: @mcollina

@sgallagher
Yes, you are right, this appears to be an error in the 6.0.7 CMakeLists.txt, unrelated to the incorrect release branch, which is now corrected. 6.0.6 builds correctly. (I use my own CMakeLists.txt, maybe inherited from older releases)

Looking more at this , the section "configure file" at line 41 of CMakelists.txt should be removed: Its not appropriate for the "release" (just the "main" branch). The error is not a missing file, but an incorrect dependency on that file in CMakeLists.txt.

Remove this, then things build.

remove these lines from CMakeLists.txt at line 41

configure_file(
  ${CMAKE_CURRENT_SOURCE_DIR}/libllhttp.pc.in
  ${CMAKE_CURRENT_SOURCE_DIR}/libllhttp.pc
  @ONLY
)

Also the name of the release directory should probably be llhttp-release-v6.0.7 for consistency, not llhttp-6.0.7, and the file should have "release" in its name: llhttp-release-v6.0.7.zip, etc.

[sgallagher]

The .pc file is actually desirable to have, as it makes life easier for other projects that want to link against libllhttp.

[fduncanh]

I agree: in that case its a new feature for the v6.0.7 release that has not properly been integrated

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue?

[fduncanh]

here is the missing file in main that needed
https://github.com/nodejs/llhttp/blob/main/libllhttp.pc.in

I verified that simply adding it to release branch allows a build

The issue of the directory name with "-release" in it is more complicated.
I dont know what the release procedure for creating the release is (some script somewhere)

[fduncanh]

It looks like the main branch has the script to correctly create the release including libllhttp.pc.in
This doesnt seem to need a PR
maybe one just needs to do something line "make release" in the main code to create the release directory and contents?

618aea8

[fduncanh]

I dont have installed whatever it takes to build main, but I think "make all" or "make release"

should produce a directory "release"
which should be renamed to llhttp-release-v6.0.7 and then tarred and zipped as llhttp-release-v6.0.7.zip etc.,

and should contain libllhttp.pc.in, unless the Makefile is broken.

The commit for this was by @JackLiar
May the committer could check that this is working?

[mcollina]

v6.0.7 was retagged due to #166.

[sunpoet]

FreeBSD port (www/llhttp) uses the source code from tag release/v6.0.7 which does not have libllhttp.pc.in.

[fduncanh]

@mcollina @JackLiar
Its clear from the commit 618aea8
that it was intended that liblhttp.pc.in should have been in release/v6.0.7 and CMakeLists.txt requires it.

either (1) the relevant entries should be removed from CMakeLists.txt or (2) liblhttp.pc.in should be added.

either way, a further adjustment to the release would seem to be needed.

[mcollina]

I don't have any idea on what to do, or what's the use case.

I would need step-by-step instructions on how to fix this.

[fduncanh]

best answer: ask @indutny

for fix (2) (the best) add the missing file libhttp.pc.in to the release branch:

in some directory
git clone https://github.com/nodejs/llhttp.git
cd llhttp
git clone https://github.com/nodejs/llhttp.git
cd llhttp
git checkout release
cp -p ../libllhttp.pc.in .
git add libllhttpc.pc.in
git commit (commit message "add missing libhttp.pc.in")
git push

now do a new release of v6.0.7 as v6.0.7a (from the release branch)

I'm not sure of the github procedure to release from the release branch (I don't use a release branch, and release from the main branch). The release should have "assets" llhttp-release-v6.0.7a.zip" and "llhttp-release-v6.0.7a.tar.gz"
which unpack to a directory containing llhttp-release-v6.0.7a containing the contents of the release branch.

The 6.0.6 release has all this correctly, but the 6.0.7 release just was in a directory llhttp-6.0.7. Whatever was done for
6.0.6 should be done for 6.0.7a

The llhttp release process might be using an automated procedure "git-flow .." which I don't use, and don't understand.

Best thing is to ask @indutny if he is still involved.

[mcollina]

Done https://github.com/nodejs/llhttp/releases/tag/v6.0.7a.

[fduncanh]

great!

Now it builds correctly!
Thanks!

[fduncanh]

Unfortunately, I found another error:
line 4 of CMakeLists.txt is
project(llhttp VERSION 6.0.5)

It should be 6.0.7a ( or 6.0.7b if you make a second re-release to fix it)
This shows up as an incorrect entry in the libllhttp.pc file that gets generated......

Looks like you need to edit CMakeLists.txt in the release branch when doing a release.

[fduncanh]

and also edit CMakeLists.txt in the main branch to fix it there too.

[mcollina]

Please send a PR that automate all of this, it's kind of an impossible goal to meet by hand.

There is already a script

https://github.com/nodejs/llhttp/blob/main/Makefile#L45

Which is what I execute.

I welcome a PR that enables us to release this as it would work for you.

[fduncanh]

@mcollina

I fixed the two errors in the script.
(1) It was not updating VERSION to $(TAG) in release/CMakelists.txt
(2) It was not updated to add the new v6.0.7 feature libhttpp.pc.in to the release branch

see PR #170

[mcollina]

released, hopefully should be good

[fduncanh]

@mcollina

Unfortunately, cmake doesnt like the VERSION 6.0.7b in CMakeLists.txtt, so it doesnt build.
I had only tested with 6.0.7

cmake accepts VERSION 6.0.7.1 (numbers) , appropriate for a minor release of 6.0.7.

The best would be either to do it again with a tag 6.0.7.1 or just redo it as 6.0.8

Sorry!
At least for future releases it is automated properly again.

[mcollina]

v6.0.8 is out and it should be good to go

[fduncanh]

@mcollina @sgallagher
I tested the 6.0.8 build. Downloaded from branch "release".

Now builds fine! Yes its good to go. The generated libllhttp.pc file seems correct.

You still need to post 6.0.8 to the "Releases" sidebar on git hub by releasing 6.0.8 (its still showing 6.0.7b as latest)