-
Notifications
You must be signed in to change notification settings - Fork 175
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
Introduction of the cargo.build rule to manage patternsleuth as a target #516
Conversation
[ 87%]: linking.Game__Shipping__Win64 ArgsParser.exe |
Did it look like your build hit the incremental cache at any point? If you try |
First time started at 62% when usually I think when it starts incremental build from like 50% ish? [ 87%]: linking.Game__Shipping__Win64 ArgsParser.exe |
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 |
I still get the same error (with --rebuild) |
How about |
Same error. |
Oh shoot. Hadn't realized that I change I made in xmake hasn't been shipped yet... If you want to test against the dev scripts, you can update your scripts only by running 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 plan on enforcing the minimum required with the project-wide |
|
xmake v2.9.2 is now publicly available, so I'm resurrecting this PR with bolstered documentation. |
The VS changes/documentation here likely resolves #526 |
I tested |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
@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 |
# WARNINGThis change is waiting on a fix to be released in xmake v2.9.2 xmake-io/xmake#5034I'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 canxmake 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.
#384
#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
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.