UE4SS-RE / RE-UE4SS

Injectable LUA scripting system, SDK generator, live property editor and other dumping utilities for UE4/5 games
http://docs.ue4ss.com/
MIT License
1.39k stars 187 forks source link

Introduction of the cargo.build rule to manage patternsleuth as a target #516

Closed bitonality closed 6 months ago

bitonality commented 6 months ago

# WARNING

This change is waiting on a fix to be released in xmake v2.9.2 https://github.com/xmake-io/xmake/pull/5034

I'm downgrading this to a draft until users can run this script with a stable release of xmake.

If you want to test it, you can xmake update -s dev which will update your xmake version with the latest dev scripts.

PR NOTES

Continuation of previous discussions in which we decided to build patternsleuth as a cargo target.

https://github.com/UE4SS-RE/RE-UE4SS/pull/384 https://github.com/UE4SS-RE/RE-UE4SS/pull/494

This promotes patternsleuth to its own target that can be built with xmake build patternsleuth.

It also leverages cargo's incremental build and parses the cargo .d files for robust detection if rebuilds are needed or not. I also cache the release/debug dependency crates to allow for faster builds across modes on the same machines/CI machines.

xmake clean patternsleuth xmake build patternsleuth xmake build --rebuild patternsleuth etc...

This also fixes somewhat of a glaring oversight in the cargo package pipeline involving msvcrt.lib and msvcrtd.lib. The cargo rule I wrote has the logic for detecting windows runtimes and correctly linking them.

This is also an improvement over the cargo package pipeline since I leveraged cargo to report precisely what libraries it expects to be linked with instead of us guessing upstream.

We now can remove

add_links("ws2_32", "advapi32", "userenv", "ntdll", "oleaut32", "bcrypt", "ole32", { public = true })

because cargo now directly reports what it needs for linking and we automatically add those flags at link time. This should help with cross compatibility since we won't have to generate a magical linking matrix for specific platforms/etc.

I'm also happy with the generic Cargo.toml solution I implemented because we can very easily add more pure-rust Cargo packages into the build system with very little additional work. The cargo specific logic has all been abstracted away in the rust_rules.lua file and people should only have to write vanilla xmake definitions.

Buckminsterfullerene02 commented 6 months ago

@bitonality

[ 87%]: linking.GameShippingWin64 ArgsParser.exe error: cannot open file: Intermediates.deps\patternsleuth_bind\windows\x64\GameShippingWin64\native-libs.txt, No such file or directory

bitonality commented 6 months ago

@bitonality

[ 87%]: linking.GameShippingWin64 ArgsParser.exe error: cannot open file: Intermediates.deps\patternsleuth_bind\windows\x64\GameShippingWin64\native-libs.txt, No such file or directory

Did it look like your build hit the incremental cache at any point? If you try xmake build --rebuild do you get different results?

Buckminsterfullerene02 commented 6 months ago

@bitonality [ 87%]: linking.GameShippingWin64 ArgsParser.exe error: cannot open file: Intermediates.deps\patternsleuth_bind\windows\x64\GameShippingWin64\native-libs.txt, No such file or directory

Did it look like your build hit the incremental cache at any point? If you try xmake build --rebuild do you get different results?

First time started at 62% when usually I think when it starts incremental build from like 50% ish?

[ 87%]: linking.GameShippingWin64 ArgsParser.exe [ 91%]: linking.GameShippingWin64 patternsleuth_bind.lib [ 91%]: archiving.GameShippingWin64 LuaMadeSimple.lib [ 91%]: archiving.GameShippingWin64 DynamicOutput.lib [ 91%]: archiving.GameShippingWin64 ParserBase.lib [ 91%]: linking.GameShippingWin64 proxy_generator.exe error: error: crate name ` passed to--extern` is not a valid ASCII identifier

bitonality commented 6 months ago

@bitonality [ 87%]: linking.GameShippingWin64 ArgsParser.exe error: cannot open file: Intermediates.deps\patternsleuth_bind\windows\x64\GameShippingWin64\native-libs.txt, No such file or directory

Did it look like your build hit the incremental cache at any point? If you try xmake build --rebuild do you get different results?

First time started at 62% when usually I think when it starts incremental build from like 50% ish?

[ 87%]: linking.GameShippingWin64 ArgsParser.exe [ 91%]: linking.GameShippingWin64 patternsleuth_bind.lib [ 91%]: archiving.GameShippingWin64 LuaMadeSimple.lib [ 91%]: archiving.GameShippingWin64 DynamicOutput.lib [ 91%]: archiving.GameShippingWin64 ParserBase.lib [ 91%]: linking.GameShippingWin64 proxy_generator.exe error: error: crate name ` passed to--extern` is not a valid ASCII identifier

I think I caught an edge case for people updating with existing cached config/incremental.

Could you pull latest and see if it works with just regular xmake build and if that has issues, can you try xmake build --rebuild

Buckminsterfullerene02 commented 6 months ago

I still get the same error (with --rebuild)

bitonality commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Buckminsterfullerene02 commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Same error.

bitonality commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Same error.

Oh shoot. Hadn't realized that I change I made in xmake hasn't been shipped yet... https://github.com/xmake-io/xmake/pull/5034

If you want to test against the dev scripts, you can update your scripts only by running xmake update -s dev

No worries if not. I guess I'll have to pump the brakes on this change until xmake v2.9.2 releases.

UE4SS commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Same error.

Oh shoot. Hadn't realized that I change I made in xmake hasn't been shipped yet... xmake-io/xmake#5034

If you want to test against the dev scripts, you can update your scripts only by running xmake update -s dev

No worries if not. I guess I'll have to pump the brakes on this change until xmake v2.9.2 releases.

I think you should edit the build requirements in this PR to state the required xmake version in that case. I just noticed we don't currently mention which version is required, and clearly this change will require a particular version or greater and people need to know this.

bitonality commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Same error.

Oh shoot. Hadn't realized that I change I made in xmake hasn't been shipped yet... xmake-io/xmake#5034 If you want to test against the dev scripts, you can update your scripts only by running xmake update -s dev No worries if not. I guess I'll have to pump the brakes on this change until xmake v2.9.2 releases.

I think you should edit the build requirements in this PR to state the required xmake version in that case. I just noticed we don't currently mention which version is required, and clearly this change will require a particular version or greater and people need to know this.

I plan on enforcing the minimum required with the project-wide set_xmakever(). I just need to wait for that version to exist :P

bitonality commented 6 months ago

I still get the same error (with --rebuild)

How about xmake clean --all and then xmake build?

Same error.

Oh shoot. Hadn't realized that I change I made in xmake hasn't been shipped yet... xmake-io/xmake#5034 If you want to test against the dev scripts, you can update your scripts only by running xmake update -s dev No worries if not. I guess I'll have to pump the brakes on this change until xmake v2.9.2 releases.

I think you should edit the build requirements in this PR to state the required xmake version in that case. I just noticed we don't currently mention which version is required, and clearly this change will require a particular version or greater and people need to know this.

I plan on enforcing the minimum required with the project-wide set_xmakever(). I just need to wait for that version to exist :P

image Example output for what that looks like from a user pov

bitonality commented 6 months ago

xmake v2.9.2 is now publicly available, so I'm resurrecting this PR with bolstered documentation.

bitonality commented 6 months ago

The VS changes/documentation here likely resolves #526 Need a couple of more testers to confirm

UE4SS commented 6 months ago

The VS changes/documentation here likely resolves #526 Need a couple of more testers to confirm

I tested xmake project -k vsxmake2022 -m "Game__Debug__Win64,Game__Shipping__Win64" -y, and configuration succeeded, and building both also succeeded. This will definitely make the development experience less annoying for me, thanks.

bitonality commented 6 months ago

Accidentally approved, didn't realize the scope was bigger than I thought, I don't have the capacity to know if this PR is good enough to be merged.

I appreciate the caution. All new codepaths are behind the new --patternsleuth flag which is defaulted to be package (the current configuration in main), so the blast radius of potential breakage is small. That said, I have high confidence in this change and would like to start getting more executions on the new code path on an opt-in basis (particularly for the linux port).

narknon commented 6 months ago

@bitonality are your new commits intended to be on this PR?

bitonality commented 6 months ago

@bitonality are your new commits intended to be on this PR?

Thanks for catching that - was testing CI on my fork and messed up the branch filters.

The unrelated commits are backed out and no longer part of this change