Merged #2469 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#event-9091587159
You are receiving this because you are subscribed to this thread.
Message ID:
___
Rpm-maint
Looks nice and the "heavy lifting" of actually locating Lua is still somebody
elses headache.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1521673440
You are receiving this because you are subscribed to this thread.
OK, reworked to an imported target.
Note that we can't use `IMPORTED_LOCATION` as with e.g. libmagic since there
can be multiple libraries to link against, all available via `LUA_LIBRARIES`.
Instead, define an "imported interface" which seems like a better fit, anyway.
I'll probably rework
@dmnks pushed 2 commits.
e2be683d34c5aab1cf17ae3957a59780446a1130 Use CMake Lua module
ac142060b907ddeccaf844f8fc2e4ce3e9e1c603 Fix leftover "lua.h" import from days
long gone
--
View it on GitHub:
> Have we tried advocating to Lua upstream to include a pkgconfig file
> recently? If not, maybe that's worth doing, and then we can keep relying on
> pkgconfig.
Feel free.
--
Reply to this email directly or view it on GitHub:
Yup, we may suggest that upstream. In the interim, until/if that's implemented
on their side, let's just go with the custom imported target as mentioned
above. I'll update this PR accordingly.
--
Reply to this email directly or view it on GitHub:
Have we tried advocating to Lua upstream to include a pkgconfig file recently?
If not, maybe that's worth doing, and then we can keep relying on pkgconfig.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1501028061
You
Just realized we "always" relied on pkg-config for Lua so this is not a
regression and no need to rush it into 4.19 alpha, so milestone postponed.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1499018933
You are
We're not planning to write our own FindLua, just wrap it with an import
target. It would be nice to use pkg-config where available but, doing that
tends to complicate things instead of making it easier. IFF it's possible to
ALIAS that points to either our own target or the pkgconfig one, then
If we're doing our own FindLua module, can we still leverage pkgconfig if it's
available?
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1498748502
You are receiving this because you are subscribed to this thread.
> It seems pretty simple, see (otherwise unrelated) #2472
Oh, thanks! I'll just do the same here, then.
> I'm starting to think we should make this a policy of handling external
> libraries as imported targets everywhere, it's so much nicer...
Yup, seems reasonable and keeps the cmake files
It seems pretty simple, see (otherwise unrelated) #2472
I'm starting to think we should make this a policy of handling external
libraries as imported targets everywhere, it's so much nicer...
--
Reply to this email directly or view it on GitHub:
> Other than that... why oh why doesn't the FindLua module implement an IMPORT
> interface
Yeah, I was quite disappointed to learn that, too. The actual "user interface"
is thus no prettier than a plain `find_library()` approach.
> We could do that by ourselves though, imported interfaces are
> > Fix #include lines to use "lua.h" instead of as per
> > cmake-modules(7):
>
> The actual issue in the FindLua doc is cautioning against `#include
> ` because of the `lua/` _directory_ is not standardized. The
> guidance to use "lua.h" seems quite odd, as quotes imply per-project files
Other than that... why oh why doesn't the FindLua module implement an IMPORT
interface :facepalm:
We could do that by ourselves though, imported interfaces are so much nicer to
use for the rest of the build, especially as Lua is used and linked from many
different places. Here's an example of
> Fix #include lines to use "lua.h" instead of as per cmake-modules(7):
The actual issue in the FindLua doc is cautioning against `#include
` because of the `lua/` *directory* is not standardized. The
guidance to use "lua.h" seems quite odd, as quotes imply per-project files
rather than
Filed #2470 now so we don't forget again.
Until that gets sorted out, if you suspect an intermittent random failure
(including network timeouts and whatnot), just hit "rebuild this version" on
Semaphore.
--
Reply to this email directly or view it on GitHub:
> It's the absence of the following error in test 423 (Dependency generation
> 3), has anyone seen this before?
>
> ```
> error: Illegal char '*' (0x2a) in: *
> ```
I know I have, but always ended up brushing it aside from something "more
important".
So this is NOT a new thing, in fact it's
... and the same test passes on the very same VM that the CI job ran on (I've
checked manually using the SSH feature in Semaphore). Seems like we've got a
new random false negative in our test suite... :cry:
--
Reply to this email directly or view it on GitHub:
It's this error in test 423 (Dependency generation 3), has anyone seen this
before?
```
error: Illegal char '*' (0x2a) in: *
```
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1497698026
You are receiving this because
Hmm, so the CI failure doesn't seem to have anything to do with this commit.
The same podman image passes just fine for me locally...
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2469#issuecomment-1497696590
You are receiving this
I'm not entirely sure about the `#include ` vs. `#include "lua.h"` thing
here. It does seem more appropriate and consistent to use the bracket style for
external libraries. Let me know your thoughts, I can drop that part from the
commit if needed :smile:
--
Reply to this email directly or
While many distros ship a pkg-config file for Lua downstream, the upstream
source tree does not, and so we shouldnt rely on it. Turns out, CMake
provides a native Lua package, so just use that.
Unfortunately, the package doesnt define any IMPORTED target, it only does
the LUA_LIBRARIES and
23 matches
Mail list logo
