Fix docs.rs

[avantgardnerio]

Which issue does this PR close?

Closes #3538 maybe.

Rationale for this change

Fix docs.rs build.

What changes are included in this PR?

Try to fix docs.rs, but no idea how to test.

Are there any user-facing changes?

Docs should work again.

[avantgardnerio]

@andygrove please review and let me know if you know how to test.

[avantgardnerio]

Not ready for merge... one minute.

[avantgardnerio]

Not ready for merge

Okay, ready.

[codecov-commenter]

Codecov Report

Merging #3580 (c7fcac2) into master (0a2b0a7) will increase coverage by 0.13%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3580      +/-   ##
==========================================
+ Coverage   85.92%   86.05%   +0.13%     
==========================================
  Files         301      300       -1     
  Lines       56249    56328      +79     
==========================================
+ Hits        48330    48475     +145     
+ Misses       7919     7853      -66     

Impacted Files	Coverage Δ
datafusion/proto/src/lib.rs	94.35% <ø> (+0.07%)	:arrow_up:
.../src/physical_plan/file_format/delimited_stream.rs	91.75% <100.00%> (ø)
datafusion/core/src/physical_plan/hash_join.rs	95.03% <100.00%> (ø)
datafusion/expr/src/logical_plan/plan.rs	77.10% <0.00%> (-1.16%)	:arrow_down:
datafusion/core/src/physical_plan/sorts/sort.rs	93.86% <0.00%> (-0.60%)	:arrow_down:
...e/src/physical_plan/sorts/sort_preserving_merge.rs	93.49% <0.00%> (-0.36%)	:arrow_down:
datafusion/expr/src/binary_rule.rs	84.31% <0.00%> (-0.29%)	:arrow_down:
datafusion/core/tests/sql/decimal.rs	100.00% <0.00%> (ø)
datafusion/core/src/execution/context.rs	79.33% <0.00%> (ø)
datafusion/physical-expr/src/expressions/binary.rs	97.63% <0.00%> (ø)
... and 22 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tustvold]

According to https://github.com/rust-lang/docs.rs/issues/147#issuecomment-648844656 we can use an environment variable instead of a feature flag, perhaps in combination with a cfg key, which would avoid this leaking into the crates public interface.

FWIW I'm not sure if any IDEs need this source hack, it was fixed for CLion a year ago (https://github.com/intellij-rust/intellij-rust/issues/1908), and so perhaps we might just revisit this decision? I'm not sure if rust-analyser has issues though? Edit: apparently rust-analyser doesn't understand (https://github.com/rust-lang/rust-analyzer/issues/3767) :sob:

[avantgardnerio]

Thanks @tustvold for the links. I'll investigate further today. It would clearly be best if we didn't have to hack our build to work-around IDE limitations.

It sounds like you have no problem with "go to definition" on protobuf generated code in CLion? I'm definitely using the latest version and it was not working for me. I assume @andygrove is in the same boat.

I'll investigate further today and document how to make it work if I figure it out. Otherwise, how would a config setting (vs feature) sound to you? I don't think an environment variable is sufficient in and of itself as we need to flip this switch at compile time.

[tustvold]

To get CLion to work you need to run a full build so that the files exist, and then "Refresh Cargo Projects". It then finds the files. There is also an experimental option "evaluate build scripts" but I haven't had a need for this.

A config flag seems fine to me, ofc better if we didn't need to, but provided it isn't externally visible, nobody needs to know 😅

I am somewhat curious what happens when consuming the crate from crates.io, does the build script generate files in the cargo "checkout"? That feels like it might cause issues

[andygrove]

To get CLion to work you need to run a full build so that the files exist, and then "Refresh Cargo Projects". It then finds the files.

I wasn't aware of this action. I just updated to CLion 2022.2.3 and tried this out with the Ballista project but I still cannot "go to definition" for the generated code.

My steps were:

• Delete .idea folder
• Open project in CLion
• Build in CLion
• Refresh Cargo Projects

Am I missing a step perhaps?

[avantgardnerio]

To get CLion to work

From what I can tell, there are two potential issues:

1. The fact that code is generated into the target/ folder, which is marked as "excluded" (for good reasons)
2. the usage of the import! macro is confusing CLion

I haven't done a test to figure out which is the issue, but given the fix listed above, I'm inclined to think it is #1. With Java, the solution is to go find the target/myfolder/generated and mark it as a "generated sources root" overriding the "exclude" on the parent folder. With Cargo, this isn't so easy as everything under the target/ folder all gets random hash looking identifiers that change periodically making it not possible to re-include them continuously.

[ursabot]

Benchmark runs are scheduled for baseline = b02753c7b4aa4f74d048b4df62b01ec30d598f94 and contender = 49b9c675abd2be96a8426038c128bb46acc76240. 49b9c675abd2be96a8426038c128bb46acc76240 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. Conbench compare runs links: [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2 [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q Buildkite builds: Supported benchmarks: ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True test-mac-arm: Supported benchmark langs: C++, Python, R ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

[tustvold]

I wasn't aware of this action. I just updated to CLion 2022.2.3 and tried this out with the Ballista project but I still cannot "go to definition" for the generated code.

I just did this and it worked correctly, although I note that the generated code it is finding is in src/generated.

To test this out I created a very basic project with the following code.

main.rs

include!(concat!(env!("OUT_DIR"), "/test.rs"));

fn main() {
    Foo {};
    println!("Hello, world!");
}

build.rs

use std::fs::File;
use std::io::Write;
use std::path::PathBuf;

fn main() {
    let path = PathBuf::from(std::env::var_os("OUT_DIR").unwrap()).join("test.rs");
    let mut file = File::create(path).unwrap();
    writeln!(file, "struct Foo {{}}").unwrap();
}

I then compiled and ran the binary, code completion was not working at this point. Running "Refresh Cargo Projects" also didn't work, nor did "Invalidate Caches and Restart".

Fortunately the fix is relatively straightforward:

• Open Help > Find Action (CTRL + SHIFT + A)
• Enter "Experimental" and select "Experimental Features"
• Scroll down to "org.rust.cargo.evaluate.build.scripts" and enable it
• File > Invalidate Caches

On restart everything should be working. I used to have this turned on, I turned it off at some point and left it off because everything continued to work, but I suspect this was the result of some caching shenanigans. Either way I think the above should work, let me know. Either way until rust-analyser fixes support for this, I suspect we will need to continue to work around it, but perhaps framing it as a rust-analyser limitation might encourage some movement towards fixing it.

Edit: I also created https://github.com/intellij-rust/intellij-rust/issues/9402 to potentially get some feedback from the plugin maintainers

[avantgardnerio]

@tustvold awesome work, thank you! It's great to have a resolution for why you saw different results than myself and @andygrove . As you note, probably for dev-x reasons we should work around it for now, but at least there's an issue to track so we know when to un-hack this. (I'm linking to it in a comment in my other PR).

[Undin]

File > Invalidate Caches

@tustvold users shouldn't invalidate caches to make build script evaluation work. You just need to reload the project model after turning the feature on via Refresh Cargo Projects in Cargo tool window

[tustvold]

This doesn't appear to have worked with datafusion 13.0.0, the docs build is still broken, I'm getting a PR up to just check in the code like we do for arrow-rs.