On 15.01.26 06:51, Srirama Kucherlapati wrote:
This patch appears to be incomplete.  It references a file mkldexport.sh
but that file does not exist and is not included in the patch.

The initial patch contained only Meson-specific changes addressing the comments provided regarding Meson in this discussion. https://www.postgresql.org/message-id/ DF3QX1B91OKO.182K0IH9QDQUY%40partin.io <https://www.postgresql.org/ message-id/DF3QX1B91OKO.182K0IH9QDQUY%40partin.io>

As requested, I have now included the mkldexport.sh changes in the attached file. We want to initially close all the meson review comments.

Please review the changes and let me know your thoughts or if any further adjustments are needed.

I took this idea of disabling static libraries in meson and made it a separate patch; see [0]. It looks like this patch is getting close to consensus, so we could commit it soon. Then you could rebase your patch over it, which would make it quite a bit simpler.

I think in general, the meson changes are ok. But I needed some changes, for example, your patch contains

+if not dep_static_lib.disabled()

but the method .disabled() doesn't exist, it should be .found(). So I'm wondering how this patch was tested.

Another patch of interest to you could be [1], which moves the MAXIMUM_ALIGNOF computation into c.h. This should also simplify your patch. But that patch has not received any discussion so far.

In any case, you should post complete patch series. It's ok to split changes into multiple patches, and then recommend which parts you want reviewed first. But we need to see at least a rough outline of the complete plan before spending significant effort on reviewing the pieces.


[0]: https://www.postgresql.org/message-id/e8aa97db-872b-4087-b073-f296baae948d%40eisentraut.org

[1]: https://www.postgresql.org/message-id/58cedbc7-5658-468d-868e-a4d06de04ca6%40eisentraut.org


Reply via email to