Skip to content

[brigand] Fix pkgconfig file and modernize portfile.cmake.#21009

Merged
BillyONeal merged 5 commits intomicrosoft:masterfrom
BillyONeal:brigand
Oct 28, 2021
Merged

[brigand] Fix pkgconfig file and modernize portfile.cmake.#21009
BillyONeal merged 5 commits intomicrosoft:masterfrom
BillyONeal:brigand

Conversation

@BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Oct 26, 2021

Brigand was producing a pkgconfig file that looked like it was not a header-only library, but it is a header-only library. Remove the incorrect patch and place the pkgconfig file in the correct location.

@BillyONeal BillyONeal added category:port-bug The issue is with a library, which is something the port should already support info:internal labels Oct 26, 2021
@PhoebeHui PhoebeHui self-assigned this Oct 27, 2021
@BillyONeal
Copy link
Member Author

@autoantwort Is my understanding of "that .pc file is just broken" correct?

@autoantwort
Copy link
Contributor

Does this lib produces a *.lib file? I tried to build it on mac but I get build errors (on master)

@BillyONeal
Copy link
Member Author

Does this lib produces a *.lib file? I tried to build it on mac but I get build errors (on master)

It produces a .a file.

@BillyONeal
Copy link
Member Author

(Specifically installed/x64-linux/lib/libbrigand.a and installed/x64-linux/debug/lib/libbrigandd.a)

@autoantwort
Copy link
Contributor

autoantwort commented Oct 27, 2021

When I understand the CMakeLists.txt right its a header only lib and the generated lib does only contain test files. I think we should remove the generation of the test lib and the Libs: line in the *.pc.in file

@autoantwort
Copy link
Contributor

autoantwort commented Oct 27, 2021

I would propose the following patch (not tested):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2763ff0..f94b68d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -289,16 +289,12 @@ endif()
 
 source_group(tests FILES ${test_files})
 
-add_executable(brigand_test ${test_files})
 
 IF(${CMAKE_MAJOR_VERSION} GREATER 2)
     add_library(brigand INTERFACE)
     add_custom_target(brigand_headers SOURCES ${BRIGAND_HEADERS})
-    target_link_libraries(brigand_test brigand)
 ENDIF()
 
-add_test(NAME brigand_test COMMAND brigand_test)
-
 configure_file(libbrigand.pc.in
     libbrigand.pc
     @ONLY
@@ -309,10 +305,9 @@ install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/brigand
 )
 
 install(FILES ${CMAKE_BINARY_DIR}/libbrigand.pc
-    DESTINATION lib/pkgconfig
+    DESTINATION share/pkgconfig
 )
 
 include(SetupDoxygen)
 include(SetupStandardese)
 
-add_subdirectory(examples)
diff --git a/libbrigand.pc.in b/libbrigand.pc.in
index 2ed570e..218215b 100644
--- a/libbrigand.pc.in
+++ b/libbrigand.pc.in
@@ -6,5 +6,4 @@ Name: Brigand
 Description: Light-weight, fully functional, instant-compile time C++ 11 meta-programming library
 URL: https://github.com/edouarda/brigand
 Version: 1.2.0
-Libs:
 Cflags: -I${includedir}

@BillyONeal BillyONeal marked this pull request as draft October 27, 2021 01:23
@BillyONeal BillyONeal changed the title [brigand] Delete broken pkgconfig file and modernize portfile.cmake. [brigand] Fix pkgconfig file and modernize portfile.cmake. Oct 27, 2021
@BillyONeal BillyONeal marked this pull request as ready for review October 27, 2021 02:13
@BillyONeal
Copy link
Member Author

I would propose the following patch (not tested):

That patch doesn't appear to match the sources we're currently downloading, but I did a similar thing. Can you take a look?

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d8b9fa9a535de1d3b4effe8fca5c18f573af88e8 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/b-/brigand.json b/versions/b-/brigand.json
index f43f2cf..489f080 100644
--- a/versions/b-/brigand.json
+++ b/versions/b-/brigand.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "2eb18256137a5fac1a25275484c952fb18911261",
+      "version-string": "1.3.0",
+      "port-version": 2
+    },
     {
       "git-tree": "99395d0e7569b8b32f76d99cf2183a2a92679a7d",
       "version-string": "1.3.0",
diff --git a/versions/baseline.json b/versions/baseline.json
index 389900a..e37d078 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1130,7 +1130,7 @@
     },
     "brigand": {
       "baseline": "1.3.0",
-      "port-version": 1
+      "port-version": 2
     },
     "brotli": {
       "baseline": "1.0.9",

"homepage": "https://github.com/edouarda/brigand",
"dependencies": [
"boost"
"boost",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove boost from the dependency, it looks only its tests rely on boost, and the tests should be disabled.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it looks only headers from boost-fusion and boost-variant are used in brigand, so we can make it reply on the two ports instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, applied your suggestion, thanks!

@autoantwort
Copy link
Contributor

@BillyONeal Your changes looks good to me

Co-authored-by: Phoebe <20694052+PhoebeHui@users.noreply.github.com>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/brigand/vcpkg.json b/ports/brigand/vcpkg.json
index 8606119..99a5e02 100644
--- a/ports/brigand/vcpkg.json
+++ b/ports/brigand/vcpkg.json
@@ -5,8 +5,8 @@
   "description": "Brigand is a light-weight, fully functional, instant-compile time C++ 11 meta-programming library.",
   "homepage": "https://github.com/edouarda/brigand",
   "dependencies": [
-   "boost-fusion",
-   "boost-variant",
+    "boost-fusion",
+    "boost-variant",
     {
       "name": "vcpkg-cmake",
       "host": true
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d8b9fa9a535de1d3b4effe8fca5c18f573af88e8 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/b-/brigand.json b/versions/b-/brigand.json
index f386a12..d5b9f4b 100644
--- a/versions/b-/brigand.json
+++ b/versions/b-/brigand.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "b16f7b84a7f87b9befd3ac63c8c9d7cb3d50a6d2",
+      "git-tree": "a27641e46efb007b46792b7bd803f32437fe4178",
       "version-string": "1.3.0",
       "port-version": 2
     },

@BillyONeal
Copy link
Member Author

@BillyONeal Your changes looks good to me

Thanks for double checking!

@BillyONeal BillyONeal merged commit 95db766 into microsoft:master Oct 28, 2021
@BillyONeal BillyONeal deleted the brigand branch October 28, 2021 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants