From 9733e80c2ae7517f44c658cd2914d99454470dd1 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 22 Mar 2013 10:44:45 -0700 Subject: [PATCH 1/6] Add has_cr_in_index check to CRLF filter This adds a check to the drop_crlf filter path to check it the file in the index already has a CR in it, in which case this will not drop the CRs from the workdir file contents. This uncovered a "bug" in `git_blob_create_fromworkdir` where the full path to the file was passed to look up the attributes instead of the relative path from the working directory root. This meant that the check in the index for a pre-existing entry of the same name was failing. --- src/blob.c | 14 +++++++++++-- src/crlf.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/blob.c b/src/blob.c index bcb6ac96b..3ff141c7f 100644 --- a/src/blob.c +++ b/src/blob.c @@ -221,7 +221,9 @@ int git_blob_create_fromworkdir(git_oid *oid, git_repository *repo, const char * return -1; } - error = blob_create_internal(oid, repo, git_buf_cstr(&full_path), git_buf_cstr(&full_path), true); + error = blob_create_internal( + oid, repo, git_buf_cstr(&full_path), + git_buf_cstr(&full_path) + strlen(workdir), true); git_buf_free(&full_path); return error; @@ -231,13 +233,21 @@ int git_blob_create_fromdisk(git_oid *oid, git_repository *repo, const char *pat { int error; git_buf full_path = GIT_BUF_INIT; + const char *workdir, *hintpath; if ((error = git_path_prettify(&full_path, path, NULL)) < 0) { git_buf_free(&full_path); return error; } - error = blob_create_internal(oid, repo, git_buf_cstr(&full_path), git_buf_cstr(&full_path), true); + hintpath = git_buf_cstr(&full_path); + workdir = git_repository_workdir(repo); + + if (workdir && !git__prefixcmp(hintpath, workdir)) + hintpath += strlen(workdir); + + error = blob_create_internal( + oid, repo, git_buf_cstr(&full_path), hintpath, true); git_buf_free(&full_path); return error; diff --git a/src/crlf.c b/src/crlf.c index 060d39d37..84347ac6c 100644 --- a/src/crlf.c +++ b/src/crlf.c @@ -10,8 +10,8 @@ #include "hash.h" #include "filter.h" #include "repository.h" - #include "git2/attr.h" +#include "git2/blob.h" struct crlf_attrs { int crlf_action; @@ -21,6 +21,8 @@ struct crlf_attrs { struct crlf_filter { git_filter f; struct crlf_attrs attrs; + git_repository *repo; + char path[GIT_FLEX_ARRAY]; }; static int check_crlf(const char *value) @@ -132,7 +134,46 @@ static int drop_crlf(git_buf *dest, const git_buf *source) return 0; } -static int crlf_apply_to_odb(git_filter *self, git_buf *dest, const git_buf *source) +static int has_cr_in_index(git_filter *self) +{ + struct crlf_filter *filter = (struct crlf_filter *)self; + git_index *index; + const git_index_entry *entry; + git_blob *blob; + const void *blobcontent; + git_off_t blobsize; + bool found_cr; + + if (git_repository_index__weakptr(&index, filter->repo) < 0) { + giterr_clear(); + return false; + } + + if (!(entry = git_index_get_bypath(index, filter->path, 0)) && + !(entry = git_index_get_bypath(index, filter->path, 1))) + return false; + + if (!S_ISREG(entry->mode)) /* don't crlf filter non-blobs */ + return true; + + if (git_blob_lookup(&blob, filter->repo, &entry->oid) < 0) + return false; + + blobcontent = git_blob_rawcontent(blob); + blobsize = git_blob_rawsize(blob); + if (!git__is_sizet(blobsize)) + blobsize = (size_t)-1; + + found_cr = (blobcontent != NULL && + blobsize > 0 && + memchr(blobcontent, '\r', (size_t)blobsize) != NULL); + + git_blob_free(blob); + return found_cr; +} + +static int crlf_apply_to_odb( + git_filter *self, git_buf *dest, const git_buf *source) { struct crlf_filter *filter = (struct crlf_filter *)self; @@ -162,16 +203,14 @@ static int crlf_apply_to_odb(git_filter *self, git_buf *dest, const git_buf *sou if (stats.cr != stats.crlf) return -1; -#if 0 - if (crlf_action == CRLF_GUESS) { + if (filter->attrs.crlf_action == GIT_CRLF_GUESS) { /* * If the file in the index has any CR in it, do not convert. * This is the new safer autocrlf handling. */ - if (has_cr_in_index(path)) - return 0; + if (has_cr_in_index(self)) + return -1; } -#endif if (!stats.cr) return -1; @@ -266,6 +305,7 @@ static int find_and_add_filter( { struct crlf_attrs ca; struct crlf_filter *filter; + size_t pathlen; int error; /* Load gitattributes for the path */ @@ -293,12 +333,15 @@ static int find_and_add_filter( /* If we're good, we create a new filter object and push it * into the filters array */ - filter = git__malloc(sizeof(struct crlf_filter)); + pathlen = strlen(path); + filter = git__malloc(sizeof(struct crlf_filter) + pathlen + 1); GITERR_CHECK_ALLOC(filter); filter->f.apply = apply; filter->f.do_free = NULL; memcpy(&filter->attrs, &ca, sizeof(struct crlf_attrs)); + filter->repo = repo; + memcpy(filter->path, path, pathlen + 1); return git_vector_insert(filters, filter); } From b8acb775e25c76f07dac8855ad0a88b37f7b2f17 Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Thu, 7 Mar 2013 22:15:40 +0100 Subject: [PATCH 2/6] Added some tests for issue #1397 Signed-off-by: Sven Strickroth --- tests-clar/checkout/crlf.c | 8 + tests-clar/checkout/index.c | 17 ++ tests-clar/checkout/tree.c | 34 ++++ tests-clar/diff/tree.c | 25 +++ tests-clar/diff/workdir.c | 179 ++++++++++++++++++ tests-clar/index/tests.c | 40 ++++ tests-clar/resources/issue_1397/.gitted/HEAD | 1 + .../resources/issue_1397/.gitted/config | 6 + tests-clar/resources/issue_1397/.gitted/index | Bin 0 -> 233 bytes .../7f/483a738f867e5b21c8f377d70311f011eb48b5 | 3 + .../83/12e0889a9cbab77c732b6bc39b51a683e3a318 | Bin 0 -> 48 bytes .../8a/7ef047fc933edb62e84e7977b0612ec3f6f283 | Bin 0 -> 141 bytes .../8e/8f80088a9274fd23584992f587083ca1bcbbac | Bin 0 -> 63 bytes .../f2/c62dea0372a0578e053697d5c1ba1ac05e774a | Bin 0 -> 94 bytes .../ff/3578d64d199d5b48d92bbb569e0a273e411741 | Bin 0 -> 73 bytes .../issue_1397/.gitted/refs/heads/master | 1 + tests-clar/resources/issue_1397/crlf_file.txt | 3 + .../issue_1397/some_other_crlf_file.txt | 3 + tests-clar/status/worktree.c | 15 ++ 19 files changed, 335 insertions(+) create mode 100644 tests-clar/resources/issue_1397/.gitted/HEAD create mode 100644 tests-clar/resources/issue_1397/.gitted/config create mode 100644 tests-clar/resources/issue_1397/.gitted/index create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/7f/483a738f867e5b21c8f377d70311f011eb48b5 create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/83/12e0889a9cbab77c732b6bc39b51a683e3a318 create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/8a/7ef047fc933edb62e84e7977b0612ec3f6f283 create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/8e/8f80088a9274fd23584992f587083ca1bcbbac create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/f2/c62dea0372a0578e053697d5c1ba1ac05e774a create mode 100644 tests-clar/resources/issue_1397/.gitted/objects/ff/3578d64d199d5b48d92bbb569e0a273e411741 create mode 100644 tests-clar/resources/issue_1397/.gitted/refs/heads/master create mode 100644 tests-clar/resources/issue_1397/crlf_file.txt create mode 100644 tests-clar/resources/issue_1397/some_other_crlf_file.txt diff --git a/tests-clar/checkout/crlf.c b/tests-clar/checkout/crlf.c index 74da27652..39889a181 100644 --- a/tests-clar/checkout/crlf.c +++ b/tests-clar/checkout/crlf.c @@ -35,6 +35,7 @@ void test_checkout_crlf__detect_crlf_autocrlf_false(void) git_checkout_head(g_repo, &opts); test_file_contents("./crlf/all-lf", ALL_LF_TEXT_RAW); + test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); #endif } @@ -55,6 +56,9 @@ void test_checkout_crlf__autocrlf_false_index_size_is_unfiltered_size(void) cl_assert((entry = git_index_get_bypath(index, "all-lf", 0)) != NULL); cl_assert(entry->file_size == strlen(ALL_LF_TEXT_RAW)); + cl_assert((entry = git_index_get_bypath(index, "all-crlf", 0)) != NULL); + cl_assert(entry->file_size == strlen(ALL_CRLF_TEXT_RAW)); + git_index_free(index); #endif } @@ -70,6 +74,7 @@ void test_checkout_crlf__detect_crlf_autocrlf_true(void) git_checkout_head(g_repo, &opts); test_file_contents("./crlf/all-lf", ALL_LF_TEXT_AS_CRLF); + test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); #endif } @@ -90,6 +95,9 @@ void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void) cl_assert((entry = git_index_get_bypath(index, "all-lf", 0)) != NULL); cl_assert(entry->file_size == strlen(ALL_LF_TEXT_AS_CRLF)); + cl_assert((entry = git_index_get_bypath(index, "all-crlf", 0)) != NULL); + cl_assert(entry->file_size == strlen(ALL_CRLF_TEXT_RAW)); + git_index_free(index); #endif } diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 3976dd20e..6dfa95dd3 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -488,3 +488,20 @@ void test_checkout_index__can_checkout_a_newly_initialized_repository(void) cl_git_pass(git_checkout_index(g_repo, NULL, NULL)); } + +void test_checkout_index__issue_1397(void) +{ + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; + + test_checkout_index__cleanup(); + + g_repo = cl_git_sandbox_init("issue_1397"); + + set_core_autocrlf_to(true); + + opts.checkout_strategy = GIT_CHECKOUT_FORCE; + + cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); + + test_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); +} diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index 8309cd721..0f0ac6982 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -481,3 +481,37 @@ void test_checkout_tree__can_checkout_with_last_workdir_item_missing(void) git_commit_free(commit); git_index_free(index); } + +void test_checkout_tree__issue_1397(void) +{ + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; + git_config *cfg; + const char *partial_oid = "8a7ef04"; + size_t len = strlen(partial_oid); + git_oid oid; + git_object *obj = NULL; + git_tree *tree = NULL; + + g_repo = cl_git_sandbox_init("issue_1397"); + + cl_git_pass(git_repository_config(&cfg, g_repo)); + cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); + git_config_free(cfg); + + if (git_oid_fromstrn(&oid, partial_oid, len) == 0) + git_object_lookup_prefix(&obj, g_repo, &oid, len, GIT_OBJ_ANY); + cl_assert(obj); + cl_assert(git_object_type(obj) == GIT_OBJ_COMMIT); + cl_git_pass(git_commit_tree(&tree, (git_commit *)obj)); + git_object_free(obj); + + opts.checkout_strategy = GIT_CHECKOUT_FORCE; + + cl_assert(tree != NULL); + + git_checkout_tree(g_repo, (git_object *)tree, &opts); + + test_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); + + git_tree_free(tree); +} diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index e86f8d538..06e649979 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -431,3 +431,28 @@ void test_diff_tree__regular_blob_mode_changed_to_executable_file(void) cl_assert_equal_i(0, expect.file_status[GIT_DELTA_ADDED]); cl_assert_equal_i(0, expect.file_status[GIT_DELTA_TYPECHANGE]); } + +void test_diff_tree__issue_1397(void) +{ + // this test shows, that it is not needed + git_config *cfg; + g_repo = cl_git_sandbox_init("issue_1397"); + + cl_git_pass(git_repository_config(&cfg, g_repo)); + cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); + git_config_free(cfg); + + cl_assert((a = resolve_commit_oid_to_tree(g_repo, "8a7ef04")) != NULL); + cl_assert((b = resolve_commit_oid_to_tree(g_repo, "7f483a7")) != NULL); + + cl_git_pass(git_diff_tree_to_tree(&diff, g_repo, a, b, &opts)); + + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &expect)); + + cl_assert_equal_i(1, expect.files); + cl_assert_equal_i(0, expect.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(1, expect.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, expect.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(0, expect.file_status[GIT_DELTA_TYPECHANGE]); +} diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 1ac56311c..7e66671b2 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -1084,3 +1084,182 @@ void test_diff_workdir__can_diff_empty_file(void) git_diff_patch_free(patch); git_diff_list_free(diff); } + +static void set_config_entry_to(const char *entry_name, bool value) +{ + git_config *cfg; + + cl_git_pass(git_repository_config(&cfg, g_repo)); + cl_git_pass(git_config_set_bool(cfg, entry_name, value)); + + git_config_free(cfg); +} + +static void set_core_autocrlf_to(bool value) +{ + set_config_entry_to("core.autocrlf", value); +} + +void test_diff_workdir__to_index_issue_1397(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + diff_expects exp; + int use_iterator; + + g_repo = cl_git_sandbox_init("issue_1397"); + + set_core_autocrlf_to(true); + + opts.context_lines = 3; + opts.interhunk_lines = 1; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + + for (use_iterator = 0; use_iterator <= 1; use_iterator++) { + memset(&exp, 0, sizeof(exp)); + + if (use_iterator) + cl_git_pass(diff_foreach_via_iterator( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + else + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); + + cl_assert_equal_i(0, exp.hunks); + + cl_assert_equal_i(0, exp.lines); + cl_assert_equal_i(0, exp.line_ctxt); + cl_assert_equal_i(0, exp.line_adds); + cl_assert_equal_i(0, exp.line_dels); + } + + git_diff_list_free(diff); + diff = NULL; + memset(&exp, 0, sizeof(exp)); + + cl_git_rewritefile("issue_1397/crlf_file.txt", "first line\r\nsecond line modified\r\nboth with crlf"); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + + for (use_iterator = 0; use_iterator <= 1; use_iterator++) { + memset(&exp, 0, sizeof(exp)); + + if (use_iterator) + cl_git_pass(diff_foreach_via_iterator( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + else + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + + cl_assert_equal_i(1, exp.files); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(1, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); + + cl_assert_equal_i(1, exp.hunks); + + cl_assert_equal_i(5, exp.lines); + cl_assert_equal_i(3, exp.line_ctxt); + cl_assert_equal_i(1, exp.line_adds); + cl_assert_equal_i(1, exp.line_dels); + } + + git_diff_list_free(diff); +} + +void test_diff_workdir__to_tree_issue_1397(void) +{ + /* grabbed a couple of commit oids from the history of the attr repo */ + const char *a_commit = "51883ad"; /* the current HEAD */ + git_tree *a; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + git_diff_list *diff2 = NULL; + diff_expects exp; + int use_iterator; + + g_repo = cl_git_sandbox_init("issue_1397"); + + set_core_autocrlf_to(true); + + a = resolve_commit_oid_to_tree(g_repo, a_commit); + + opts.context_lines = 3; + opts.interhunk_lines = 1; + + cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, a, &opts)); + + for (use_iterator = 0; use_iterator <= 1; use_iterator++) { + memset(&exp, 0, sizeof(exp)); + + if (use_iterator) + cl_git_pass(diff_foreach_via_iterator( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + else + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); + } + + /* Since there is no git diff equivalent, let's just assume that the + * text diffs produced by git_diff_foreach are accurate here. We will + * do more apples-to-apples test comparison below. + */ + + git_diff_list_free(diff); + diff = NULL; + memset(&exp, 0, sizeof(exp)); + + /* This is a compatible emulation of "git diff " which looks like + * a workdir to tree diff (even though it is not really). This is what + * you would get from "git diff --name-status 26a125ee1bf" + */ + cl_git_pass(git_diff_tree_to_index(&diff, g_repo, a, NULL, &opts)); + cl_git_pass(git_diff_index_to_workdir(&diff2, g_repo, NULL, &opts)); + cl_git_pass(git_diff_merge(diff, diff2)); + git_diff_list_free(diff2); + + for (use_iterator = 0; use_iterator <= 1; use_iterator++) { + memset(&exp, 0, sizeof(exp)); + + if (use_iterator) + cl_git_pass(diff_foreach_via_iterator( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + else + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); + cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); + + cl_assert_equal_i(0, exp.hunks); + + cl_assert_equal_i(0, exp.lines); + cl_assert_equal_i(0, exp.line_ctxt); + cl_assert_equal_i(0, exp.line_adds); + cl_assert_equal_i(0, exp.line_dels); + } + + git_diff_list_free(diff); + git_tree_free(a); +} diff --git a/tests-clar/index/tests.c b/tests-clar/index/tests.c index 64f547ead..49defd03f 100644 --- a/tests-clar/index/tests.c +++ b/tests-clar/index/tests.c @@ -251,6 +251,46 @@ void test_index_tests__add(void) git_repository_free(repo); } +void test_index_tests__add_issue_1397(void) +{ + git_index *index; + git_config *cfg; + git_repository *repo; + const git_index_entry *entry; + git_oid id1; + + cl_set_cleanup(&cleanup_myrepo, NULL); + + repo = cl_git_sandbox_init("issue_1397"); + + cl_git_pass(git_repository_config(&cfg, repo)); + cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); + git_config_free(cfg); + + /* Ensure we're the only guy in the room */ + cl_git_pass(git_repository_index(&index, repo)); + + /* Store the expected hash of the file/blob + * This has been generated by executing the following + * $ git hash-object crlf_file.txt + */ + cl_git_pass(git_oid_fromstr(&id1, "8312e0889a9cbab77c732b6bc39b51a683e3a318")); + + /* Make sure the initial SHA-1 is correct */ + cl_assert((entry = git_index_get_bypath(index, "crlf_file.txt", 0)) != NULL); + cl_assert(git_oid_cmp(&id1, &entry->oid) == 0); + + /* Update the index */ + cl_git_pass(git_index_add_bypath(index, "crlf_file.txt")); + + /* Check the new SHA-1 */ + cl_assert((entry = git_index_get_bypath(index, "crlf_file.txt", 0)) != NULL); + cl_assert(git_oid_cmp(&id1, &entry->oid) == 0); + + git_index_free(index); + git_repository_free(repo); +} + void test_index_tests__add_bypath_to_a_bare_repository_returns_EBAREPO(void) { git_repository *bare_repo; diff --git a/tests-clar/resources/issue_1397/.gitted/HEAD b/tests-clar/resources/issue_1397/.gitted/HEAD new file mode 100644 index 000000000..cb089cd89 --- /dev/null +++ b/tests-clar/resources/issue_1397/.gitted/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests-clar/resources/issue_1397/.gitted/config b/tests-clar/resources/issue_1397/.gitted/config new file mode 100644 index 000000000..ba5bbde24 --- /dev/null +++ b/tests-clar/resources/issue_1397/.gitted/config @@ -0,0 +1,6 @@ +[core] + bare = false + repositoryformatversion = 0 + filemode = false + logallrefupdates = true + ignorecase = true diff --git a/tests-clar/resources/issue_1397/.gitted/index b/tests-clar/resources/issue_1397/.gitted/index new file mode 100644 index 0000000000000000000000000000000000000000..fa0f541d682c27cb907103903c5f5cee2a914f43 GIT binary patch literal 233 zcmZ?q402{*U|<5_K#QOIfiy#)#ovAqjR1{H5NrnZW}yciv*zsDUQ?`{eRy`@vgXH& zB^Y>?2X)ol9JgS(^-#Fcx! Zt96*&*|Irjp3UNDk*o2SHn6w70ssOqMw$Qs literal 0 HcmV?d00001 diff --git a/tests-clar/resources/issue_1397/.gitted/objects/7f/483a738f867e5b21c8f377d70311f011eb48b5 b/tests-clar/resources/issue_1397/.gitted/objects/7f/483a738f867e5b21c8f377d70311f011eb48b5 new file mode 100644 index 000000000..63bcb5d76 --- /dev/null +++ b/tests-clar/resources/issue_1397/.gitted/objects/7f/483a738f867e5b21c8f377d70311f011eb48b5 @@ -0,0 +1,3 @@ +xQN0 as +!'nTBC+e\vӯǾofPPL8FΆ%_ċb4IܗtkULdeId<3/|0j1֣\Gcwe]~ߊS +)GxEqsUڇ8mk~yI \ No newline at end of file diff --git a/tests-clar/resources/issue_1397/.gitted/objects/83/12e0889a9cbab77c732b6bc39b51a683e3a318 b/tests-clar/resources/issue_1397/.gitted/objects/83/12e0889a9cbab77c732b6bc39b51a683e3a318 new file mode 100644 index 0000000000000000000000000000000000000000..06b59fede0a6cf2f69a233ec6b0193ee2ea260cc GIT binary patch literal 48 zcmV-00MGw;0ZYosPf{?pWJt>_DlSpT$;?aTbjN= z==`BKX2@BY(UH}J-jj*ikso4=#(I`v7!_;8QKJa2KGz1*o1ZZCR??%@KEs9AP?lTT vPJ3v4CcXkHJ6LC=F>sV9rAXhi^B>ylpB8r_Cg^*G)RqjnEWC*i1sy#$=MO`P literal 0 HcmV?d00001 diff --git a/tests-clar/resources/issue_1397/.gitted/objects/8e/8f80088a9274fd23584992f587083ca1bcbbac b/tests-clar/resources/issue_1397/.gitted/objects/8e/8f80088a9274fd23584992f587083ca1bcbbac new file mode 100644 index 0000000000000000000000000000000000000000..f5c776b1704a072e0b229fa2b8a67c37f2d79a9a GIT binary patch literal 63 zcmV-F0Korv0ZYosPf{>7V@S&^DlSpT$;?aT%mOEMIS^K(-bk~0$X V(t)x``9Kk{aB@*j8UVhP7870q8eaea literal 0 HcmV?d00001 diff --git a/tests-clar/resources/issue_1397/.gitted/objects/f2/c62dea0372a0578e053697d5c1ba1ac05e774a b/tests-clar/resources/issue_1397/.gitted/objects/f2/c62dea0372a0578e053697d5c1ba1ac05e774a new file mode 100644 index 0000000000000000000000000000000000000000..f932f36183338a2623adff268e0d6116ecea66d5 GIT binary patch literal 94 zcmV-k0HObQ0V^p=O;xZoW-v4`Ff%bxNG{4ri%-kUN!2T#oF12 zX9q58e!N%$s<1deH#I)LBqOyb9#v0Ye*;I?q>{hN5uTI2wsY7l+_QTP0D0~xg5r)R A!2kdN literal 0 HcmV?d00001 diff --git a/tests-clar/resources/issue_1397/.gitted/objects/ff/3578d64d199d5b48d92bbb569e0a273e411741 b/tests-clar/resources/issue_1397/.gitted/objects/ff/3578d64d199d5b48d92bbb569e0a273e411741 new file mode 100644 index 0000000000000000000000000000000000000000..fbd7317276615c933f2300b066f6140b3ca973fd GIT binary patch literal 73 zcmV-P0Ji^l0V^p=O;xZoW-v4`Ff%bxNG{4ri%-kUN!2T#oF12 fX9q58e!N%$s<1deH#I)LBqOybp0FMOg{~ Date: Fri, 22 Mar 2013 14:52:29 -0700 Subject: [PATCH 3/6] Test fixes and cleanup This fixes some places where the new tests were leaving the test area in a bad state or were freeing data they should not free. It also removes code that is extraneous to the core issue and fixes an invalid SHA being looked up in one of the tests (which was failing, but for the wrong reason). --- tests-clar/checkout/index.c | 2 +- tests-clar/checkout/tree.c | 25 ++---- tests-clar/diff/tree.c | 8 +- tests-clar/diff/workdir.c | 149 ++++++++--------------------------- tests-clar/index/tests.c | 18 +++-- tests-clar/status/worktree.c | 5 +- 6 files changed, 55 insertions(+), 152 deletions(-) diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 6dfa95dd3..33506a669 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -497,7 +497,7 @@ void test_checkout_index__issue_1397(void) g_repo = cl_git_sandbox_init("issue_1397"); - set_core_autocrlf_to(true); + cl_repo_set_bool(g_repo, "core.autocrlf", true); opts.checkout_strategy = GIT_CHECKOUT_FORCE; diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index 0f0ac6982..2a8fbc457 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -485,33 +485,22 @@ void test_checkout_tree__can_checkout_with_last_workdir_item_missing(void) void test_checkout_tree__issue_1397(void) { git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; - git_config *cfg; const char *partial_oid = "8a7ef04"; - size_t len = strlen(partial_oid); - git_oid oid; - git_object *obj = NULL; - git_tree *tree = NULL; + git_object *tree = NULL; + + test_checkout_tree__cleanup(); /* cleanup default checkout */ g_repo = cl_git_sandbox_init("issue_1397"); - cl_git_pass(git_repository_config(&cfg, g_repo)); - cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); - git_config_free(cfg); + cl_repo_set_bool(g_repo, "core.autocrlf", true); - if (git_oid_fromstrn(&oid, partial_oid, len) == 0) - git_object_lookup_prefix(&obj, g_repo, &oid, len, GIT_OBJ_ANY); - cl_assert(obj); - cl_assert(git_object_type(obj) == GIT_OBJ_COMMIT); - cl_git_pass(git_commit_tree(&tree, (git_commit *)obj)); - git_object_free(obj); + cl_git_pass(git_revparse_single(&tree, g_repo, partial_oid)); opts.checkout_strategy = GIT_CHECKOUT_FORCE; - cl_assert(tree != NULL); - - git_checkout_tree(g_repo, (git_object *)tree, &opts); + cl_git_pass(git_checkout_tree(g_repo, tree, &opts)); test_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); - git_tree_free(tree); + git_object_free(tree); } diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index 06e649979..850feefde 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -434,13 +434,11 @@ void test_diff_tree__regular_blob_mode_changed_to_executable_file(void) void test_diff_tree__issue_1397(void) { - // this test shows, that it is not needed - git_config *cfg; + /* this test shows that it is not needed */ + g_repo = cl_git_sandbox_init("issue_1397"); - cl_git_pass(git_repository_config(&cfg, g_repo)); - cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); - git_config_free(cfg); + cl_repo_set_bool(g_repo, "core.autocrlf", true); cl_assert((a = resolve_commit_oid_to_tree(g_repo, "8a7ef04")) != NULL); cl_assert((b = resolve_commit_oid_to_tree(g_repo, "7f483a7")) != NULL); diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 7e66671b2..f67f09ae8 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -1085,112 +1085,66 @@ void test_diff_workdir__can_diff_empty_file(void) git_diff_list_free(diff); } -static void set_config_entry_to(const char *entry_name, bool value) -{ - git_config *cfg; - - cl_git_pass(git_repository_config(&cfg, g_repo)); - cl_git_pass(git_config_set_bool(cfg, entry_name, value)); - - git_config_free(cfg); -} - -static void set_core_autocrlf_to(bool value) -{ - set_config_entry_to("core.autocrlf", value); -} - void test_diff_workdir__to_index_issue_1397(void) { git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; - int use_iterator; g_repo = cl_git_sandbox_init("issue_1397"); - set_core_autocrlf_to(true); + cl_repo_set_bool(g_repo, "core.autocrlf", true); opts.context_lines = 3; opts.interhunk_lines = 1; cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); - for (use_iterator = 0; use_iterator <= 1; use_iterator++) { - memset(&exp, 0, sizeof(exp)); + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - if (use_iterator) - cl_git_pass(diff_foreach_via_iterator( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - else - cl_git_pass(git_diff_foreach( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - - cl_assert_equal_i(0, exp.files); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); - - cl_assert_equal_i(0, exp.hunks); - - cl_assert_equal_i(0, exp.lines); - cl_assert_equal_i(0, exp.line_ctxt); - cl_assert_equal_i(0, exp.line_adds); - cl_assert_equal_i(0, exp.line_dels); - } + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.hunks); + cl_assert_equal_i(0, exp.lines); git_diff_list_free(diff); diff = NULL; - memset(&exp, 0, sizeof(exp)); - cl_git_rewritefile("issue_1397/crlf_file.txt", "first line\r\nsecond line modified\r\nboth with crlf"); + cl_git_rewritefile("issue_1397/crlf_file.txt", + "first line\r\nsecond line modified\r\nboth with crlf"); cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); - for (use_iterator = 0; use_iterator <= 1; use_iterator++) { - memset(&exp, 0, sizeof(exp)); + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - if (use_iterator) - cl_git_pass(diff_foreach_via_iterator( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - else - cl_git_pass(git_diff_foreach( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); + cl_assert_equal_i(1, exp.files); + cl_assert_equal_i(1, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(1, exp.files); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(1, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); + cl_assert_equal_i(1, exp.hunks); - cl_assert_equal_i(1, exp.hunks); - - cl_assert_equal_i(5, exp.lines); - cl_assert_equal_i(3, exp.line_ctxt); - cl_assert_equal_i(1, exp.line_adds); - cl_assert_equal_i(1, exp.line_dels); - } + cl_assert_equal_i(5, exp.lines); + cl_assert_equal_i(3, exp.line_ctxt); + cl_assert_equal_i(1, exp.line_adds); + cl_assert_equal_i(1, exp.line_dels); git_diff_list_free(diff); } void test_diff_workdir__to_tree_issue_1397(void) { - /* grabbed a couple of commit oids from the history of the attr repo */ - const char *a_commit = "51883ad"; /* the current HEAD */ + const char *a_commit = "7f483a738"; /* the current HEAD */ git_tree *a; git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; git_diff_list *diff2 = NULL; diff_expects exp; - int use_iterator; g_repo = cl_git_sandbox_init("issue_1397"); - set_core_autocrlf_to(true); + cl_repo_set_bool(g_repo, "core.autocrlf", true); a = resolve_commit_oid_to_tree(g_repo, a_commit); @@ -1199,66 +1153,29 @@ void test_diff_workdir__to_tree_issue_1397(void) cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, a, &opts)); - for (use_iterator = 0; use_iterator <= 1; use_iterator++) { - memset(&exp, 0, sizeof(exp)); + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - if (use_iterator) - cl_git_pass(diff_foreach_via_iterator( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - else - cl_git_pass(git_diff_foreach( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - - cl_assert_equal_i(0, exp.files); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); - } - - /* Since there is no git diff equivalent, let's just assume that the - * text diffs produced by git_diff_foreach are accurate here. We will - * do more apples-to-apples test comparison below. - */ + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.hunks); + cl_assert_equal_i(0, exp.lines); git_diff_list_free(diff); diff = NULL; - memset(&exp, 0, sizeof(exp)); - /* This is a compatible emulation of "git diff " which looks like - * a workdir to tree diff (even though it is not really). This is what - * you would get from "git diff --name-status 26a125ee1bf" - */ cl_git_pass(git_diff_tree_to_index(&diff, g_repo, a, NULL, &opts)); cl_git_pass(git_diff_index_to_workdir(&diff2, g_repo, NULL, &opts)); cl_git_pass(git_diff_merge(diff, diff2)); git_diff_list_free(diff2); - for (use_iterator = 0; use_iterator <= 1; use_iterator++) { - memset(&exp, 0, sizeof(exp)); + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_foreach( + diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - if (use_iterator) - cl_git_pass(diff_foreach_via_iterator( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - else - cl_git_pass(git_diff_foreach( - diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp)); - - cl_assert_equal_i(0, exp.files); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_UNTRACKED]); - - cl_assert_equal_i(0, exp.hunks); - - cl_assert_equal_i(0, exp.lines); - cl_assert_equal_i(0, exp.line_ctxt); - cl_assert_equal_i(0, exp.line_adds); - cl_assert_equal_i(0, exp.line_dels); - } + cl_assert_equal_i(0, exp.files); + cl_assert_equal_i(0, exp.hunks); + cl_assert_equal_i(0, exp.lines); git_diff_list_free(diff); git_tree_free(a); diff --git a/tests-clar/index/tests.c b/tests-clar/index/tests.c index 49defd03f..88e374e6e 100644 --- a/tests-clar/index/tests.c +++ b/tests-clar/index/tests.c @@ -251,21 +251,24 @@ void test_index_tests__add(void) git_repository_free(repo); } +static void cleanup_1397(void *opaque) +{ + GIT_UNUSED(opaque); + cl_git_sandbox_cleanup(); +} + void test_index_tests__add_issue_1397(void) { git_index *index; - git_config *cfg; git_repository *repo; const git_index_entry *entry; git_oid id1; - cl_set_cleanup(&cleanup_myrepo, NULL); + cl_set_cleanup(&cleanup_1397, NULL); repo = cl_git_sandbox_init("issue_1397"); - cl_git_pass(git_repository_config(&cfg, repo)); - cl_git_pass(git_config_set_bool(cfg, "core.autocrlf", true)); - git_config_free(cfg); + cl_repo_set_bool(repo, "core.autocrlf", true); /* Ensure we're the only guy in the room */ cl_git_pass(git_repository_index(&index, repo)); @@ -278,17 +281,16 @@ void test_index_tests__add_issue_1397(void) /* Make sure the initial SHA-1 is correct */ cl_assert((entry = git_index_get_bypath(index, "crlf_file.txt", 0)) != NULL); - cl_assert(git_oid_cmp(&id1, &entry->oid) == 0); + cl_assert_(git_oid_cmp(&id1, &entry->oid) == 0, "first oid check"); /* Update the index */ cl_git_pass(git_index_add_bypath(index, "crlf_file.txt")); /* Check the new SHA-1 */ cl_assert((entry = git_index_get_bypath(index, "crlf_file.txt", 0)) != NULL); - cl_assert(git_oid_cmp(&id1, &entry->oid) == 0); + cl_assert_(git_oid_cmp(&id1, &entry->oid) == 0, "second oid check"); git_index_free(index); - git_repository_free(repo); } void test_index_tests__add_bypath_to_a_bare_repository_returns_EBAREPO(void) diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c index 61f0982cd..a9b8a12ed 100644 --- a/tests-clar/status/worktree.c +++ b/tests-clar/status/worktree.c @@ -544,12 +544,9 @@ void test_status_worktree__line_endings_dont_count_as_changes_with_autocrlf(void void test_status_worktree__line_endings_dont_count_as_changes_with_autocrlf_issue_1397(void) { git_repository *repo = cl_git_sandbox_init("issue_1397"); - git_config *config; unsigned int status; - cl_git_pass(git_repository_config(&config, repo)); - cl_git_pass(git_config_set_bool(config, "core.autocrlf", true)); - git_config_free(config); + cl_repo_set_bool(repo, "core.autocrlf", true); cl_git_pass(git_status_file(&status, repo, "crlf_file.txt")); From 4a15ea869ca097dca0b45b1202429cc12cb94219 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 21 Mar 2013 14:02:25 -0700 Subject: [PATCH 4/6] don't convert CRLF to CRCRLF --- src/crlf.c | 13 +++++++---- tests-clar/checkout/crlf.c | 46 +++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/crlf.c b/src/crlf.c index 84347ac6c..cd6d9825f 100644 --- a/src/crlf.c +++ b/src/crlf.c @@ -223,12 +223,17 @@ static int crlf_apply_to_odb( static int convert_line_endings(git_buf *dest, const git_buf *source, const char *ending) { const char *scan = git_buf_cstr(source), - *next, - *scan_end = git_buf_cstr(source) + git_buf_len(source); + *next, + *line_end, + *scan_end = git_buf_cstr(source) + git_buf_len(source); while ((next = memchr(scan, '\n', scan_end - scan)) != NULL) { - if (next > scan) - git_buf_put(dest, scan, next-scan); + if (next > scan) { + line_end = *(next - 1) == '\r' ? next - 1 : next; + git_buf_put(dest, scan, line_end - scan); + scan = next + 1; + } + git_buf_puts(dest, ending); scan = next + 1; } diff --git a/tests-clar/checkout/crlf.c b/tests-clar/checkout/crlf.c index 39889a181..40f083c1c 100644 --- a/tests-clar/checkout/crlf.c +++ b/tests-clar/checkout/crlf.c @@ -10,7 +10,9 @@ #define MORE_CRLF_TEXT_RAW "crlf\r\ncrlf\r\nlf\ncrlf\r\ncrlf\r\n" #define MORE_LF_TEXT_RAW "lf\nlf\ncrlf\r\nlf\nlf\n" -#define ALL_LF_TEXT_AS_CRLF "lf\r\nlf\r\nlf\r\nlf\r\nlf\r\n" +#define ALL_LF_TEXT_AS_CRLF "lf\r\nlf\r\nlf\r\nlf\r\nlf\r\n" +#define MORE_CRLF_TEXT_AS_CRLF "crlf\r\ncrlf\r\nlf\r\ncrlf\r\ncrlf\r\n" +#define MORE_LF_TEXT_AS_CRLF "lf\r\nlf\r\ncrlf\r\nlf\r\nlf\r\n" static git_repository *g_repo; @@ -78,6 +80,48 @@ void test_checkout_crlf__detect_crlf_autocrlf_true(void) #endif } +void test_checkout_crlf__more_lf_autocrlf_true(void) +{ +#ifdef GIT_WIN32 + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; + opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; + + cl_repo_set_bool(g_repo, "core.autocrlf", true); + + git_checkout_head(g_repo, &opts); + + test_file_contents("./crlf/more-lf", MORE_LF_TEXT_AS_CRLF); +#endif +} + +void test_checkout_crlf__more_crlf_autocrlf_true(void) +{ +#ifdef GIT_WIN32 + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; + opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; + + cl_repo_set_bool(g_repo, "core.autocrlf", true); + + git_checkout_head(g_repo, &opts); + + test_file_contents("./crlf/more-crlf", MORE_CRLF_TEXT_AS_CRLF); +#endif +} + +void test_checkout_crlf__all_crlf_autocrlf_true(void) +{ +#ifdef GIT_WIN32 + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; + opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; + + cl_repo_set_bool(g_repo, "core.autocrlf", true); + + git_checkout_head(g_repo, &opts); + + test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); +#endif +} + void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void) { #ifdef GIT_WIN32 From 050ab9950d089c55e874b63e5ab9c8d0b948cf46 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 25 Mar 2013 14:13:53 -0700 Subject: [PATCH 5/6] Fix up checkout file contents checks This fixes of the file contents checks in checkout to give slightly better error messages by directly calling the underlying clar assertions so the file and line number of the top level call can be reported correctly, and renames the helpers to not start with "test_" since that is kind of reserved by clar. This also enables some of the CRLF tests on all platforms that were previously Windows only (by pushing a check of the native line endings into the test body). --- tests-clar/checkout/checkout_helpers.c | 29 ++++++++++------ tests-clar/checkout/checkout_helpers.h | 16 +++++++-- tests-clar/checkout/crlf.c | 46 +++++++++++++------------- tests-clar/checkout/index.c | 42 +++++++++++------------ tests-clar/checkout/tree.c | 6 ++-- 5 files changed, 80 insertions(+), 59 deletions(-) diff --git a/tests-clar/checkout/checkout_helpers.c b/tests-clar/checkout/checkout_helpers.c index 79e80c13a..ab93a89bd 100644 --- a/tests-clar/checkout/checkout_helpers.c +++ b/tests-clar/checkout/checkout_helpers.c @@ -50,35 +50,44 @@ void reset_index_to_treeish(git_object *treeish) git_index_free(index); } -static void test_file_contents_internal( - const char *path, const char *expectedcontents, bool strip_cr) +static void check_file_contents_internal( + const char *path, + const char *expected_content, + bool strip_cr, + const char *file, + int line, + const char *msg) { int fd; char data[1024] = {0}; git_buf buf = GIT_BUF_INIT; - size_t expectedlen = strlen(expectedcontents); + size_t expected_len = expected_content ? strlen(expected_content) : 0; fd = p_open(path, O_RDONLY); cl_assert(fd >= 0); buf.ptr = data; - buf.size = p_read(fd, buf.ptr, 1024); + buf.size = p_read(fd, buf.ptr, sizeof(data)); cl_git_pass(p_close(fd)); if (strip_cr) strip_cr_from_buf(&buf); - cl_assert_equal_i((int)expectedlen, (int)buf.size); - cl_assert_equal_s(expectedcontents, buf.ptr); + clar__assert_equal_i((int)expected_len, (int)buf.size, file, line, "strlen(expected_content) != strlen(actual_content)", 1); + clar__assert_equal_s(expected_content, buf.ptr, file, line, msg, 1); } -void test_file_contents(const char *path, const char *expected) +void check_file_contents_at_line( + const char *path, const char *expected, + const char *file, int line, const char *msg) { - test_file_contents_internal(path, expected, false); + check_file_contents_internal(path, expected, false, file, line, msg); } -void test_file_contents_nocr(const char *path, const char *expected) +void check_file_contents_nocr_at_line( + const char *path, const char *expected, + const char *file, int line, const char *msg) { - test_file_contents_internal(path, expected, true); + check_file_contents_internal(path, expected, true, file, line, msg); } diff --git a/tests-clar/checkout/checkout_helpers.h b/tests-clar/checkout/checkout_helpers.h index 2c3a4b5bb..34053809d 100644 --- a/tests-clar/checkout/checkout_helpers.h +++ b/tests-clar/checkout/checkout_helpers.h @@ -5,5 +5,17 @@ extern void strip_cr_from_buf(git_buf *buf); extern void assert_on_branch(git_repository *repo, const char *branch); extern void reset_index_to_treeish(git_object *treeish); -extern void test_file_contents(const char *path, const char *expected); -extern void test_file_contents_nocr(const char *path, const char *expected); + +extern void check_file_contents_at_line( + const char *path, const char *expected, + const char *file, int line, const char *msg); + +extern void check_file_contents_nocr_at_line( + const char *path, const char *expected, + const char *file, int line, const char *msg); + +#define check_file_contents(PATH,EXP) \ + check_file_contents_at_line(PATH,EXP,__FILE__,__LINE__,"String mismatch: " #EXP " != " #PATH) + +#define check_file_contents_nocr(PATH,EXP) \ + check_file_contents_nocr_at_line(PATH,EXP,__FILE__,__LINE__,"String mismatch: " #EXP " != " #PATH) diff --git a/tests-clar/checkout/crlf.c b/tests-clar/checkout/crlf.c index 40f083c1c..285b1f272 100644 --- a/tests-clar/checkout/crlf.c +++ b/tests-clar/checkout/crlf.c @@ -28,7 +28,6 @@ void test_checkout_crlf__cleanup(void) void test_checkout_crlf__detect_crlf_autocrlf_false(void) { -#ifdef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; @@ -36,14 +35,12 @@ void test_checkout_crlf__detect_crlf_autocrlf_false(void) git_checkout_head(g_repo, &opts); - test_file_contents("./crlf/all-lf", ALL_LF_TEXT_RAW); - test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); -#endif + check_file_contents("./crlf/all-lf", ALL_LF_TEXT_RAW); + check_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); } void test_checkout_crlf__autocrlf_false_index_size_is_unfiltered_size(void) { -#ifdef GIT_WIN32 git_index *index; const git_index_entry *entry; git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; @@ -62,12 +59,10 @@ void test_checkout_crlf__autocrlf_false_index_size_is_unfiltered_size(void) cl_assert(entry->file_size == strlen(ALL_CRLF_TEXT_RAW)); git_index_free(index); -#endif } void test_checkout_crlf__detect_crlf_autocrlf_true(void) { -#ifdef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; @@ -75,14 +70,16 @@ void test_checkout_crlf__detect_crlf_autocrlf_true(void) git_checkout_head(g_repo, &opts); - test_file_contents("./crlf/all-lf", ALL_LF_TEXT_AS_CRLF); - test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); -#endif + if (GIT_EOL_NATIVE == GIT_EOL_LF) + check_file_contents("./crlf/all-lf", ALL_LF_TEXT_RAW); + else + check_file_contents("./crlf/all-lf", ALL_LF_TEXT_AS_CRLF); + + check_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); } void test_checkout_crlf__more_lf_autocrlf_true(void) { -#ifdef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; @@ -90,13 +87,14 @@ void test_checkout_crlf__more_lf_autocrlf_true(void) git_checkout_head(g_repo, &opts); - test_file_contents("./crlf/more-lf", MORE_LF_TEXT_AS_CRLF); -#endif + if (GIT_EOL_NATIVE == GIT_EOL_LF) + check_file_contents("./crlf/more-lf", MORE_LF_TEXT_RAW); + else + check_file_contents("./crlf/more-lf", MORE_LF_TEXT_AS_CRLF); } void test_checkout_crlf__more_crlf_autocrlf_true(void) { -#ifdef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; @@ -104,13 +102,14 @@ void test_checkout_crlf__more_crlf_autocrlf_true(void) git_checkout_head(g_repo, &opts); - test_file_contents("./crlf/more-crlf", MORE_CRLF_TEXT_AS_CRLF); -#endif + if (GIT_EOL_NATIVE == GIT_EOL_LF) + check_file_contents("./crlf/more-crlf", MORE_CRLF_TEXT_RAW); + else + check_file_contents("./crlf/more-crlf", MORE_CRLF_TEXT_AS_CRLF); } void test_checkout_crlf__all_crlf_autocrlf_true(void) { -#ifdef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; @@ -118,13 +117,11 @@ void test_checkout_crlf__all_crlf_autocrlf_true(void) git_checkout_head(g_repo, &opts); - test_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); -#endif + check_file_contents("./crlf/all-crlf", ALL_CRLF_TEXT_RAW); } void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void) { -#ifdef GIT_WIN32 git_index *index; const git_index_entry *entry; git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; @@ -137,11 +134,14 @@ void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void) git_repository_index(&index, g_repo); cl_assert((entry = git_index_get_bypath(index, "all-lf", 0)) != NULL); - cl_assert(entry->file_size == strlen(ALL_LF_TEXT_AS_CRLF)); + + if (GIT_EOL_NATIVE == GIT_EOL_LF) + cl_assert_equal_sz(strlen(ALL_LF_TEXT_RAW), entry->file_size); + else + cl_assert_equal_sz(strlen(ALL_LF_TEXT_AS_CRLF), entry->file_size); cl_assert((entry = git_index_get_bypath(index, "all-crlf", 0)) != NULL); - cl_assert(entry->file_size == strlen(ALL_CRLF_TEXT_RAW)); + cl_assert_equal_sz(strlen(ALL_CRLF_TEXT_RAW), entry->file_size); git_index_free(index); -#endif } diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 33506a669..78ff5ac62 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -48,9 +48,9 @@ void test_checkout_index__can_create_missing_files(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/README", "hey there\n"); - test_file_contents("./testrepo/branch_file.txt", "hi\nbye!\n"); - test_file_contents("./testrepo/new.txt", "my new file\n"); + check_file_contents("./testrepo/README", "hey there\n"); + check_file_contents("./testrepo/branch_file.txt", "hi\nbye!\n"); + check_file_contents("./testrepo/new.txt", "my new file\n"); } void test_checkout_index__can_remove_untracked_files(void) @@ -88,8 +88,8 @@ void test_checkout_index__honor_the_specified_pathspecs(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); cl_assert_equal_i(false, git_path_isfile("./testrepo/README")); - test_file_contents("./testrepo/branch_file.txt", "hi\nbye!\n"); - test_file_contents("./testrepo/new.txt", "my new file\n"); + check_file_contents("./testrepo/branch_file.txt", "hi\nbye!\n"); + check_file_contents("./testrepo/new.txt", "my new file\n"); } void test_checkout_index__honor_the_gitattributes_directives(void) @@ -106,9 +106,9 @@ void test_checkout_index__honor_the_gitattributes_directives(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/README", "hey there\n"); - test_file_contents("./testrepo/new.txt", "my new file\n"); - test_file_contents("./testrepo/branch_file.txt", "hi\r\nbye!\r\n"); + check_file_contents("./testrepo/README", "hey there\n"); + check_file_contents("./testrepo/new.txt", "my new file\n"); + check_file_contents("./testrepo/branch_file.txt", "hi\r\nbye!\r\n"); } void test_checkout_index__honor_coreautocrlf_setting_set_to_true(void) @@ -124,7 +124,7 @@ void test_checkout_index__honor_coreautocrlf_setting_set_to_true(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/README", expected_readme_text); + check_file_contents("./testrepo/README", expected_readme_text); #endif } @@ -139,7 +139,7 @@ void test_checkout_index__honor_coresymlinks_setting_set_to_true(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); #ifdef GIT_WIN32 - test_file_contents("./testrepo/link_to_new.txt", "new.txt"); + check_file_contents("./testrepo/link_to_new.txt", "new.txt"); #else { char link_data[1024]; @@ -149,7 +149,7 @@ void test_checkout_index__honor_coresymlinks_setting_set_to_true(void) link_data[link_size] = '\0'; cl_assert_equal_i(link_size, strlen("new.txt")); cl_assert_equal_s(link_data, "new.txt"); - test_file_contents("./testrepo/link_to_new.txt", "my new file\n"); + check_file_contents("./testrepo/link_to_new.txt", "my new file\n"); } #endif } @@ -164,7 +164,7 @@ void test_checkout_index__honor_coresymlinks_setting_set_to_false(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/link_to_new.txt", "new.txt"); + check_file_contents("./testrepo/link_to_new.txt", "new.txt"); } void test_checkout_index__donot_overwrite_modified_file_by_default(void) @@ -180,7 +180,7 @@ void test_checkout_index__donot_overwrite_modified_file_by_default(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/new.txt", "This isn't what's stored!"); + check_file_contents("./testrepo/new.txt", "This isn't what's stored!"); } void test_checkout_index__can_overwrite_modified_file(void) @@ -193,7 +193,7 @@ void test_checkout_index__can_overwrite_modified_file(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/new.txt", "my new file\n"); + check_file_contents("./testrepo/new.txt", "my new file\n"); } void test_checkout_index__options_disable_filters(void) @@ -207,14 +207,14 @@ void test_checkout_index__options_disable_filters(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/new.txt", "my new file\r\n"); + check_file_contents("./testrepo/new.txt", "my new file\r\n"); p_unlink("./testrepo/new.txt"); opts.disable_filters = true; cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/new.txt", "my new file\n"); + check_file_contents("./testrepo/new.txt", "my new file\n"); } void test_checkout_index__options_dir_modes(void) @@ -274,7 +274,7 @@ void test_checkout_index__options_open_flags(void) opts.checkout_strategy = GIT_CHECKOUT_FORCE; cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./testrepo/new.txt", "hi\nmy new file\n"); + check_file_contents("./testrepo/new.txt", "hi\nmy new file\n"); } struct notify_data { @@ -469,9 +469,9 @@ void test_checkout_index__can_update_prefixed_files(void) /* remove untracked will remove the .gitattributes file before the blobs * were created, so they will have had crlf filtering applied on Windows */ - test_file_contents_nocr("./testrepo/README", "hey there\n"); - test_file_contents_nocr("./testrepo/branch_file.txt", "hi\nbye!\n"); - test_file_contents_nocr("./testrepo/new.txt", "my new file\n"); + check_file_contents_nocr("./testrepo/README", "hey there\n"); + check_file_contents_nocr("./testrepo/branch_file.txt", "hi\nbye!\n"); + check_file_contents_nocr("./testrepo/new.txt", "my new file\n"); cl_assert(!git_path_exists("testrepo/READ")); cl_assert(!git_path_exists("testrepo/README.after")); @@ -503,5 +503,5 @@ void test_checkout_index__issue_1397(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); - test_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); + check_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); } diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index 2a8fbc457..5a2eacea1 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -248,7 +248,7 @@ void test_checkout_tree__can_update_only(void) cl_assert(!git_path_isdir("testrepo/a")); - test_file_contents_nocr("testrepo/branch_file.txt", "hi\nbye!\n"); + check_file_contents_nocr("testrepo/branch_file.txt", "hi\nbye!\n"); /* now checkout branch but with update only */ @@ -269,7 +269,7 @@ void test_checkout_tree__can_update_only(void) cl_assert(!git_path_isdir("testrepo/a")); /* but this file still should have been updated */ - test_file_contents_nocr("testrepo/branch_file.txt", "hi\n"); + check_file_contents_nocr("testrepo/branch_file.txt", "hi\n"); git_object_free(obj); } @@ -500,7 +500,7 @@ void test_checkout_tree__issue_1397(void) cl_git_pass(git_checkout_tree(g_repo, tree, &opts)); - test_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); + check_file_contents("./issue_1397/crlf_file.txt", "first line\r\nsecond line\r\nboth with crlf"); git_object_free(tree); } From 3658e81e3499f874dc9f323d4d9127df7e219e12 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 25 Mar 2013 14:20:07 -0700 Subject: [PATCH 6/6] Move crlf conversion into buf_text This adds crlf/lf conversion functions into buf_text with more efficient implementations that bypass the high level buffer functions. They attempt to minimize the number of reallocations done and they directly write the buffer data as needed if they know that there is enough memory allocated to memcpy data. Tests are added for these new functions. The crlf.c code is updated to use the new functions. Removed the include of buf_text.h from filter.h and just include it more narrowly in the places that need it. --- src/blob.c | 1 + src/buf_text.c | 77 +++++++++++++++++++++++++++++++ src/buf_text.h | 14 ++++++ src/checkout.c | 1 + src/crlf.c | 80 +++++++------------------------- src/diff_output.c | 1 + src/filter.h | 1 - tests-clar/core/buffer.c | 82 +++++++++++++++++++++++++++++++++ tests-clar/object/blob/filter.c | 1 + 9 files changed, 194 insertions(+), 64 deletions(-) diff --git a/src/blob.c b/src/blob.c index 3ff141c7f..c0514fc13 100644 --- a/src/blob.c +++ b/src/blob.c @@ -12,6 +12,7 @@ #include "common.h" #include "blob.h" #include "filter.h" +#include "buf_text.h" const void *git_blob_rawcontent(const git_blob *blob) { diff --git a/src/buf_text.c b/src/buf_text.c index 3a8f442b4..443454b5f 100644 --- a/src/buf_text.c +++ b/src/buf_text.c @@ -60,6 +60,83 @@ void git_buf_text_unescape(git_buf *buf) buf->size = git__unescape(buf->ptr); } +int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src) +{ + const char *scan = src->ptr; + const char *scan_end = src->ptr + src->size; + const char *next = memchr(scan, '\r', src->size); + char *out; + + assert(tgt != src); + + if (!next) + return GIT_ENOTFOUND; + + /* reduce reallocs while in the loop */ + if (git_buf_grow(tgt, src->size) < 0) + return -1; + out = tgt->ptr; + tgt->size = 0; + + /* Find the next \r and copy whole chunk up to there to tgt */ + for (; next; scan = next + 1, next = memchr(scan, '\r', scan_end - scan)) { + if (next > scan) { + size_t copylen = next - scan; + memcpy(out, scan, copylen); + out += copylen; + } + + /* Do not drop \r unless it is followed by \n */ + if (next[1] != '\n') + *out++ = '\r'; + } + + /* Copy remaining input into dest */ + memcpy(out, scan, scan_end - scan + 1); /* +1 for NUL byte */ + out += (scan_end - scan); + tgt->size = out - tgt->ptr; + + return 0; +} + +int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) +{ + const char *start = src->ptr; + const char *end = start + src->size; + const char *scan = start; + const char *next = memchr(scan, '\n', src->size); + + assert(tgt != src); + + if (!next) + return GIT_ENOTFOUND; + + /* attempt to reduce reallocs while in the loop */ + if (git_buf_grow(tgt, src->size + (src->size >> 4) + 1) < 0) + return -1; + tgt->size = 0; + + for (; next; scan = next + 1, next = memchr(scan, '\n', end - scan)) { + size_t copylen = next - scan; + /* don't convert existing \r\n to \r\r\n */ + size_t extralen = (next > start && next[-1] == '\r') ? 1 : 2; + size_t needsize = tgt->size + copylen + extralen + 1; + + if (tgt->asize < needsize && git_buf_grow(tgt, needsize) < 0) + return -1; + + if (next > scan) { + memcpy(tgt->ptr + tgt->size, scan, copylen); + tgt->size += copylen; + } + if (extralen == 2) + tgt->ptr[tgt->size++] = '\r'; + tgt->ptr[tgt->size++] = '\n'; + } + + return git_buf_put(tgt, scan, end - scan); +} + int git_buf_text_common_prefix(git_buf *buf, const git_strarray *strings) { size_t i; diff --git a/src/buf_text.h b/src/buf_text.h index 458ee33c9..58e4e26a7 100644 --- a/src/buf_text.h +++ b/src/buf_text.h @@ -55,6 +55,20 @@ GIT_INLINE(int) git_buf_text_puts_escape_regex(git_buf *buf, const char *string) */ extern void git_buf_text_unescape(git_buf *buf); +/** + * Replace all \r\n with \n (or do nothing if no \r\n are found) + * + * @return 0 on success, GIT_ENOTFOUND if no \r\n, -1 on memory error + */ +extern int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src); + +/** + * Replace all \n with \r\n (or do nothing if no \n are found) + * + * @return 0 on success, GIT_ENOTFOUND if no \n, -1 on memory error + */ +extern int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src); + /** * Fill buffer with the common prefix of a array of strings * diff --git a/src/checkout.c b/src/checkout.c index e52649aec..5a2698e41 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -23,6 +23,7 @@ #include "blob.h" #include "diff.h" #include "pathspec.h" +#include "buf_text.h" /* See docs/checkout-internals.md for more information */ diff --git a/src/crlf.c b/src/crlf.c index cd6d9825f..81268da83 100644 --- a/src/crlf.c +++ b/src/crlf.c @@ -9,6 +9,7 @@ #include "fileops.h" #include "hash.h" #include "filter.h" +#include "buf_text.h" #include "repository.h" #include "git2/attr.h" #include "git2/blob.h" @@ -105,35 +106,6 @@ static int crlf_load_attributes(struct crlf_attrs *ca, git_repository *repo, con return -1; } -static int drop_crlf(git_buf *dest, const git_buf *source) -{ - const char *scan = source->ptr, *next; - const char *scan_end = git_buf_cstr(source) + git_buf_len(source); - - /* Main scan loop. Find the next carriage return and copy the - * whole chunk up to that point to the destination buffer. - */ - while ((next = memchr(scan, '\r', scan_end - scan)) != NULL) { - /* copy input up to \r */ - if (next > scan) - git_buf_put(dest, scan, next - scan); - - /* Do not drop \r unless it is followed by \n */ - if (*(next + 1) != '\n') - git_buf_putc(dest, '\r'); - - scan = next + 1; - } - - /* If there was no \r, then tell the library to skip this filter */ - if (scan == source->ptr) - return -1; - - /* Copy remaining input into dest */ - git_buf_put(dest, scan, scan_end - scan); - return 0; -} - static int has_cr_in_index(git_filter *self) { struct crlf_filter *filter = (struct crlf_filter *)self; @@ -217,29 +189,7 @@ static int crlf_apply_to_odb( } /* Actually drop the carriage returns */ - return drop_crlf(dest, source); -} - -static int convert_line_endings(git_buf *dest, const git_buf *source, const char *ending) -{ - const char *scan = git_buf_cstr(source), - *next, - *line_end, - *scan_end = git_buf_cstr(source) + git_buf_len(source); - - while ((next = memchr(scan, '\n', scan_end - scan)) != NULL) { - if (next > scan) { - line_end = *(next - 1) == '\r' ? next - 1 : next; - git_buf_put(dest, scan, line_end - scan); - scan = next + 1; - } - - git_buf_puts(dest, ending); - scan = next + 1; - } - - git_buf_put(dest, scan, scan_end - scan); - return 0; + return git_buf_text_crlf_to_lf(dest, source); } static const char *line_ending(struct crlf_filter *filter) @@ -282,26 +232,28 @@ line_ending_error: return NULL; } -static int crlf_apply_to_workdir(git_filter *self, git_buf *dest, const git_buf *source) +static int crlf_apply_to_workdir( + git_filter *self, git_buf *dest, const git_buf *source) { struct crlf_filter *filter = (struct crlf_filter *)self; const char *workdir_ending = NULL; - assert (self && dest && source); + assert(self && dest && source); /* Empty file? Nothing to do. */ if (git_buf_len(source) == 0) - return 0; + return -1; /* Determine proper line ending */ workdir_ending = line_ending(filter); - if (!workdir_ending) return -1; + if (!workdir_ending) + return -1; + if (!strcmp("\n", workdir_ending)) /* do nothing for \n ending */ + return -1; - /* If the line ending is '\n', just copy the input */ - if (!strcmp(workdir_ending, "\n")) - return git_buf_puts(dest, git_buf_cstr(source)); - - return convert_line_endings(dest, source, workdir_ending); + /* for now, only lf->crlf conversion is supported here */ + assert(!strcmp("\r\n", workdir_ending)); + return git_buf_text_lf_to_crlf(dest, source); } static int find_and_add_filter( @@ -351,12 +303,14 @@ static int find_and_add_filter( return git_vector_insert(filters, filter); } -int git_filter_add__crlf_to_odb(git_vector *filters, git_repository *repo, const char *path) +int git_filter_add__crlf_to_odb( + git_vector *filters, git_repository *repo, const char *path) { return find_and_add_filter(filters, repo, path, &crlf_apply_to_odb); } -int git_filter_add__crlf_to_workdir(git_vector *filters, git_repository *repo, const char *path) +int git_filter_add__crlf_to_workdir( + git_vector *filters, git_repository *repo, const char *path) { return find_and_add_filter(filters, repo, path, &crlf_apply_to_workdir); } diff --git a/src/diff_output.c b/src/diff_output.c index fba6129b7..e8dd5b317 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -12,6 +12,7 @@ #include #include "fileops.h" #include "filter.h" +#include "buf_text.h" static int read_next_int(const char **str, int *value) { diff --git a/src/filter.h b/src/filter.h index 0ca71656b..42a44ebdb 100644 --- a/src/filter.h +++ b/src/filter.h @@ -9,7 +9,6 @@ #include "common.h" #include "buffer.h" -#include "buf_text.h" #include "git2/odb.h" #include "git2/repository.h" diff --git a/tests-clar/core/buffer.c b/tests-clar/core/buffer.c index cb5f0161b..3d8221e04 100644 --- a/tests-clar/core/buffer.c +++ b/tests-clar/core/buffer.c @@ -904,3 +904,85 @@ void test_core_buffer__similarity_metric_whitespace(void) git_buf_free(&buf); } + +#define check_buf(expected,buf) do { \ + cl_assert_equal_s(expected, buf.ptr); \ + cl_assert_equal_sz(strlen(expected), buf.size); } while (0) + +void test_core_buffer__lf_and_crlf_conversions(void) +{ + git_buf src = GIT_BUF_INIT, tgt = GIT_BUF_INIT; + + /* LF source */ + + git_buf_sets(&src, "lf\nlf\nlf\nlf\n"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("lf\r\nlf\r\nlf\r\nlf\r\n", tgt); + + cl_assert_equal_i(GIT_ENOTFOUND, git_buf_text_crlf_to_lf(&tgt, &src)); + /* no conversion needed if all LFs already */ + + git_buf_sets(&src, "\nlf\nlf\nlf\nlf\nlf"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("\r\nlf\r\nlf\r\nlf\r\nlf\r\nlf", tgt); + + cl_assert_equal_i(GIT_ENOTFOUND, git_buf_text_crlf_to_lf(&tgt, &src)); + /* no conversion needed if all LFs already */ + + /* CRLF source */ + + git_buf_sets(&src, "crlf\r\ncrlf\r\ncrlf\r\ncrlf\r\n"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("crlf\r\ncrlf\r\ncrlf\r\ncrlf\r\n", tgt); + check_buf(src.ptr, tgt); + + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("crlf\ncrlf\ncrlf\ncrlf\n", tgt); + + git_buf_sets(&src, "\r\ncrlf\r\ncrlf\r\ncrlf\r\ncrlf\r\ncrlf"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("\r\ncrlf\r\ncrlf\r\ncrlf\r\ncrlf\r\ncrlf", tgt); + check_buf(src.ptr, tgt); + + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("\ncrlf\ncrlf\ncrlf\ncrlf\ncrlf", tgt); + + /* CRLF in LF text */ + + git_buf_sets(&src, "\nlf\nlf\ncrlf\r\nlf\nlf\ncrlf\r\n"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("\r\nlf\r\nlf\r\ncrlf\r\nlf\r\nlf\r\ncrlf\r\n", tgt); + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("\nlf\nlf\ncrlf\nlf\nlf\ncrlf\n", tgt); + + /* LF in CRLF text */ + + git_buf_sets(&src, "\ncrlf\r\ncrlf\r\nlf\ncrlf\r\ncrlf"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("\r\ncrlf\r\ncrlf\r\nlf\r\ncrlf\r\ncrlf", tgt); + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("\ncrlf\ncrlf\nlf\ncrlf\ncrlf", tgt); + + /* bare CR test */ + + git_buf_sets(&src, "\rcrlf\r\nlf\nlf\ncr\rcrlf\r\nlf\ncr\r"); + + cl_git_pass(git_buf_text_lf_to_crlf(&tgt, &src)); + check_buf("\rcrlf\r\nlf\r\nlf\r\ncr\rcrlf\r\nlf\r\ncr\r", tgt); + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("\rcrlf\nlf\nlf\ncr\rcrlf\nlf\ncr\r", tgt); + + git_buf_sets(&src, "\rcr\r"); + cl_assert_equal_i(GIT_ENOTFOUND, git_buf_text_lf_to_crlf(&tgt, &src)); + cl_git_pass(git_buf_text_crlf_to_lf(&tgt, &src)); + check_buf("\rcr\r", tgt); + + git_buf_free(&src); + git_buf_free(&tgt); +} diff --git a/tests-clar/object/blob/filter.c b/tests-clar/object/blob/filter.c index b9bbfff0c..042bddab7 100644 --- a/tests-clar/object/blob/filter.c +++ b/tests-clar/object/blob/filter.c @@ -2,6 +2,7 @@ #include "posix.h" #include "blob.h" #include "filter.h" +#include "buf_text.h" static git_repository *g_repo = NULL; #define NUM_TEST_OBJECTS 8