+ improve CMake-based package config files. #246

moodyhunter · 2021-06-30T10:56:28Z

CMake package config files for uvw, they are now installed as ${LIBDIR}/cmake/uvw/uvwConfig.cmake with target files for appropriate configuration (e.g. debug, release, etc...).
A uvw::uvw ALIAS target is created for uvw static library, to make persistency with what is done to the header-only version,

Callers can now use find_package(uvw) and link with the uvw::uvw target to use uvw in their projects directly using CMake.

skypjack · 2021-06-30T13:47:02Z

@stefanofiorentino it looks good to me but could you review it too?
I'm far from being a cmake expert, so two extra eyes can help here. 😅

stefanofiorentino · 2021-06-30T16:18:01Z

I’ll check it later this week.

CMakeLists.txt

conanfile.py

src/CMakeLists.txt

stefanofiorentino

Change request: I would spin-off the conan-related commit in an other PR.

I fully agree with the uvwConfig.cmake file installation along with the libs path.
As I cannot check all config/flags paths, I'm not 100% sure.
The CI/CD are green, though. ;) so I'll approve it once conan was stripped out or approved by @skypjack .

NOTE: the pkg-config related stuff was taken as is from libuv

skypjack · 2021-07-04T12:10:41Z

Change request: I would spin-off the conan-related commit in an other PR.

I'm fine with the request from @stefanofiorentino I don't have a strong opinion here, so I follow him. 🙂

Previously, uvw provides only a pkg-config .pc file, although a uvw.cmake presents, it's not properly installed and placed in to a correct cmake `find_package` search path. This leads to many problems, where in this commit, a `uvw::uvw` ALIAS target is created for uvw static library, - if `BUILD_UVW_LIBS` is set, FETCH_LIBUV must be set as well (otherwise it'll not build, it's the current behavior). - if `BUILD_UVW_LIBS` is NOT set: - when `FETCH_LIBUV` is set, it's guaranteed that uv::uv-static target exists, so a genex is removed. - when `FETCH_LIBUV` is NOT set, it's the caller's responsibility to make sure libuv can be properly linked This commit also fixes CMake package config files for uvw, they are now installed as ${LIBDIR}/cmake/uvw/uvwConfig.cmake with target files for appropriate configuration (e.g. debug, release, etc...). Callers can now use `find_package(uvw)` and link with the `uvw::uvw` target to use uvw in their projects directly using CMake.

moodyhunter · 2021-07-04T12:25:09Z

Hi all, I've just force pushed and removed that conanfile related commit from the git log, for now, it should be clean and only contains a single commit.

stefanofiorentino · 2021-07-27T21:32:30Z

@skypjack should we put this into an experimental branch and let us time to be tried by someone else?

moodyhunter · 2021-07-28T05:12:42Z

I agree, I deleted my fork by accident :( , it'll be better if an experimental branch can be created, this also allows more people to test the changes.

skypjack · 2021-07-28T08:51:49Z

Makes sense. I would update uvw to wrap libuv v1.42 though, then we can create a branch from the last version.

moodyhunter · 2021-07-30T12:05:34Z

It seems that a dl library is missing, (See https://github.com/moodyhunter/QvPersonal/runs/3201566023?check_suite_focus=true#step:8:273 and https://github.com/skypjack/uvw/blob/master/src/CMakeLists.txt#L80) however the fork is deleted and I can't update this PR.

stefanofiorentino · 2021-07-30T22:22:46Z

It seems that a dl library is missing, (See https://github.com/moodyhunter/QvPersonal/runs/3201566023?check_suite_focus=true#step:8:273 and https://github.com/skypjack/uvw/blob/master/src/CMakeLists.txt#L80) however the fork is deleted and I can't update this PR.

This dependency comes from libuv (and that's the only one IIRC), as they have API regarding the management of shared libraries.
Now, is it uvw responsibility to link under-the-hood to dl or is better to leave it to-do to the final linker?
That's a matter of taste and philosophy up-to-me.
So I guess @skypjack has to be invoked to the final decision: uvw users would link to dl implicitly or explicitly?

skypjack · 2021-07-31T08:10:04Z

I'd make users link it explicitly. dl isn't a direct dependency of uvw. Instead, it's a dependency of the underlying library.
As an user, I wouldn't feel comfortable with an indirect dependency pulled in by a third party library honestly.

stefanofiorentino · 2021-07-31T17:08:39Z

I'd make users link it explicitly. dl isn't a direct dependency of uvw. Instead, it's a dependency of the underlying library.
As an user, I wouldn't feel comfortable with an indirect dependency pulled in by a third party library honestly.

I like when the linker breaks and I discover hidden dependencies. I feel the developers are playing fair with me. So we agreed.

Close skypjack#246 Signed-off-by: Fiorentino Ing. Stefano <[email protected]>

stefanofiorentino · 2021-10-19T13:47:34Z

Ciao, I've committed some more modifications to this PR here: experimental.
As already stated somewhere, this additions will encapsulate the right version of libuv in the uvw subfolder of system folders during installation. This avoid system folders pollution/overwriting.
I guess that's the best we can achieve up to now.
Do you agree?

stefanofiorentino · 2021-10-30T09:31:57Z

@moodyhunter could you please try this and see if it finally solve your issue?
https://github.com/skypjack/uvw/tree/experimental
Long story short: the installation phase of the dependent libuv is done in the uvw subfolder.

moodyhunter · 2021-10-30T12:50:58Z

Yeah, it seems to be good, all header-only, shared and static libraries are properly installed now.

skypjack · 2021-11-03T16:05:47Z

@stefanofiorentino I would merge this upstream. Any objection?

moodyhunter · 2021-11-04T09:06:30Z

No, I certainly agree.

stefanofiorentino · 2021-11-04T12:59:10Z

@stefanofiorentino I would merge this upstream. Any objection?

Let's proceed with this. I mean upstream is to me your repo as my origin is my fork.
Anyway I think we will get it to master too, right? That's ok to me.

Close #246 Signed-off-by: Fiorentino Ing. Stefano <[email protected]>

moodyhunter mentioned this pull request Jun 30, 2021

CMake package configuration files for uvw #245

Closed

skypjack added the build system label Jun 30, 2021

skypjack self-assigned this Jun 30, 2021

skypjack requested a review from stefanofiorentino June 30, 2021 13:47

stefanofiorentino reviewed Jul 4, 2021

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

stefanofiorentino reviewed Jul 4, 2021

View reviewed changes

conanfile.py Outdated Show resolved Hide resolved

stefanofiorentino reviewed Jul 4, 2021

View reviewed changes

src/CMakeLists.txt Show resolved Hide resolved

stefanofiorentino requested changes Jul 4, 2021

View reviewed changes

moodyhunter mentioned this pull request Jul 4, 2021

fixed a typo in conanfile.py #247

Merged

moodyhunter changed the title ~~+ fixed a typo in conanfile.py and fixed CMake-based package config files.~~ + improve CMake-based package config files. Jul 5, 2021

Merge branch 'master' into master

8587245

pemensik mentioned this pull request Sep 4, 2021

Install cmake package module libuv/libuv#3293

Merged

stefanofiorentino added a commit to stefanofiorentino/uvw that referenced this pull request Oct 19, 2021

install either static or shared lib only

277b7c7

Close skypjack#246 Signed-off-by: Fiorentino Ing. Stefano <[email protected]>

stefanofiorentino approved these changes Oct 30, 2021

View reviewed changes

skypjack approved these changes Nov 11, 2021

View reviewed changes

skypjack merged commit b388750 into skypjack:master Nov 11, 2021

skypjack pushed a commit that referenced this pull request Feb 1, 2022

install either static or shared lib only

472245a

Close #246 Signed-off-by: Fiorentino Ing. Stefano <[email protected]>

Uh oh!

+ improve CMake-based package config files. #246

+ improve CMake-based package config files. #246

Uh oh!

Conversation

moodyhunter commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skypjack commented Jun 30, 2021

Uh oh!

stefanofiorentino commented Jun 30, 2021 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stefanofiorentino left a comment

Choose a reason for hiding this comment

Uh oh!

skypjack commented Jul 4, 2021

Uh oh!

moodyhunter commented Jul 4, 2021

Uh oh!

stefanofiorentino commented Jul 27, 2021

Uh oh!

moodyhunter commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skypjack commented Jul 28, 2021

Uh oh!

moodyhunter commented Jul 30, 2021

Uh oh!

stefanofiorentino commented Jul 30, 2021

Uh oh!

skypjack commented Jul 31, 2021

Uh oh!

stefanofiorentino commented Jul 31, 2021

Uh oh!

stefanofiorentino commented Oct 19, 2021

Uh oh!

stefanofiorentino commented Oct 30, 2021

Uh oh!

moodyhunter commented Oct 30, 2021

Uh oh!

skypjack commented Nov 3, 2021

Uh oh!

moodyhunter commented Nov 4, 2021

Uh oh!

stefanofiorentino commented Nov 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

moodyhunter commented Jun 30, 2021 •

edited

Loading

stefanofiorentino commented Jun 30, 2021 via email •

edited

Loading

moodyhunter commented Jul 28, 2021 •

edited

Loading

stefanofiorentino commented Nov 4, 2021 •

edited

Loading