Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pugixml to v1.15 #6166

Merged
merged 15 commits into from
Jan 20, 2025
Merged

Update pugixml to v1.15 #6166

merged 15 commits into from
Jan 20, 2025

Conversation

luadebug
Copy link
Contributor

@luadebug luadebug commented Jan 15, 2025

zeux/pugixml@v1.11.4...v1.12
I think STATIC_CRT renamed to PUGIXML_STATIC_CRT, but it seems xmake tricky enough to pass it, is it truth?

"Again WASM, sorry but cure is only this..."

idk upon PUGIXML_API

        if package:config("exports") and package:is_plat("windows") then
            if package:config("shared") then
                io.replace("src/pugiconfig.hpp", "// #define PUGIXML_CLASS __declspec(dllimport)", "#define PUGIXML_CLASS __declspec(dllimport)", {plain = true})
            else
                io.replace("src/pugiconfig.hpp", "// #define PUGIXML_API __declspec(dllexport)", "#define PUGIXML_API __declspec(dllexport)", {plain = true})
            end
        end

@star-hengxing
Copy link
Contributor

Don't worry, xmake will pass CMAKE_MSVC_RUNTIME_LIBRARY flag.
https://github.com/zeux/pugixml/blob/9d7fcbf7410e20720b093f9bb3d81dfe6500d30d/CMakeLists.txt#L66-L69

This package missing some important option, exceptions, wchar_t mode, PUGIXML_API etc...Can you fix it?

@luadebug
Copy link
Contributor Author

Don't worry, xmake will pass CMAKE_MSVC_RUNTIME_LIBRARY flag. https://github.com/zeux/pugixml/blob/9d7fcbf7410e20720b093f9bb3d81dfe6500d30d/CMakeLists.txt#L66-L69

This package missing some important option, exceptions, wchar_t mode, PUGIXML_API etc...Can you fix it?

Sorry Im too confused for PUGIXML_API 🥴🥴🥴

@luadebug
Copy link
Contributor Author

luadebug commented Jan 16, 2025

@star-hengxing any news?
well if some options need to be default false feel free to change such as headeronly or something else %_%

@star-hengxing
Copy link
Contributor

@star-hengxing any news? well if some options need to be default false feel free to change such as headeronly or something else %_%

The behaviour of the previous package was non-header only. To avoid breaking downstream users, it is best to set it to false by default.

@luadebug
Copy link
Contributor Author

luadebug commented Jan 16, 2025

@star-hengxing any news? well if some options need to be default false feel free to change such as headeronly or something else %_%

The behaviour of the previous package was non-header only. To avoid breaking downstream users, it is best to set it to false by default.

I am free to rely your choice upon picking defaults. I hope wchar wont affect, this. If it would turn it off as well.
It looks like a toggle that is turned off by default so it is supposed and should be off, since it is a toggle between utf8 interfaces.
LGTM.

@star-hengxing star-hengxing merged commit 376edcc into xmake-io:dev Jan 20, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants