{"thread":{"id":"13933","subject":"[PATCH] git-apply doesn't handle same name patches well","startedAt":"2008-06-13T16:55:31Z","lastAt":"2008-06-16T09:01:41Z","messageCount":15,"participants":["Don Zickus","Johannes Schindelin","Miklos Vajna","Olivier Marin","Junio C Hamano","Jakub Narebski","Mike Ralphson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"79741","messageId":"1213376131-20967-1-git-send-email-dzickus@redhat.com","threadId":"13933","inReplyTo":null,"subject":"[PATCH] git-apply doesn't handle same name patches well","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-06-13T16:55:31Z","receivedAt":"2008-06-13T16:55:31Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"When working with a lot of people who backport patches all day long, every\nonce in a while I get a patch that modifies the same file more than once\ninside the same patch.  git-apply either fails if the second change relies\non the first change or silently drops the first change if the second change\nis independent.\n\nThe silent part is the scary scenario for us.  Also this behaviour is\ndifferent from the patch-utils.\n\nI have modified git-apply to cache the filenames of files it modifies such\nthat if a later patch chunk modifies a file in the cache it will buffer the\npreviously changed file instead of reading the original file from disk.\n\nLogic has been put in to handle creations/deletions/renames/copies.  All the\nrelevant tests of git-apply succeed.\n\nA new test has been added to cover the two cases I addressed.\n\nThe fix is relatively straight-forward.  But I'm not sure if this new\nbehaviour is something the git community wants.\n\nSigned-off-by: Don Zickus <dzickus@redhat.com>\n---\n builtin-apply.c          |   69 +++++++++++++++++++++++++++++++++++++++++++++-\n t/t4127-apply-same-fn.sh |   40 ++++++++++++++++++++++++++\n 2 files changed, 108 insertions(+), 1 deletions(-)\n create mode 100755 t/t4127-apply-same-fn.sh\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c497889..8330517 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -185,6 +185,18 @@ struct image {\n \tstruct line *line;\n };\n \n+/*\n+ * Caches patch filenames to handle the case where a\n+ * patch chunk reuses a filename\n+ */\n+struct fn_cache {\n+\tchar *name;\n+\tstruct patch *patch;\n+\tstruct fn_cache *next;\n+};\n+\n+struct fn_cache *fn_cache_top = NULL;\n+\n static uint32_t hash_line(const char *cp, size_t len)\n {\n \tsize_t i;\n@@ -2176,6 +2188,51 @@ static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n \treturn 0;\n }\n \n+struct patch *in_fn_cache(char *name)\n+{\n+\tstruct fn_cache *p;\n+\n+\tfor (p=fn_cache_top; p; p=p->next) {\n+\t\tif (!strcmp(name, p->name))\n+\t\t\treturn p->patch;\n+\t}\n+\n+\treturn NULL;\n+}\n+\n+void add_to_fn_cache(char *name, struct patch *patch)\n+{\n+\tstruct fn_cache *fn_cache;\n+\n+\t/* Always add new_name unless patch is a deletion */\n+\tif (name != NULL) {\n+\t\tfn_cache = xmalloc(sizeof(*fn_cache));\n+\n+\t\t/* assuming the pointer to filename won't disappear */\n+\t\tfn_cache->name = name;\n+\t\tfn_cache->patch = patch;\n+\t\tfn_cache->next = fn_cache_top;\n+\n+\t\tfn_cache_top = fn_cache;\n+\t}\n+\n+\t/* skip normal diffs, creations and copies */\n+\t/*\n+\t * store a failure on rename/deletion cases because\n+\t * later chunks shouldn't patch old names\n+\t */\n+\tif ((name == NULL) || (patch->is_rename)) {\n+\t\tfn_cache = xmalloc(sizeof(*fn_cache));\n+\n+\t\t/* assuming the pointer to filename won't disappear */\n+\t\tfn_cache->name = patch->old_name;\n+\t\tfn_cache->patch = (struct patch *) -1;\n+\t\tfn_cache->next = fn_cache_top;\n+\n+\t\tfn_cache_top = fn_cache;\n+\t}\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tstruct strbuf buf;\n@@ -2188,7 +2245,16 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \t\tif (read_file_or_gitlink(ce, &buf))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t} else if (patch->old_name) {\n-\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\tstruct patch *tpatch = in_fn_cache(patch->old_name);\n+\n+\t\tif (tpatch != NULL) {\n+\t\t\tif (tpatch == (struct patch *) -1) {\n+\t\t\t\treturn error(\"patch %s has been renamed/deleted\",\n+\t\t\t\t\tpatch->old_name);\n+\t\t\t}\n+\t\t\t/* We have a patched copy in memory use that */\n+\t\t\tstrbuf_add(&buf, tpatch->result, tpatch->resultsize);\n+\t\t} else if (S_ISGITLINK(patch->old_mode)) {\n \t\t\tif (ce) {\n \t\t\t\tread_file_or_gitlink(ce, &buf);\n \t\t\t} else {\n@@ -2211,6 +2277,7 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \t\treturn -1; /* note with --reject this succeeds. */\n \tpatch->result = image.buf;\n \tpatch->resultsize = image.len;\n+\tadd_to_fn_cache(patch->new_name, patch);\n \tfree(image.line_allocated);\n \n \tif (0 < patch->is_delete && patch->resultsize)\ndiff --git a/t/t4127-apply-same-fn.sh b/t/t4127-apply-same-fn.sh\nnew file mode 100755\nindex 0000000..a20bd8e\n--- /dev/null\n+++ b/t/t4127-apply-same-fn.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='apply same filename'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tfor i in a b c d e f g h i j k l m\n+\tdo\n+\t\techo $i\n+\tdone >same_fn &&\n+\tgit add same_fn &&\n+\tgit commit -m initial\n+'\n+test_expect_success 'apply same filename with independent changes' '\n+\tsed -i -e \"s/^d/z/\" same_fn &&\n+\tgit diff > patch0 &&\n+\tgit add same_fn &&\n+\tsed -i -e \"s/^i/y/\" same_fn &&\n+\tgit diff >> patch0 &&\n+\tcp same_fn same_fn2 &&\n+\tgit reset --hard &&\n+\tgit-apply patch0 &&\n+\tdiff same_fn same_fn2\n+'\n+\n+test_expect_success 'apply same filename with overlapping changes' '\n+\tgit reset --hard\n+\tsed -i -e \"s/^d/z/\" same_fn &&\n+\tgit diff > patch0 &&\n+\tgit add same_fn &&\n+\tsed -i -e \"s/^e/y/\" same_fn &&\n+\tgit diff >> patch0 &&\n+\tcp same_fn same_fn2 &&\n+\tgit reset --hard &&\n+\tgit-apply patch0 &&\n+\tdiff same_fn same_fn2\n+'\n+\n+test_done\n-- \n1.5.6.rc2.45.gdc92c\n"},{"id":"79779","messageId":"alpine.DEB.1.00.0806132131180.6439@racer","threadId":"13933","inReplyTo":"1213376131-20967-1-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH] git-apply doesn't handle same name patches well","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-13T20:32:52Z","receivedAt":"2008-06-13T20:32:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 13 Jun 2008, Don Zickus wrote:\n\n> When working with a lot of people who backport patches all day long, \n> every once in a while I get a patch that modifies the same file more \n> than once inside the same patch.  git-apply either fails if the second \n> change relies on the first change or silently drops the first change if \n> the second change is independent.\n> \n> The silent part is the scary scenario for us.  Also this behaviour is \n> different from the patch-utils.\n> \n> I have modified git-apply to cache the filenames of files it modifies \n> such that if a later patch chunk modifies a file in the cache it will \n> buffer the previously changed file instead of reading the original file \n> from disk.\n> \n> Logic has been put in to handle creations/deletions/renames/copies.  All the\n> relevant tests of git-apply succeed.\n> \n> A new test has been added to cover the two cases I addressed.\n> \n> The fix is relatively straight-forward.  But I'm not sure if this new\n> behaviour is something the git community wants.\n\nThe scary part is about adding a linked list for file names you want to \nlook up.\n\nNot that performance matters here, I guess, but we _already_ have \nsomething much more efficient in Git, namely path-lists.\n\nYou could use that, and end up with a substantially smaller patch.\n\nCiao,\nDscho\n"},{"id":"79780","messageId":"20080613204219.GE7703@redhat.com","threadId":"13933","inReplyTo":"alpine.DEB.1.00.0806132131180.6439@racer","subject":"Re: [PATCH] git-apply doesn't handle same name patches well","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-06-13T20:42:19Z","receivedAt":"2008-06-13T20:42:19Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Fri, Jun 13, 2008 at 09:32:52PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Fri, 13 Jun 2008, Don Zickus wrote:\n> \n> > When working with a lot of people who backport patches all day long, \n> > every once in a while I get a patch that modifies the same file more \n> > than once inside the same patch.  git-apply either fails if the second \n> > change relies on the first change or silently drops the first change if \n> > the second change is independent.\n> > \n> > The silent part is the scary scenario for us.  Also this behaviour is \n> > different from the patch-utils.\n> > \n> > I have modified git-apply to cache the filenames of files it modifies \n> > such that if a later patch chunk modifies a file in the cache it will \n> > buffer the previously changed file instead of reading the original file \n> > from disk.\n> > \n> > Logic has been put in to handle creations/deletions/renames/copies.  All the\n> > relevant tests of git-apply succeed.\n> > \n> > A new test has been added to cover the two cases I addressed.\n> > \n> > The fix is relatively straight-forward.  But I'm not sure if this new\n> > behaviour is something the git community wants.\n> \n> The scary part is about adding a linked list for file names you want to \n> look up.\n> \n> Not that performance matters here, I guess, but we _already_ have \n> something much more efficient in Git, namely path-lists.\n> \n> You could use that, and end up with a substantially smaller patch.\n\nThanks for the feedback.  I was unaware of path-lists.  I'll try to find\nan example and implement it if it works.\n\nCheers,\nDon\n"},{"id":"79790","messageId":"1213397300-23224-1-git-send-email-vmiklos@frugalware.org","threadId":"13933","inReplyTo":"alpine.DEB.1.00.0806132131180.6439@racer","subject":"[PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-13T22:48:20Z","receivedAt":"2008-06-13T22:48:20Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Fri, Jun 13, 2008 at 09:32:52PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> Not that performance matters here, I guess, but we _already_ have\n> something much more efficient in Git, namely path-lists.\n>\n> You could use that, and end up with a substantially smaller patch.\n\nI just noticed that Documentation/technical/api-path-list.txt is almost\nempty. Here is an attempt to document the path-list API.\n\n Documentation/technical/api-path-list.txt |   92 +++++++++++++++++++++++++++-\n 1 files changed, 88 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/api-path-list.txt b/Documentation/technical/api-path-list.txt\nindex d077683..844eee9 100644\n--- a/Documentation/technical/api-path-list.txt\n+++ b/Documentation/technical/api-path-list.txt\n@@ -1,9 +1,93 @@\n path-list API\n =============\n \n-Talk about <path-list.h>, things like\n+The path_list API offers a data structure and functions to handle sorted\n+and unsorted string lists.\n \n-* it is not just paths but strings in general;\n-* the calling sequence.\n+The name is a bit misleading, a path_list may store not only paths but\n+strings in general.\n \n-(Dscho)\n+The caller:\n+\n+. Allocates and clears (`memset(&list, '0', sizeof(path_list));`) a\n+  `struct path_list` variable.\n+\n+. Initializes the members. You can manually set the `items` member, but\n+  then you have to set `nr`, accordingly. Also don't forget to set\n+  `strdup_paths` if you need it.\n+\n+. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n+\n+. Can check if a string is in the list using `path_list_has_path` or\n+  `unsorted_path_list_has_path` and get it from the list using\n+  `path_list_lookup` for sorted lists.\n+\n+. Can sort an unsorted list using `sort_path_list`.\n+\n+. Finally it should free the list using `path_list_clear`.\n+\n+Functions\n+---------\n+\n+* General ones (works with sorted and unsorted lists as well)\n+\n+`print_path_list`::\n+\n+\tDump a path_list to stdout, useful mainly for debugging purposes. It\n+\tcan take an optional header argument and it writes out the\n+\tstring-pointer pairs of the path_list, each one in its own line.\n+\n+`path_list_clear`::\n+\n+\tFree a path_list. The `path` pointer of the items will be freed in case\n+\tthe `strdup_paths` member of the path_list is set. The second parameter\n+\tcontrols if the `util` pointer of the items should be freed or not.\n+\n+* Functions for sorted lists only\n+\n+`path_list_has_path`::\n+\n+\tDetermine if the path_list has a given string or not.\n+\n+`path_list_insert`::\n+\n+\tInsert a new element to the path_list. The returned pointer can be handy\n+\tif you want to write something to the `util` pointer of the\n+\tpath_list_item containing the just added string.\n+\n+`path_list_lookup`::\n+\n+\tLook up a given string in the path_list, returning the containing\n+\tpath_list_item. If the string is not found, NULL is returned.\n+\n+* Functions for unsorted lists only\n+\n+`path_list_append`::\n+\n+\tAppend a new string to the end of the path_list.\n+\n+`sort_path_list`::\n+\n+\tMake an unsorted list sorted.\n+\n+`unsorted_path_list_has_path`::\n+\n+\tIt's like `path_list_has_path()` but for unsorted lists.\n+\n+Data structures\n+---------------\n+\n+* `struct path_list_item`\n+\n+Represent an item of the list. The `path` member is a pointer to the\n+string, and you may use the `util` member for any purpose, if you want.\n+\n+* `struct path_list`\n+\n+Represents the list itself.\n+\n+. The array of items are available via the `items` member.\n+. The `nr` member contains the number of items stored in the list.\n+. The `alloc` member is used for `ALLOC_GROW()`.\n+. Setting the `strdup_paths` member to 1 means that the added paths are\n+  copied to the path list and not just a pointer to them is stored.\n-- \n1.5.6.rc2.dirty\n"},{"id":"79794","messageId":"48530331.70807@free.fr","threadId":"13933","inReplyTo":"1213397300-23224-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-13T23:30:57Z","receivedAt":"2008-06-13T23:30:57Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Hi,\n\nNice work!\n\nMiklos Vajna a écrit :\n> \n> +The caller:\n> +\n> +. Allocates and clears (`memset(&list, '0', sizeof(path_list));`) a\n> +  `struct path_list` variable.\n\nDon't you mean sizeof(list) here?\n\nOlivier.\n"},{"id":"79796","messageId":"1213402428-24025-1-git-send-email-vmiklos@frugalware.org","threadId":"13933","inReplyTo":"48530331.70807@free.fr","subject":"[PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-14T00:13:48Z","receivedAt":"2008-06-14T00:13:48Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Sat, Jun 14, 2008 at 01:30:57AM +0200, Olivier Marin <dkr+ml.git@free.fr> wrote:\n> > +. Allocates and clears (`memset(&list, '0', sizeof(path_list));`) a\n> > +  `struct path_list` variable.\n>\n> Don't you mean sizeof(list) here?\n\nRight, it was a typo. Thanks for the correction.\n\nAlso I just noticed that the '0' did not render properly in asciidoc,\nusing \\'0' fixes the issue.\n\nUpdated patch below.\n\n Documentation/technical/api-path-list.txt |   92 +++++++++++++++++++++++++++-\n 1 files changed, 88 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/api-path-list.txt b/Documentation/technical/api-path-list.txt\nindex d077683..313d088 100644\n--- a/Documentation/technical/api-path-list.txt\n+++ b/Documentation/technical/api-path-list.txt\n@@ -1,9 +1,93 @@\n path-list API\n =============\n \n-Talk about <path-list.h>, things like\n+The path_list API offers a data structure and functions to handle sorted\n+and unsorted string lists.\n \n-* it is not just paths but strings in general;\n-* the calling sequence.\n+The name is a bit misleading, a path_list may store not only paths but\n+strings in general.\n \n-(Dscho)\n+The caller:\n+\n+. Allocates and clears (`memset(&list, \\'0', sizeof(list));`) a\n+  `struct path_list` variable.\n+\n+. Initializes the members. You can manually set the `items` member, but\n+  then you have to set `nr`, accordingly. Also don't forget to set\n+  `strdup_paths` if you need it.\n+\n+. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n+\n+. Can check if a string is in the list using `path_list_has_path` or\n+  `unsorted_path_list_has_path` and get it from the list using\n+  `path_list_lookup` for sorted lists.\n+\n+. Can sort an unsorted list using `sort_path_list`.\n+\n+. Finally it should free the list using `path_list_clear`.\n+\n+Functions\n+---------\n+\n+* General ones (works with sorted and unsorted lists as well)\n+\n+`print_path_list`::\n+\n+\tDump a path_list to stdout, useful mainly for debugging purposes. It\n+\tcan take an optional header argument and it writes out the\n+\tstring-pointer pairs of the path_list, each one in its own line.\n+\n+`path_list_clear`::\n+\n+\tFree a path_list. The `path` pointer of the items will be freed in case\n+\tthe `strdup_paths` member of the path_list is set. The second parameter\n+\tcontrols if the `util` pointer of the items should be freed or not.\n+\n+* Functions for sorted lists only\n+\n+`path_list_has_path`::\n+\n+\tDetermine if the path_list has a given string or not.\n+\n+`path_list_insert`::\n+\n+\tInsert a new element to the path_list. The returned pointer can be handy\n+\tif you want to write something to the `util` pointer of the\n+\tpath_list_item containing the just added string.\n+\n+`path_list_lookup`::\n+\n+\tLook up a given string in the path_list, returning the containing\n+\tpath_list_item. If the string is not found, NULL is returned.\n+\n+* Functions for unsorted lists only\n+\n+`path_list_append`::\n+\n+\tAppend a new string to the end of the path_list.\n+\n+`sort_path_list`::\n+\n+\tMake an unsorted list sorted.\n+\n+`unsorted_path_list_has_path`::\n+\n+\tIt's like `path_list_has_path()` but for unsorted lists.\n+\n+Data structures\n+---------------\n+\n+* `struct path_list_item`\n+\n+Represent an item of the list. The `path` member is a pointer to the\n+string, and you may use the `util` member for any purpose, if you want.\n+\n+* `struct path_list`\n+\n+Represents the list itself.\n+\n+. The array of items are available via the `items` member.\n+. The `nr` member contains the number of items stored in the list.\n+. The `alloc` member is used for `ALLOC_GROW()`.\n+. Setting the `strdup_paths` member to 1 means that the added paths are\n+  copied to the path list and not just a pointer to them is stored.\n-- \n1.5.6.rc2.dirty\n"},{"id":"79797","messageId":"1213404964-25161-1-git-send-email-vmiklos@frugalware.org","threadId":"13933","inReplyTo":"48530331.70807@free.fr","subject":"[PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-14T00:56:04Z","receivedAt":"2008-06-14T00:56:04Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nUpdated patch, obviously we want to init the struct with '\\0', not '0'.\n\n Documentation/technical/api-path-list.txt |   92 +++++++++++++++++++++++++++-\n 1 files changed, 88 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/api-path-list.txt b/Documentation/technical/api-path-list.txt\nindex d077683..313d088 100644\n--- a/Documentation/technical/api-path-list.txt\n+++ b/Documentation/technical/api-path-list.txt\n@@ -1,9 +1,93 @@\n path-list API\n =============\n \n-Talk about <path-list.h>, things like\n+The path_list API offers a data structure and functions to handle sorted\n+and unsorted string lists.\n \n-* it is not just paths but strings in general;\n-* the calling sequence.\n+The name is a bit misleading, a path_list may store not only paths but\n+strings in general.\n \n-(Dscho)\n+The caller:\n+\n+. Allocates and clears (`memset(&list, \\'\\0', sizeof(list));`) a\n+  `struct path_list` variable.\n+\n+. Initializes the members. You can manually set the `items` member, but\n+  then you have to set `nr`, accordingly. Also don't forget to set\n+  `strdup_paths` if you need it.\n+\n+. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n+\n+. Can check if a string is in the list using `path_list_has_path` or\n+  `unsorted_path_list_has_path` and get it from the list using\n+  `path_list_lookup` for sorted lists.\n+\n+. Can sort an unsorted list using `sort_path_list`.\n+\n+. Finally it should free the list using `path_list_clear`.\n+\n+Functions\n+---------\n+\n+* General ones (works with sorted and unsorted lists as well)\n+\n+`print_path_list`::\n+\n+\tDump a path_list to stdout, useful mainly for debugging purposes. It\n+\tcan take an optional header argument and it writes out the\n+\tstring-pointer pairs of the path_list, each one in its own line.\n+\n+`path_list_clear`::\n+\n+\tFree a path_list. The `path` pointer of the items will be freed in case\n+\tthe `strdup_paths` member of the path_list is set. The second parameter\n+\tcontrols if the `util` pointer of the items should be freed or not.\n+\n+* Functions for sorted lists only\n+\n+`path_list_has_path`::\n+\n+\tDetermine if the path_list has a given string or not.\n+\n+`path_list_insert`::\n+\n+\tInsert a new element to the path_list. The returned pointer can be handy\n+\tif you want to write something to the `util` pointer of the\n+\tpath_list_item containing the just added string.\n+\n+`path_list_lookup`::\n+\n+\tLook up a given string in the path_list, returning the containing\n+\tpath_list_item. If the string is not found, NULL is returned.\n+\n+* Functions for unsorted lists only\n+\n+`path_list_append`::\n+\n+\tAppend a new string to the end of the path_list.\n+\n+`sort_path_list`::\n+\n+\tMake an unsorted list sorted.\n+\n+`unsorted_path_list_has_path`::\n+\n+\tIt's like `path_list_has_path()` but for unsorted lists.\n+\n+Data structures\n+---------------\n+\n+* `struct path_list_item`\n+\n+Represent an item of the list. The `path` member is a pointer to the\n+string, and you may use the `util` member for any purpose, if you want.\n+\n+* `struct path_list`\n+\n+Represents the list itself.\n+\n+. The array of items are available via the `items` member.\n+. The `nr` member contains the number of items stored in the list.\n+. The `alloc` member is used for `ALLOC_GROW()`.\n+. Setting the `strdup_paths` member to 1 means that the added paths are\n+  copied to the path list and not just a pointer to them is stored.\n-- \n1.5.6.rc2.dirty\n"},{"id":"79853","messageId":"4853BE8E.4030009@free.fr","threadId":"13933","inReplyTo":"1213404964-25161-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Olivier Marin","fromEmail":"dkr+ml.git@free.fr","sentAt":"2008-06-14T12:50:22Z","receivedAt":"2008-06-14T12:50:22Z","isPatch":true,"sender":{"key":"dkr+ml.git@free.fr","avatar":null},"body":"Miklos Vajna a écrit :\n> \n> +. Allocates and clears (`memset(&list, \\'\\0', sizeof(list));`) a\n> +  `struct path_list` variable.\n\nWhat about just `memset(&list, 0, sizeof(list))` instead?\n\nIt's readable in the text format, clean in html and this is the way\nmemset() is used.\n\nOlivier.\n"},{"id":"79868","messageId":"alpine.DEB.1.00.0806141705050.6439@racer","threadId":"13933","inReplyTo":"1213404964-25161-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-14T18:08:19Z","receivedAt":"2008-06-14T18:08:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Jun 2008, Miklos Vajna wrote:\n\n> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n\nThanks for doing this... I meant to document it after pushing the \npath_list -> string_list patch.\n\nSpeaking of which: Junio, could you give me any clue how you would like to \nproceed with that patch?\n\n> @@ -1,9 +1,93 @@\n>  path-list API\n>  =============\n>  \n> -Talk about <path-list.h>, things like\n> +The path_list API offers a data structure and functions to handle sorted\n> +and unsorted string lists.\n>  \n> -* it is not just paths but strings in general;\n> -* the calling sequence.\n> +The name is a bit misleading, a path_list may store not only paths but\n> +strings in general.\n>  \n> -(Dscho)\n> +The caller:\n> +\n> +. Allocates and clears (`memset(&list, \\'\\0', sizeof(list));`) a\n> +  `struct path_list` variable.\n\nSome callers use global variables; these are already initialized.  Also, I \nwould not write code here, but later in a concise example.\n\n> +. Initializes the members. You can manually set the `items` member, but\n> +  then you have to set `nr`, accordingly. Also don't forget to set\n> +  `strdup_paths` if you need it.\n\nI would not promote the manual setting of the items member, until later.  \nThis is advanced usage, and you have to malloc() the list if you add \nthings later, and you should set the `alloc` member in that case, too.\n\nFurther, I would like to have an explanation of the variable \n\"strdup_paths\" first.\n\nSomething like: \"You might want to set the flag `strdup_paths` if the \nstrings should be strdup()ed.  For example, this is necessary when you add \nsomething like git_path(\"...\"), since that function returns a static \nbuffer that will change with the next call to git_path().\"\n\n> +. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n> +\n> +. Can check if a string is in the list using `path_list_has_path` or\n> +  `unsorted_path_list_has_path` and get it from the list using\n> +  `path_list_lookup` for sorted lists.\n> +\n> +. Can sort an unsorted list using `sort_path_list`.\n> +\n> +. Finally it should free the list using `path_list_clear`.\n\nHere, you should add a note that it is more efficient to build an unsorted \nlist and sort it afterwards, instead of building a sorted list (O(n log n) \ninstead of O(n^2)).\n\nHowever, if you use the list to check if a certain string was added \nalready, you should not do that (using unsorted_path_list_has_path()), \nbecause the complexity would be quadratic again (but with a worse factor).\n\n> +`path_list_insert`::\n> +\n> +\tInsert a new element to the path_list. The returned pointer can be handy\n> +\tif you want to write something to the `util` pointer of the\n> +\tpath_list_item containing the just added string.\n\n\tSince this function uses xrealloc() (which die()s if it fails) if \n\tthe list needs to grow, it is safe not to check the pointer.  I.e.\n\tyou may write \"path_list_insert(...)->util = ...;\"\n\n> +`unsorted_path_list_has_path`::\n> +\n> +\tIt's like `path_list_has_path()` but for unsorted lists.\n\n\tObviously, this function needs to look through all items, as \n\topposed to its counterpart for sorted lists, which performs a \n\tbinary search.\n\n> +Data structures\n> +---------------\n> +\n> +* `struct path_list_item`\n> +\n> +Represent an item of the list. The `path` member is a pointer to the\n\ns/sent/&s/\n\n> +string, and you may use the `util` member for any purpose, if you want.\n> +\n> +* `struct path_list`\n> +\n> +Represents the list itself.\n> +\n> +. The array of items are available via the `items` member.\n> +. The `nr` member contains the number of items stored in the list.\n> +. The `alloc` member is used for `ALLOC_GROW()`.\n\nMaybe \"The `alloc` member is used to avoid reallocating at every \ninsertion.  You should not tamper with it.\"\n\n> +. Setting the `strdup_paths` member to 1 means that the added paths are\n> +  copied to the path list and not just a pointer to them is stored.\n\nLike I said, I would say that it will strdup() the strings before adding \nthem, and then motivate it (by presenting a case where it helps, e.g. \ngit_path()).\n\nOf course, a short and concise example how to use path_lists would be \nnice... ;-)\n\nCiao,\nDscho\n"},{"id":"79890","messageId":"20080614225029.GM29404@genesis.frugalware.org","threadId":"13933","inReplyTo":"4853BE8E.4030009@free.fr","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-14T22:50:29Z","receivedAt":"2008-06-14T22:50:29Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Jun 14, 2008 at 02:50:22PM +0200, Olivier Marin <dkr+ml.git@free.fr> wrote:\n> What about just `memset(&list, 0, sizeof(list))` instead?\n> \n> It's readable in the text format, clean in html and this is the way\n> memset() is used.\n\nGood idea, thanks. Will do in a bit.\n"},{"id":"79892","messageId":"1213485725-6755-1-git-send-email-vmiklos@frugalware.org","threadId":"13933","inReplyTo":"alpine.DEB.1.00.0806141705050.6439@racer","subject":"[PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-14T23:22:05Z","receivedAt":"2008-06-14T23:22:05Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Sat, Jun 14, 2008 at 07:08:19PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> Thanks for doing this... I meant to document it after pushing the\n> path_list -> string_list patch.\n\nHere is an updated version, hopefully I added all your suggestion, and\nappended a short example as well.\n\n(Sending to the list only, as I accidently removed the list from Cc, sorry for that.)\n\n Documentation/technical/api-path-list.txt |  125 ++++++++++++++++++++++++++++-\n 1 files changed, 121 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/api-path-list.txt b/Documentation/technical/api-path-list.txt\nindex d077683..654470d 100644\n--- a/Documentation/technical/api-path-list.txt\n+++ b/Documentation/technical/api-path-list.txt\n@@ -1,9 +1,126 @@\n path-list API\n =============\n \n-Talk about <path-list.h>, things like\n+The path_list API offers a data structure and functions to handle sorted\n+and unsorted string lists.\n \n-* it is not just paths but strings in general;\n-* the calling sequence.\n+The name is a bit misleading, a path_list may store not only paths but\n+strings in general.\n \n-(Dscho)\n+The caller:\n+\n+. Allocates and clears a `struct path_list` variable.\n+\n+. Initializes the members. You might want to set the flag `strdup_paths`\n+  if the strings should be strdup()ed. For example, this is necessary\n+  when you add something like git_path(\"...\"), since that function returns\n+  a static buffer that will change with the next call to git_path().\n++\n+If you need something advanced, you can manually malloc() the `items`\n+member (you need this if you add things later) and you should set the\n+`nr` and `alloc` members in that case, too.\n+\n+. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n+\n+. Can check if a string is in the list using `path_list_has_path` or\n+  `unsorted_path_list_has_path` and get it from the list using\n+  `path_list_lookup` for sorted lists.\n+\n+. Can sort an unsorted list using `sort_path_list`.\n+\n+. Finally it should free the list using `path_list_clear`.\n+\n+Example:\n+\n+----\n+struct path_list list;\n+int i;\n+\n+memset(&list, 0, sizeof(struct path_list));\n+path_list_append(\"foo\", &list);\n+path_list_append(\"bar\", &list);\n+for (i = 0; i < list.nr; i++)\n+\tprintf(\"%s\\n\", list.items[i].path)\n+----\n+\n+NOTE: It is more efficient to build an unsorted list and sort it\n+afterwards, instead of building a sorted list `(O(n log n)` instead of\n+`O(n^2))`.\n++\n+However, if you use the list to check if a certain string was added\n+already, you should not do that (using unsorted_path_list_has_path()),\n+because the complexity would be quadratic again (but with a worse factor).\n+\n+Functions\n+---------\n+\n+* General ones (works with sorted and unsorted lists as well)\n+\n+`print_path_list`::\n+\n+\tDump a path_list to stdout, useful mainly for debugging purposes. It\n+\tcan take an optional header argument and it writes out the\n+\tstring-pointer pairs of the path_list, each one in its own line.\n+\n+`path_list_clear`::\n+\n+\tFree a path_list. The `path` pointer of the items will be freed in case\n+\tthe `strdup_paths` member of the path_list is set. The second parameter\n+\tcontrols if the `util` pointer of the items should be freed or not.\n+\n+* Functions for sorted lists only\n+\n+`path_list_has_path`::\n+\n+\tDetermine if the path_list has a given string or not.\n+\n+`path_list_insert`::\n+\n+\tInsert a new element to the path_list. The returned pointer can be handy\n+\tif you want to write something to the `util` pointer of the\n+\tpath_list_item containing the just added string.\n++\n+Since this function uses xrealloc() (which die()s if it fails) if the\n+list needs to grow, it is safe not to check the pointer. I.e. you may\n+write `path_list_insert(...)->util = ...;`.\n+\n+`path_list_lookup`::\n+\n+\tLook up a given string in the path_list, returning the containing\n+\tpath_list_item. If the string is not found, NULL is returned.\n+\n+* Functions for unsorted lists only\n+\n+`path_list_append`::\n+\n+\tAppend a new string to the end of the path_list.\n+\n+`sort_path_list`::\n+\n+\tMake an unsorted list sorted.\n+\n+`unsorted_path_list_has_path`::\n+\n+\tIt's like `path_list_has_path()` but for unsorted lists.\n++\n+This function needs to look through all items, as opposed to its\n+counterpart for sorted lists, which performs a binary search.\n+\n+Data structures\n+---------------\n+\n+* `struct path_list_item`\n+\n+Represents an item of the list. The `path` member is a pointer to the\n+string, and you may use the `util` member for any purpose, if you want.\n+\n+* `struct path_list`\n+\n+Represents the list itself.\n+\n+. The array of items are available via the `items` member.\n+. The `nr` member contains the number of items stored in the list.\n+. The `alloc` member is used to avoid reallocating at every insertion.\n+  You should not tamper with it.\n+. Setting the `strdup_paths` member to 1 will strdup() the strings\n+  before adding them, see above.\n-- \n1.5.6.rc2.dirty\n"},{"id":"79902","messageId":"7vhcbve1ad.fsf@gitster.siamese.dyndns.org","threadId":"13933","inReplyTo":"alpine.DEB.1.00.0806141705050.6439@racer","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-15T05:04:58Z","receivedAt":"2008-06-15T05:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Speaking of which: Junio, could you give me any clue how you would like to \n> proceed with that patch?\n\nIt would be most convenient to do so when\n\n    git diff master pu | grep path.list\n\nshrinks to the minimum.  I think very early after 1.5.6 would be the best,\nas there is nothing outstanding that adds new use or removes existing use\nof path_list.\n"},{"id":"79913","messageId":"m3k5groyw8.fsf@localhost.localdomain","threadId":"13933","inReplyTo":"1213485725-6755-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] path-list documentation: document all functions and data structures","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-15T09:01:19Z","receivedAt":"2008-06-15T09:01:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> +NOTE: It is more efficient to build an unsorted list and sort it\n> +afterwards, instead of building a sorted list `(O(n log n)` instead of\n> +`O(n^2))`.\n\nI think there is typo here (misplaced backticks '`' on the wrong side\nof enclosing parentheses), and this fragment should read:\n\n+afterwards, instead of building a sorted list (`O(n log n)` instead of\n+`O(n^2)`).\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"79925","messageId":"1213531603-8364-1-git-send-email-vmiklos@frugalware.org","threadId":"13933","inReplyTo":"m3k5groyw8.fsf@localhost.localdomain","subject":"[PATCH] path-list documentation: document all functions and data structures","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-06-15T12:06:43Z","receivedAt":"2008-06-15T12:06:43Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Sun, Jun 15, 2008 at 02:01:19AM -0700, Jakub Narebski <jnareb@gmail.com> wrote:\n> > +NOTE: It is more efficient to build an unsorted list and sort it\n> > +afterwards, instead of building a sorted list `(O(n log n)` instead\n> > of\n> > +`O(n^2))`.\n>\n> I think there is typo here (misplaced backticks '`' on the wrong side\n> of enclosing parentheses), and this fragment should read:\n>\n> +afterwards, instead of building a sorted list (`O(n log n)` instead\n> of\n> +`O(n^2)`).\n\nExactly, thanks for pointing out. Updated patch below.\n\n Documentation/technical/api-path-list.txt |  125 ++++++++++++++++++++++++++++-\n 1 files changed, 121 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/technical/api-path-list.txt b/Documentation/technical/api-path-list.txt\nindex d077683..9dbedd0 100644\n--- a/Documentation/technical/api-path-list.txt\n+++ b/Documentation/technical/api-path-list.txt\n@@ -1,9 +1,126 @@\n path-list API\n =============\n \n-Talk about <path-list.h>, things like\n+The path_list API offers a data structure and functions to handle sorted\n+and unsorted string lists.\n \n-* it is not just paths but strings in general;\n-* the calling sequence.\n+The name is a bit misleading, a path_list may store not only paths but\n+strings in general.\n \n-(Dscho)\n+The caller:\n+\n+. Allocates and clears a `struct path_list` variable.\n+\n+. Initializes the members. You might want to set the flag `strdup_paths`\n+  if the strings should be strdup()ed. For example, this is necessary\n+  when you add something like git_path(\"...\"), since that function returns\n+  a static buffer that will change with the next call to git_path().\n++\n+If you need something advanced, you can manually malloc() the `items`\n+member (you need this if you add things later) and you should set the\n+`nr` and `alloc` members in that case, too.\n+\n+. Adds new items to the list, using `path_list_append` or `path_list_insert`.\n+\n+. Can check if a string is in the list using `path_list_has_path` or\n+  `unsorted_path_list_has_path` and get it from the list using\n+  `path_list_lookup` for sorted lists.\n+\n+. Can sort an unsorted list using `sort_path_list`.\n+\n+. Finally it should free the list using `path_list_clear`.\n+\n+Example:\n+\n+----\n+struct path_list list;\n+int i;\n+\n+memset(&list, 0, sizeof(struct path_list));\n+path_list_append(\"foo\", &list);\n+path_list_append(\"bar\", &list);\n+for (i = 0; i < list.nr; i++)\n+\tprintf(\"%s\\n\", list.items[i].path)\n+----\n+\n+NOTE: It is more efficient to build an unsorted list and sort it\n+afterwards, instead of building a sorted list (`O(n log n)` instead of\n+`O(n^2)`).\n++\n+However, if you use the list to check if a certain string was added\n+already, you should not do that (using unsorted_path_list_has_path()),\n+because the complexity would be quadratic again (but with a worse factor).\n+\n+Functions\n+---------\n+\n+* General ones (works with sorted and unsorted lists as well)\n+\n+`print_path_list`::\n+\n+\tDump a path_list to stdout, useful mainly for debugging purposes. It\n+\tcan take an optional header argument and it writes out the\n+\tstring-pointer pairs of the path_list, each one in its own line.\n+\n+`path_list_clear`::\n+\n+\tFree a path_list. The `path` pointer of the items will be freed in case\n+\tthe `strdup_paths` member of the path_list is set. The second parameter\n+\tcontrols if the `util` pointer of the items should be freed or not.\n+\n+* Functions for sorted lists only\n+\n+`path_list_has_path`::\n+\n+\tDetermine if the path_list has a given string or not.\n+\n+`path_list_insert`::\n+\n+\tInsert a new element to the path_list. The returned pointer can be handy\n+\tif you want to write something to the `util` pointer of the\n+\tpath_list_item containing the just added string.\n++\n+Since this function uses xrealloc() (which die()s if it fails) if the\n+list needs to grow, it is safe not to check the pointer. I.e. you may\n+write `path_list_insert(...)->util = ...;`.\n+\n+`path_list_lookup`::\n+\n+\tLook up a given string in the path_list, returning the containing\n+\tpath_list_item. If the string is not found, NULL is returned.\n+\n+* Functions for unsorted lists only\n+\n+`path_list_append`::\n+\n+\tAppend a new string to the end of the path_list.\n+\n+`sort_path_list`::\n+\n+\tMake an unsorted list sorted.\n+\n+`unsorted_path_list_has_path`::\n+\n+\tIt's like `path_list_has_path()` but for unsorted lists.\n++\n+This function needs to look through all items, as opposed to its\n+counterpart for sorted lists, which performs a binary search.\n+\n+Data structures\n+---------------\n+\n+* `struct path_list_item`\n+\n+Represents an item of the list. The `path` member is a pointer to the\n+string, and you may use the `util` member for any purpose, if you want.\n+\n+* `struct path_list`\n+\n+Represents the list itself.\n+\n+. The array of items are available via the `items` member.\n+. The `nr` member contains the number of items stored in the list.\n+. The `alloc` member is used to avoid reallocating at every insertion.\n+  You should not tamper with it.\n+. Setting the `strdup_paths` member to 1 will strdup() the strings\n+  before adding them, see above.\n-- \n1.5.6.rc2.dirty\n"},{"id":"80008","messageId":"e2b179460806160201j133dac0eg37c88b5670541ff8@mail.gmail.com","threadId":"13933","inReplyTo":"1213376131-20967-1-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH] git-apply doesn't handle same name patches well","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2008-06-16T09:01:41Z","receivedAt":"2008-06-16T09:01:41Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2008/6/13 Don Zickus <dzickus@redhat.com>:\n> When working with a lot of people who backport patches all day long, every\n> once in a while I get a patch that modifies the same file more than once\n> inside the same patch...\n>\n> I have modified git-apply to cache the filenames of files it modifies such\n> that if a later patch chunk modifies a file in the cache it will buffer the\n> previously changed file instead of reading the original file from disk.\n\nExcellent spot. A couple of things you might want to add to your new\ntest cases would be examples where the first patch renames or removes\na file (or two files are swapped) and a subsequent patch then touches\nthe same path(s).\n\nMike\n"}]}