[GH-ISSUE #108] Reducing rspotify's core dependencies #36

Closed
opened 2026-02-27 20:22:43 +03:00 by kerem · 10 comments
Owner

Originally created by @marioortizmanero on GitHub (Aug 12, 2020).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/108

I've been creating various PRs to clean up some unnecessary dependencies I found and such. Not only does this decrease compilation times, but this may also reduce binary size in case the optimizer is unable to know a module isn't used at all.

Here are some of my proposals, don't hesitate on criticizing them; I want to discuss these changes with @ramsayleung and if possible with more users of rspotify. It might also be too much to remove some of them:

Optional features

  • Make webbrowser optional, under some cli feature. This dependency isn't used by all the users, since some may not need to open a browser for the authentication process. It's only used for the util::request_token function currently.
  • Make dotenv optional, under some env feature. This is also only used by some users, mostly web servers.

These changes will break backward compatibility, so a new version would have to be released. I could create an UPGRADING.md file that explains what changes were made in order to make this process easier.

Removing dependencies

  • [accepted PR] 2 less dependencies after #100
  • [pending PR accept] 4 less dependencies after #106
  • [pending PR accept] upgrading the dependencies curiously goes from 191 dependencies with cargo run --example current_playing to 177, so ~14 less dependencies after #105
  • [discussion] lazy_static is only used in client.rs for CLIENT here. This CLIENT variable is then only used once here. I haven't used lazy_static myself, but I wanted to re-evaluate the following questions:
    - Is it necessary that CLIENT is a global, static, public variable?
    - Is lazy_static is necessary here?
  • [discussion] Is rand necessary only for util::generate_random_string here? rand is a considerably big dependency because it includes lots of things that are often unnecessary. I recently saw this PR https://github.com/rust-analyzer/rust-analyzer/pull/5574 that replaced it with oorandom, a much simpler and straightforward crate. Could this also be applied to this use case?
Originally created by @marioortizmanero on GitHub (Aug 12, 2020). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/108 I've been creating various PRs to clean up some unnecessary dependencies I found and such. Not only does this decrease compilation times, but this may also reduce binary size in case the optimizer is unable to know a module isn't used at all. Here are some of my proposals, don't hesitate on criticizing them; I want to discuss these changes with @ramsayleung and if possible with more users of rspotify. It might also be too much to remove some of them: ## Optional features * Make `webbrowser` optional, under some `cli` feature. This dependency isn't used by all the users, since some may not need to open a browser for the authentication process. It's only used for the `util::request_token` function currently. * Make `dotenv` optional, under some `env` feature. This is also only used by some users, mostly web servers. These changes will break backward compatibility, so a new version would have to be released. I could create an `UPGRADING.md` file that explains what changes were made in order to make this process easier. ## Removing dependencies + **[accepted PR]** 2 less dependencies after #100 + **[pending PR accept]** 4 less dependencies after #106 + **[pending PR accept]** upgrading the dependencies curiously goes from 191 dependencies with `cargo run --example current_playing` to 177, so ~14 less dependencies after #105 + **[discussion]** `lazy_static` is only used in `client.rs` for `CLIENT` [here](https://github.com/ramsayleung/rspotify/blob/master/src/client.rs#L40). This `CLIENT` variable is then only used *once* [here](https://github.com/ramsayleung/rspotify/blob/master/src/client.rs#L185). I haven't used `lazy_static` myself, but I wanted to re-evaluate the following questions: - Is it necessary that `CLIENT` is a *global, static, public* variable? - Is `lazy_static` is necessary here? + **[discussion]** Is `rand` necessary only for `util::generate_random_string` [here](https://github.com/ramsayleung/rspotify/blob/master/src/util.rs#L20)? `rand` is a considerably big dependency because it includes lots of things that are often unnecessary. I recently saw this PR https://github.com/rust-analyzer/rust-analyzer/pull/5574 that replaced it with `oorandom`, a much simpler and straightforward crate. Could this also be applied to this use case?
kerem 2026-02-27 20:22:43 +03:00
Author
Owner

@ramsayleung commented on GitHub (Aug 13, 2020):

Make webbrowser optional, under some cli feature.

It's a great suggestion, which makes it more easier to build automatic tests without webbrowser. But when we need credential to call Spotify's endpoints, how to get credential without pulling up a browser? It's the original reason of why I used webbrowser.

Make dotenv optional, under some env feature. This is also only used by some users, mostly web servers.

Yes, I originally use dotenv to parse environment variable like CLIENT_ID, CLIENT_SECRET. It will be fine if users prefer pass those parameters directly, so we could just make dotenv optional.

[discussion] Is rand necessary only for util::generate_random_string here?

Honestly, it's the only usage of rand, just generating a random string. If we could find a lightweight replacement(or could we implement the generate_random_string without any additional crate?), it's great to remove rand to reduce compile time and binary size.

Could this also be applied to this use case?

If it's the best choice for us, why not?

<!-- gh-comment-id:673431428 --> @ramsayleung commented on GitHub (Aug 13, 2020): > Make `webbrowser` optional, under some cli feature. It's a great suggestion, which makes it more easier to build automatic tests without `webbrowser`. But when we need credential to call Spotify's endpoints, how to get credential without pulling up a browser? It's the original reason of why I used `webbrowser`. > Make `dotenv` optional, under some `env` feature. This is also only used by some users, mostly web servers. Yes, I originally use `dotenv` to parse environment variable like `CLIENT_ID`, `CLIENT_SECRET`. It will be fine if users prefer pass those parameters directly, so we could just make `dotenv` optional. > [discussion] Is rand necessary only for util::generate_random_string [here](https://github.com/ramsayleung/rspotify/blob/master/src/util.rs?rgh-link-date=2020-08-12T18%3A47%3A52Z#L20)? Honestly, it's the only usage of `rand`, just generating a random string. If we could find a lightweight replacement(or could we implement the `generate_random_string` without any additional crate?), it's great to remove `rand` to reduce compile time and binary size. > Could this also be applied to this use case? If it's the best choice for us, why not?
Author
Owner

@marioortizmanero commented on GitHub (Aug 13, 2020):

It's a great suggestion, which makes it more easier to build automatic tests without webbrowser. But when we need credential to call Spotify's endpoints, how to get credential without pulling up a browser? It's the original reason of why I used webbrowser.

My point is that opening a new browser window/tab is only needed for those creating CLI applications:

  • If you are using this library for a web server, you won't need that (I haven't tried that so please correct me if I'm wrong)
  • If you use a function other than util::request_token because you need a more user-friendly authentication process within a GUI (my case, where I run a small HTTP server to have an user-friendly and automatic redirect uri), webbrowser isn't needed for rspotify, only for your binary.

Yes, I originally use dotenv to parse environment variable like CLIENT_ID, CLIENT_SECRET. It will be fine if users prefer pass those parameters directly, so we could just make dotenv optional.

As long as it's well documented I think it will be fine.

Honestly, it's the only usage of rand, just generating a random string. If we could find a lightweight replacement(or could we implement the generate_random_string without any additional crate?), it's great to remove rand to reduce compile time and binary size.

Great. I'll try to come up with something for that and I'll open a new PR once I have it ready.

Do you want me to create these features in Cargo.toml and the documentation regarding them in a new PR, or do you prefer to do it yourself? In case webbrowser is made optional, most examples will have to be changed as well. Also the tests for dotenv.

<!-- gh-comment-id:673444236 --> @marioortizmanero commented on GitHub (Aug 13, 2020): > It's a great suggestion, which makes it more easier to build automatic tests without webbrowser. But when we need credential to call Spotify's endpoints, how to get credential without pulling up a browser? It's the original reason of why I used webbrowser. My point is that opening a new browser window/tab is only needed for those creating CLI applications: * If you are using this library for a web server, you won't need that (I haven't tried that so please correct me if I'm wrong) * If you use a function other than `util::request_token` because you need a more user-friendly authentication process within a GUI (my case, where I run a small HTTP server to have an user-friendly and automatic redirect uri), `webbrowser` isn't needed for rspotify, only for your binary. > Yes, I originally use dotenv to parse environment variable like CLIENT_ID, CLIENT_SECRET. It will be fine if users prefer pass those parameters directly, so we could just make dotenv optional. As long as it's well documented I think it will be fine. > Honestly, it's the only usage of rand, just generating a random string. If we could find a lightweight replacement(or could we implement the generate_random_string without any additional crate?), it's great to remove rand to reduce compile time and binary size. Great. I'll try to come up with something for that and I'll open a new PR once I have it ready. Do you want me to create these features in `Cargo.toml` and the documentation regarding them in a new PR, or do you prefer to do it yourself? In case `webbrowser` is made optional, most examples will have to be changed as well. Also the tests for `dotenv`.
Author
Owner

@ramsayleung commented on GitHub (Aug 13, 2020):

Do you want me to create these features in Cargo.toml and the documentation regarding them in a new PR, or do you prefer to do it yourself?

I prefer to open a PR to implement this reduction, we both could commit to this PR. And now I am trying to replace rand with oorand, it's funny. I can't help to think, how much improvment will this reduction make.

<!-- gh-comment-id:673453418 --> @ramsayleung commented on GitHub (Aug 13, 2020): > Do you want me to create these features in Cargo.toml and the documentation regarding them in a new PR, or do you prefer to do it yourself? I prefer to open a PR to implement this reduction, we both could commit to this PR. And now I am trying to replace `rand` with `oorand`, it's funny. I can't help to think, how much improvment will this reduction make.
Author
Owner

@marioortizmanero commented on GitHub (Aug 13, 2020):

I prefer to open a PR to implement this reduction, we both could commit to this PR. And now I am trying to replace rand with oorand, it's funny. I can't help to think, how much improvment will this reduction make.

I just noticed oorandom points out in their README.md that it's not cryptographically secure. Knowing that get_random_string is used for the state parameter in the authentication process. From the OAuth 2.0 specification:

The binding value used for CSRF protection MUST contain a non-guessable value (as described in Section 10.10), and the user-agent's authenticated state (e.g., session cookie, HTML5 local storage) MUST be kept in a location accessible only to the client and the user-agent (i.e., protected by same-origin policy).

So I don't think removing rand is worth it. That, or finding another lightweight random number generator that is cryptographically secure, like the getrandom crate.

<!-- gh-comment-id:673494539 --> @marioortizmanero commented on GitHub (Aug 13, 2020): > I prefer to open a PR to implement this reduction, we both could commit to this PR. And now I am trying to replace rand with oorand, it's funny. I can't help to think, how much improvment will this reduction make. I just noticed `oorandom` points out in their README.md that [it's not cryptographically secure](https://sr.ht/~icefox/oorandom/#this-is-not). Knowing that `get_random_string` is used for the `state` parameter in the authentication process. From the [OAuth 2.0 specification](https://tools.ietf.org/html/rfc6749#section-10.12): > The binding value used for CSRF protection MUST contain a non-guessable value (as described in Section 10.10), and the user-agent's authenticated state (e.g., session cookie, HTML5 local storage) MUST be kept in a location accessible only to the client and the user-agent (i.e., protected by same-origin policy). So I don't think removing `rand` is worth it. That, or finding another lightweight random number generator that is cryptographically secure, like the `getrandom` crate.
Author
Owner

@ramsayleung commented on GitHub (Aug 13, 2020):

I just create a code snippet with getrandom to generate a random string:

use getrandom::getrandom;
static ALPHANUM: &str = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
fn main() {
    let random_string = generate_random_string(16);
    println!("{:?}", random_string);
}

fn generate_random_string(length: usize) -> String {
    let mut buf = vec![0u8; length];
    if let Err(why) = getrandom(&mut buf) {
        panic!("Fail {:?}", why)
    };
    let length = ALPHANUM.len();
    let mut random_string = String::from("");
    for byte in buf.iter() {
        let c = ALPHANUM.as_bytes()[*byte as usize % length] as char;
        random_string.push(c);
    }
    random_string
}

it works.

in getrandom's README, I noticed that it requires calling some external system API, I am not sure will it become a problem of cross-compilation or not?

However, getting randomness usually requires calling some external system API. This means most platforms will require linking against system libraries (i.e. libc for Unix, Advapi32.dll for Windows, Security framework on iOS, etc...).

<!-- gh-comment-id:673570746 --> @ramsayleung commented on GitHub (Aug 13, 2020): I just create a code snippet with `getrandom` to generate a random string: ```rust use getrandom::getrandom; static ALPHANUM: &str = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; fn main() { let random_string = generate_random_string(16); println!("{:?}", random_string); } fn generate_random_string(length: usize) -> String { let mut buf = vec![0u8; length]; if let Err(why) = getrandom(&mut buf) { panic!("Fail {:?}", why) }; let length = ALPHANUM.len(); let mut random_string = String::from(""); for byte in buf.iter() { let c = ALPHANUM.as_bytes()[*byte as usize % length] as char; random_string.push(c); } random_string } ``` it works. in `getrandom`'s README, I noticed that it requires calling some external system API, I am not sure will it become a problem of cross-compilation or not? > However, getting randomness usually requires calling some external system API. This means most platforms will require linking against system libraries (i.e. libc for Unix, Advapi32.dll for Windows, Security framework on iOS, etc...).
Author
Owner

@marioortizmanero commented on GitHub (Aug 13, 2020):

Looks great! Here's what I came up with:

use getrandom::getrandom;

fn main() {
    let random_string = generate_random_string(16);
    println!("{:?}", random_string);
}

fn generate_random_string(length: usize) -> String {
    static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes();
    let mut buf = vec![0u8; length];
    getrandom(&mut buf).unwrap();
    let range = ALPHANUM.len();
    return buf.iter().map(|byte| {
        ALPHANUM[*byte as usize % range] as char
    }).collect()
}

Iterators are cool! :)

in getrandom's README, I noticed that it requires calling some external system API, I am not sure will it become a problem of cross-compilation or not?

getrandom just uses random number generators from the operating system rather than implementing them from "scratch", like rand. I don't think it's a problem, as it supports all operating systems I know of.

<!-- gh-comment-id:673577801 --> @marioortizmanero commented on GitHub (Aug 13, 2020): Looks great! Here's what I came up with: ```rust use getrandom::getrandom; fn main() { let random_string = generate_random_string(16); println!("{:?}", random_string); } fn generate_random_string(length: usize) -> String { static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes(); let mut buf = vec![0u8; length]; getrandom(&mut buf).unwrap(); let range = ALPHANUM.len(); return buf.iter().map(|byte| { ALPHANUM[*byte as usize % range] as char }).collect() } ``` Iterators are cool! :) > in getrandom's README, I noticed that it requires calling some external system API, I am not sure will it become a problem of cross-compilation or not? `getrandom` just uses random number generators from the operating system rather than implementing them from "scratch", like `rand`. I don't think it's a problem, as it supports all operating systems I know of.
Author
Owner

@marioortizmanero commented on GitHub (Aug 13, 2020):

Here's a comparison I've made:

rand version:

use rand::distributions::Alphanumeric;
use rand::{self, Rng};

fn main() {
    for _ in 1..100000 {
        let random_string = generate_random_string(16);
        println!("{:?}", random_string);
    }
}

fn generate_random_string(length: usize) -> String {
    rand::thread_rng()
        .sample_iter(&Alphanumeric)
        .take(length)
        .collect()
}

getrandom version:

use getrandom::getrandom;

fn main() {
    for _ in 1..100000 {
        let random_string = generate_random_string(16);
        println!("{:?}", random_string);
    }
}

fn generate_random_string(length: usize) -> String {
    static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes();
    let mut buf = vec![0u8; length];
    getrandom(&mut buf).unwrap();
    let range = ALPHANUM.len();
    return buf.iter().map(|byte| {
        ALPHANUM[*byte as usize % range] as char
    }).collect()
}
use std::time::{SystemTime, UNIX_EPOCH};
use oorandom::Rand32;

fn main() {
   for _ in 1..100000 {
       let random_string = generate_random_string(16);
       println!("{:?}", random_string);
   }
}

fn generate_random_string(length: usize) -> String {
   // Using a deterministic seed such as the Unix timestamp. For a non
   // deterministic seed, the `getrandom` crate would have to be used, which
   // makes no sense because it can be used directly to generate the random
   // numbers.
   //
   // If this method were to be used more than once per second, the results
   // would also be identical. This could be fixed by using `.as_nanos` instead
   // and converting to `u64`, which should have a similar performance.
   //
   // In conclusion, using oorandom doesn't make sense in this case at all,
   // but I'll benchmark it just out of curiosity.
   let seed = SystemTime::now()
       .duration_since(UNIX_EPOCH)
       .unwrap()
       .as_secs();
   static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes();
   let mut rng = Rand32::new(seed);
   let mut buf = String::with_capacity(length);
   let range = ALPHANUM.len();
   for _ in 0..length {
       buf.push(ALPHANUM[rng.rand_u32() as usize % range] as char)
   }
   buf
}
built modules compilation time compilation time (--release) execution time (--release) size with du -h after running strip, with --release
rand 12 ~3.22s ~3.51s 0.10s user 0.13s system 22% cpu 1.027 total 260K
getrandom 6 ~1.46s ~1.64s 0.10s user 0.21s system 29% cpu 1.017 total 240K
oorandom 2 ~0.36s ~0.37s ./target/release/with_oorandom 0.08s user 0.15s system 23% cpu 1.005 total 240K

The cargo tree output from the rand version actually includes getrandom, so I suspect the speed is similar just because rand in the end uses getrandom. The change to getrandom seems worth it to me.

<!-- gh-comment-id:673587185 --> @marioortizmanero commented on GitHub (Aug 13, 2020): Here's a comparison I've made: `rand` version: ```rust use rand::distributions::Alphanumeric; use rand::{self, Rng}; fn main() { for _ in 1..100000 { let random_string = generate_random_string(16); println!("{:?}", random_string); } } fn generate_random_string(length: usize) -> String { rand::thread_rng() .sample_iter(&Alphanumeric) .take(length) .collect() } ``` `getrandom` version: ```rust use getrandom::getrandom; fn main() { for _ in 1..100000 { let random_string = generate_random_string(16); println!("{:?}", random_string); } } fn generate_random_string(length: usize) -> String { static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes(); let mut buf = vec![0u8; length]; getrandom(&mut buf).unwrap(); let range = ALPHANUM.len(); return buf.iter().map(|byte| { ALPHANUM[*byte as usize % range] as char }).collect() } ``` ```rust use std::time::{SystemTime, UNIX_EPOCH}; use oorandom::Rand32; fn main() { for _ in 1..100000 { let random_string = generate_random_string(16); println!("{:?}", random_string); } } fn generate_random_string(length: usize) -> String { // Using a deterministic seed such as the Unix timestamp. For a non // deterministic seed, the `getrandom` crate would have to be used, which // makes no sense because it can be used directly to generate the random // numbers. // // If this method were to be used more than once per second, the results // would also be identical. This could be fixed by using `.as_nanos` instead // and converting to `u64`, which should have a similar performance. // // In conclusion, using oorandom doesn't make sense in this case at all, // but I'll benchmark it just out of curiosity. let seed = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap() .as_secs(); static ALPHANUM: &[u8] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789".as_bytes(); let mut rng = Rand32::new(seed); let mut buf = String::with_capacity(length); let range = ALPHANUM.len(); for _ in 0..length { buf.push(ALPHANUM[rng.rand_u32() as usize % range] as char) } buf } ``` | | built modules | compilation time | compilation time (--release) | execution time (--release) | size with `du -h` after running `strip`, with --release | |-----------|---------------|------------------|------------------------------|---------------------------------------------|--| | rand | 12 | ~3.22s | ~3.51s | 0.10s user 0.13s system 22% cpu 1.027 total | 260K | | getrandom | 6 | ~1.46s | ~1.64s | 0.10s user 0.21s system 29% cpu 1.017 total | 240K | | oorandom | 2 | ~0.36s | ~0.37s | ./target/release/with_oorandom 0.08s user 0.15s system 23% cpu 1.005 total | 240K | The `cargo tree` output from the `rand` version actually includes `getrandom`, so I suspect the speed is similar just because `rand` in the end uses `getrandom`. The change to `getrandom` seems worth it to me.
Author
Owner

@ramsayleung commented on GitHub (Aug 14, 2020):

Cool, I just open a PR https://github.com/ramsayleung/rspotify/pull/110 to reduce core dependencies, feel free to contribute to this.

Here's a comparison I've made:

Just out of curiosity, would you like to add oorandom to this contest, I am just curious about how does it perform, is it 10x lightweight than rand as it claims in README.

<!-- gh-comment-id:674121055 --> @ramsayleung commented on GitHub (Aug 14, 2020): Cool, I just open a PR https://github.com/ramsayleung/rspotify/pull/110 to reduce core dependencies, feel free to contribute to this. > Here's a comparison I've made: Just out of curiosity, would you like to add `oorandom` to this contest, I am just curious about how does it perform, is it 10x lightweight than `rand` as it claims in `README`.
Author
Owner

@marioortizmanero commented on GitHub (Aug 14, 2020):

Just out of curiosity, would you like to add oorandom to this contest, I am just curious about how does it perform, is it 10x lightweight than rand as it claims in README.

Sure, I've added a benchmark with oorandom just out of curiosity, since it doesn't make sense to use it for this crate.

<!-- gh-comment-id:674195685 --> @marioortizmanero commented on GitHub (Aug 14, 2020): > Just out of curiosity, would you like to add oorandom to this contest, I am just curious about how does it perform, is it 10x lightweight than rand as it claims in README. Sure, I've added a benchmark with `oorandom` just out of curiosity, since it doesn't make sense to use it for this crate.
Author
Owner

@marioortizmanero commented on GitHub (Aug 28, 2020):

This can be closed now that #110 was merged.

<!-- gh-comment-id:682621366 --> @marioortizmanero commented on GitHub (Aug 28, 2020): This can be closed now that #110 was merged.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/rspotify#36
No description provided.