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

Fix build error when using specific or no Cargo compression features #339

Merged
merged 1 commit into from
Apr 13, 2024

Conversation

joseluisq
Copy link
Collaborator

Description

This PR fixes a build issue when SWS is used as a dependency and the Cargo compression feature is disabled completely or just some specific compression features are enabled.

Also, if only one compression feature is enabled (e.g. compression-gzip) then the preferred encoding gets re-evaluated and assigned to that compression feature format (e.g. gzip) but only if it was part of the accept-encoding request header value.

For example, now the following scenarios will work:

#....
[dependencies]
static-web-server = { version = "2.28.0", default-features = false }
# or
static-web-server = { version = "2.28.0", default-features = false, features = ["compression-brotli"] }

In addition, some logs are added/improved to reflect the changes.

Related Issue

Fixes #338

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

also, if only one compression feature is
enabled (e.g. compression-gzip) then the preferred encoding gets
re-evaluated and assigned to that compression feature but only if it
is part of the `accept-encoding` header value.

now the following scenarios will work:

[dependencies]
static-web-server = { version = "2.28.0", default-features = false }

or

 [dependencies]
static-web-server = { version = "2.28.0", default-features = false, features = ["compression-brotli"] }

in addition, some tracing logs are added to reflect the changes.
@joseluisq joseluisq added v2 v2 release bugfix This is PR fixes a bug library Related to the static-web-server crate labels Apr 13, 2024
@joseluisq joseluisq self-assigned this Apr 13, 2024
Copy link

semanticdiff-com bot commented Apr 13, 2024

Review changes with SemanticDiff.

Analyzed 12 of 12 files.

Overall, the semantic diff is 42% smaller than the GitHub diff.

Filename Status
✔️ src/compression.rs 35.24% smaller
✔️ src/compression_static.rs 3.33% smaller
✔️ src/handler.rs 46.22% smaller
✔️ src/lib.rs 51.61% smaller
✔️ src/server.rs 44.03% smaller
✔️ src/settings/cli.rs 53.8% smaller
✔️ src/settings/file.rs 53.8% smaller
✔️ src/settings/mod.rs 46.67% smaller
✔️ src/static_files.rs 40.57% smaller
✔️ src/testing.rs Analyzed
✔️ tests/compression_static.rs 39.57% smaller
✔️ tests/static_files.rs 39.34% smaller

@joseluisq joseluisq merged commit 183102d into master Apr 13, 2024
27 checks passed
@joseluisq joseluisq deleted the fix-build-from-source-no-compression-feature branch April 13, 2024 02:45
feature = "compression",
feature = "compression-deflate",
feature = "compression-gzip",
feature = "compression-deflate",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you are checking for compression-deflate twice here and a bunch of other places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one, yeah it has to be removed.

@@ -8,13 +8,13 @@

// Part of the file is borrowed from <https://github.com/seanmonstar/warp/pull/513>*

#[cfg(feature = "compression-brotli")]
#[cfg(any(feature = "compression", feature = "compression-brotli"))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: With the compression feature activating the compression-brotli feature, I’m fairly certain that this change (and a bunch of similar ones) is unnecessary. Unless you are doing this for readability purposes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I'm doing this just for readability because using compression already implies the other ones.

But it could be removed without a problem.

Feel free to clean this up if you think so.

@joseluisq joseluisq added this to the v2.30.0 milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This is PR fixes a bug library Related to the static-web-server crate v2 v2 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to compile without compression feature
2 participants