Fix compilation for android (plotly.rs)

[frroossst]

I prefer having cli programs on my android device as well. So when I tried to install numbat-cli with termux on Android it failed.

How to replicate

git clone https://github.com/sharkdp/numbat.git
cd numbat
cargo build

The error we get is

error[E0599]: no function or associated item named `show_with_default_app` found for struct `Plot` in the current scope
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:273:15
    |
178 | pub struct Plot {
    | --------------- function or associated item `show_with_default_app` not found for this struct
...
273 |         Plot::show_with_default_app(temp_path);
    |               ^^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `Plot`
    |
note: if you're trying to build a new `Plot`, consider using `Plot::new` which returns `Plot`
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:190:5
    |
190 |     pub fn new() -> Plot {
    |     ^^^^^^^^^^^^^^^^^^^^

error[E0599]: no function or associated item named `show_with_default_app` found for struct `Plot` in the current scope
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:286:15
    |
178 | pub struct Plot {
    | --------------- function or associated item `show_with_default_app` not found for this struct
...
286 |         Plot::show_with_default_app(path);
    |               ^^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `Plot`
    |
note: if you're trying to build a new `Plot`, consider using `Plot::new` which returns `Plot`
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:190:5
    |
190 |     pub fn new() -> Plot {
    |     ^^^^^^^^^^^^^^^^^^^^

error[E0599]: no function or associated item named `show_with_default_app` found for struct `Plot` in the current scope
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:315:15
    |
178 | pub struct Plot {
    | --------------- function or associated item `show_with_default_app` not found for this struct
...
315 |         Plot::show_with_default_app(temp_path);
    |               ^^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `Plot`
    |
note: if you're trying to build a new `Plot`, consider using `Plot::new` which returns `Plot`
   --> /data/data/com.termux/files/home/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/plotly-0.12.1/src/plot.rs:190:5
    |
190 |     pub fn new() -> Plot {
    |     ^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0599`.
error: could not compile `plotly` (lib) due to 3 previous errors

Solution

Looking around I found this particular issue on the plotly github.

plotly/plotly.rs#282

There does not seem to be an update out at this point for plotly that fixes compilation on android. (Last release was 6 months ago)

So, the simple update would be to change numbat/Cargo.toml

# plotly = { version = "0.12.0", optional = true }
plotly = { git = "https://github.com/plotly/plotly.rs", branch = "main" , optional = true }

This will fail with

error[E0599]: no method named `show` found for struct `Plot` in the current scope                                  
  --> numbat/src/ffi/plot.rs:93:10
   |
93 |     plot.show();
   |          ^^^^ method not found in `Plot`

For more information about this error, try `rustc --explain E0599`.                                                
error: could not compile `numbat` (lib) due to 1 previous error

Currently, I just have a simple fix to add the plot.show() stub for android.

In numbat/src/ffi/plot.rs

fn show_plot(plot: Plot) -> CompactString {
    #[cfg(target_os = "android")]
    {
        let _ = plot;
        unimplemented!("cannot show_plot on android")
    }

    #[cfg(not(target_os = "android"))]
    {
        plot.show();

        CompactString::const_new("Plot will be opened in the browser")
    }
}

If the author(s)/ maintainer(s) feel like this might be a useful thing to add, I can open a pull request against master.

Thank you for creating and working on numbat!

[frroossst]

Feel free to checkout the code that is ready to be merged at
https://github.com/frroossst/numbat/tree/36e8d059683f05a0bcf32bbb0d14447f19011563

I have run cargo test and all tests seem to pass.

[Goju-Ryu]

Can this not be solved by compiling Numbat without the plotting feature? I'm not too familiar with rust compilation yet, but I think that would be the preferable solution if possible.

[frroossst]

shouldn't plotting be an additional feature that could be turned on, but should be disabled on default installs?

cargo install numbat-cli --features plotting

[Goju-Ryu]

Whether it should or not I'm not sure, but currently it is part of the default feature set. To get a default installation except excluding plotting I think you can run:

cargo install numbat-cli --no-default-features --features fetch-exchangerates

[Goju-Ryu]

Although I might be forgetting features from the cli now that I think about it.

[frroossst]

https://doc.rust-lang.org/cargo/reference/features.html?highlight=additive#feature-unification

I was just going off of this which says cargo features must be additive

[Goju-Ryu]

I am not super well versed in rust compilation, but I think it is allowed since we use the resolver version 2 as described in this section. I may be wrong though.

[frroossst]

So, should I close this as won't fix?