From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 8824A6EC40; Tue, 17 Aug 2021 15:15:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8824A6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629202539; bh=V24D2HfypDAYNlAwsd4sGUTgLyOaJeBQC7nMUFSaen4=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=VanORXfd02yZgcDZzRqFNJpKXvgQTtjCEPzuolWT3KQB35pEG/+WXHN+o5tvezkEc iK7m4Zy2hgSB3iV2QiUd0uQQcp53Q9TMPiXkyTlyte7CoxNXvgjeSLiMOuRbv0xUGG OQNl/xzpZj0A5Uf0jYKB6IvDMpcl7b1lwD+fR39A= Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2F8F76EC40 for ; Tue, 17 Aug 2021 15:15:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2F8F76EC40 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1mFxzx-00076O-R8; Tue, 17 Aug 2021 15:15:18 +0300 To: Timur Safin References: <25f9e4a85319d7b4d1fa4fe70d9e9fb30b43c78d.1629071531.git.tsafin@tarantool.org> Message-ID: Date: Tue, 17 Aug 2021 15:15:17 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <25f9e4a85319d7b4d1fa4fe70d9e9fb30b43c78d.1629071531.git.tsafin@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9ECFD080E047A606F6525B29142351271182A05F5380850400345BF5912F20D263B61F0AE4234F58FD783C745ECEB78F06D5BB667966A6E4C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BCC85671EC7A750CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378D8F1FE4E764DFCDEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F2042A8439C65313A19101A640E808B7CACC7F00164DA146DAFE8445B8C89999728AA50765F7900637F6B57BC7E64490618DEB871D839B7333395957E7521B51C2DFABB839C843B9C08941B15DA834481F8AA50765F7900637F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BB9CEE4F2B4A90F8475ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5864A4555F107C8D6BEB2A1BC2780592E338E16C4693A5AD0D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7567C209D01CC1E34B410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3453741B480A6503C3A29D7E538A1111DDA6598B8C73769A3A55F3C3A6A4CE86B54465BE6482AB2F4C1D7E09C32AA3244C04896B9971CC6F50410E599092C6AAEE3C6EB905E3A8056B83B48618A63566E0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tpAx3C2753gOg== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446A6FDA86844F6A5F935FC101E6C029A3F329FE07998C0EACE424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 1/8] build: add Christian Hansen c-dt to the build X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 16.08.2021 02:59, Timur Safin via Tarantool-patches пишет: > * Integrated chansen/c-dt parser as 3rd party module to the > Tarantool cmake build process; > * Points to tarantool/c-dt instead of original chansen/c-dt to > have easier build integration, because there are additional > commits which have integrated cmake support and have established > symbols renaming facilities (similar to those we see in xxhash > or icu). > We have to be able to rename externally public symbols to avoid > name clashes with 3rd party modules. We prefix c-dt symbols > in the Tarantool build with `tnt_` prefix; > * took special care that generated build artifacts not override > in-source files, but use different build/ directory. > > * added datetime parsing unit tests, for literals - with and > without trailing timezones; > * also we check that strftime is reversible and produce consistent > results after roundtrip from/to strings; > * discovered the harder way that on *BSD/MacOSX `strftime()` format > `%z` outputs local time-zone if passed `tm_gmtoff` is 0. > This behaviour is different to that we observed on Linux, thus we > might have different execution results. Made test to not use `%z` > and only operate with normalized date time formats `%F` and `%T` > > Part of #5941 Hi! Thanks for the patch! Please, find a couple of style comments below. Also I think you may squash all the commits regarding c-dt cmake integration into one. And push all the commits from your branch to tarantool/c-dt master. It's not good that they live on a separate branch > --- > .gitmodules | 3 + > CMakeLists.txt | 8 + > cmake/BuildCDT.cmake | 8 + > src/CMakeLists.txt | 3 +- > test/unit/CMakeLists.txt | 3 +- > test/unit/datetime.c | 223 ++++++++++++++++++++++++ > test/unit/datetime.result | 358 ++++++++++++++++++++++++++++++++++++++ > third_party/c-dt | 1 + > 8 files changed, 605 insertions(+), 2 deletions(-) > create mode 100644 cmake/BuildCDT.cmake > create mode 100644 test/unit/datetime.c > create mode 100644 test/unit/datetime.result > create mode 160000 third_party/c-dt > > diff --git a/.gitmodules b/.gitmodules > index f2f91ee72..aa3fbae4e 100644 > --- a/.gitmodules > +++ b/.gitmodules > @@ -43,3 +43,6 @@ > [submodule "third_party/xxHash"] > path = third_party/xxHash > url =https://github.com/tarantool/xxHash > +[submodule "third_party/c-dt"] > + path = third_party/c-dt > + url =https://github.com/tarantool/c-dt.git > diff --git a/CMakeLists.txt b/CMakeLists.txt > index e25b81eac..53c86f2a5 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -571,6 +571,14 @@ endif() > # zstd > # > > +# > +# Chritian Hanson c-dt > +# > + > +include(BuildCDT) > +libccdt_build() > +add_dependencies(build_bundled_libs cdt) > + > # > # Third-Party misc > # > diff --git a/cmake/BuildCDT.cmake b/cmake/BuildCDT.cmake > new file mode 100644 > index 000000000..343fb1b99 > --- /dev/null > +++ b/cmake/BuildCDT.cmake > @@ -0,0 +1,8 @@ > +macro(libccdt_build) > + set(LIBCDT_INCLUDE_DIRS ${PROJECT_SOURCE_DIR}/third_party/c-dt/) > + set(LIBCDT_LIBRARIES cdt) > + > + file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/third_party/c-dt/build/) > + add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/c-dt > + ${CMAKE_CURRENT_BINARY_DIR}/third_party/c-dt/build/) > +endmacro() > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt > index adb03b3f4..97b0cb326 100644 > --- a/src/CMakeLists.txt > +++ b/src/CMakeLists.txt > @@ -193,7 +193,8 @@ target_link_libraries(server core coll http_parser bit uri uuid swim swim_udp > # Rule of thumb: if exporting a symbol from a static library, list the > # library here. > set (reexport_libraries server core misc bitset csv swim swim_udp swim_ev > - shutdown ${LUAJIT_LIBRARIES} ${MSGPUCK_LIBRARIES} ${ICU_LIBRARIES} ${CURL_LIBRARIES} ${XXHASH_LIBRARIES}) > + shutdown ${LUAJIT_LIBRARIES} ${MSGPUCK_LIBRARIES} ${ICU_LIBRARIES} > + ${CURL_LIBRARIES} ${XXHASH_LIBRARIES} ${LIBCDT_LIBRARIES}) > > set (common_libraries > ${reexport_libraries} > diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt > index 5bb7cd6e7..31b183a8f 100644 > --- a/test/unit/CMakeLists.txt > +++ b/test/unit/CMakeLists.txt > @@ -56,7 +56,8 @@ add_executable(uuid.test uuid.c core_test_utils.c) > target_link_libraries(uuid.test uuid unit) > add_executable(random.test random.c core_test_utils.c) > target_link_libraries(random.test core unit) > - > +add_executable(datetime.test datetime.c) > +target_link_libraries(datetime.test cdt unit) > add_executable(bps_tree.test bps_tree.cc) > target_link_libraries(bps_tree.test small misc) > add_executable(bps_tree_iterator.test bps_tree_iterator.cc) > diff --git a/test/unit/datetime.c b/test/unit/datetime.c > new file mode 100644 > index 000000000..64c19dac4 > --- /dev/null > +++ b/test/unit/datetime.c I see that you only test datetime parsing in this test. Not the datetime module itself. Maybe worth renaming the test to datetime_parse, or c-dt, or any other name you find suitable? P.S. never mind, I see more tests are added to this file later on. > + > +/* avoid introducing external datetime.h dependency - > + just copy paste it for today > +*/ Please, fix comment formatting: /* Something you have to say. */ /*  * Something you have to say  * spanning a couple of lines.  */ > +#define SECS_PER_DAY 86400 > +#define DT_EPOCH_1970_OFFSET 719163 > + > +struct datetime { > + double secs; > + int32_t nsec; > + int32_t offset; > +}; > + > +static int > +local_rd(const struct datetime *dt) > +{ > + return (int)((int64_t)dt->secs / SECS_PER_DAY) + DT_EPOCH_1970_OFFSET; > +} > + > +static int > +local_dt(const struct datetime *dt) > +{ > + return dt_from_rdn(local_rd(dt)); > +} > + > +struct tm * > +datetime_to_tm(struct datetime *dt) > +{ > + static struct tm tm; > + > + memset(&tm, 0, sizeof(tm)); > + dt_to_struct_tm(local_dt(dt), &tm); > + > + int seconds_of_day = (int64_t)dt->secs % 86400; > + tm.tm_hour = (seconds_of_day / 3600) % 24; > + tm.tm_min = (seconds_of_day / 60) % 60; > + tm.tm_sec = seconds_of_day % 60; > + > + return &tm; > +} > + > +static void datetime_test(void) > +{ > + size_t index; > + int64_t secs_expected; > + int32_t nanosecs; > + int32_t ofs; > + > + plan(355); > + parse_datetime(sample, sizeof(sample) - 1, > + &secs_expected, &nanosecs, &ofs); > + > + for (index = 0; index < DIM(tests); index++) { > + int64_t secs; > + int rc = parse_datetime(tests[index].sz, tests[index].len, > + &secs, &nanosecs, &ofs); Please, fix argument alignment here. > + is(rc, 0, "correct parse_datetime return value for '%s'", > + tests[index].sz); > + is(secs, secs_expected, "correct parse_datetime output " > + "seconds for '%s", tests[index].sz); > + > + /* check that stringized literal produces the same date */ > + /* time fields */ Same as above, please fix comment formatting. > + static char buff[40]; > + struct datetime dt = {secs, nanosecs, ofs}; > + /* datetime_to_tm returns time in GMT zone */ > + struct tm * p_tm = datetime_to_tm(&dt); > + size_t len = strftime(buff, sizeof buff, "%F %T", p_tm); > + ok(len > 0, "strftime"); > + int64_t parsed_secs; > + int32_t parsed_nsecs, parsed_ofs; > + rc = parse_datetime(buff, len, &parsed_secs, &parsed_nsecs, &parsed_ofs); > + is(rc, 0, "correct parse_datetime return value for '%s'", buff); > + is(secs, parsed_secs, > + "reversible seconds via strftime for '%s", buff); > + } > +} > + > +int > +main(void) > +{ > + plan(1); > + datetime_test(); > + > + return check_plan(); > +} > diff --git a/test/unit/datetime.result b/test/unit/datetime.result > new file mode 100644 > index 000000000..33997d9df > --- /dev/null > +++ b/test/unit/datetime.result > diff --git a/third_party/c-dt b/third_party/c-dt > new file mode 160000 > index 000000000..5b1398ca8 > --- /dev/null > +++ b/third_party/c-dt > @@ -0,0 +1 @@ > +Subproject commit 5b1398ca8f53513985670a2e832b5f6e253addf7 > -- > Serge Petrenko