Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network requests in build.rs and heavy build dependencies #34

Closed
mitsuhiko opened this issue Dec 11, 2022 · 6 comments
Closed

Network requests in build.rs and heavy build dependencies #34

mitsuhiko opened this issue Dec 11, 2022 · 6 comments

Comments

@mitsuhiko
Copy link

The alpha release of the crate requires tokio and reqwest during build time which are very heavy dependencies. More importantly it seems to be doing HTTP requests in build.rs which is a bad idea.

Could this be implemented in different ways?

@wooorm
Copy link
Owner

wooorm commented Dec 11, 2022

There are two network requests but they’re only fired if there is no file yet, e.g.,:

markdown-rs/build.rs

Lines 15 to 23 in af96212

let value = if let Ok(value) = fs::read_to_string(data_url) {
value
} else {
let value = reqwest::get(url).await.unwrap().text().await.unwrap();
fs::write(data_url, value.clone()).unwrap();
value
};
.

I’ve noticed Rust gets very slow with build scripts though.

Could this be implemented in different ways?

Pretty sure you know more about Rust (ecosystem) than I do, so I’m all ears on what a good way to build files in a Rust project is, other than, ehh, a build script 😅

@mitsuhiko
Copy link
Author

The build script itself is not the issue, it's mostly about how heavy it is. A reasonable alternative would be to put this logic behind an optional feature. On second look the build script also does something it's not supposed to do which is to change the contents in src (for the unicode) file.

In some ways what this build script does is probably better done in a separate utility for development purposes only and to check-in the output of it regularly. That's what I also generally do myself for similar situations.

@wooorm
Copy link
Owner

wooorm commented Dec 11, 2022

Why are build scripts not supposed to change src?

separate utility for development purposes only and to check-in the output of it regularly

That’s exactly what I’m using build scripts for. I guess that isn’t what build scripts in the rust ecosystem are supposed to do. Rust ecosystem is weird sometimes.

@mitsuhiko
Copy link
Author

Why are build scripts not supposed to change src?

Build scripts are supposed to place content in OUT_DIR and then have a script in src/ reference it. If you do place stuff there you end up with bugs like this: rust-lang/cargo#3076

That’s exactly what I’m using build scripts for. I guess that isn’t what build scripts in the rust ecosystem are supposed to do. Rust ecosystem is weird sometimes.

build scripts in the rust ecosystem are just there to run additional time at build time. This however seems to be a task that is run during development not at build time (eg: artifacts are checked out).

@wooorm
Copy link
Owner

wooorm commented Dec 12, 2022

This however seems to be a task that is run during development

Correct.

Is this something you would like to work on, or can you otherwise share how you would do this?

@wooorm
Copy link
Owner

wooorm commented Dec 13, 2022

Fixed in https://github.com/wooorm/markdown-rs/releases/tag/1.0.0-alpha.5!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants