fix: fix complex cargo struct project build error#1
fix: fix complex cargo struct project build error#1oowl wants to merge 2 commits intomlua-rs:mainfrom
Conversation
Signed-off-by: oowl <[email protected]>
|
Thanks! Also, would setting |
|
@khvzak I am not just want to resolve CI build failed if i want to |
oowl
changed the title
| end | ||
|
|
||
| table.insert(cmd, "cargo build --release --features " .. table.concat(features, ",")) | ||
| table.insert(cmd, "CARGO_TARGET_DIR=./target cargo build --release --features " .. table.concat(features, ",")) |
There was a problem hiding this comment.
Regarding @khvzak's concern:
My only concern is when the env forcely set, it would not inherit external defs.
What do we know / can we assume about the shell this command will run in? If it's a POSIX shell we can avoid overriding an existing variable while still setting it if not:
| table.insert(cmd, "CARGO_TARGET_DIR=./target cargo build --release --features " .. table.concat(features, ",")) | |
| table.insert(cmd, "CARGO_TARGET_DIR=${CARGO_TARGET_DIR:=./target} cargo build --release --features " .. table.concat(features, ",")) |
There was a problem hiding this comment.
But in opendal use case, we do not just want to avoid overriding it , the root case is https://github.com/khvzak/luarocks-build-rust-mlua/blob/main/src/luarocks/build/rust-mlua.lua#L74 can not make sure this path is correct
There was a problem hiding this comment.
Oh, I know your mean, That makes sense. we can not pollute some env.
There was a problem hiding this comment.
But I still think we need to resolve the path problem, make luarocks-build-rust-mlua find correct target path, my PR seems like some tricks
There was a problem hiding this comment.
One can find the correct target path, but it's a PITA. If you have something to parse JSON available, you can query cargo for it:
$ cargo test -q --no-run --message-format=json | jq 'select(.reason == "compiler-artifact" and .target.test and (.target.kind | index("bin"))) | .executable'Obviously adjust for release or other builds and for just getting the directory if you won't want the final executable name.
There was a problem hiding this comment.
I made a PR with a proposed fix: #2
Instead of rewriting CARGO_TARGET_DIR, can you set the correct path in rockspec file?
3 participants
use the current directory as the cargo build target. If you want to know more information, please check apache/opendal#2558