nginxinc / ngx-rust

Rust binding for NGINX
Apache License 2.0
713 stars 59 forks source link

feat: modify cache & NGINX dir #72

Closed JyJyJcr closed 3 months ago

JyJyJcr commented 3 months ago

Proposed changes

This PR add some environment variable options useful to develop modules using the SDK.

CACHE_DIR

Default: [nginx-sys's root]/.cache

Specify cache directory. With relative -> absolute path converting option provided in .cargo/config.toml like below, we are able to generate a project-owned cache dir:

[env]
CACHE_DIR = { value = ".cache", relative = true }

This option makes it easier to save and restore the cache, which speeds up CI/CD.

Sidenote

Originally the default location of the cache dir is in the parent dir of the manifest dir (= root dir) of nginx-sys: this is equivalent to set the default of CACHE_DIR as [nginx-sys's root]/../.cache. This setting resulted the cache dir to be generated in the root dir of ngx-rust as long as ngx-rust is treated as the main project (this is likely the originally intended behavior). However, once nginx-sys is whether directly or indirectly used as a dependency of other projects, the repository was copied in $CARGO_HOME/registry/cache/index.crates.io-***/nginx-sys-***/ or $CARGO_HOME/git/checkouts/ngx-rust-***/***/, causing the cache dir to be generated in the same depth with the copies of crates. This was apparently undesirable. So the default value of CACHE_DIR is changed to [nginx-sys's root]/.cache to prevent the "pollution" described above. Instead, ngx-rust also use CACHE_DIR to move the cache dir to the original location.

NGINX_INSTALL_ROOT_DIR

Default: {CACHE_DIR}/nginx

Specify the directory where the NGINX compiled in the series of build will be installed. By default, each of Nginx is installed in the sub-subdirectory labeled by the version, OS and Architecture. This option is useful at developing stage to try multiple versions with your own conf files (for the latter aim only, NGINX_INSTALL_DIR is more suited).

NGINX_INSTALL_DIR

Default: {NGINX_INSTALL_ROOT_DIR}/{NGX_VERSION}/{OS}-{Arch}

Specify the directory where the NGINX compiled in this build will be installed. This option is mainly intended to facilitate the test automation in CI/CD.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

dekobon commented 3 months ago

Let's add this to the .gitignore file

nginx-sys/.cache/
JyJyJcr commented 3 months ago

Thank you for your rapid review.

Let's add this to the .gitignore file

nginx-sys/.cache/

We don't have to add this, because ngx-rust set CACHE_DIR to [ngx-rust's root]/.cache: https://github.com/nginxinc/ngx-rust/blob/29076150102b5524df635ab114bca1160f8e7989/.cargo/config.toml#L15-L16

ngx-rust keeps generating cache in its root dir, same as before.

dekobon commented 3 months ago

Thank you for your rapid review.

Let's add this to the .gitignore file

nginx-sys/.cache/

We don't have to add this, because ngx-rust set CACHE_DIR to [ngx-rust's root]/.cache:

https://github.com/nginxinc/ngx-rust/blob/29076150102b5524df635ab114bca1160f8e7989/.cargo/config.toml#L15-L16

ngx-rust keeps generating cache in its root dir, same as before.

Oh, that's my oversight. Thank you for clarifying.

JyJyJcr commented 3 months ago

I changed the src dir name in the same manner to the Nginx dir: https://github.com/nginxinc/ngx-rust/blob/0c930cabf7505e3f31cf021b74cfe810d60cdf5f/nginx-sys/build.rs#L275-L279 In addition, I added target-triple to dev-dependencies to fix log_test.rs to support new style of the directory structure: https://github.com/nginxinc/ngx-rust/blob/0c930cabf7505e3f31cf021b74cfe810d60cdf5f/tests/log_test.rs#L18-L25

JyJyJcr commented 3 months ago

Updated README.md. Thank you for the continuous support!