1fanwang opened a new pull request, #7614:
URL: https://github.com/apache/datafusion-comet/pull/7614

   ## Which issue does this PR close?
   
   <!--
   We generally require a GitHub issue to be filed for all bug fixes and 
enhancements and this helps us generate change logs for our releases. You can 
link an issue to this PR using the GitHub syntax. For example `Closes #123` 
indicates that this PR will close issue #123.
   -->
   
   Closes #7609 (https://github.com/apache/datafusion-comet/issues/7609)
   
   ## Rationale for this change
   
   <!--
    Why are you proposing this change? If this is already explained clearly in 
the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your 
changes and offer better suggestions for fixes.
   -->
   
   Rust 1.100 makes Comet's native UDF bridge fail to load a valid test 
library: the path points under target/debug/build although Cargo places the 
library in target/debug. Cargo's split build-directory layout made the 
fixed-depth path calculation select the wrong directory.
   
   ## What changes are included in this PR?
   
   <!--
   There is no need to duplicate the description in the issue here but it is 
sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   The build script now derives the library directory from Cargo's build layout 
instead of assuming a fixed directory depth. Regression tests cover both 
layouts, custom profiles and target triples, and an invalid path.
   
   ## How are these changes tested?
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are 
they covered by existing tests)?
   -->
   
   I ran the same native UDF loader smoke test against unmodified main at 
https://github.com/apache/datafusion-comet/commit/c09d8e857892432c41c3a2c16d14dfa0dc35d6ee
 and this fix at 
https://github.com/apache/datafusion-comet/commit/5dc241ab45776a990f40a47ec692d047e9eaf9ab,
 using Rust 1.100.0 and Temurin JDK 27 on macOS. The reproducer source is below.
   
   ```shell
   cd native
   export JAVA_HOME="$(/usr/libexec/java_home -v 27)"
   export DYLD_LIBRARY_PATH="$JAVA_HOME/lib/server"
   cargo +1.100.0 build -p comet-test-udfs
   cargo +1.100.0 run -p datafusion-comet --example comet_test_udf_smoke
   ```
   
   Before the fix, the real loader failed on the wrong path:
   
   ```text
   failed to open .../native/target/debug/build/libcomet_test_udfs.dylib
   BASELINE_SMOKE_EXIT=101
   ```
   
   After the fix, it loaded the exported UDF from the profile directory:
   
   ```text
   loaded and validated add_one_c from 
.../native/target/debug/libcomet_test_udfs.dylib
   exit code: 0
   ```
   
   <details>
   <summary>Reproducer source: comet_test_udf_smoke.rs</summary>
   
   ```rust
   fn main() {
       let path = env!("COMET_TEST_UDFS_LIB");
       comet::comet_native_udf_bridge::validate_native_udf_library(
           path,
           "add_one_c",
           true,
           "",
       )
       .expect("test UDF library should load");
       println!("loaded and validated add_one_c from {path}");
   }
   ```
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to