# # # patch "cmd_ws_commit.cc" # from [388166b33195e7202dc63d0f762ab4b47d19bc30] # to [1d0701613003c35e113c6ce99baa5328912c4992] # # patch "dates.cc" # from [8a01802be5f9f514b73a29a9480a09d8415df3c4] # to [c9c15e57ebd09330e4a6a5b2d8abe324b4f6b179] # # patch "dates.hh" # from [e25c6a24f11ef3e948b44d7adb694661183b1d1a] # to [3a730e5536cece2437e9caad0e4f8f2767e4887d] # # patch "rev_output.cc" # from [95e5dd5794fa8133c73498002169065ba86d6faa] # to [9919b72bc029e80d06c4b443a35236f8f1eb4f6a] # # patch "unit-tests/dates.cc" # from [af8a777e15c6cccc98f7c006dfe4d84ffd5636ba] # to [8632e759182b511458d9e0ffe3ee7c7e5786ea36] # ============================================================ --- cmd_ws_commit.cc 388166b33195e7202dc63d0f762ab4b47d19bc30 +++ cmd_ws_commit.cc 1d0701613003c35e113c6ce99baa5328912c4992 @@ -67,7 +67,7 @@ get_log_message_interactively(lua_hooks get_log_message_interactively(lua_hooks & lua, workspace & work, revision_id const rid, revision_t const & rev, string & author, date_t & date, branch_name & branch, - utf8 & log_message) + string const & date_fmt, utf8 & log_message) { utf8 instructions( _("Ensure the values for Author, Date and Branch are correct, then enter\n" @@ -78,12 +78,12 @@ get_log_message_interactively(lua_hooks utf8 changelog; work.read_user_log(changelog); - string text = changelog(); - // ensure the changelog message ends with a newline. an empty changelog is // replaced with a single newline so that the ChangeLog: cert line is // produced by revision_header and there is somewhere to enter a message + string text = changelog(); + if (text.empty() || text[text.length()-1] != '\n') { text += '\n'; @@ -93,7 +93,6 @@ get_log_message_interactively(lua_hooks utf8 header; utf8 summary; - string date_fmt; revision_header(rid, rev, author, date, branch, changelog, date_fmt, header); revision_summary(rev, summary); @@ -110,6 +109,8 @@ get_log_message_interactively(lua_hooks system_to_utf8(output_message, full_message); + // FIXME: save the full message in _MTN/changelog so its not lost + string raw(full_message()); // Check the message carefully to make sure the user didn't edit somewhere @@ -124,28 +125,40 @@ get_log_message_interactively(lua_hooks E(raw.find(instructions()) == 0, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing instructions).")); + "Commit failed (missing or modified instructions).")); if (!summary().empty()) { size_t pos = raw.find(summary()); - // ignore the trailing blank line from the header as well - E(pos >= instructions().length() + header().length() - 1, + // ignore the blank lines around the changelog as well + E(pos != string::npos && + pos >= instructions().length() + header().length() - changelog().length() - 2, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing summary).")); + "Commit failed (missing or modified summary).")); + + E(raw.length() == summary().length() + pos, + origin::user, + F("Modifications outside of Author, Date, Branch or ChangeLog.\n" + "Commit failed (text following summary).")); + raw.resize(pos); // remove the change summary } raw = raw.substr(instructions().length()); // remove the instructions + string const AUTHOR(_("Author: ")); + string const DATE(_("Date: ")); + string const BRANCH(_("Branch: ")); + string const CHANGELOG(_("ChangeLog: ")); + // ensure the first 3 or 4 lines from the header still match - size_t pos = header().find("Author: "); + size_t pos = header().find(AUTHOR); E(header().substr(0, pos) == raw.substr(0, pos), origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing revision or parent header).")); + "Commit failed (missing Revision or Parent header).")); raw = raw.substr(pos); // remove the leading unchanged header lines @@ -158,37 +171,41 @@ get_log_message_interactively(lua_hooks "Commit failed (missing lines).")); vector::const_iterator line = lines.begin(); - E(line->find(_("Author: ")) == 0, + E(line->find(AUTHOR) == 0, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing author).")); + "Commit failed (missing Author header).")); - author = trim(line->substr(8)); + author = trim(line->substr(AUTHOR.length())); ++line; - E(line->find(_("Date: ")) == 0, + E(line->find(DATE) == 0, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing date).")); + "Commit failed (missing Date header).")); - date = trim(line->substr(6)); + if (date_fmt.empty()) + date = date_t(trim(line->substr(DATE.length()))); + else + date = date_t::from_formatted_localtime(trim(line->substr(DATE.length())), + date_fmt); ++line; - E(line->find(_("Branch: ")) == 0, + E(line->find(BRANCH) == 0, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing branch).")); + "Commit failed (missing Branch header).")); - branch = branch_name(trim(line->substr(8)), origin::user); + branch = branch_name(trim(line->substr(BRANCH.length())), origin::user); ++line; - E(*line == _("ChangeLog: "), + E(*line == CHANGELOG, origin::user, F("Modifications outside of Author, Date, Branch or ChangeLog.\n" - "Commit failed (missing changelog).")); + "Commit failed (missing ChangeLog header).")); // now pointing at the optional blank line after ChangeLog - ++line; + ++line; //FIXME if the optional blank line is missing this will probably join_lines(line, lines.end(), raw); raw = trim(raw) + "\n"; @@ -633,6 +650,20 @@ CMD(status, "status", "", CMD_REF(inform app.lua.hook_get_date_format_spec(date_fmt); } + // check that the specified date format can be parsed (for commit) + date_t now = date_t::now(); + date_t parsed; + try + { + string formatted = now.as_formatted_localtime(date_fmt); + parsed = date_t::from_formatted_localtime(formatted, date_fmt); + } + catch (recoverable_failure const & e) + { } + + if (parsed != now) + W(F("date format '%s' cannot be used for commit") % date_fmt); + work.get_parent_rosters(db, old_rosters); work.get_current_roster_shape(db, nis, new_roster); @@ -688,11 +719,13 @@ CMD(status, "status", "", CMD_REF(inform utf8 changelog; work.read_user_log(changelog); - // ensure the changelog message ends with a newline. an empty changelog message - // is left as-is so that no ChangeLog: line is produced by revision_header. + // ensure the changelog message ends with a newline. an empty changelog is + // replaced with a single newline so that the ChangeLog: cert line is + // produced by revision_header for consistency with commit. string text = changelog(); - if (!text.empty() && text[text.length()-1] != '\n') + + if (text.empty() || text[text.length()-1] != '\n') { text += '\n'; changelog = utf8(text, origin::user); @@ -1168,9 +1201,10 @@ CMD(commit, "commit", "ci", CMD_REF(work CMD(commit, "commit", "ci", CMD_REF(workspace), N_("[PATH]..."), N_("Commits workspace changes to the database"), "", - options::opts::branch | options::opts::message | options::opts::msgfile - | options::opts::date | options::opts::author | options::opts::depth - | options::opts::exclude) + options::opts::branch | options::opts::message | options::opts::msgfile | + options::opts::date | options::opts::author | options::opts::depth | + options::opts::exclude | + options::opts::format_dates | options::opts::date_fmt) { database db(app); key_store keys(app); @@ -1185,6 +1219,15 @@ CMD(commit, "commit", "ci", CMD_REF(work temp_node_id_source nis; cset excluded; + string date_fmt; + if (app.opts.format_dates) + { + if (!app.opts.date_fmt.empty()) + date_fmt = app.opts.date_fmt; + else + app.lua.hook_get_date_format_spec(date_fmt); + } + work.get_parent_rosters(db, old_rosters); work.get_current_roster_shape(db, nis, new_roster); @@ -1260,13 +1303,26 @@ CMD(commit, "commit", "ci", CMD_REF(work author = key.official_name(); } + // check that the current date format can be parsed + date_t parsed; + try + { + string formatted = now.as_formatted_localtime(date_fmt); + parsed = date_t::from_formatted_localtime(formatted, date_fmt); + } + catch (recoverable_failure const & e) + { } + + E(parsed == now, origin::user, + F("date format '%s' cannot be used for commit") % date_fmt); + if (!log_message_given) { // This call handles _MTN/log. get_log_message_interactively(app.lua, work, restricted_rev_id, restricted_rev, - author, date, app.opts.branch, - log_message); + author, date, app.opts.branch, + date_fmt, log_message); // We only check for empty log messages when the user entered them // interactively. Consensus was that if someone wanted to explicitly @@ -1306,7 +1362,7 @@ CMD(commit, "commit", "ci", CMD_REF(work app.opts.ignore_suspend_certs); unsigned int old_head_size = heads.size(); - P(F("beginning commit on branch '%s'") % app.opts.branch); + P(F("beginning commit on branch '%s'") % app.opts.branch); { transaction_guard guard(db); ============================================================ --- dates.cc 8a01802be5f9f514b73a29a9480a09d8415df3c4 +++ dates.cc c9c15e57ebd09330e4a6a5b2d8abe324b4f6b179 @@ -50,6 +50,9 @@ #undef SEC #undef MILLISEC +using std::localtime; +using std::mktime; +using std::numeric_limits; using std::ostream; using std::string; using std::time_t; @@ -143,7 +146,7 @@ our_gmtime(s64 ts, broken_down_time & tb our_gmtime(s64 ts, broken_down_time & tb) { // validate our assumptions about which basic type is u64 (see above). - I(PROBABLE_S64_MAX == std::numeric_limits::max()); + I(PROBABLE_S64_MAX == numeric_limits::max()); I(LATEST_SUPPORTED_DATE < PROBABLE_S64_MAX); I(valid_ms_count(ts)); @@ -312,7 +315,8 @@ date_t::date_t(int year, int month, int broken_down_time t; t.millisec = millisec; t.sec = sec; - t.min = min; t.hour = hour; + t.min = min; + t.hour = hour; t.day = day; t.month = month; t.year = year; @@ -327,7 +331,7 @@ date_t::now() date_t::now() { time_t t = std::time(0); - s64 tu = s64(t) * 1000 + get_epoch_offset(); + s64 tu = MILLISEC(t) + get_epoch_offset(); E(valid_ms_count(tu), origin::system, F("current date '%s' is outside usable range\n" "(your system clock may not be set correctly)") @@ -363,8 +367,26 @@ date_t::as_formatted_localtime(string co string date_t::as_formatted_localtime(string const & fmt) const { - time_t t(d/1000 - get_epoch_offset()); - tm tb(*std::localtime(&t)); + // note that the time_t value here may underflow or overflow if our date + // is outside of the representable range. for 32 bit time_t's the earliest + // representable time is 1901-12-13 20:45:52 UTC and the latest + // representable time is 2038-01-19 03:14:07 UTC. assert that the value is + // within range for the current time_t type so that localtime doesn't + // produce a bad result. + + s64 seconds = d/1000 - get_epoch_offset(); + + E(seconds >= numeric_limits::min(), origin::user, + F("date '%s' is out of range and cannot be formatted") + % as_iso_8601_extended()); + + E(seconds <= numeric_limits::max(), origin::user, + F("date '%s' is out of range and cannot be formatted") + % as_iso_8601_extended()); + + time_t t(seconds); // seconds since unix epoch in UTC + tm tb(*localtime(&t)); // converted to local timezone values + char buf[128]; // Poison the buffer so we can tell whether strftime() produced @@ -393,6 +415,51 @@ date_t::as_formatted_localtime(string co % (sizeof buf - 1)); } +date_t +date_t::from_formatted_localtime(string const & s, string const & fmt) +{ + tm tb; + memset(&tb, 0, sizeof(tb)); + char *p = strptime(s.c_str(), fmt.c_str(), &tb); // local timezone values + + E(p, origin::user, // strptime failed to match all of the format string + F("unable to parse date '%s' with format '%s'") % s % fmt); + + E(*p == 0, origin::user, // extraneous characters in input string + F("invalid date '%s' not matched by format '%s'") % s % fmt); + + // note that the time_t value here may underflow or overflow if our date + // is outside of the representable range. for 32 bit time_t's the earliest + // representable time is 1901-12-13 20:45:52 UTC and the latest + // representable time is 2038-01-19 03:14:07 UTC. mktime seems to detect this + // and return -1 for values it cannot handle. + + time_t t = mktime(&tb); // converted to seconds since unix epoch in UTC + + // -1 is also 1960-12-31 23:59:59 but mktime uses it to indicate errors + + E(t != -1, origin::user, + F("date '%s' is out of range and cannot be parsed") + % s); + + tm check(*localtime(&t)); // back to local timezone values + + E(tb.tm_sec == check.tm_sec && + tb.tm_min == check.tm_min && + tb.tm_hour == check.tm_hour && + tb.tm_mday == check.tm_mday && + tb.tm_mon == check.tm_mon && + tb.tm_year == check.tm_year && + tb.tm_wday == check.tm_wday && + tb.tm_yday == check.tm_yday && + tb.tm_isdst == check.tm_isdst, + origin::user, + F("date '%s' is out of range and cannot be parsed") + % s); + + return date_t(MILLISEC(t) + get_epoch_offset()); +} + s64 date_t::as_millisecs_since_unix_epoch() const { ============================================================ --- dates.hh e25c6a24f11ef3e948b44d7adb694661183b1d1a +++ dates.hh 3a730e5536cece2437e9caad0e4f8f2767e4887d @@ -45,6 +45,9 @@ struct date_t // display only. std::string as_formatted_localtime(std::string const & fmt) const; + static date_t from_formatted_localtime(std::string const & s, + std::string const & fmt); + // Retrieve the internal milliseconds count since the Unix epoch. s64 as_millisecs_since_unix_epoch() const; ============================================================ --- rev_output.cc 95e5dd5794fa8133c73498002169065ba86d6faa +++ rev_output.cc 9919b72bc029e80d06c4b443a35236f8f1eb4f6a @@ -42,9 +42,8 @@ revision_header(revision_id const rid, r empty_key)); certs.push_back(cert(rid, branch_cert_name, cert_value(branch(), origin::user), empty_key)); - if (!changelog().empty()) - certs.push_back(cert(rid, changelog_cert_name, - cert_value(changelog(), origin::user), empty_key)); + certs.push_back(cert(rid, changelog_cert_name, + cert_value(changelog(), origin::user), empty_key)); revision_header(rid, rev, certs, date_fmt, header); } ============================================================ --- unit-tests/dates.cc af8a777e15c6cccc98f7c006dfe4d84ffd5636ba +++ unit-tests/dates.cc 8632e759182b511458d9e0ffe3ee7c7e5786ea36 @@ -188,6 +188,64 @@ UNIT_TEST(from_string) #undef NO } +UNIT_TEST(roundtrip_localtimes) +{ +#define OK(x) do { \ + string iso8601 = x.as_iso_8601_extended(); \ + string formatted = x.as_formatted_localtime("%c"); \ + L(FL("iso 8601 date '%s' local date '%s'\n") % iso8601 % formatted); \ + date_t parsed = date_t::from_formatted_localtime(formatted, "%c"); \ + UNIT_TEST_CHECK(parsed == x); \ + } while (0) + + // this is the valid range of dates supported by 32 bit time_t + date_t start("1901-12-13T20:45:52"); + date_t end("2038-01-19T03:14:07"); + + OK(start); + OK(end); + + // stagger the millisecond values to hit different times of day + for (date_t date = start; date <= end; date += MILLISEC(DAY+HOUR+MIN+SEC)) + OK(date); + + start -= 1000; + end += 1000; + + // these tests run with LANG=C and TZ=UTC so the %c format seems to work + // however strptime does not like the timezone name when %c is used in + // other locales. with LANG=en_CA.UTF-8 this test fails. + + if (sizeof(time_t) <= 4) + { + UNIT_TEST_CHECK_THROW(start.as_formatted_localtime("%c"), + recoverable_failure); + UNIT_TEST_CHECK_THROW(date_t::from_formatted_localtime("Fri Dec 13 20:45:51 1901", "%c"), + recoverable_failure); + + UNIT_TEST_CHECK_THROW(end.as_formatted_localtime("%c"), + recoverable_failure); + UNIT_TEST_CHECK_THROW(date_t::from_formatted_localtime("Tue Jan 19 03:14:08 2038", "%c"), + recoverable_failure); + } + else + { + OK(start); + OK(end); + } + + // this date represents 1 second before the unix epoch which has a time_t + // value of -1. mktime returns -1 to indicate that it was unable to + // convert a struct tm into a valid time_t value even though dates + // before/after this date are valid. + date_t mktime1("1969-12-31T23:59:59"); + + // this can be formatted but not parsed. 64 bit time_t probably doesn't help + mktime1.as_formatted_localtime("%c"); + UNIT_TEST_CHECK_THROW(date_t::from_formatted_localtime("Wed Dec 31 23:59:59 1969", "%c"), + recoverable_failure); +#undef OK +} UNIT_TEST(from_unix_epoch) { #define OK_(x,y) do { \