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

Use pre-built timezone/directory maps by default. #193

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions chrono-tz-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::collections::BTreeSet;
use std::env;
use std::fs::File;
use std::io::{self, BufRead, BufReader, Write};
use std::path::Path;
use std::path::{Path, PathBuf};

use parse_zoneinfo::line::{Line, LineParser};
use parse_zoneinfo::structure::{Child, Structure};
Expand Down Expand Up @@ -135,6 +135,7 @@ fn write_timezone_file(timezone_file: &mut File, table: &Table) -> io::Result<()

#[cfg(feature = "case-insensitive")]
{
writeln!(timezone_file, r#"#[cfg(feature = "case-insensitive")]"#)?;
writeln!(timezone_file, "use uncased::UncasedStr;\n",)?;
let mut map = phf_codegen::Map::new();
for zone in &zones {
Expand All @@ -143,6 +144,7 @@ fn write_timezone_file(timezone_file: &mut File, table: &Table) -> io::Result<()
&format!("Tz::{}", convert_bad_chars(zone)),
);
}
writeln!(timezone_file, r#"#[cfg(feature = "case-insensitive")]"#)?;
writeln!(
timezone_file,
"static TIMEZONES_UNCASED: ::phf::Map<&'static uncased::UncasedStr, Tz> = \n{};",
Expand Down Expand Up @@ -555,13 +557,25 @@ pub fn main() {

let mut table = table.build();
filter::maybe_filter_timezone_table(&mut table);
let iana_db_version = detect_iana_db_version();

let timezone_path = Path::new(&env::var("OUT_DIR").unwrap()).join("timezones.rs");
if env::var("CHRONO_TZ_UPDATE_PREBUILT").as_deref() == Ok("1") {
let src_dir = env::current_dir().unwrap();
let timezone_path = src_dir.join("src").join("prebuilt_timezones.rs");
let mut timezone_file = File::create(timezone_path).unwrap();
write_timezone_file(&mut timezone_file, &table).unwrap();

let directory_path = src_dir.join("src").join("prebuilt_directory.rs");
let mut directory_file = File::create(directory_path).unwrap();
write_directory_file(&mut directory_file, &table, &iana_db_version).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract the duplicated code out into a function instead?

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

}

let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
let timezone_path = out_dir.join("timezones.rs");
let mut timezone_file = File::create(timezone_path).unwrap();
write_timezone_file(&mut timezone_file, &table).unwrap();

let directory_path = Path::new(&env::var("OUT_DIR").unwrap()).join("directory.rs");
let directory_path = out_dir.join("directory.rs");
let mut directory_file = File::create(directory_path).unwrap();
let version = detect_iana_db_version();
write_directory_file(&mut directory_file, &table, &version).unwrap();
write_directory_file(&mut directory_file, &table, &iana_db_version).unwrap();
}
6 changes: 3 additions & 3 deletions chrono-tz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ uncased = { version = "0.9", optional = true, default-features = false }
default = ["std"]
std = []
serde = ["dep:serde"]
filter-by-regex = ["chrono-tz-build/filter-by-regex"]
case-insensitive = ["dep:uncased", "chrono-tz-build/case-insensitive", "phf/uncased"]
filter-by-regex = ["dep:chrono-tz-build", "chrono-tz-build/filter-by-regex"]
case-insensitive = ["dep:uncased", "chrono-tz-build?/case-insensitive", "phf/uncased"]

[build-dependencies]
chrono-tz-build = { path = "../chrono-tz-build", version = "0.4" }
chrono-tz-build = { path = "../chrono-tz-build", version = "0.4", optional = true }

[dev-dependencies]
serde_test = "1"
Expand Down
8 changes: 8 additions & 0 deletions chrono-tz/build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
fn main() {
#[cfg(feature = "filter-by-regex")]
chrono_tz_build::main();

#[cfg(not(feature = "filter-by-regex"))]
{
if std::env::var("CHRONO_TZ_TIMEZONE_FILTER").is_ok() {
println!("cargo:warning=CHRONO_TZ_TIMEZONE_FILTER set without enabling filter-by-regex feature")
Copy link
Member

Choose a reason for hiding this comment

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

From the docs:

The warning instruction tells Cargo to display a warning after the build script has finished running. Warnings are only shown for path dependencies (that is, those you’re working on locally), so for example warnings printed out in crates.io crates are not emitted by default, unless the build fails.

Not sure this is useful?

(Also the docs seem to indicate it should be cargo::warning, with two colons.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, maybe we also need to fail the build in that case, in addition to the warning. My hope was to give a simple warning without a hard failure.

I tried using cargo::warning but apparently that is new syntax. The single-colon syntax is required for the MSRV of chrono-tz.

Copy link
Author

Choose a reason for hiding this comment

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

I've switched this now to use cargo::error which is only supported on newer versions. So either the library is larger than expected but still works on older versions, or you get a helpful error telling you to enable the feature.

}
}
}
5 changes: 5 additions & 0 deletions chrono-tz/src/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@
#![allow(non_snake_case)]
#![allow(non_upper_case_globals)]
#![allow(dead_code)]

#[cfg(feature = "filter-by-regex")]
include!(concat!(env!("OUT_DIR"), "/directory.rs"));

#[cfg(not(feature = "filter-by-regex"))]
include!("prebuilt_directory.rs");
Copy link
Member

Choose a reason for hiding this comment

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

Could we instead have #[path = "prebuilt_directory.rs"] on the mod declaration in lib.rs conditional on a cfg(), or does that not work? Otherwise, can we use a mod declaration here and re-export all symbols? I'd like the prebuilt modules to be visible to Rust-Analyzer et al.

Copy link
Author

Choose a reason for hiding this comment

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

I think that'd work (the first suggestion), I'll try it.

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately I can't get this to work, because we need the OUT_DIR environment variable but the path attribute only accepts string literals.

Copy link
Author

Choose a reason for hiding this comment

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

For what it's worth rust-analyzer seems to be able to pierce through the include! barrier.

Loading
Loading