[lhasa] add new port #32793

Ghabry · 2023-07-27T22:00:57Z

The library uses autotools. I created a CMakeLists based on the one from bzip2 available here in vcpkg.

I'm unsure about the "usage text". Am I required to create a unofficial::lhasa target that a few other libraries offer?

dg0yt · 2023-07-28T04:56:15Z

The library uses autotools.

So why don't we use it?

FrankXie05 · 2023-07-28T07:27:21Z

@Ghabry As #32793 (comment) said, we support autotools. :)

Ghabry · 2023-07-28T09:37:02Z

ooh. My last port contribution was a long time ago so I wasn't aware of the autotools support. 😅

will make it a draft until I resolve this.

Ghabry · 2023-07-28T16:55:39Z

@FrankXie05 migrated to autotools. macos builder looks stuck though.

FrankXie05 · 2023-07-31T05:50:07Z

ports/lhasa/portfile.cmake

+set(LHASA_VERSION 0.4.0)
+vcpkg_download_distfile(ARCHIVE
+    URLS "https://github.com/fragglet/lhasa/releases/download/v0.4.0/lhasa-${LHASA_VERSION}.tar.gz"
+    FILENAME "lhasa-${LHASA_VERSION}.tar.gz"


Suggested change

set(LHASA_VERSION 0.4.0)

vcpkg_download_distfile(ARCHIVE

URLS "https://github.com/fragglet/lhasa/releases/download/v0.4.0/lhasa-${LHASA_VERSION}.tar.gz"

FILENAME "lhasa-${LHASA_VERSION}.tar.gz"

vcpkg_download_distfile(ARCHIVE

URLS "https://github.com/fragglet/lhasa/releases/download/v0.4.0/lhasa-${VERSION}.tar.gz"

FILENAME "lhasa-${VERSION}.tar.gz"

You can use ${VERSION} directly, it reads the "version" field in vcpkg.json.

FrankXie05 · 2023-07-31T05:50:52Z

ports/lhasa/portfile.cmake

+vcpkg_install_copyright(
+    COMMENT "lhasa is under ISC license."
+    FILE_LIST
+        "${SOURCE_PATH}/COPYING"
+)


Suggested change

vcpkg_install_copyright(

COMMENT "lhasa is under ISC license."

FILE_LIST

"${SOURCE_PATH}/COPYING"

)

vcpkg_install_copyright( FILE_LIST "${SOURCE_PATH}/COPYING")

FrankXie05 · 2023-07-31T05:51:14Z

ports/lhasa/vcpkg.json

+{
+  "name": "lhasa",
+  "version-semver": "0.4.0",
+  "port-version": 1,


Suggested change

"port-version": 1,

ports/lhasa/vcpkg.json

FrankXie05 · 2023-07-31T05:54:06Z

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".
That way, I can be aware that you've responded since you can't modify the tags.

FrankXie05 · 2023-08-14T06:41:42Z

@Ghabry The port provides a pc file, but does not provide a usage. We need find_package(PkgConfig). Refer to port liburing.

Ghabry · 2023-09-20T15:15:56Z

sorry for the delay. Was on vacation and didn't notice the build failure because the usage file was not installed. :/

Now resolved.

FrankXie05 · 2023-09-21T09:22:49Z

ports/lhasa/vcpkg.json

+  "version": "0.4.0",
+  "description": "LZH/LHA archive library",
+  "homepage": "https://fragglet.github.io/lhasa/",
+  "license": "ISC"


Suggested change

"license": "ISC"

"license": "ISC",

"dependencies": [

{

"name": "pkgconf",

"host": true,

"default-features": false

}

]

Please add the dependencies to use the pkg-config.

@FrankXie05 Why?

For the usage.

For using this port? (I have pkg-config.)
Or for building this port? (Don't we we still use vcpkg_find_acquire_program(PKGCONFG)?)

Of course this method can be used, what I mean is that this port requires pkgconfg to be installed. :)

Of course this method can be used, what I mean is that this port requires pkgconfg to be installed. :)

I don't see why it requires port pkgconf.

Or add vcpkg_find_acquire_program(PKGCONFIG) to portfile.cmake.

Ghabry · 2023-11-12T08:49:42Z

Hey this was approved 3 weeks ago. Is there anything else missing before merging?

ports/lhasa/vcpkg.json

dg0yt

It might be possible to drop the patch (unless upstreamed) and just pass COPY_SOURCE to vcpkg_configure_make.

ports/lhasa/portfile.cmake

dg0yt · 2023-11-12T10:30:04Z

ports/lhasa/portfile.cmake

+if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic")
+    # fixes error: libtool: can't build x86_64-pc-mingw32 shared library unless -no-undefined is specified
+    list(APPEND OPTIONS "LDFLAGS=-no-undefined")


Suggested change

if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic")

# fixes error: libtool: can't build x86_64-pc-mingw32 shared library unless -no-undefined is specified

list(APPEND OPTIONS "LDFLAGS=-no-undefined")

if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic" AND VCPKG_TARGET_IS_MINGW)

# fixes error: libtool: can't build x86_64-pc-mingw32 shared library unless -no-undefined is specified

list(APPEND OPTIONS "LDFLAGS=\$LDFLAGS -no-undefined")

@dg0yt

This patch did not compile. I had to use

if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic" AND VCPKG_TARGET_IS_WINDOWS)

instead of

if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic" AND VCPKG_TARGET_IS_MINGW)

Now is all green.

This is strange. CI doesn't even test mingw. And no other port needs to tell mingw about this flag. Does libtool convert it to some MSVC flag?

ports/lhasa/portfile.cmake

Ghabry · 2023-11-14T19:31:16Z

that patch is directly taken from an upstream commit. There is just no new release since that fix was commited.

FrankXie05 · 2024-04-02T06:55:18Z

Test the usage on x64-windows:

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
CMake Error at C:/Program Files/CMake/share/cmake-3.27/Modules/FindPkgConfig.cmake:663 (message):
  pkg-config tool not found
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.27/Modules/FindPkgConfig.cmake:829 (_pkg_check_modules_internal)
  CMakeLists.txt:20 (pkg_check_modules)


-- Configuring incomplete, errors occurred!

Ghabry · 2024-04-02T13:55:32Z

I was told by @dg0yt to remove the pkg-config dependency. 🤔

Technically pkg-config is not a dependency for the library. You can e.g. manually link it through a makefile.

dg0yt · 2024-04-02T18:45:05Z

I was told by @dg0yt to remove the pkg-config dependency. 🤔

Technically pkg-config is not a dependency for the library. You can e.g. manually link it through a makefile.

You need

vcpkg_find_acquire_program(PKGCONFIG)

vcpkg_cmake_configure(
    ...
    OPTIONS
        ...
         "-DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}"

Ghabry · 2024-04-03T09:32:59Z

I use vcpkg_configure_make, not vcpkg_cmake_configure. How do I correctly provide the argument to it? I looked through all 30 ports that call vcpkg_find_acquire_program(PKGCONFIG) and all of them use CMake, not Make.

BillyONeal · 2024-06-27T20:52:40Z

I think @dg0yt was speaking to @FrankXie05 ?

BillyONeal · 2024-06-27T23:19:49Z

/azp run

azure-pipelines · 2024-06-27T23:20:00Z

Azure Pipelines successfully started running 1 pipeline(s).

dg0yt · 2024-06-28T04:21:57Z

I use vcpkg_configure_make, not vcpkg_cmake_configure. How do I correctly provide the argument to it? I looked through all 30 ports that call vcpkg_find_acquire_program(PKGCONFIG) and all of them use CMake, not Make.

hm, maybe an ugly corner.

I guess the ports with vcpkg_configure_make just lack explicit handling. pkg-config comes from the system or from msys2, just as make, so it just works if it is not missing.

It only confirms that the new port doesn't need a pkgconf dependency.

dg0yt · 2024-06-28T04:23:52Z

ports/lhasa/usage

+The package lhasa can be imported via CMake FindPkgConfig module:
+
+    find_package(PkgConfig)
+    pkg_check_modules(LHASA REQUIRED IMPORTED_TARGET liblhasa)
+
+    target_link_libraries(main PRIVATE PkgConfig::LHASA)


You may want to review the heuristically output which is shown when no usage file gets installed. Formatting and wording were changed.

Yeah, despite the name 'info:reviewed' here I just set it to make sure I looked at it after confirmation that the merge succeeded build since I resurrected a PR that is a year old

* Update usage text * Use the patch from upstream

BillyONeal

I would like @dg0yt 's question #32793 (comment) answered before merging

Ghabry force-pushed the lhasa branch 2 times, most recently from 8d11b2f to bf37a1b Compare July 27, 2023 22:21

Ghabry mentioned this pull request Jul 27, 2023

Add inih and lhasa EasyRPG/buildscripts#150

Merged

Cheney-W assigned FrankXie05 Jul 28, 2023

FrankXie05 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jul 28, 2023

Ghabry marked this pull request as draft July 28, 2023 09:37

Ghabry force-pushed the lhasa branch from bf37a1b to 15608eb Compare July 28, 2023 11:21

Ghabry marked this pull request as ready for review July 28, 2023 15:03

Ghabry force-pushed the lhasa branch 5 times, most recently from 97bd3e0 to 87f5d1f Compare July 28, 2023 16:22

FrankXie05 requested changes Jul 31, 2023

View reviewed changes

FrankXie05 marked this pull request as draft July 31, 2023 05:54

Ghabry force-pushed the lhasa branch from 87f5d1f to 8e747f9 Compare August 12, 2023 14:27

Ghabry marked this pull request as ready for review August 12, 2023 22:41

Ghabry force-pushed the lhasa branch from 8e747f9 to 25f4d74 Compare August 18, 2023 17:49

Ghabry force-pushed the lhasa branch from 25f4d74 to e972806 Compare September 20, 2023 14:38

FrankXie05 requested changes Sep 21, 2023

View reviewed changes

Ghabry force-pushed the lhasa branch from e972806 to 0c33d1e Compare October 21, 2023 19:35

FrankXie05 previously approved these changes Oct 24, 2023

View reviewed changes

dg0yt reviewed Nov 12, 2023

View reviewed changes

ports/lhasa/vcpkg.json Outdated Show resolved Hide resolved

dg0yt reviewed Nov 12, 2023

View reviewed changes

Ghabry dismissed FrankXie05’s stale review via ef7d5cd November 14, 2023 19:28

Ghabry force-pushed the lhasa branch from ef7d5cd to 0ef6152 Compare November 14, 2023 19:30

Ghabry added 2 commits March 29, 2024 11:59

[lhasa] new port

d5870d1

[lhasa] run x-add-version

6c066cd

Ghabry force-pushed the lhasa branch from 0ef6152 to 6c066cd Compare March 29, 2024 11:01

BillyONeal added the info:reviewed Pull Request changes follow basic guidelines label Jun 27, 2024

Merge remote-tracking branch 'origin/master' into HEAD

6b8a7be

FrankXie05 previously approved these changes Jun 28, 2024

View reviewed changes

dg0yt reviewed Jun 28, 2024

View reviewed changes

* Use vcpkg_from_github

015f7b2

* Update usage text * Use the patch from upstream

BillyONeal dismissed FrankXie05’s stale review via 015f7b2 June 29, 2024 06:03

BillyONeal approved these changes Jun 29, 2024

View reviewed changes

BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Jun 29, 2024

BillyONeal requested changes Jun 29, 2024

View reviewed changes

JavierMatosD marked this pull request as draft October 8, 2024 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lhasa] add new port #32793

[lhasa] add new port #32793

Ghabry commented Jul 27, 2023 •

edited

Loading

dg0yt commented Jul 28, 2023

FrankXie05 commented Jul 28, 2023

Ghabry commented Jul 28, 2023

Ghabry commented Jul 28, 2023

FrankXie05 Jul 31, 2023

FrankXie05 Jul 31, 2023

FrankXie05 Jul 31, 2023

FrankXie05 commented Jul 31, 2023

FrankXie05 commented Aug 14, 2023

Ghabry commented Sep 20, 2023 •

edited

Loading

FrankXie05 Sep 21, 2023

dg0yt Sep 21, 2023

FrankXie05 Sep 22, 2023

dg0yt Sep 22, 2023

FrankXie05 Sep 22, 2023

dg0yt Sep 22, 2023

FrankXie05 Sep 22, 2023

Ghabry commented Nov 12, 2023

dg0yt left a comment

dg0yt Nov 12, 2023

Ghabry Mar 29, 2024

dg0yt Jun 28, 2024

Ghabry commented Nov 14, 2023

FrankXie05 commented Apr 2, 2024

Ghabry commented Apr 2, 2024

dg0yt commented Apr 2, 2024

Ghabry commented Apr 3, 2024

BillyONeal commented Jun 27, 2024

BillyONeal commented Jun 27, 2024

azure-pipelines bot commented Jun 27, 2024

dg0yt commented Jun 28, 2024

dg0yt Jun 28, 2024

BillyONeal Jun 28, 2024 •

edited

Loading

BillyONeal left a comment

-  "license": "ISC"
+  "license": "ISC",
+  "dependencies": [
+    {
+      "name": "pkgconf",
+      "host": true,
+      "default-features": false
+    }
+  ]

[lhasa] add new port #32793

Are you sure you want to change the base?

[lhasa] add new port #32793

Conversation

Ghabry commented Jul 27, 2023 • edited Loading

dg0yt commented Jul 28, 2023

FrankXie05 commented Jul 28, 2023

Ghabry commented Jul 28, 2023

Ghabry commented Jul 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrankXie05 commented Jul 31, 2023

FrankXie05 commented Aug 14, 2023

Ghabry commented Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ghabry commented Nov 12, 2023

dg0yt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ghabry commented Nov 14, 2023

FrankXie05 commented Apr 2, 2024

Ghabry commented Apr 2, 2024

dg0yt commented Apr 2, 2024

Ghabry commented Apr 3, 2024

BillyONeal commented Jun 27, 2024

BillyONeal commented Jun 27, 2024

azure-pipelines bot commented Jun 27, 2024

dg0yt commented Jun 28, 2024

Choose a reason for hiding this comment

BillyONeal Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

BillyONeal left a comment

Choose a reason for hiding this comment

Ghabry commented Jul 27, 2023 •

edited

Loading

Ghabry commented Sep 20, 2023 •

edited

Loading

BillyONeal Jun 28, 2024 •

edited

Loading