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.

Sign up for GitHub

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

[asmjit] Update to 2024-06-05 #39090

Closed
wants to merge 4 commits into from
Closed

[asmjit] Update to 2024-06-05 #39090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2351722
[asmjit] Update to 2024-06-05
kobalicek Jun 5, 2024
e8deb41
Update version file
stevemk14ebr Jun 3, 2024
23f6ed7
Update vcpkg for asmjit package update
stevemk14ebr Jun 3, 2024
8f9112b
Removed arm support from fbgemm as it's not a supported platform
kobalicek Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions ports/asmjit/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO asmjit/asmjit
REF 51b10b19b6631434d3f9ad536a6fb140944a36d2 # commited on 2023-03-25
SHA512 1fba5159d2adad64e9a2b07a1f90de6988d1da47b9802ca8b57c61a89d8a90924525f6d0d6607279994bdbadcf693b2cc96cd7e4bf7f018ad64127b640dc38fb
REF 4a61c23ab65a8feb58d1bcdb7cbe8c8bdc36b7d1 # commited on 2024-06-05
SHA512 ccc04655d031bd5a89d217aac85f6a8bdb756684257877de8df2162b2ec6e0d95cd7e01c0d46f817465fccb0fa52a4552d5034c48f361f2854f2e05aa32b7c89
HEAD_REF master
)

Expand All @@ -16,16 +16,7 @@ vcpkg_cmake_configure(

vcpkg_cmake_install()
vcpkg_copy_pdbs()

vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/asmjit)

file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

if(VCPKG_LIBRARY_LINKAGE STREQUAL "static")
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/include/asmjit/core/api-config.h"
"#if !defined(ASMJIT_STATIC)"
"#if 0"
)
endif()
Comment on lines -24 to -29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @dg0yt pointed out, these lines are necessary for the library to work properly for MSBuild users and shouldn't affect the package adversely.

Copy link
Contributor Author

@kobalicek kobalicek Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you elaborate?

Are msbuild users not able to setup compile-time macros? If anyone wants statically linked AsmJit there is ASMJIT_STATIC macro that does exactly that and this is a documented behavior (basically a contract).

So, are you suggesting that ALL vcpkg packages patch source code to define/undefine a visibility macro? Because I don't see that in other packages, so I don't think AsmJit should be an exception here.

What I'm trying to do is to have vanilla AsmJit on vcpkg - not patched, 100% upstream. The cmake build system works correctly and dependents link properly. So far I haven't seen any non-cmake requirement in vcpkg documentation. There are no tests in vcpkg registry that would explicitly test msbuild without cmake integration. So, I'm really confused about this.

Moreover, if there is any extra requirement that ALL ports must follow, please link me the relevant documentation so I can at least read about those requirements - I haven't found any, the whole vcpkg build system is based on cmake, and AsmJit's cmake integration works as advertised.

AsmJit is not a difficult project to compile - it doesn't even need a build system, and the ASMJIT_STATIC macro is the only thing that must be defined in order to consume it statically. I think that if you have a custom msbuild project, that consumes dependencies from vcpkg but totally ignores the configuration, this is your burden, and not vcpkg port burden (or in other words - my burden).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I don't see that in other packages, so I don't think AsmJit should be an exception here.

look again. (aws-sdk-cpp; cjson; freerdp; graphviz; hunspell; libheif, etc)

What I'm trying to do is to have vanilla AsmJit here - not patched, 100% upstream

Not possible if there are visibility macros

I know that this patching is not right

Incorrect. The patching is correct since vcpkg always only installs one type (static/shared) of a library. Downstream consumers shouldn't need to know if something is build static or shared. However, the macro needs additional knowledge of that otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please link me the documentation that describes this behavior first. I want to read about this flaw.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole vcpkg build system is based on cmake,

Vcpkg uses cmake as a scripting language, and it provides some integration.

But there are many ports which do not provide CMake packages. Some provide only pkg-config. Some provide nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I just don't like this situation.

They want from me to do something that is not documented nor stated anywhere and I dislike it very much from a maintainer's perspective. And all of this is about some phantom "MSBuild users" that need flawless integration with vcpkg that means patching source code of other projects... But where are they? Can at least one MSBuild user explain to me why adding ASMJIT_STATIC to MSBuild file and thus following the AsmJit contract is such a big deal?

I feel this is just very hypothetical and I'm being nitpicked to support a scenario that doesn't exist in practice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kobalicek: #20936 found via git blame.
asmjit installs wrong headers since it only allows building either static or shared as such the installed headers should reflect that. There is no reason to have a magic define for downstream users which is not necessary since it is decided at build time which library will be installed and thus used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by installing wrong headers? The headers are the same in both configurations!

I think that the issue above clearly describes the problem. Instead of helping the user to properly setup his project to consume the library, the port was patched instead. However, this essentially did create a problem which I'm now trying to undo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headers are the same in both configurations!

No they are not. You are just hiding that fact with a preprocessor define. If they would be the same you wouldn't need the preprocessor to change them conditionally. (which on windows even changes the symbol names!). This is conceptually the same as having configure_file in cmake and directly install the header with the correct defines set or have the cmake build use https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html.

However, this essentially did create a problem which I'm now trying to undo.

There is nothing to undo. You just don't like the solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, MSBuild is an important customer that doesn't understand CMake configs but it is far from the only customer that doesn't understand them. Plain make or similar downstream customers prefer not needing to replicate settings that are known at library build time too.


vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE.md")
7 changes: 4 additions & 3 deletions ports/asmjit/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
{
"name": "asmjit",
"version-date": "2023-03-25",
"description": "Complete x86/x64 JIT and Remote Assembler for C++",
"version-date": "2024-06-05",
"description": "AsmJit is a lightweight library for machine code generation written in C++ language",
"homepage": "https://github.com/asmjit/asmjit",
"supports": "!arm",
"documentation": "https://asmjit.com/doc/index.html",
"license": "Zlib",
"dependencies": [
{
"name": "vcpkg-cmake",
Expand Down
3 changes: 2 additions & 1 deletion ports/fbgemm/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
{
"name": "fbgemm",
"version": "0.4.1",
"port-version": 1,
"description": "FB (Facebook) + GEMM (General Matrix-Matrix Multiplication)",
"homepage": "https://code.fb.com/ml-applications/fbgemm/",
"license": "BSD-3-Clause",
"supports": "!(x86 | uwp)",
"supports": "!arm & !x86 & !uwp",
"dependencies": [
"asmjit",
"cpuinfo",
Expand Down
66 changes: 0 additions & 66 deletions ports/polyhook2/fix-dep.patch
View file
Open in desktop

This file was deleted.

5 changes: 2 additions & 3 deletions ports/polyhook2/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO stevemk14ebr/PolyHook_2_0
REF fd2a88f09c8ae89440858fc52573656141013c7f
SHA512 09eef1b0ca75f9420fa258c4290b6b62a3115531e77c91f9d223b2c4a10e8ee95b243bf9f6598d7c55b76ca4e4a185cdf1231b40d5ef87d7d405b8b434eb6cb0
REF 8cd6cb4ef0f2a599f35bdd46ac5833843aea6523
SHA512 93e6fcbb47f4b894c325766e1b2c146cebfe73424473ce0f52c8c923bd5b1276aebb3af03da9fc7027e2027994663c47a5a4f5a4f3c4a997a322886ac43401ec
HEAD_REF master
PATCHES fix-dep.patch
)

vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
Expand Down
2 changes: 1 addition & 1 deletion ports/polyhook2/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "polyhook2",
"version-date": "2024-02-08",
"version-date": "2024-06-03",
"description": "C++17, x86/x64 Hooking Library v2.0",
"homepage": "https://github.com/stevemk14ebr/PolyHook_2_0",
"license": "MIT",
Expand Down
5 changes: 5 additions & 0 deletions versions/a-/asmjit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "6e65362a730457a3168f8a7ad59358472eed0fcd",
"version-date": "2024-06-05",
"port-version": 0
},
{
"git-tree": "e226149f3b22da2162e7d93eacc9f19411401e62",
"version-date": "2023-03-25",
Expand Down
6 changes: 3 additions & 3 deletions versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@
"port-version": 7
},
"asmjit": {
"baseline": "2023-03-25",
"baseline": "2024-06-05",
"port-version": 0
},
"asmtk": {
Expand Down Expand Up @@ -2662,7 +2662,7 @@
},
"fbgemm": {
"baseline": "0.4.1",
"port-version": 0
"port-version": 1
},
"fbthrift": {
"baseline": "2024.05.27.00",
Expand Down Expand Up @@ -6905,7 +6905,7 @@
"port-version": 12
},
"polyhook2": {
"baseline": "2024-02-08",
"baseline": "2024-06-03",
"port-version": 0
},
"polymorphic-value": {
Expand Down
7 changes: 6 additions & 1 deletion versions/f-/fbgemm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
{
"versions": [
{
"git-tree": "e19b797fb42fad6dae0f9aca0890ddc62535800f",
"git-tree": "27b4707b88bb2ed527d372438c406d1a97323229",
"version": "0.4.1",
"port-version": 1
},
{
"git-tree": "2aa09f4d6344a0c379f6b65fae72dc8117759805",
"version": "0.4.1",
"port-version": 0
},
Expand Down
5 changes: 5 additions & 0 deletions versions/p-/polyhook2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "6039d71effaafe4610f614a64d90c84f386fa23b",
"version-date": "2024-06-03",
"port-version": 0
},
{
"git-tree": "2380bd82c3b07238558d4cfb7a2a38011b75faf5",
"version-date": "2024-02-08",
Expand Down
Loading