[bug#53015,2/4] gnu: spdlog: Build with '-fpic'.

Empty Message

Hello,

This patch (2/4 build option for spdlog, part of the MangoHud patch series) may be superseded by https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b1542d59606919d0da04914fa6916b85354e2f89  (CCing Nicolas, hope that's okay!) I've built MangoHud with the shared spdlog before, seems to work.

However, given what spdlog has in its documentation, I wonder if it would be better to default to the static version as before? I think the "-fpic" build option would also work for shared linking in general? Or does that not work for nheko, which this change was for I believe?

I'm not really sure though, nor have a strong opinion, just going based on spdlog's info and that some projects (like MangoHud) build spdlog as a static library by default when bundling. I'm guessing it is for performance reasons, though I don't know if that is realized.

John

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

> Hello,
>
> This patch (2/4 build option for spdlog, part of the MangoHud patch
> series) may be superseded by
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b1542d59606919d0da04914fa6916b85354e2f89
> (CCing Nicolas, hope that's okay!) I've built MangoHud with the shared
> spdlog before, seems to work.

That should solve the problem, yes!

> However, given what spdlog has in its documentation, I wonder if it
> would be better to default to the static version as before? I think
> the "-fpic" build option would also work for shared linking in
> general? Or does that not work for nheko, which this change was for I
> believe?

Do you mean this:
https://github.com/gabime/spdlog/wiki/How-to-use-spdlog-in-DLLs ?

> I'm not really sure though, nor have a strong opinion, just going
> based on spdlog's info and that some projects (like MangoHud) build
> spdlog as a static library by default when bundling. I'm guessing it
> is for performance reasons, though I don't know if that is realized.

Typically in Guix the libraries are offered as shared objects by
default; and when desirable a static output can be added to which the
static objects are copied.

You could add a static output if you have such a need :-).

Maxim

Hi Maxim,

------- Original Message -------

On Sunday, February 20th, 2022 at 11:11 PM, Maxim Cournoyer wrote:

> Hi John,
>
> John Kehayias john.kehayias@protonmail.com writes:
>
> > Hello,
> >
> > This patch (2/4 build option for spdlog, part of the MangoHud patch
> > series) may be superseded by
> > https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b1542d59606919d0da04914fa6916b85354e2f89
> > (CCing Nicolas, hope that's okay!) I've built MangoHud with the shared
> > spdlog before, seems to work.
>
> That should solve the problem, yes!
>
> > However, given what spdlog has in its documentation, I wonder if it
> > would be better to default to the static version as before? I think
> > the "-fpic" build option would also work for shared linking in
> > general? Or does that not work for nheko, which this change was for I
> > believe?
>
> Do you mean this:
> https://github.com/gabime/spdlog/wiki/How-to-use-spdlog-in-DLLs ?
>

Yes, that's what I meant. I'm not familiar with spdlog, but was just going based on that and seeing packages build a static spdlog to use.

> > I'm not really sure though, nor have a strong opinion, just going
> > based on spdlog's info and that some projects (like MangoHud) build
> > spdlog as a static library by default when bundling. I'm guessing it
> > is for performance reasons, though I don't know if that is realized.
>
> Typically in Guix the libraries are offered as shared objects by
> default; and when desirable a static output can be added to which the
> static objects are copied.
>
> You could add a static output if you have such a need :-).
>

Got it. Personally, it doesn't matter much to me, I'm only assuming projects use the static version due to what upstream says and/or for performance reasons (no idea if that is realized though).

In the interest of moving this forward, I'm definitely fine leaving spdlog as it was updated. We could add a note that upstream and other projects prefer a static build and leave that to add as a TODO. I did take a stab at it some time ago, but it got slightly messy in trying to do it all together (I suppose I could just run the build a second time, but I think it also required modifications to other files to make the static build work separately? I forget now.)

Any thoughts on the imgui details? For me, I'd go for keeping both current (old) dear-imgui as hidden for ogre, adding the one I have here (hidden too? though useful Debian additions in the build) for mangohud, and consolidating to one up to date version once everything can use that. We could use the imgui you added for that, or wait for the more complete Debian build files we are using for dear-imgui right now (the pkg-config, for instance, is helpful).

Finally, the pciutils hardware data pci.ids came up in a new patch: https://issues.guix.gnu.org/54069 I commented on that issue about doing something similar here. I think we can figure out what to do in a core updates change (due to pciutils dependents). In the meantime, a pciutils variant and/or a separate hwdata package would be helpful. Funny enough, this is the last non-trivial change in this patch series, brought up by someone else. I guess the repeated work shows this would be useful though.

Would love to finally get this merged, with whatever changes we decide for imgui and pciutils. WDYT?

Thanks!
John

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

[...]

> Any thoughts on the imgui details? For me, I'd go for keeping both
> current (old) dear-imgui as hidden for ogre, adding the one I have
> here (hidden too? though useful Debian additions in the build) for
> mangohud, and consolidating to one up to date version once everything
> can use that. We could use the imgui you added for that, or wait for
> the more complete Debian build files we are using for dear-imgui right
> now (the pkg-config, for instance, is helpful).

I sorted out the imgui vs dear-imgui mess I had created yesterday by
upgrading our 'ogre' package to its latest version and using imgui 1.86
for it (it wouldn't work with 1.87).

So now we have imgui@1.87 and imgui@1.86 available.  I suggest you try
one of these and if it fails you can try adding yet another public
version for it.

I've looked at the pciutils issue; it seems suboptimal that pciutils
support having the database compressed yet doesn't provide an easy API
to look things from it (or perhaps it does and this is what mangohud
should use?  to be investigated).

I'll try to have a look later into pciutils and whether it offers some
kind of API that users of the DB should use instead of manually parsing
their way through :-).

Thanks,

Maxim