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

git-date parse draft #498

Merged
merged 11 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion git-date/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ serde1 = ["serde", "bstr/serde1"]
bstr = { version = "0.2.13", default-features = false, features = ["std"]}
serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
itoa = "1.0.1"
time = { version = "0.3.2", default-features = false, features = ["local-offset", "formatting", "macros"] }
time = { version = "0.3.2", default-features = false, features = ["local-offset", "formatting", "macros", "parsing"] }
thiserror = "1.0.32"

document-features = { version = "0.2.0", optional = true }

[dev-dependencies]
git-testtools = { path = "../tests/tools"}
once_cell = "1.12.0"

[package.metadata.docs.rs]
all-features = true
Expand Down
114 changes: 111 additions & 3 deletions git-date/src/parse.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,119 @@
use crate::time::format::{DEFAULT, ISO8601, ISO8601_STRICT, RFC2822, SHORT};
use crate::time::Sign;
use crate::Time;
use std::convert::TryInto;
use std::num::TryFromIntError;
use std::str::FromStr;
use std::time::SystemTime;
use time::{Date, OffsetDateTime};

#[derive(thiserror::Error, Debug)]
#[allow(missing_docs)]
pub fn parse(input: &str) -> Option<Time> {
pub enum Error {
#[error("Date string can not be parsed")]
InvalidDateString,
#[error("Relative period can not be parsed")]
InvalidPeriod,
#[error("Dates past 2038 can not be represented.")]
InvalidDate(#[from] TryFromIntError),
#[error("Current time is missing.")]
MissingCurrentTime,
}

#[allow(missing_docs)]
pub fn parse(input: &str, now: Option<SystemTime>) -> Result<Time, Error> {
// TODO: actual implementation, this is just to not constantly fail
if input == "1979-02-26 18:30:00" {
Some(Time::new(42, 1800))
Ok(Time::new(42, 1800))
} else {
None
return if let Ok(val) = Date::parse(input, SHORT) {
Copy link
Member

Choose a reason for hiding this comment

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

Explicit return isn't needed here.
Also you can wrap the whole if expression expression into an Ok to save on all the Ok in each branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to not explicit return, thanks but don't understand "wrap the whole if expression", example may be?

let val = val.with_hms(0, 0, 0).expect("date is in range").assume_utc();
Byron marked this conversation as resolved.
Show resolved Hide resolved
Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else if let Ok(val) = OffsetDateTime::parse(input, RFC2822) {
Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else if let Ok(val) = OffsetDateTime::parse(input, ISO8601) {
Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else if let Ok(val) = OffsetDateTime::parse(input, ISO8601_STRICT) {
Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else if let Ok(val) = OffsetDateTime::parse(input, DEFAULT) {
Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else if let Ok(val) = u32::from_str(input) {
// Format::Unix
Ok(Time::new(val, 0))
} else if let Some(val) = parse_raw(input) {
// Format::Raw
Ok(val)
} else if let Some(val) = relative::parse(input, now.ok_or(Error::MissingCurrentTime)?) {
Copy link
Member

Choose a reason for hiding this comment

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

This would cause unnecessary errors if no time is given but it's not a relative date either. Instead, let the relative::parse() function return a time offset (it's relative, after all), and if it succeeded and there is no current time, we have to fail. There should be a test for that, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would cause unnecessary errors if no time is given but it's not a relative date either.

See 1eac4de#diff-fe1f4921b662e3ab738c66e74ec7da02aa5ffde9cc94fe9c1a17e90326f54813R74 no error if passing None

Ok(Time::new(
val.unix_timestamp().try_into()?,
val.offset().whole_seconds(),
))
} else {
Err(Error::InvalidDateString)
};
}
}

fn parse_raw(input: &str) -> Option<Time> {
let mut split = input.split_whitespace();
let seconds_since_unix_epoch: u32 = split.next()?.parse().ok()?;
let offset = split.next()?;
if offset.len() != 5 {
return None;
}
let sign = if &offset[..1] == "-" { Sign::Plus } else { Sign::Minus };
let hours: i32 = offset[1..3].parse().ok()?;
let minutes: i32 = offset[3..5].parse().ok()?;
let offset_in_seconds = hours * 3600 + minutes * 60;
let time = Time {
seconds_since_unix_epoch,
offset_in_seconds,
sign,
};
Some(time)
}

mod relative {
use crate::parse::Error;
use std::str::FromStr;
use std::time::SystemTime;
use time::{Duration, OffsetDateTime};

pub(crate) fn parse(input: &str, now: SystemTime) -> Option<OffsetDateTime> {
let mut split = input.split_whitespace();
let multiplier = i64::from_str(split.next()?).ok()?;
let period = split.next()?;
if split.next()? != "ago" {
return None;
}
OffsetDateTime::from(now).checked_sub(duration(period, multiplier).ok()?)
}

fn duration(period: &str, multiplier: i64) -> Result<Duration, Error> {
let period = period.strip_suffix("s").unwrap_or(period);
return match period {
"second" => Ok(Duration::seconds(multiplier)),
"minute" => Ok(Duration::minutes(multiplier)),
"hour" => Ok(Duration::hours(multiplier)),
"day" => Ok(Duration::days(multiplier)),
"week" => Ok(Duration::weeks(multiplier)),
// TODO months & years
_ => Err(Error::InvalidPeriod),
};
}
}
46 changes: 46 additions & 0 deletions git-date/tests/fixtures/generate_git_date_baseline.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/bin/bash
set -eu -o pipefail

git init;

function baseline() {
local test_date=$1 # first argument is the date to test

git -c section.key="$test_date" config --type=expiry-date section.key && status=0 || status=$?
{
echo "$test_date"
echo "$status"
if [ $status == 0 ]
then
git -c section.key="$test_date" config --type=expiry-date section.key
else
echo "-1"
fi
} >> baseline.git
}

# success

# date formats following to https://git-scm.com/docs/git-log#Documentation/git-log.txt---dateltformatgt

# short
# ODO
#baseline '2022-08-22'
# rfc2822
baseline 'Thu, 18 Aug 2022 12:45:06 +0800'
# iso8601
baseline '2022-08-17 22:04:58 +0200'
# iso8601_strict
baseline '2022-08-17T21:43:13+08:00'
# default
baseline 'Thu Sep 04 2022 10:45:06 -0400'
# unix
baseline '123456789'
# raw
baseline '1660874655 +0800'

# failing

# empty_input
baseline ""

Binary file not shown.
86 changes: 85 additions & 1 deletion git-date/tests/time/parse.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,98 @@
use bstr::{BString, ByteSlice};
use git_date::time::Sign;
use git_date::Time;
use once_cell::sync::Lazy;
use std::collections::HashMap;
use std::str::FromStr;
use std::time::SystemTime;
use time::{Duration, OffsetDateTime};

type Result<T = ()> = std::result::Result<T, Box<dyn std::error::Error>>;

static BASELINE: Lazy<HashMap<BString, (usize, BString)>> = Lazy::new(|| {
let base = git_testtools::scripted_fixture_repo_read_only("generate_git_date_baseline.sh").unwrap();

(|| -> Result<_> {
let mut map = HashMap::new();
let baseline = std::fs::read(base.join("baseline.git"))?;
let mut lines = baseline.lines();
while let Some(date_str) = lines.next() {
let exit_code = lines.next().expect("three lines per baseline").to_str()?.parse()?;
let output = lines.next().expect("three lines per baseline").into();
map.insert(date_str.into(), (exit_code, output));
}
Ok(map)
})()
.unwrap()
});

#[test]
fn baseline() {
for (pattern, (exit_code, output)) in BASELINE.iter() {
let res = git_date::parse(pattern.to_str().expect("valid pattern"), Some(SystemTime::now()));
assert_eq!(
res.is_ok(),
*exit_code == 0,
"{pattern:?} disagrees with baseline: {res:?}"
);
if *exit_code == 0 {
let actual = res.unwrap().seconds_since_unix_epoch;
let expected = u32::from_str(output.to_str().expect("valid utf")).expect("valid epoch value");
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to parse the output while parsing the baseline file, that way it can be used natively here.

assert_eq!(actual, expected, "{pattern:?} disagrees with baseline: {actual:?}")
}
}
}

#[test]
fn special_time_is_ok_for_now() {
assert_eq!(
git_date::parse("1979-02-26 18:30:00").unwrap(),
git_date::parse("1979-02-26 18:30:00", Some(SystemTime::now())).unwrap(),
Time {
seconds_since_unix_epoch: 42,
offset_in_seconds: 1800,
sign: Sign::Plus,
}
);
}

#[test]
fn short() {
assert_eq!(
git_date::parse("1979-02-26", Some(SystemTime::now())).expect("parsed date"),
Time {
seconds_since_unix_epoch: 288835200,
offset_in_seconds: 0,
sign: Sign::Plus,
},
"could not parse with SHORT format"
);
}

#[test]
fn rfc2822() {
assert_eq!(
git_date::parse("Thu, 18 Aug 2022 12:45:06 +0800", Some(SystemTime::now())).expect("parsed rfc2822 string"),
Time {
seconds_since_unix_epoch: 1660797906,
offset_in_seconds: 28800,
sign: Sign::Plus,
},
"could not parse with RFC2822 format"
);
}

#[test]
fn relative() {
let now = Some(SystemTime::now());
let two_weeks_ago = git_date::parse("2 weeks ago", now).expect("valid time");
assert_eq!(Sign::Plus, two_weeks_ago.sign);
assert_eq!(0, two_weeks_ago.offset_in_seconds);
let expected = OffsetDateTime::from(now.unwrap()).saturating_sub(Duration::weeks(2));
// account for the loss of precision when creating `Time` with seconds
let expected = expected.replace_nanosecond(0).unwrap();
assert_eq!(
OffsetDateTime::from_unix_timestamp(two_weeks_ago.seconds_since_unix_epoch as i64).expect("valid datetime"),
expected,
"relative times differ"
);
}
5 changes: 3 additions & 2 deletions git-repository/src/repository/identity.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Cow;
use std::time::SystemTime;

use crate::{bstr::BString, permission};

Expand Down Expand Up @@ -125,13 +126,13 @@ impl Personas {
committer_email = committer_email.or_else(|| env_var("GIT_COMMITTER_EMAIL"));
committer_date = std::env::var("GIT_COMMITTER_DATE")
.ok()
.and_then(|date| git_date::parse(&date));
.and_then(|date| git_date::parse(&date, Some(SystemTime::now())).ok());

author_name = author_name.or_else(|| env_var("GIT_AUTHOR_NAME"));
author_email = author_email.or_else(|| env_var("GIT_AUTHOR_EMAIL"));
author_date = std::env::var("GIT_AUTHOR_DATE")
.ok()
.and_then(|date| git_date::parse(&date));
.and_then(|date| git_date::parse(&date, Some(SystemTime::now())).ok());

user_email = user_email.or_else(|| env_var("EMAIL")); // NOTE: we don't have permission for this specific one…
}
Expand Down
3 changes: 2 additions & 1 deletion git-revision/src/spec/parse/function.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::time::SystemTime;
use std::{convert::TryInto, str::FromStr};

use bstr::{BStr, BString, ByteSlice, ByteVec};
Expand Down Expand Up @@ -435,7 +436,7 @@ where
let time = nav
.to_str()
.ok()
.and_then(git_date::parse)
.and_then(|v| git_date::parse(v, Some(SystemTime::now())).ok())
.ok_or_else(|| Error::Time { input: nav.into() })?;
delegate
.reflog(delegate::ReflogLookup::Date(time))
Expand Down