{"thread":{"id":"16113","subject":"getting list of objects for packing","startedAt":"2008-10-31T19:32:18Z","lastAt":"2008-11-13T00:50:26Z","messageCount":61,"participants":["Brandon Casey","Nicolas Pitre","Junio C Hamano","Jakub Narebski","drafnel@gmail.com","Andreas Ericsson","Shawn O. Pearce","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"94442","messageId":"TtAUShKh7lOR5rkf1iyWwpMOWoYpT8Mnw-t3Wemdy3tTCd0XiQHdag@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":null,"subject":"getting list of objects for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-31T19:32:18Z","receivedAt":"2008-10-31T19:32:18Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nI'm trying to write a script that will repack large binary or compressed\nobjects into their own non-compressed, non-delta'ed pack file.\n\nTo make the decision about whether an object should go into this special\npack file or not, I want the output from 'git cat-file --batch-check'.\nI get it with something similar to:\n\n git rev-list --objects --all |\n sed -e 's/^\\([0-9a-f]\\{40\\}\\).*/\\1/' |\n git cat-file --batch-check\n\nFirst question: Is the rev-list call correct?\n -If I am understanding things right, then the list of objects produced\n by rev-list will be in the right order for piping to pack-objects. \n -The sed statement is stripping off anything after the sha1. Any way to\n get rev-list to print out just the sha1 so that sed is not necessary?\n\nThen I want to parse the output from cat-file and use an external program\nto detect the file format. Here is a simplified version:\n\n | while read sha1 type size; do\n\n if [ $type = \"blob\" ]; then\n if ! ( git cat-file blob \"$sha1\" | file -b - | grep text ) &&\n [ $size -ge $threshhold ]; then\n # pack into special pack\n else\n # pack normally into normal pack\n fi\n fi\n done\n\nAll of this has actually been rewritten into a perl script, so ignore any\nsyntax mistakes.\n\nI have successfully created two of the pack files that I have been trying to\nmake. Where the definition of successful means that after removing the existing\npacks and objects, and putting in place the two pack files that I generated,\n'git fsck --full' prints no errors and exits successfully.\n\nThese two packs will be placed into a central repository.\n\nISSUE TWO:\n\nI have placed these two packs into my own personal repo, and I have unpacked all\nof the other objects so that they are loose.\n\nI thought I could use a similar sequence of commands to pack those loose objects\ninto a normal and special pack. I added the --unpacked option to my rev-list\ncommand, but it still lists many more objects than exist loosely in the repository.\n\n git rev-list --objects --unpacked --all\n\nThe man page says:\n\n --objects\n Print the object IDs of any object referenced by the listed\n commits. --objects foo ^bar thus means \"send me all object IDs\n which I need to download if I have the commit object bar, but\n not foo\".\n\n --unpacked\n Only useful with --objects; print the object IDs that are not\n in packs.\n\nIs this the correct behavior for rev-list --unpacked?\nAm I mis-reading the --unpacked text, or should it be changed?\n\n-brandon\n"},{"id":"94456","messageId":"alpine.LFD.2.00.0810311625450.13034@xanadu.home","threadId":"16113","inReplyTo":"TtAUShKh7lOR5rkf1iyWwpMOWoYpT8Mnw-t3Wemdy3tTCd0XiQHdag@cipher.nrlssc.navy.mil","subject":"Re: getting list of objects for packing","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-10-31T20:40:40Z","receivedAt":"2008-10-31T20:40:40Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 31 Oct 2008, Brandon Casey wrote:\n\n> \n> I'm trying to write a script that will repack large binary or compressed\n> objects into their own non-compressed, non-delta'ed pack file.\n> \n> To make the decision about whether an object should go into this special\n> pack file or not, I want the output from 'git cat-file --batch-check'.\n> I get it with something similar to:\n> \n> git rev-list --objects --all |\n> sed -e 's/^\\([0-9a-f]\\{40\\}\\).*/\\1/' |\n> git cat-file --batch-check\n> \n> First question: Is the rev-list call correct?\n\nyes.\n\n> -If I am understanding things right, then the list of objects produced\n> by rev-list will be in the right order for piping to pack-objects. \n> -The sed statement is stripping off anything after the sha1. Any way to\n> get rev-list to print out just the sha1 so that sed is not necessary?\n\nIf you strip the data after the SHA1 when pipping into pack-objects then \nyou'll have horrible delta compression results. The path names after \neach SHA1 is used to sort objects when trying to find best matches for \ndelta compression. So you should preserve those and feed it back \nespecially with those packs that you still want delta compression for.\n\n> ISSUE TWO:\n> \n> I have placed these two packs into my own personal repo, and I have unpacked all\n> of the other objects so that they are loose.\n> \n> I thought I could use a similar sequence of commands to pack those loose objects\n> into a normal and special pack. I added the --unpacked option to my rev-list\n> command, but it still lists many more objects than exist loosely in the repository.\n> \n> git rev-list --objects --unpacked --all\n> \n> The man page says:\n> \n> --objects\n> Print the object IDs of any object referenced by the listed\n> commits. --objects foo ^bar thus means \"send me all object IDs\n> which I need to download if I have the commit object bar, but\n> not foo\".\n> \n> --unpacked\n> Only useful with --objects; print the object IDs that are not\n> in packs.\n> \n> Is this the correct behavior for rev-list --unpacked?\n\nWell, don't think so. Although I have zero unpacked objects in my git \nrepository, the listing still gives me a hundred objects or so, and they \nall appear to be tag objects. There is probably a bug here.\n\n\nNicolas\n"},{"id":"94457","messageId":"JhY9the71dfsAJuojZF2S4BG-SEkshM7XxIWGPBeY9M@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"alpine.LFD.2.00.0810311625450.13034@xanadu.home","subject":"Re: getting list of objects for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-31T20:48:52Z","receivedAt":"2008-10-31T20:48:52Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Nicolas Pitre wrote:\n> On Fri, 31 Oct 2008, Brandon Casey wrote:\n\n>> -The sed statement is stripping off anything after the sha1. Any way to\n>> get rev-list to print out just the sha1 so that sed is not necessary?\n> \n> If you strip the data after the SHA1 when pipping into pack-objects then \n> you'll have horrible delta compression results. The path names after \n> each SHA1 is used to sort objects when trying to find best matches for \n> delta compression. So you should preserve those and feed it back \n> especially with those packs that you still want delta compression for.\n\nAh, I'll have to rethink my script then. Thanks!\n\n-brandon\n"},{"id":"94460","messageId":"7v7i7o8nc5.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"JhY9the71dfsAJuojZF2S4BG-SEkshM7XxIWGPBeY9M@cipher.nrlssc.navy.mil","subject":"Re: getting list of objects for packing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-31T21:30:02Z","receivedAt":"2008-10-31T21:30:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Nicolas Pitre wrote:\n>> On Fri, 31 Oct 2008, Brandon Casey wrote:\n>\n>>> -The sed statement is stripping off anything after the sha1. Any way to\n>>> get rev-list to print out just the sha1 so that sed is not necessary?\n>> \n>> If you strip the data after the SHA1 when pipping into pack-objects then \n>> you'll have horrible delta compression results. The path names after \n>> each SHA1 is used to sort objects when trying to find best matches for \n>> delta compression. So you should preserve those and feed it back \n>> especially with those packs that you still want delta compression for.\n>\n> Ah, I'll have to rethink my script then. Thanks!\n\nYeah, but wasn't the purpose of your whole exercise to list objects that\ndo not delta nor compress well with each other, in which case the delta\ncompression order (aka name hash) would not matter, no?\n"},{"id":"94466","messageId":"fAUegZ3bxPo8HquZjUM9syW-giYefuAzBtb1XXHQ-TwbmvJvMZvmDA@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"7v7i7o8nc5.fsf@gitster.siamese.dyndns.org","subject":"Re: getting list of objects for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-31T21:40:03Z","receivedAt":"2008-10-31T21:40:03Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n> Brandon Casey writes:\n> \n>> Nicolas Pitre wrote:\n>>> On Fri, 31 Oct 2008, Brandon Casey wrote:\n>>>> -The sed statement is stripping off anything after the sha1. Any way to\n>>>> get rev-list to print out just the sha1 so that sed is not necessary?\n>>> If you strip the data after the SHA1 when pipping into pack-objects then \n>>> you'll have horrible delta compression results. The path names after \n>>> each SHA1 is used to sort objects when trying to find best matches for \n>>> delta compression. So you should preserve those and feed it back \n>>> especially with those packs that you still want delta compression for.\n>> Ah, I'll have to rethink my script then. Thanks!\n> \n> Yeah, but wasn't the purpose of your whole exercise to list objects that\n> do not delta nor compress well with each other, in which case the delta\n> compression order (aka name hash) would not matter, no?\n\nThe script I wrote actually starts up two pack-objects instances and I was\nwriting the objects I wanted to pack _normally_ to one, and the ones that I\ndid not want compressed/deltafied to the other (which was started with\n--no-reuse-object --window=0 --depth=0 --compression=0).\n\nI didn't mentioned that fact in my first email, but I'm very glad Nico\nmade his point.\n\n-brandon\n"},{"id":"94476","messageId":"m3y704flrf.fsf@localhost.localdomain","threadId":"16113","inReplyTo":"fAUegZ3bxPo8HquZjUM9syW-giYefuAzBtb1XXHQ-TwbmvJvMZvmDA@cipher.nrlssc.navy.mil","subject":"Re: getting list of objects for packing","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-10-31T22:23:40Z","receivedAt":"2008-10-31T22:23:40Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Brandon Casey writes:\n\n> Junio C Hamano wrote:\n\n> > Yeah, but wasn't the purpose of your whole exercise to list objects that\n> > do not delta nor compress well with each other, in which case the delta\n> > compression order (aka name hash) would not matter, no?\n> \n> The script I wrote actually starts up two pack-objects instances and I was\n> writing the objects I wanted to pack _normally_ to one, and the ones that I\n> did not want compressed/deltafied to the other (which was started with\n> --no-reuse-object --window=0 --depth=0 --compression=0).\n> \n> I didn't mentioned that fact in my first email, but I'm very glad Nico\n> made his point.\n\nWasn't there some gitattribute which prohibited deltification of some\nfiles (`delta` or something)? Or wasn't this patch accepted, as I\ncannot find such attribute in documentation (gitattributes(5))...\n... err, it was added in commit a74db82e15cd8a2c53a4a83e9a36dc7bf7a4c750\n(Teach \"delta\" attribute to pack-objects.) by Junio C Hamano in May\n19, _without_ documentation.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"94501","messageId":"1S3xpaVP1Cy1Rei_ODwlXsBdu64BGiPve-lj_4fN6cA@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"fAUegZ3bxPo8HquZjUM9syW-giYefuAzBtb1XXHQ-TwbmvJvMZvmDA@cipher.nrlssc.navy.mil","subject":"Re: getting list of objects for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-01T00:00:06Z","receivedAt":"2008-11-01T00:00:06Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Brandon Casey wrote:\n> Junio C Hamano wrote:\n>> Brandon Casey writes:\n>>\n>>> Nicolas Pitre wrote:\n>>>> On Fri, 31 Oct 2008, Brandon Casey wrote:\n>>>>> -The sed statement is stripping off anything after the sha1. Any way to\n>>>>> get rev-list to print out just the sha1 so that sed is not necessary?\n>>>> If you strip the data after the SHA1 when pipping into pack-objects then \n>>>> you'll have horrible delta compression results. The path names after \n>>>> each SHA1 is used to sort objects when trying to find best matches for \n>>>> delta compression. So you should preserve those and feed it back \n>>>> especially with those packs that you still want delta compression for.\n>>> Ah, I'll have to rethink my script then. Thanks!\n>> Yeah, but wasn't the purpose of your whole exercise to list objects that\n>> do not delta nor compress well with each other, in which case the delta\n>> compression order (aka name hash) would not matter, no?\n> \n> The script I wrote actually starts up two pack-objects instances and I was\n> writing the objects I wanted to pack _normally_ to one, and the ones that I\n> did not want compressed/deltafied to the other (which was started with\n> --no-reuse-object --window=0 --depth=0 --compression=0).\n\nSo, my script created two pack files: one packed normally, and one packed without\ncompression or delta. I removed my original packs, and put these two new ones in\nmy pack directory and ran 'git fsck --full' and it completed successfully. There\nare no loose objects in the repo.\n\nI added a .keep file for each pack.\n\nSince my script removed the extra info from rev-parse's output, I removed the\n.keep file from the appropriate pack and ran 'git gc --aggressive'.\n\nThe 1.7GB pack that had the .keep file removed has been replaced with a +3GB\npack file. The other pack file which still has the .keep file is 2.3GB.\n\nIn another repo with 3 packs marked .keep, and one 388KB pack with ~300\nobjects in it, and 3 loose dangling objects, the 388KB pack was replaced with\na 3.5GB pack.\n\nIt appears that the entire repository is being packed into the new pack file\neven though there are existing pack files with .keep files.\n\nIf I compare the output from 'git verify-pack -v' I can see that many of the\nobjects in the packs marked with a .keep file are indeed in the new pack file.\nBut not all of them.\n\n-brandon\n"},{"id":"94620","messageId":"14536526.1225596838300.JavaMail.teamon@b301.teamon.com","threadId":"16113","inReplyTo":"1S3xpaVP1Cy1Rei_ODwlXsBdu64BGiPve-lj_4fN6cA@cipher.nrlssc.navy.mil","subject":"[PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-02T03:35:19Z","receivedAt":"2008-11-02T03:35:19Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nObjects residing in pack files that have an associated .keep file are not\nsupposed to be repacked into new pack files, but they are.\n\nSigned-off-by: Brandon Casey \n---\n t/t7700-repack.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7700-repack.sh\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nnew file mode 100755\nindex 0000000..1489e68\n--- /dev/null\n+++ b/t/t7700-repack.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git repack works correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_failure 'objects in packs marked .keep are not repacked' '\n+\techo content1 > file1 &&\n+\techo content2 > file2 &&\n+\tgit add . &&\n+\tgit commit -m initial_commit &&\n+\t# Create two packs \n+\t# The first pack will contain all of the objects except one\n+\tgit rev-list --objects --all | head -n -1 |\n+\t\tgit pack-objects pack > /dev/null &&\n+\t# The second pack will contain the excluded object\n+\tpacksha1=$(git rev-list --objects --all | tail -n 1 |\n+\t\tgit pack-objects pack) &&\n+\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n+\t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n+\tmv pack-* .git/objects/pack/ &&\n+\tgit repack -A -d -l &&\n+\tgit prune-packed &&\n+\tfor p in .git/objects/pack/*.idx; do\n+\t\tidx=$(basename $p)\n+\t\ttest \"pack-$packsha1.idx\" = \"$idx\" && continue\n+\t\tif git verify-pack -v $p | egrep \"^$objsha1\"; then\n+\t\t\tfound_duplicate_object=1\n+\t\t\techo \"DUPLICATE OBJECT FOUND\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone &&\n+\ttest -z \"$found_duplicate_object\"\n+'\n+\n+test_done\n+\n-- \n1.6.0.2.588.g3102\n"},{"id":"94666","messageId":"6141358.1225643400587.JavaMail.teamon@b303.teamon.com","threadId":"16113","inReplyTo":"14536526.1225596838300.JavaMail.teamon@b301.teamon.com","subject":"[PATCH 1/3] packed_git: convert pack_local flag into generic bit mask","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-02T16:31:15Z","receivedAt":"2008-11-02T16:31:15Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nThis converts the pack_local flag of the packed_git structure into a generic\nbit mask and introduces a PACK_LOCAL mask and an ispacklocal() access macro.\n\nSo instead of this:\n\n if (p->pack_local)\n do_something\n\nyou would do this:\n\n if (ispacklocal(p))\n do_something\n\nThis is in preparation for adding a flag indicating whether a .keep file is\npresent.\n\nSigned-off-by: Brandon Casey \n---\n builtin-count-objects.c | 2 +-\n builtin-gc.c | 2 +-\n builtin-pack-objects.c | 2 +-\n cache.h | 5 ++++-\n pack-redundant.c | 4 ++--\n server-info.c | 4 ++--\n sha1_file.c | 5 +++--\n 7 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-count-objects.c b/builtin-count-objects.c\nindex ab35b65..3f981d6 100644\n--- a/builtin-count-objects.c\n+++ b/builtin-count-objects.c\n@@ -108,7 +108,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tif (!packed_git)\n \t\t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next) {\n-\t\t\tif (!p->pack_local)\n+\t\t\tif (!ispacklocal(p))\n \t\t\t\tcontinue;\n \t\t\tif (open_pack_index(p))\n \t\t\t\tcontinue;\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 7af65bb..0473158 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -135,7 +135,7 @@ static int too_many_packs(void)\n \t\tsize_t len;\n \t\tint keep;\n \n-\t\tif (!p->pack_local)\n+\t\tif (!ispacklocal(p))\n \t\t\tcontinue;\n \t\tlen = strlen(p->pack_name);\n \t\tif (PATH_MAX <= len + 1)\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 59c30d1..6a8b9bf 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t\tbreak;\n \t\t\tif (incremental)\n \t\t\t\treturn 0;\n-\t\t\tif (local && !p->pack_local)\n+\t\t\tif (local && !ispacklocal(p))\n \t\t\t\treturn 0;\n \t\t}\n \t}\ndiff --git a/cache.h b/cache.h\nindex b0edbf9..0cb9350 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -679,12 +679,15 @@ extern struct packed_git {\n \tint index_version;\n \ttime_t mtime;\n \tint pack_fd;\n-\tint pack_local;\n+\tunsigned int flags;\n \tunsigned char sha1[20];\n \t/* something like \".git/objects/pack/xxxxx.pack\" */\n \tchar pack_name[FLEX_ARRAY]; /* more */\n } *packed_git;\n \n+#define PACK_LOCAL\t1\n+#define ispacklocal(p) ((p)->flags & PACK_LOCAL)\n+\n struct pack_entry {\n \toff_t offset;\n \tunsigned char sha1[20];\ndiff --git a/pack-redundant.c b/pack-redundant.c\nindex 25b81a4..964f18f 100644\n--- a/pack-redundant.c\n+++ b/pack-redundant.c\n@@ -544,7 +544,7 @@ static struct pack_list * add_pack(struct packed_git *p)\n \tunsigned long off = 0, step;\n \tconst unsigned char *base;\n \n-\tif (!p->pack_local && !(alt_odb || verbose))\n+\tif (!ispacklocal(p) && !(alt_odb || verbose))\n \t\treturn NULL;\n \n \tl.pack = p;\n@@ -562,7 +562,7 @@ static struct pack_list * add_pack(struct packed_git *p)\n \t}\n \t/* this list will be pruned in cmp_two_packs later */\n \tl.unique_objects = llist_copy(l.all_objects);\n-\tif (p->pack_local)\n+\tif (ispacklocal(p))\n \t\treturn pack_list_insert(&local_packs, &l);\n \telse\n \t\treturn pack_list_insert(&altodb_packs, &l);\ndiff --git a/server-info.c b/server-info.c\nindex c1c073b..2eb20f5 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -168,14 +168,14 @@ static void init_pack_info(const char *infofile, int force)\n \t\t/* we ignore things on alternate path since they are\n \t\t * not available to the pullers in general.\n \t\t */\n-\t\tif (!p->pack_local)\n+\t\tif (!ispacklocal(p))\n \t\t\tcontinue;\n \t\ti++;\n \t}\n \tnum_pack = i;\n \tinfo = xcalloc(num_pack, sizeof(struct pack_info *));\n \tfor (i = 0, p = packed_git; p; p = p->next) {\n-\t\tif (!p->pack_local)\n+\t\tif (!ispacklocal(p))\n \t\t\tcontinue;\n \t\tinfo[i] = xcalloc(1, sizeof(struct pack_info));\n \t\tinfo[i]->p = p;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex ab2b520..e4141c9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -851,7 +851,8 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \t * actually mapping the pack file.\n \t */\n \tp->pack_size = st.st_size;\n-\tp->pack_local = local;\n+\tif (local)\n+\t\tp->flags |= PACK_LOCAL;\n \tp->mtime = st.st_mtime;\n \tif (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))\n \t\thashclr(p->sha1);\n@@ -941,7 +942,7 @@ static int sort_pack(const void *a_, const void *b_)\n \t * remote ones could be on a network mounted filesystem.\n \t * Favor local ones for these reasons.\n \t */\n-\tst = a->pack_local - b->pack_local;\n+\tst = ispacklocal(a) - ispacklocal(b);\n \tif (st)\n \t\treturn -st;\n \n-- \n1.6.0.2.588.g3102\n"},{"id":"94667","messageId":"14846251.1225643403413.JavaMail.teamon@b303.teamon.com","threadId":"16113","inReplyTo":"1225643477-32319-1-git-send-email-foo@foo.com","subject":"[PATCH 2/3] packed_git: add new PACK_KEEP flag and haspackkeep() access macro","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-02T16:31:16Z","receivedAt":"2008-11-02T16:31:16Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nIf you want to tell whether a pack has an associated \".keep\" file you\nwould do:\n\n if (haspackkeep(p))\n do_something\n\nSigned-off-by: Brandon Casey \n---\n cache.h | 2 ++\n sha1_file.c | 5 +++++\n 2 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 0cb9350..48cd366 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -686,7 +686,9 @@ extern struct packed_git {\n } *packed_git;\n \n #define PACK_LOCAL\t1\n+#define PACK_KEEP\t2\n #define ispacklocal(p) ((p)->flags & PACK_LOCAL)\n+#define haspackkeep(p) ((p)->flags & PACK_KEEP)\n \n struct pack_entry {\n \toff_t offset;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e4141c9..8a027e9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -841,6 +841,11 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \t\treturn NULL;\n \t}\n \tmemcpy(p->pack_name, path, path_len);\n+\n+\tstrcpy(p->pack_name + path_len, \".keep\");\n+\tif (!access(p->pack_name, F_OK))\n+\t\tp->flags |= PACK_KEEP;\n+\n \tstrcpy(p->pack_name + path_len, \".pack\");\n \tif (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {\n \t\tfree(p);\n-- \n1.6.0.2.588.g3102\n"},{"id":"94668","messageId":"4002818.1225643406819.JavaMail.teamon@b303.teamon.com","threadId":"16113","inReplyTo":"1225643477-32319-2-git-send-email-foo@foo.com","subject":"[PATCH 3/3] pack-objects: honor '.keep' files","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-02T16:31:17Z","receivedAt":"2008-11-02T16:31:17Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nBy default, pack-objects creates a pack file with every object specified by\nthe user. There are two options which can be used to exclude objects which\nare accessible by the repository.\n\n 1) --incremental\n This excludes any object which already exists in an accessible pack.\n\n 2) --local\n This excludes any object which exists in a non-local pack.\n\nWith this patch, both arguments also cause objects which exist in packs\nmarked with a .keep file to be excluded. Only the --local option requires\nan explicit check for the .keep file. If the user doesn't want the objects\nin a pack marked with .keep to be exclude, then the .keep file should be\nremoved.\n\nAdditionally, this fixes the repack bug which allowed porcelain repack to\ncreate packs which contained objects already contained in existing packs\nmarked with a .keep file.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis seems to be the correct fix.\n\nI thought about two alternatives:\n\n 1) New option to pack-objects which causes it to honor .keep files\n\n I didn't think this was necessary since that is why we have .keep\n files in the first place.\n\n So, now there are three variants:\n\n Neither --incremental or --local is used)\n Ignore repo packs, pack is created using all objects specified by\n user.\n --incremental is used)\n Look at repo packs, exclude already packed objects (including those\n marked with .keep file)\n --local is used)\n Look at repo packs, exclude objects in non-local packs (including local\n objects marked with .keep file)\n\n 2) Always honor .keep files, even when --incremental or --local is\n not specified. \n\n I think this may be counter-intuitive, since if you specify all of\n the objects explicitly, without using any other options, you expect\n all of those objects to be placed into the created pack file. If\n .keep is always honored, then a new option would be required to\n indicate ignoring .keep files, or we would adopt the platform that\n \"the user must just remove the .keep file\".\n\n\nSo, honoring .keep files only with --incremental (which is a side effect) or\n --local seems to be the most appropriate.\n\nComments welcome.\n\n-brandon\n\n\n builtin-pack-objects.c | 2 +-\n t/t7700-repack.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 6a8b9bf..5199e54 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t\tbreak;\n \t\t\tif (incremental)\n \t\t\t\treturn 0;\n-\t\t\tif (local && !ispacklocal(p))\n+\t\t\tif (local && (!ispacklocal(p) || haspackkeep(p)))\n \t\t\t\treturn 0;\n \t\t}\n \t}\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 1489e68..b6b02da 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -4,7 +4,7 @@ test_description='git repack works correctly'\n \n . ./test-lib.sh\n \n-test_expect_failure 'objects in packs marked .keep are not repacked' '\n+test_expect_success 'objects in packs marked .keep are not repacked' '\n \techo content1 > file1 &&\n \techo content2 > file2 &&\n \tgit add . &&\n-- \n1.6.0.2.588.g3102\n"},{"id":"94747","messageId":"490ED3FE.8040103@op5.se","threadId":"16113","inReplyTo":"14536526.1225596838300.JavaMail.teamon@b301.teamon.com","subject":"Re: [PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-03T10:35:42Z","receivedAt":"2008-11-03T10:35:42Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"drafnel@gmail.com wrote:\n> From: Brandon Casey \n> \n> Objects residing in pack files that have an associated .keep file are not\n> supposed to be repacked into new pack files, but they are.\n> \n\nI think that's a misconception. Packfiles that are marked with .keep files\nshould never be deleted. There are, afaik, no rules against packing the\nsame objects into other packfiles as well. This is nifty for dumb ref\nwalkers, as they can use a small pack for incremental fetching while using\na mega-pack for initial cloning.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94773","messageId":"20081103161202.GJ15463@spearce.org","threadId":"16113","inReplyTo":"6141358.1225643400587.JavaMail.teamon@b303.teamon.com","subject":"Re: [PATCH 1/3] packed_git: convert pack_local flag into generic bit mask","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-03T16:12:02Z","receivedAt":"2008-11-03T16:12:02Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"drafnel@gmail.com wrote:\n> This is in preparation for adding a flag indicating whether a .keep file is\n> present.\n\nGood idea.\n \n> diff --git a/cache.h b/cache.h\n> index b0edbf9..0cb9350 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -679,12 +679,15 @@ extern struct packed_git {\n> \tint index_version;\n> \ttime_t mtime;\n> \tint pack_fd;\n> -\tint pack_local;\n> +\tunsigned int flags;\n\nHmm, isn't this a smaller change to make?\n\n-\tint pack_local;\n+\tunsigned pack_local:1;\n\nThen later you can do:\n\n-\tunsigned pack_local:1;\n+\tunsigned pack_local:1,\n+\t\tpack_keep:1;\n\nand the compiler handles all the bitmask stuff for you?\n\nIn general in git.git we like to use the struct bitmask stuff when\npossible as the code is easier to follow. We only use explicit\nmask constants and mask operations when the data is being stored\non disk or written over the network and we need to ensure it is\nconsistent across compilers. But for in-core only stuff, struct\nbitmasks are easier.\n\n-- \nShawn.\n"},{"id":"94774","messageId":"20081103161734.GK15463@spearce.org","threadId":"16113","inReplyTo":"4002818.1225643406819.JavaMail.teamon@b303.teamon.com","subject":"Re: [PATCH 3/3] pack-objects: honor '.keep' files","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-03T16:17:34Z","receivedAt":"2008-11-03T16:17:34Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"drafnel@gmail.com wrote:\n> From: Brandon Casey \n> \n> By default, pack-objects creates a pack file with every object specified by\n> the user. There are two options which can be used to exclude objects which\n> are accessible by the repository.\n> \n> 1) --incremental\n> This excludes any object which already exists in an accessible pack.\n> \n> 2) --local\n> This excludes any object which exists in a non-local pack.\n> \n> With this patch, both arguments also cause objects which exist in packs\n> marked with a .keep file to be excluded. Only the --local option requires\n> an explicit check for the .keep file. If the user doesn't want the objects\n> in a pack marked with .keep to be exclude, then the .keep file should be\n> removed.\n> \n> Additionally, this fixes the repack bug which allowed porcelain repack to\n> create packs which contained objects already contained in existing packs\n> marked with a .keep file.\n\nThis looks sane to me, but I'm holding off on the ack because\nI don't like the way the flags are managed in struct packed_git.\n(See my reply to 1/3 in the series.)\n\n \n> Signed-off-by: Brandon Casey \n> ---\n> \n> \n> This seems to be the correct fix.\n> \n> I thought about two alternatives:\n> \n> 1) New option to pack-objects which causes it to honor .keep files\n> \n> I didn't think this was necessary since that is why we have .keep\n> files in the first place.\n> \n> So, now there are three variants:\n> \n> Neither --incremental or --local is used)\n> Ignore repo packs, pack is created using all objects specified by\n> user.\n> --incremental is used)\n> Look at repo packs, exclude already packed objects (including those\n> marked with .keep file)\n> --local is used)\n> Look at repo packs, exclude objects in non-local packs (including local\n> objects marked with .keep file)\n> \n> 2) Always honor .keep files, even when --incremental or --local is\n> not specified. \n> \n> I think this may be counter-intuitive, since if you specify all of\n> the objects explicitly, without using any other options, you expect\n> all of those objects to be placed into the created pack file. If\n> .keep is always honored, then a new option would be required to\n> indicate ignoring .keep files, or we would adopt the platform that\n> \"the user must just remove the .keep file\".\n> \n> \n> So, honoring .keep files only with --incremental (which is a side effect) or\n> --local seems to be the most appropriate.\n> \n> Comments welcome.\n> \n> -brandon\n> \n> \n> builtin-pack-objects.c | 2 +-\n> t/t7700-repack.sh | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 6a8b9bf..5199e54 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n> \t\t\t\tbreak;\n> \t\t\tif (incremental)\n> \t\t\t\treturn 0;\n> -\t\t\tif (local && !ispacklocal(p))\n> +\t\t\tif (local && (!ispacklocal(p) || haspackkeep(p)))\n> \t\t\t\treturn 0;\n> \t\t}\n> \t}\n> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\n> index 1489e68..b6b02da 100755\n> --- a/t/t7700-repack.sh\n> +++ b/t/t7700-repack.sh\n> @@ -4,7 +4,7 @@ test_description='git repack works correctly'\n> \n> . ./test-lib.sh\n> \n> -test_expect_failure 'objects in packs marked .keep are not repacked' '\n> +test_expect_success 'objects in packs marked .keep are not repacked' '\n> \techo content1 > file1 &&\n> \techo content2 > file2 &&\n> \tgit add . &&\n> -- \n> 1.6.0.2.588.g3102\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n-- \nShawn.\n"},{"id":"94789","messageId":"RqVk2AkdyUcFTIGofSkQwl1GtBTXMYzMqaOQiAOmBXAyPDuWlQug-w@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"490ED3FE.8040103@op5.se","subject":"Re: [PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T18:20:04Z","receivedAt":"2008-11-03T18:20:04Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Andreas Ericsson wrote:\n> drafnel@gmail.com wrote:\n>> From: Brandon Casey \n>>\n>> Objects residing in pack files that have an associated .keep file are not\n>> supposed to be repacked into new pack files, but they are.\n>>\n> \n> I think that's a misconception. Packfiles that are marked with .keep files\n> should never be deleted. There are, afaik, no rules against packing the\n> same objects into other packfiles as well. This is nifty for dumb ref\n> walkers, as they can use a small pack for incremental fetching while using\n> a mega-pack for initial cloning.\n\nHaving no rules against an object residing in more than one pack is different\nfrom intending for git to produce pack files with redundant objects.\n\nI think one intention for the .keep mechanism was to allow for a size optimized\npack to be produced and distributed. Currently, if I am handed such a pack file,\nI can not merely place it into my pack directory (along with the .idx and .keep\nfiles) and then run git-gc to remove any redundancy. Instead, I would get\na _new_ pack file which would contain all of the objects in the repository and\neffectively double the size of my objects store. That doesn't seem like\nsomething a user would expect or should expect.\n\n-brandon\n"},{"id":"94790","messageId":"CU90KJ5IESAvyuMdaPcAuWOyXIdTgXenJ-InGZLiFn8@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"20081103161202.GJ15463@spearce.org","subject":"Re: [PATCH 1/3] packed_git: convert pack_local flag into generic bit mask","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T18:24:56Z","receivedAt":"2008-11-03T18:24:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Shawn O. Pearce wrote:\n> drafnel@gmail.com wrote:\n\n>> diff --git a/cache.h b/cache.h\n>> index b0edbf9..0cb9350 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -679,12 +679,15 @@ extern struct packed_git {\n>> \tint index_version;\n>> \ttime_t mtime;\n>> \tint pack_fd;\n>> -\tint pack_local;\n>> +\tunsigned int flags;\n> \n> Hmm, isn't this a smaller change to make?\n\nheh, well if you want to do it the \"easy\" way. :)\n\nFor some reason I've never used this bit field mechanism, but\nI agree it is more readable and simpler, and you can't argue\nwith that.\n\n-brandon\n"},{"id":"94804","messageId":"490F5E2E.2050207@op5.se","threadId":"16113","inReplyTo":"RqVk2AkdyUcFTIGofSkQwl1GtBTXMYzMqaOQiAOmBXAyPDuWlQug-w@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-03T20:25:18Z","receivedAt":"2008-11-03T20:25:18Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> Andreas Ericsson wrote:\n>> drafnel@gmail.com wrote:\n>>> From: Brandon Casey \n>>>\n>>> Objects residing in pack files that have an associated .keep file are not\n>>> supposed to be repacked into new pack files, but they are.\n>>>\n>> I think that's a misconception. Packfiles that are marked with .keep files\n>> should never be deleted. There are, afaik, no rules against packing the\n>> same objects into other packfiles as well. This is nifty for dumb ref\n>> walkers, as they can use a small pack for incremental fetching while using\n>> a mega-pack for initial cloning.\n> \n> Having no rules against an object residing in more than one pack is different\n> from intending for git to produce pack files with redundant objects.\n> \n> I think one intention for the .keep mechanism was to allow for a size optimized\n> pack to be produced and distributed. Currently, if I am handed such a pack file,\n> I can not merely place it into my pack directory (along with the .idx and .keep\n> files) and then run git-gc to remove any redundancy. Instead, I would get\n> a _new_ pack file which would contain all of the objects in the repository and\n> effectively double the size of my objects store. That doesn't seem like\n> something a user would expect or should expect.\n> \n\nSo long as \"git repack -a\" still creates a mega-pack, I'm fine with whatever.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94807","messageId":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"20081103161202.GJ15463@spearce.org","subject":"[PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T20:37:05Z","receivedAt":"2008-11-03T20:37:05Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nObjects residing in pack files that have an associated .keep file are not\nsupposed to be repacked into new pack files, but they are.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis version replaces the use of 'head -n -1' with a grep, and should work on\nall platforms.\n\n-brandon\n\n\n t/t7700-repack.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7700-repack.sh\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nnew file mode 100755\nindex 0000000..27af5ab\n--- /dev/null\n+++ b/t/t7700-repack.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git repack works correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_failure 'objects in packs marked .keep are not repacked' '\n+\techo content1 > file1 &&\n+\techo content2 > file2 &&\n+\tgit add . &&\n+\tgit commit -m initial_commit &&\n+\t# Create two packs\n+\t# The first pack will contain all of the objects except one\n+ git rev-list --objects --all | grep -v file2 |\n+\t\tgit pack-objects pack > /dev/null &&\n+\t# The second pack will contain the excluded object\n+ packsha1=$(git rev-list --objects --all | grep file2 |\n+\t\tgit pack-objects pack) &&\n+\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n+\t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n+\tmv pack-* .git/objects/pack/ &&\n+\tgit repack -A -d -l &&\n+\tgit prune-packed &&\n+\tfor p in .git/objects/pack/*.idx; do\n+\t\tidx=$(basename $p)\n+\t\ttest \"pack-$packsha1.idx\" = \"$idx\" && continue\n+\t\tif git verify-pack -v $p | egrep \"^$objsha1\"; then\n+\t\t\tfound_duplicate_object=1\n+\t\t\techo \"DUPLICATE OBJECT FOUND\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone &&\n+\ttest -z \"$found_duplicate_object\"\n+'\n+\n+test_done\n+\n-- \n1.6.0.3.552.g12334\n"},{"id":"94808","messageId":"-RiFxYEd9Wiq2fWX74zYGUiEwrzLeoFDb1KuG3-Xo-s@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","subject":"[PATCH v2 2/3] packed_git: convert pack_local flag into a bitfield and add pack_keep","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T20:41:34Z","receivedAt":"2008-11-03T20:41:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\npack_keep will be set when a pack file has an associated .keep file.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis patch and the following one redo the previous 3-patch series using a\nbitfield as prudently suggested by Shawn.\n\nIt seemed silly to keep the conversion of pack_local into a bitfield, and the\nintroduction of pack_keep separate, so all 7 lines are in this one patch.\n\n-brandon\n\n\n cache.h | 3 ++-\n sha1_file.c | 5 +++++\n 2 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex b0edbf9..b80cb08 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -679,7 +679,8 @@ extern struct packed_git {\n \tint index_version;\n \ttime_t mtime;\n \tint pack_fd;\n-\tint pack_local;\n+\tunsigned pack_local:1,\n+\t\t pack_keep:1;\n \tunsigned char sha1[20];\n \t/* something like \".git/objects/pack/xxxxx.pack\" */\n \tchar pack_name[FLEX_ARRAY]; /* more */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex ab2b520..f2b25bd 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -841,6 +841,11 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \t\treturn NULL;\n \t}\n \tmemcpy(p->pack_name, path, path_len);\n+\n+\tstrcpy(p->pack_name + path_len, \".keep\");\n+\tif (!access(p->pack_name, F_OK))\n+\t\tp->pack_keep = 1;\n+\n \tstrcpy(p->pack_name + path_len, \".pack\");\n \tif (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {\n \t\tfree(p);\n-- \n1.6.0.3.552.g12334\n"},{"id":"94809","messageId":"LSyxMgVV7zAWRvSezvxyUc6-kz2gK6MRVKonKSf1pAmdqO-jeuMFIw@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"-RiFxYEd9Wiq2fWX74zYGUiEwrzLeoFDb1KuG3-Xo-s@cipher.nrlssc.navy.mil","subject":"[PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T20:43:22Z","receivedAt":"2008-11-03T20:43:22Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nBy default, pack-objects creates a pack file with every object specified by\nthe user. There are two options which can be used to exclude objects which\nare accessible by the repository.\n\n 1) --incremental\n This excludes any object which already exists in an accessible pack.\n\n 2) --local\n This excludes any object which exists in a non-local pack.\n\nWith this patch, both arguments also cause objects which exist in packs\nmarked with a .keep file to be excluded. Only the --local option requires\nan explicit check for the .keep file. If the user doesn't want the objects\nin a pack marked with .keep to be exclude, then the .keep file should be\nremoved.\n\nAdditionally, this fixes the repack bug which allowed porcelain repack to\ncreate packs which contained objects already contained in existing packs\nmarked with a .keep file.\n\nSigned-off-by: Brandon Casey \n---\n builtin-pack-objects.c | 2 +-\n t/t7700-repack.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 15b80db..8be9113 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t\tbreak;\n \t\t\tif (incremental)\n \t\t\t\treturn 0;\n-\t\t\tif (local && !p->pack_local)\n+\t\t\tif (local && (!p->pack_local || p->pack_keep))\n \t\t\t\treturn 0;\n \t\t}\n \t}\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 27af5ab..5b1cd05 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -4,7 +4,7 @@ test_description='git repack works correctly'\n \n . ./test-lib.sh\n \n-test_expect_failure 'objects in packs marked .keep are not repacked' '\n+test_expect_success 'objects in packs marked .keep are not repacked' '\n \techo content1 > file1 &&\n \techo content2 > file2 &&\n \tgit add . &&\n-- \n1.6.0.3.552.g12334\n"},{"id":"94810","messageId":"20081103204958.GN15463@spearce.org","threadId":"16113","inReplyTo":"LSyxMgVV7zAWRvSezvxyUc6-kz2gK6MRVKonKSf1pAmdqO-jeuMFIw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-03T20:49:58Z","receivedAt":"2008-11-03T20:49:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Brandon Casey wrote:\n> From: Brandon Casey \n> \n> By default, pack-objects creates a pack file with every object specified by\n> the user. There are two options which can be used to exclude objects which\n> are accessible by the repository.\n> \n> 1) --incremental\n> This excludes any object which already exists in an accessible pack.\n> \n> 2) --local\n> This excludes any object which exists in a non-local pack.\n> \n> With this patch, both arguments also cause objects which exist in packs\n> marked with a .keep file to be excluded. Only the --local option requires\n> an explicit check for the .keep file. If the user doesn't want the objects\n> in a pack marked with .keep to be exclude, then the .keep file should be\n> removed.\n> \n> Additionally, this fixes the repack bug which allowed porcelain repack to\n> create packs which contained objects already contained in existing packs\n> marked with a .keep file.\n> \n> Signed-off-by: Brandon Casey \n\nThis one and the one before it (2/3):\n\nAcked-by: Shawn O. Pearce \n\n> ---\n> builtin-pack-objects.c | 2 +-\n> t/t7700-repack.sh | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 15b80db..8be9113 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n> \t\t\t\tbreak;\n> \t\t\tif (incremental)\n> \t\t\t\treturn 0;\n> -\t\t\tif (local && !p->pack_local)\n> +\t\t\tif (local && (!p->pack_local || p->pack_keep))\n> \t\t\t\treturn 0;\n> \t\t}\n> \t}\n> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\n> index 27af5ab..5b1cd05 100755\n> --- a/t/t7700-repack.sh\n> +++ b/t/t7700-repack.sh\n> @@ -4,7 +4,7 @@ test_description='git repack works correctly'\n> \n> . ./test-lib.sh\n> \n> -test_expect_failure 'objects in packs marked .keep are not repacked' '\n> +test_expect_success 'objects in packs marked .keep are not repacked' '\n> \techo content1 > file1 &&\n> \techo content2 > file2 &&\n> \tgit add . &&\n> -- \n> 1.6.0.3.552.g12334\n> \n\n-- \nShawn.\n"},{"id":"94816","messageId":"76IhmMdjmr4jA4xApPpvlZ4vfjtqEDsUaFal2mN5MHfI0MChFmsj5g@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"490F5E2E.2050207@op5.se","subject":"Re: [PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T22:02:41Z","receivedAt":"2008-11-03T22:02:41Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Andreas Ericsson wrote:\n\n> So long as \"git repack -a\" still creates a mega-pack, I'm fine with\n> whatever.\n\nI don't think it will after pack-objects is taught about .keep files, and\nI don't think it will _now_ if all of your packs have .keep files.\n\n'repack -a' will call pack-objects with either '--unpack=' for\neach pack file without a .keep file, or with '--unpacked --incremental' if\nthere are no pack files without .keep files.\n\nIn the first case, the modifications to pack-objects that I propose\nwill prevent objects that exist in local packs with .keep files\nfrom being packed into the new pack.\n\nIn the second case, the --incremental option would have done the same thing.\n\nSo this inconsistency already existed, but will now be removed in favor of\nhonoring .keep files.\n\nMega-pack creation will become an \"advanced operation\" along the lines of:\n\n git rev-list --objects --all | git pack-objects\n\n-brandon\n"},{"id":"94819","messageId":"5QaPpbqIy9l0n5_CfDoFgHU9t7eMkiXtyHTh8C6VkbUVh62Cajc2qw@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","subject":"[PATCH v3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-03T22:14:34Z","receivedAt":"2008-11-03T22:14:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nObjects residing in pack files that have an associated .keep file are not\nsupposed to be repacked into new pack files, but they are.\n\nSigned-off-by: Brandon Casey \n---\n\n\nWhoops, white space contaminated in two spots, this one fixes it.\n\n-brandon\n\n\n t/t7700-repack.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7700-repack.sh\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nnew file mode 100755\nindex 0000000..7aaff0b\n--- /dev/null\n+++ b/t/t7700-repack.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git repack works correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_failure 'objects in packs marked .keep are not repacked' '\n+\techo content1 > file1 &&\n+\techo content2 > file2 &&\n+\tgit add . &&\n+\tgit commit -m initial_commit &&\n+\t# Create two packs\n+\t# The first pack will contain all of the objects except one\n+\tgit rev-list --objects --all | grep -v file2 |\n+\t\tgit pack-objects pack > /dev/null &&\n+\t# The second pack will contain the excluded object\n+\tpacksha1=$(git rev-list --objects --all | grep file2 |\n+\t\tgit pack-objects pack) &&\n+\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n+\t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n+\tmv pack-* .git/objects/pack/ &&\n+\tgit repack -A -d -l &&\n+\tgit prune-packed &&\n+\tfor p in .git/objects/pack/*.idx; do\n+\t\tidx=$(basename $p)\n+\t\ttest \"pack-$packsha1.idx\" = \"$idx\" && continue\n+\t\tif git verify-pack -v $p | egrep \"^$objsha1\"; then\n+\t\t\tfound_duplicate_object=1\n+\t\t\techo \"DUPLICATE OBJECT FOUND\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone &&\n+\ttest -z \"$found_duplicate_object\"\n+'\n+\n+test_done\n+\n-- \n1.6.0.3.552.g12334\n"},{"id":"94910","messageId":"49109FD4.30003@op5.se","threadId":"16113","inReplyTo":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-04T19:17:40Z","receivedAt":"2008-11-04T19:17:40Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> From: Brandon Casey \n> \n> Objects residing in pack files that have an associated .keep file are not\n> supposed to be repacked into new pack files, but they are.\n> \n> Signed-off-by: Brandon Casey \n> ---\n> \n> \n> This version replaces the use of 'head -n -1' with a grep, and should work on\n> all platforms.\n> \n\nsed 1q is faster, as it stops parsing after the first line (the same as 'head\n-n 1' does, but in a more portable fashion).\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94911","messageId":"4910A1B8.8070506@op5.se","threadId":"16113","inReplyTo":"76IhmMdjmr4jA4xApPpvlZ4vfjtqEDsUaFal2mN5MHfI0MChFmsj5g@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-04T19:25:44Z","receivedAt":"2008-11-04T19:25:44Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> Andreas Ericsson wrote:\n> \n>> So long as \"git repack -a\" still creates a mega-pack, I'm fine with\n>> whatever.\n> \n> I don't think it will after pack-objects is taught about .keep files, and\n\nIn that case you're almost certainly breaking something.\n\n> I don't think it will _now_ if all of your packs have .keep files.\n> \n\nIt should, by copying the objects from the .keep-marked packfiles. Otherwise\neither repack or the repack docs are in error.\n\n> 'repack -a' will call pack-objects with either '--unpack=' for\n> each pack file without a .keep file, or with '--unpacked --incremental' if\n> there are no pack files without .keep files.\n> \n> In the first case, the modifications to pack-objects that I propose\n> will prevent objects that exist in local packs with .keep files\n> from being packed into the new pack.\n> \n> In the second case, the --incremental option would have done the same thing.\n> \n> So this inconsistency already existed, but will now be removed in favor of\n> honoring .keep files.\n> \n\nThat means the nifty hack of incrementally and sometimes fully repack the\nodb to speed up cloning over dumb protocols no longer works properly, then.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94913","messageId":"V78jOMhdYwpSlLU-YzsqEHZxJyrvKbXRQbKsuNPZOEtB8E0kZ5Wi7Q@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"49109FD4.30003@op5.se","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-04T19:49:41Z","receivedAt":"2008-11-04T19:49:41Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Andreas Ericsson wrote:\n> Brandon Casey wrote:\n>> From: Brandon Casey \n>>\n>> Objects residing in pack files that have an associated .keep file are not\n>> supposed to be repacked into new pack files, but they are.\n>>\n>> Signed-off-by: Brandon Casey \n>> ---\n>>\n>>\n>> This version replaces the use of 'head -n -1' with a grep, and should\n>> work on\n>> all platforms.\n>>\n> \n> sed 1q is faster, as it stops parsing after the first line (the same as\n> 'head\n> -n 1' does, but in a more portable fashion).\n\nExcept that I wanted all but the _last_ line though.\n\nI didn't think about using sed. Perhaps I could have used something like\n\n sed -n -e '$q' -e 'p'\n\nThe grep works though.\n\n-brandon\n"},{"id":"94914","messageId":"7vmygfqn92.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"V78jOMhdYwpSlLU-YzsqEHZxJyrvKbXRQbKsuNPZOEtB8E0kZ5Wi7Q@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-04T19:55:53Z","receivedAt":"2008-11-04T19:55:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Andreas Ericsson wrote:\n>\n>> sed 1q is faster, as it stops parsing after the first line (the same as\n>> 'head\n>> -n 1' does, but in a more portable fashion).\n\nIf your sed is GNU, and you are doing this for a small file, the startup\ncost of it may dwarf such gain ;-)\n\n> Except that I wanted all but the _last_ line though.\n>\n> I didn't think about using sed. Perhaps I could have used something like\n>\n> sed -n -e '$q' -e 'p'\n\nYou surely meant \"sed -e '$d'\", right?\n\n> The grep works though.\n\nIndeed.\n"},{"id":"94915","messageId":"1ophgqBEnDgFkf-pSe86Z994lgPAXHwUH75NBIVGKfTk8Pz5M-x34Q@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"7vmygfqn92.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-04T20:01:34Z","receivedAt":"2008-11-04T20:01:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n> Brandon Casey writes:\n\n>> sed -n -e '$q' -e 'p'\n> \n> You surely meant \"sed -e '$d'\", right?\n\nYour sed-fu is quite impressive.\n\n-b\n"},{"id":"94918","messageId":"4910AEBB.30404@op5.se","threadId":"16113","inReplyTo":"V78jOMhdYwpSlLU-YzsqEHZxJyrvKbXRQbKsuNPZOEtB8E0kZ5Wi7Q@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-04T20:21:15Z","receivedAt":"2008-11-04T20:21:15Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> Andreas Ericsson wrote:\n>> Brandon Casey wrote:\n>>> From: Brandon Casey \n>>>\n>>> Objects residing in pack files that have an associated .keep file are not\n>>> supposed to be repacked into new pack files, but they are.\n>>>\n>>> Signed-off-by: Brandon Casey \n>>> ---\n>>>\n>>>\n>>> This version replaces the use of 'head -n -1' with a grep, and should\n>>> work on\n>>> all platforms.\n>>>\n>> sed 1q is faster, as it stops parsing after the first line (the same as\n>> 'head\n>> -n 1' does, but in a more portable fashion).\n> \n> Except that I wanted all but the _last_ line though.\n> \n\nAch pooie. That's what I get for trying to review stuff while watching\nold 70's samurai movies. I misread your 'head' command.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94937","messageId":"7vk5bjnj1h.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-04T23:55:06Z","receivedAt":"2008-11-04T23:55:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; queued.\n"},{"id":"95007","messageId":"GV8cY3fn8l5UV5cNoPN8bHchWt9u2tbZ8j_ypkiY-ZLfO1tx9d7ebA@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"LSyxMgVV7zAWRvSezvxyUc6-kz2gK6MRVKonKSf1pAmdqO-jeuMFIw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-05T22:37:16Z","receivedAt":"2008-11-05T22:37:16Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nJunio,\n\nPlease leave this in pu for now, I have some concerns that I haven't\nhad time to write down yet.\n\n-brandon\n\n\nBrandon Casey wrote:\n> From: Brandon Casey \n> \n> By default, pack-objects creates a pack file with every object specified by\n> the user. There are two options which can be used to exclude objects which\n> are accessible by the repository.\n> \n> 1) --incremental\n> This excludes any object which already exists in an accessible pack.\n> \n> 2) --local\n> This excludes any object which exists in a non-local pack.\n> \n> With this patch, both arguments also cause objects which exist in packs\n> marked with a .keep file to be excluded. Only the --local option requires\n> an explicit check for the .keep file. If the user doesn't want the objects\n> in a pack marked with .keep to be exclude, then the .keep file should be\n> removed.\n> \n> Additionally, this fixes the repack bug which allowed porcelain repack to\n> create packs which contained objects already contained in existing packs\n> marked with a .keep file.\n> \n> Signed-off-by: Brandon Casey \n> ---\n> builtin-pack-objects.c | 2 +-\n> t/t7700-repack.sh | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 15b80db..8be9113 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n> \t\t\t\tbreak;\n> \t\t\tif (incremental)\n> \t\t\t\treturn 0;\n> -\t\t\tif (local && !p->pack_local)\n> +\t\t\tif (local && (!p->pack_local || p->pack_keep))\n> \t\t\t\treturn 0;\n> \t\t}\n> \t}\n> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\n> index 27af5ab..5b1cd05 100755\n> --- a/t/t7700-repack.sh\n> +++ b/t/t7700-repack.sh\n> @@ -4,7 +4,7 @@ test_description='git repack works correctly'\n> \n> . ./test-lib.sh\n> \n> -test_expect_failure 'objects in packs marked .keep are not repacked' '\n> +test_expect_success 'objects in packs marked .keep are not repacked' '\n> \techo content1 > file1 &&\n> \techo content2 > file2 &&\n> \tgit add . &&\n"},{"id":"95084","messageId":"HBFmgmcvgPzZ0xq-fRUt98ZOBXGCvwxHGyEwF9bNcgpDgS-t-D3viw@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"GV8cY3fn8l5UV5cNoPN8bHchWt9u2tbZ8j_ypkiY-ZLfO1tx9d7ebA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-06T23:22:41Z","receivedAt":"2008-11-06T23:22:41Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Brandon Casey wrote:\n> Junio,\n> \n> Please leave this in pu for now, I have some concerns that I haven't\n> had time to write down yet.\n\nI've been thinking about pack-objects and repack.\n\nHere's how I think the semantics of repack should be defined:\n\nrepack:\n\n \n -incremental repack which does not repack any object currently packed\n in any accessible pack.\n\n Works (it currently works this way).\n\n <-a>\n -create a new pack containing all objects required by the repository\n including those accessible through alternates, but excluding objects\n in _local_ packs with .keep\n\n Flawed, even with my recent patches.\n\n If there are no local packs (or they all have .keep files), then the\n pack-objects call will use --incremental which will exclude objects\n packed in alt object store, even though -l was not used.\n \n My patches do not differentiate local .keep files from remote .keep files,\n which a user may have no control over.\n\n <-A>\n -Like -a, but local unreferenced objects which were previously packed\n are made to be loose.\n\n Ditto.\n\n <-a -l>\n -Restrict operation to only local objects. Only has any effect with -a|-A.\n -Like -a, but additionally exclude objects in packs accessible through\n alternates.\n\n Works with my recent patches.\n\n <-A -l>\n -Like '-a -l', but loosen unreferenced local packed objects.\n\n Ditto.\n\n\n\nThat set of repack operations needs to map to a combination of pack-objects\noptions:\n\n \n -Create a pack with _all_ specified objects\n\n <--unpacked>\n -Exclude from packing any object already in an accessible pack.\n\n (Ahh, this came from rev-list interface, and rejects objects at an\n earlier stage than --incremental)\n\n <--unpacked=sha1>\n -Like '--unpacked', exclude already packed objects, but treat the objects\n in the pack with specified sha1 as unpacked.\n\n <--incremental>\n -Exclude from packing any object already in an accessible pack,\n regardless of whether it is in a pack specified by --unpacked=\n\n (How is this different from --unpacked, even though the exclusion\n operation is performed at a different stage? See my epiphany above\n about the source of the --unpacked option)\n\n <--unpacked --incremental>\n -seems redundant, is there any functional difference?\n\n <--local>\n -Exclude objects from being packed that are not in the local object store.\n\n\nThe issue is how to provide my described 'repack -a' functionality.\nThere does not seem to be a mapping between the above options and the\nrequired functionality.\n\nI see two solutions, both require introducing a new option to pack-objects.\n 1) allow specifying a set of packs such that if an object resides\n in any of the set, the object will not be included in the produced\n pack.\n\n Benefits:\n -allows keeping pack-objects ignorant of .keep mechanism\n -repack can easily be modified to produce the set of packs to ignore\n\n Drawback:\n -very round-about way just to have functionality to skip packs with\n .keep file\n\n 2) New option telling pack-objects to skip objects in local .keep'd packs\n\n Benefits:\n -easy to implement in pack-objects\n -easy to modify repack\n\n Drawbacks:\n -introduces new concept to pack-objects\n\n\nQuestions aside:\n 1) Are both --incremental and --unpacked still necessary pack-objects options?\n 2) Can --incremental become an alias for --unpacked, and go away?\n\n\npatch(es) will follow.\n\n-brandon\n"},{"id":"95085","messageId":"7v8wrwidi3.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"HBFmgmcvgPzZ0xq-fRUt98ZOBXGCvwxHGyEwF9bNcgpDgS-t-D3viw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-07T00:30:28Z","receivedAt":"2008-11-07T00:30:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> <-a>\n> -create a new pack containing all objects required by the repository\n> including those accessible through alternates, but excluding objects\n> in _local_ packs with .keep\n\nI have a feeling that it is debatable if this \"fattening to dissociate\nfrom alternates\" is what people want.\n\n> <-a -l>\n> -Restrict operation to only local objects. Only has any effect with -a|-A.\n> -Like -a, but additionally exclude objects in packs accessible through\n> alternates.\n\nPresumably you meant \"exclude objects accessible through alternates,\neither in packs or in loose form\"? If so then I think it is a good thing\nto have.\n\nI am not sure if listing the behaviour by combination of flags is a good\nway to start thinking about this. Wouldn't it be more productive to list\nwhat kinds of repacking are needed, and then label them with combination\nof flags? Otherwise you would miss a potentially useful operation that\ncannot be expressed with the current set of flags you have.\n\nI think the useful kinds are only these five:\n\n - scoop loose objects that exist in local repository into a new pack,\n without touching existing packs at all; exclude anything available in\n any existing pack or in alternate repository (either loose or packed);\n\n - pack everything that is needed by the local ref, except the ones that\n are borrowed from alternate repositories (either loose or packed), into\n a single new pack. There are two variants of this: eject what is\n currently packed but unnecessary into loose format when existing local\n packs are replaced with the new pack, or lose them (i.e. -A).\n\n - fatten local repository by packing everything that is needed by the\n local ref into a single new pack, including things that are currently\n borrowed from alternates. There are two variants of this: eject what\n is currently packed but unnecessary into loose format when existing\n local packs are replaced with the new pack, or lose them (i.e. -A).\n"},{"id":"95088","messageId":"BgEXN35P6wpio928OZi_34hs22vqUQxIAIGxR5hR8LqmfPIyw565Mg@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"7v8wrwidi3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-07T01:17:49Z","receivedAt":"2008-11-07T01:17:49Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n> Brandon Casey writes:\n> \n>> <-a>\n>> -create a new pack containing all objects required by the repository\n>> including those accessible through alternates, but excluding objects\n>> in _local_ packs with .keep\n> \n> I have a feeling that it is debatable if this \"fattening to dissociate\n> from alternates\" is what people want.\n\nI'm not sure I understand you here.\n\nAndreas has suggested previously that 'repack -a' should pack everything,\nincluding objects in packs with .keep. Is that what you mean?\n\nWith my current understanding it seems that that would muddy the semantics\nof repack. If -a does not honor packs with .keep, then would it be intuitive\nto expect that adding -l (i.e. exclude alternate packed objects) _would_\nhonor .keep?\n\n>> <-a -l>\n>> -Restrict operation to only local objects. Only has any effect with -a|-A.\n>> -Like -a, but additionally exclude objects in packs accessible through\n>> alternates.\n> \n> Presumably you meant \"exclude objects accessible through alternates,\n> either in packs or in loose form\"? If so then I think it is a good thing\n> to have.\n\nWould that be an enhancement to the current behavior? I don't think I saw\nany mechanism to exclude packing remote loose objects.\n\nThe documentation for pack-objects --local says:\n\n --local\n This flag is similar to --incremental; instead of ignoring all\n packed objects, it only ignores objects that are packed and not\n in the local object store (i.e. borrowed from an alternate).\n\nIt only mentions packed alternate objects.\n\n> \n> I am not sure if listing the behaviour by combination of flags is a good\n> way to start thinking about this. Wouldn't it be more productive to list\n> what kinds of repacking are needed, and then label them with combination\n> of flags? Otherwise you would miss a potentially useful operation that\n> cannot be expressed with the current set of flags you have.\n\nI agree. I made a list of the options because I was trying to understand what\neffect each option had, then I turned it into an email.\n\n> I think the useful kinds are only these five:\n> \n> - scoop loose objects that exist in local repository into a new pack,\n> without touching existing packs at all; exclude anything available in\n> any existing pack or in alternate repository (either loose or packed);\n>\n> - pack everything that is needed by the local ref, except the ones that\n> are borrowed from alternate repositories (either loose or packed), into\n> a single new pack. There are two variants of this: eject what is\n> currently packed but unnecessary into loose format when existing local\n> packs are replaced with the new pack, or lose them (i.e. -A).\n>\n> - fatten local repository by packing everything that is needed by the\n> local ref into a single new pack, including things that are currently\n> borrowed from alternates. There are two variants of this: eject what\n> is currently packed but unnecessary into loose format when existing\n> local packs are replaced with the new pack, or lose them (i.e. -A).\n\nYou didn't say when local .keep should be honored, i.e. objects in local\npacks with .keep should be excluded from repacking. always, never, only\nwith -l, new repack option?\n\n-brandon\n"},{"id":"95089","messageId":"oDevG_2ETMLvy6rfSqvxfmFqABeVqlDUcU6FjP07E5IzqLaopWkQbQ@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"HBFmgmcvgPzZ0xq-fRUt98ZOBXGCvwxHGyEwF9bNcgpDgS-t-D3viw@cipher.nrlssc.navy.mil","subject":"[PATCH 1/4] pack-objects: new option --honor-pack-keep","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-07T01:52:56Z","receivedAt":"2008-11-07T01:52:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This adds a new option to pack-objects which will cause it to ignore an\nobject which appears in a local pack which has a .keep file, even if it\nwas specified for packing.\n\nThis option will be used by the porcelain repack.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis series replaces the previous series starting at\n6ee726bc \"pack-objects: honor '.keep' files\"\n\nIt should be applied on top of\nf34cf12d \"packed_git: convert pack_local flag into a bitfield and add pack_keep\"\n\nI created the series on top of f34cf12d rebased on top of master.\n\nSuggestions for a more appropriate name for --honor-pack-keep are very welcome.\n\n-brandon\n\n\n Documentation/git-pack-objects.txt | 5 +++++\n builtin-pack-objects.c | 7 +++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8c354bd..f9fac2c 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -109,6 +109,11 @@ base-name::\n \tThe default is unlimited, unless the config variable\n \t`pack.packSizeLimit` is set.\n \n+--honor-pack-keep::\n+\tThis flag causes an object already in a local pack that\n+\thas a .keep file to be ignored, even if it appears in the\n+\tstandard input.\n+\n --incremental::\n \tThis flag causes an object already in a pack ignored\n \teven if it appears in the standard input.\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 15b80db..ddec341 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -71,6 +71,7 @@ static int reuse_delta = 1, reuse_object = 1;\n static int keep_unreachable, unpack_unreachable, include_tag;\n static int local;\n static int incremental;\n+static int ignore_packed_keep;\n static int allow_ofs_delta;\n static const char *base_name;\n static int progress = 1;\n@@ -703,6 +704,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t\treturn 0;\n \t\t\tif (local && !p->pack_local)\n \t\t\t\treturn 0;\n+\t\t\tif (ignore_packed_keep && p->pack_local && p->pack_keep)\n+\t\t\t\treturn 0;\n \t\t}\n \t}\n \n@@ -2048,6 +2051,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tincremental = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--honor-pack-keep\", arg)) {\n+\t\t\tignore_packed_keep = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--compression=\")) {\n \t\t\tchar *end;\n \t\t\tint level = strtoul(arg+14, &end, 0);\n-- \n1.6.0.3.552.g12334\n"},{"id":"95090","messageId":"P5afFyDEMjF9ynd9dfSfGeRo_GFY_K6ZAn2nIYo8_xGgSl4LapbSOA@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"oDevG_2ETMLvy6rfSqvxfmFqABeVqlDUcU6FjP07E5IzqLaopWkQbQ@cipher.nrlssc.navy.mil","subject":"[PATCH 2/4] repack: don't repack local objects in packs with .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-07T01:54:08Z","receivedAt":"2008-11-07T01:54:08Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"If the user created a .keep file for a local pack, then it can be inferred\nthat the user does not want those objects repacked.\n\nThis fixes the repack bug tested by t7700.\n\nSigned-off-by: Brandon Casey \n---\n git-repack.sh | 2 +-\n t/t7700-repack.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex d39eb6c..8bb2201 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -83,7 +83,7 @@ case \",$all_into_one,\" in\n esac\n \n args=\"$args $local $quiet $no_reuse$extra\"\n-names=$(git pack-objects --non-empty --all --reflog $args file1 &&\n \techo content2 > file2 &&\n \tgit add . &&\n-- \n1.6.0.3.552.g12334\n"},{"id":"95091","messageId":"-NKrpBlnD0U49OczAyUd7HiIBqhMblkOkU4lOc2bc9qfTrGNrwpLTg@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"P5afFyDEMjF9ynd9dfSfGeRo_GFY_K6ZAn2nIYo8_xGgSl4LapbSOA@cipher.nrlssc.navy.mil","subject":"[PATCH 3/4] repack: do not fall back to incremental repacking with [-a|-A]","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-07T01:55:20Z","receivedAt":"2008-11-07T01:55:20Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"When repack is called with either the -a or -A option, the user has\nrequested to repack all objects including those referenced by the\nalternates mechanism. Currently, if there are no local packs without\n.keep files, then repack will call pack-objects with the\n'--unpacked --incremental' options which causes it to exclude alternate\npacked objects. So, remove this fallback.\n\nSigned-off-by: Brandon Casey \n---\n git-repack.sh | 11 ++++-------\n 1 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 8bb2201..4d313d1 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -71,13 +71,10 @@ case \",$all_into_one,\" in\n \t\t\t\texisting=\"$existing $e\"\n \t\t\tfi\n \t\tdone\n-\tfi\n-\tif test -z \"$args\"\n-\tthen\n-\t\targs='--unpacked --incremental'\n-\telif test -n \"$unpack_unreachable\"\n-\tthen\n-\t\targs=\"$args $unpack_unreachable\"\n+\t\tif test -n \"$args\" -a -n \"$unpack_unreachable\"\n+\t\tthen\n+\t\t\targs=\"$args $unpack_unreachable\"\n+\t\tfi\n \tfi\n \t;;\n esac\n-- \n1.6.0.3.552.g12334\n"},{"id":"95092","messageId":"3ViIDELaxoBUGtbz27ICr9Qm3avB1LFH5UHtBEWeFrLNfT2qnYqDyA@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"-NKrpBlnD0U49OczAyUd7HiIBqhMblkOkU4lOc2bc9qfTrGNrwpLTg@cipher.nrlssc.navy.mil","subject":"[PATCH 4/4] builtin-gc.c: use new pack_keep bitfield to detect .keep file existence","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-07T01:56:39Z","receivedAt":"2008-11-07T01:56:39Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Signed-off-by: Brandon Casey \n---\n\n\nI think the existing code was broken. Looks like it would only skip\ncounting a pack if the pack disappeared between the prepare_packed_git()\nand the access() call. It never used the path it create with the .keep\nextension.\n\n-brandon\n\n\n builtin-gc.c | 12 +-----------\n 1 files changed, 1 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 7af65bb..781df60 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -131,19 +131,9 @@ static int too_many_packs(void)\n \n \tprepare_packed_git();\n \tfor (cnt = 0, p = packed_git; p; p = p->next) {\n-\t\tchar path[PATH_MAX];\n-\t\tsize_t len;\n-\t\tint keep;\n-\n \t\tif (!p->pack_local)\n \t\t\tcontinue;\n-\t\tlen = strlen(p->pack_name);\n-\t\tif (PATH_MAX <= len + 1)\n-\t\t\tcontinue; /* oops, give up */\n-\t\tmemcpy(path, p->pack_name, len-5);\n-\t\tmemcpy(path + len - 5, \".keep\", 6);\n-\t\tkeep = access(p->pack_name, F_OK) && (errno == ENOENT);\n-\t\tif (keep)\n+\t\tif (p->pack_keep)\n \t\t\tcontinue;\n \t\t/*\n \t\t * Perhaps check the size of the pack and count only\n-- \n1.6.0.3.552.g12334\n"},{"id":"95105","messageId":"4913F859.8070500@op5.se","threadId":"16113","inReplyTo":"BgEXN35P6wpio928OZi_34hs22vqUQxIAIGxR5hR8LqmfPIyw565Mg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-07T08:12:09Z","receivedAt":"2008-11-07T08:12:09Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> Junio C Hamano wrote:\n>> Brandon Casey writes:\n>>\n>>> <-a>\n>>> -create a new pack containing all objects required by the repository\n>>> including those accessible through alternates, but excluding objects\n>>> in _local_ packs with .keep\n>> I have a feeling that it is debatable if this \"fattening to dissociate\n>> from alternates\" is what people want.\n> \n> I'm not sure I understand you here.\n> \n> Andreas has suggested previously that 'repack -a' should pack everything,\n> including objects in packs with .keep. Is that what you mean?\n> \n> With my current understanding it seems that that would muddy the semantics\n> of repack. If -a does not honor packs with .keep, then would it be intuitive\n> to expect that adding -l (i.e. exclude alternate packed objects) _would_\n> honor .keep?\n> \n\nOnly -d should honor .keep, imo. .keep-files is nothing about \"don't copy\nobjects from this file\" and all about \"never delete this file\".\n\nThe only muddying comes from you, who decided that .keep-files should\nhave impact on anything else than deleting the protected pack. Before that,\n.keep files had a clear semantic, and repack's documentation was correct.\n\nHow do you explain .keep-files now? \"protects pack-files that will forever\nbe used\"? Then why the hell is it called \".keep\" instead of \"eternal\"?\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"95106","messageId":"4913F8A1.2050304@op5.se","threadId":"16113","inReplyTo":"oDevG_2ETMLvy6rfSqvxfmFqABeVqlDUcU6FjP07E5IzqLaopWkQbQ@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 1/4] pack-objects: new option --honor-pack-keep","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-07T08:13:21Z","receivedAt":"2008-11-07T08:13:21Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> This adds a new option to pack-objects which will cause it to ignore an\n> object which appears in a local pack which has a .keep file, even if it\n> was specified for packing.\n> \n> This option will be used by the porcelain repack.\n> \n> Signed-off-by: Brandon Casey \n> ---\n> \n> \n> This series replaces the previous series starting at\n> 6ee726bc \"pack-objects: honor '.keep' files\"\n> \n> It should be applied on top of\n> f34cf12d \"packed_git: convert pack_local flag into a bitfield and add pack_keep\"\n> \n> I created the series on top of f34cf12d rebased on top of master.\n> \n> Suggestions for a more appropriate name for --honor-pack-keep are very welcome.\n> \n> -brandon\n> \n> \n> Documentation/git-pack-objects.txt | 5 +++++\n> builtin-pack-objects.c | 7 +++++++\n> 2 files changed, 12 insertions(+), 0 deletions(-)\n> \n> diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\n> index 8c354bd..f9fac2c 100644\n> --- a/Documentation/git-pack-objects.txt\n> +++ b/Documentation/git-pack-objects.txt\n> @@ -109,6 +109,11 @@ base-name::\n> \tThe default is unlimited, unless the config variable\n> \t`pack.packSizeLimit` is set.\n> \n> +--honor-pack-keep::\n> +\tThis flag causes an object already in a local pack that\n> +\thas a .keep file to be ignored, even if it appears in the\n> +\tstandard input.\n> +\n\nKeep-files are *always* honored. Make this option \"--ignore-kept\" or\nsomething instead, otherwise people will see the synopsis and think\nthey need to always pass it to not remove .keep-protected packs,\nwhich is stupid.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"95107","messageId":"4913F8DE.1050308@op5.se","threadId":"16113","inReplyTo":"P5afFyDEMjF9ynd9dfSfGeRo_GFY_K6ZAn2nIYo8_xGgSl4LapbSOA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/4] repack: don't repack local objects in packs with .keep file","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-07T08:14:22Z","receivedAt":"2008-11-07T08:14:22Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> If the user created a .keep file for a local pack, then it can be inferred\n> that the user does not want those objects repacked.\n> \n\nI disagree. It can be inferred that the user doesn't want those packfiles\n*removed*.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"95138","messageId":"20081107192506.GB2932@spearce.org","threadId":"16113","inReplyTo":"4913F859.8070500@op5.se","subject":"Re: [PATCH v2 3/3] pack-objects: honor '.keep' files","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-07T19:25:06Z","receivedAt":"2008-11-07T19:25:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Andreas Ericsson wrote:\n>\n> Only -d should honor .keep, imo. .keep-files is nothing about \"don't copy\n> objects from this file\" and all about \"never delete this file\".\n>\n> The only muddying comes from you, who decided that .keep-files should\n> have impact on anything else than deleting the protected pack. Before that,\n> .keep files had a clear semantic, and repack's documentation was correct.\n>\n> How do you explain .keep-files now? \"protects pack-files that will forever\n> be used\"? Then why the hell is it called \".keep\" instead of \"eternal\"?\n\nThe _intent_ behind .keep files has always been to avoid copying\nthe objects into a new pack during git-repack, because the objects\nare held in a pack that cannot be deleted.\n\nSadly the implementation has never quite worked right, which is\nwhat led to this patch series I think.\n\n-- \nShawn.\n"},{"id":"95303","messageId":"2390436.1226296705080.JavaMail.teamon@b307.teamon.com","threadId":"16113","inReplyTo":"7v8wrwidi3.fsf@gitster.siamese.dyndns.org","subject":"recognize loose local objects during repack","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-10T05:59:55Z","receivedAt":"2008-11-10T05:59:55Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nThis was developed on top of the previous repack/pack-objects series.\n\n\nJunio wrote:\n> Presumably you meant \"exclude objects accessible through alternates,\n> either in packs or in loose form\"? If so then I think it is a good thing\n> to have.\n\n\nThis patch set looks like what is necessary to exclude loose objects accessible\nthrough alternates from packing.\n\n\n> I think the useful kinds are only these five:\n> \n> - scoop loose objects that exist in local repository into a new pack,\n> without touching existing packs at all; exclude anything available in\n> any existing pack or in alternate repository (either loose or packed);\n\n repack -l\n\n> - pack everything that is needed by the local ref, except the ones that\n> are borrowed from alternate repositories (either loose or packed), into\n> a single new pack. There are two variants of this: eject what is\n> currently packed but unnecessary into loose format when existing local\n> packs are replaced with the new pack, or lose them (i.e. -A).\n\n repack -a -l\n repack -A -l\n\n> - fatten local repository by packing everything that is needed by the\n> local ref into a single new pack, including things that are currently\n> borrowed from alternates. There are two variants of this: eject what\n> is currently packed but unnecessary into loose format when existing\n> local packs are replaced with the new pack, or lose them (i.e. -A).\n\n repack -a\n repack -A\n\n-brandon\n"},{"id":"95304","messageId":"11419048.1226296708027.JavaMail.teamon@b307.teamon.com","threadId":"16113","inReplyTo":"1226296798-31522-1-git-send-email-foo@foo.com","subject":"[PATCH 1/3] t7700: demonstrate mishandling of loose objects in an alternate ODB","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-10T05:59:56Z","receivedAt":"2008-11-10T05:59:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nLoose objects residing in an alternate object database should not be packed\nwhen the -l option to repack is used.\n\nSigned-off-by: Brandon Casey \n---\n t/t7700-repack.sh | 19 +++++++++++++++++++\n 1 files changed, 19 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 356afe3..43c9cf9 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -34,5 +34,24 @@ test_expect_success 'objects in packs marked .keep are not repacked' '\n \ttest -z \"$found_duplicate_object\"\n '\n \n+test_expect_failure 'loose objects in alternate ODB are not repacked' '\n+\tmkdir alt_objects &&\n+\techo `pwd`/alt_objects > .git/objects/info/alternates &&\n+\techo content3 > file3 &&\n+\tobjsha1=$(GIT_OBJECT_DIRECTORY=alt_objects git hash-object -w file3) &&\n+\tgit add file3 &&\n+\tgit commit -m commit_file3 &&\n+\tgit repack -a -d -l &&\n+\tgit prune-packed &&\n+\tfor p in .git/objects/pack/*.idx; do\n+\t\tif git verify-pack -v $p | egrep \"^$objsha1\"; then\n+\t\t\tfound_duplicate_object=1\n+\t\t\techo \"DUPLICATE OBJECT FOUND\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone &&\n+\ttest -z \"$found_duplicate_object\"\n+'\n+\n test_done\n \n-- \n1.6.0.2.588.g3102\n"},{"id":"95306","messageId":"5374142.1226296710939.JavaMail.teamon@b307.teamon.com","threadId":"16113","inReplyTo":"1226296798-31522-2-git-send-email-foo@foo.com","subject":"[PATCH 2/3] sha1_file.c: split has_loose_object() into local and non-local counterparts","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-10T05:59:57Z","receivedAt":"2008-11-10T05:59:57Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\n\nSigned-off-by: Brandon Casey \n---\n cache.h | 1 +\n sha1_file.c | 19 +++++++++++++------\n 2 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 37ab457..1ec90f1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -578,6 +578,7 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);\n \n extern int has_sha1_pack(const unsigned char *sha1, const char **ignore);\n extern int has_sha1_file(const unsigned char *sha1);\n+extern int has_loose_object_nonlocal(const unsigned char *sha1);\n \n extern int has_pack_file(const unsigned char *sha1);\n extern int has_pack_index(const unsigned char *sha1);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex f2b25bd..4912205 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -423,23 +423,30 @@ void prepare_alt_odb(void)\n \tread_info_alternates(get_object_directory(), 0);\n }\n \n-static int has_loose_object(const unsigned char *sha1)\n+static int has_loose_object_local(const unsigned char *sha1)\n {\n \tchar *name = sha1_file_name(sha1);\n-\tstruct alternate_object_database *alt;\n+\treturn !access(name, F_OK);\n+}\n \n-\tif (!access(name, F_OK))\n-\t\treturn 1;\n+int has_loose_object_nonlocal(const unsigned char *sha1)\n+{\n+\tstruct alternate_object_database *alt;\n \tprepare_alt_odb();\n \tfor (alt = alt_odb_list; alt; alt = alt->next) {\n-\t\tname = alt->name;\n-\t\tfill_sha1_path(name, sha1);\n+\t\tfill_sha1_path(alt->name, sha1);\n \t\tif (!access(alt->base, F_OK))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n }\n \n+static int has_loose_object(const unsigned char *sha1)\n+{\n+\treturn has_loose_object_local(sha1) ||\n+\t has_loose_object_nonlocal(sha1);\n+}\n+\n static unsigned int pack_used_ctr;\n static unsigned int pack_mmap_calls;\n static unsigned int peak_pack_open_windows;\n-- \n1.6.0.2.588.g3102\n"},{"id":"95305","messageId":"11455241.1226296713899.JavaMail.teamon@b307.teamon.com","threadId":"16113","inReplyTo":"1226296798-31522-3-git-send-email-foo@foo.com","subject":"[PATCH 3/3] pack-objects: extend --local to mean ignore non-local loose objects too","fromName":"","fromEmail":"drafnel@gmail.com","sentAt":"2008-11-10T05:59:58Z","receivedAt":"2008-11-10T05:59:58Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nWith this patch, --local means pack only local objects that are not already\npacked.\n\nAdditionally, this fixes t7700 testing whether loose objects in an alternate\nobject database are repacked.\n\nSigned-off-by: Brandon Casey \n---\n Documentation/git-pack-objects.txt | 2 +-\n builtin-pack-objects.c | 3 +++\n t/t7700-repack.sh | 2 +-\n 3 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex f9fac2c..7d4c1a7 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -121,7 +121,7 @@ base-name::\n --local::\n \tThis flag is similar to `--incremental`; instead of\n \tignoring all packed objects, it only ignores objects\n-\tthat are packed and not in the local object store\n+\tthat are packed and/or not in the local object store\n \t(i.e. borrowed from an alternate).\n \n --non-empty::\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex ddec341..69f351a 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -691,6 +691,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\treturn 0;\n \t}\n \n+\tif (!exclude && local && has_loose_object_nonlocal(sha1))\n+\t\treturn 0;\n+\n \tfor (p = packed_git; p; p = p->next) {\n \t\toff_t offset = find_pack_entry_one(sha1, p);\n \t\tif (offset) {\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 43c9cf9..960bff4 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -34,7 +34,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' '\n \ttest -z \"$found_duplicate_object\"\n '\n \n-test_expect_failure 'loose objects in alternate ODB are not repacked' '\n+test_expect_success 'loose objects in alternate ODB are not repacked' '\n \tmkdir alt_objects &&\n \techo `pwd`/alt_objects > .git/objects/info/alternates &&\n \techo content3 > file3 &&\n-- \n1.6.0.2.588.g3102\n"},{"id":"95387","messageId":"7vtzaf47ld.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"2390436.1226296705080.JavaMail.teamon@b307.teamon.com","subject":"Re: recognize loose local objects during repack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-10T21:03:10Z","receivedAt":"2008-11-10T21:03:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"drafnel@gmail.com writes:\n\n> This was developed on top of the previous repack/pack-objects series.\n\nThanks. Looked alright from a cursory reading.\n\nBy the way, I've been meaning to suggest that we should straighten out the\nsemantics of \"unpacked\" vs \"incremental\".\n\nWhat the latter means is quite clear. We are creating a new packfile to\nbundle loose ones into one, and after the new packfile is installed we can\nremove the loose objects.\n\nBut what --unpacked means often confuses people, primarily because it is a\nperformance heuristics that makes certain assumptions on how the objects\nare packed.\n\nNamely, \"unpacked\" is about discovery process of objects to be packed.\nWithout the option, we enumerate all objects that are reachable from the\ncommits in the given range, excluding the trees and blobs that should\nexist in commits that are excluded (e.g, if you say \"--objects A..B\", we\nexclude trees and blobs referenced by commit A).\n\nWith the option, we also omit commits that are packed. What's funny is\nthat their trees and blobs are omitted, even if they are loose ;-)\n\nThis is typically not an issue, because you do not say \"pack only this\ncommit object, without its trees or blobs\" when repacking, and because you\nmust have all the trees and blobs necessary for a commit available when\nyou pack a commit; for these reasons, the trees and blobs are typically\npacked together with the commit. It is not an issue that the rev-list\nwith --unpacked option does not list loose trees and blobs that belong to\na packed commit for this reason.\n\nYou could however arrange so that a commit itself is packed but some of\nthe tree and blob objects it refers to are loose, in which case these\nloose objects may not ever get repacked incrementally.\n\nIn an empty directory, try this:\n\n git init &&\n echo 0 >file && git add file && git commit -m initial &&\n\n P=$(git rev-list HEAD | git pack-objects pack) &&\n mv pack-$P.* .git/objects/pack/ &&\n git prune && git count-objects -v\n\n git repack && git count-objects -v\n\nIt packs only the commit object (and prunes it), leaving a tree and a blob\nloose. The repack won't find anything to pack.\n\nThis is not so bad in the sense that it will never corrupt your\nrepository, but it is confusing. Admittedly, not peeking into a commit\nthat is packed is a reasonable good heuristics for performance reasons.\n\nInterestingly enough, the object listing machinery do traverse into\nparents of packed commits when --unpacked is given. So if you pack a\ncommit and arrange to keep its parents unpacked, they are subject to the\nincremental repacking. In other words, the performance heuristics may not\nbe buying us very much --- we are traversing the history down to the root\ncommits regardless.\n"},{"id":"95530","messageId":"20081112080909.GA29176@coredump.intra.peff.net","threadId":"16113","inReplyTo":"muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T08:09:09Z","receivedAt":"2008-11-12T08:09:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:\n\n> This version replaces the use of 'head -n -1' with a grep, and should work on\n> all platforms.\n\nHmm. I'm not sure what happened, but the version in 'next' has \"head -n\n-1\" in it.\n\n-Peff\n"},{"id":"95588","messageId":"7vzlk4q38q.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"20081112080909.GA29176@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T17:10:45Z","receivedAt":"2008-11-12T17:10:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:\n>\n>> This version replaces the use of 'head -n -1' with a grep, and should work on\n>> all platforms.\n>\n> Hmm. I'm not sure what happened, but the version in 'next' has \"head -n\n> -1\" in it.\n\nYou mean this one?\n\ncommit 31d92611e45d1286b805e362dbc451936af24121\nAuthor: Brandon Casey \nAuthorDate: Sat Nov 1 22:35:19 2008 -0500\nCommit: Junio C Hamano \nCommitDate: Sun Nov 2 15:58:09 2008 -0800\n\n t7700: demonstrate mishandling of objects in packs with a .keep file\n"},{"id":"95589","messageId":"NRnq_JA3Ngz2v6EUhSwtVu5zewgvJgLsW85zZZqzrYE@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"20081112080909.GA29176@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:30:26Z","receivedAt":"2008-11-12T17:30:26Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n> On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:\n> \n>> This version replaces the use of 'head -n -1' with a grep, and should work on\n>> all platforms.\n> \n> Hmm. I'm not sure what happened, but the version in 'next' has \"head -n\n> -1\" in it.\n\nWell, there were so many revisions, I probably should have re-rolled the\nwhole series. I wasn't sure this was going to go in as is, based on the\n[Dropped] message in the \"What's Cooking\" email and Junio's last email about\nreconciling --unpacked and --incremental. I have been working through\nthe --unpacked code path, but I'm not to the point where I can suggest\na change there.\n\nBut, I think it's worse than just the wrong t7700.\n\nThese two:\n\n packed_git: convert pack_local flag into generic bit mask\n packed_git: add new PACK_KEEP flag and haspackkeep() access macro\n\nshould have been replaced by:\n\n packed_git: convert pack_local flag into a bitfield and add pack_keep\n\nwhich uses a struct bitfield rather than a bitmask.\n\nAnd then this\n\n pack-objects: honor '.keep' files\n\nwas replaced by this\n\n pack-objects: new option --honor-pack-keep\n repack: don't repack local objects in packs with .keep file\n\nif that's the way we want to go. I'm not partial to the phrase honor-pack-keep,\nbut I don't think ignore-pack-keep is appropriate, and it's the best I've come\nup with.\n\nSo,\n\n 31d92611e45d1286b805e362dbc451936af24121\n 7c335327be664751fa4c04e81b2fe3bfedceaada\n 77b5a5478a77cc04b674891b542db1ba1a1bf4f7\n 13e7f5d2f1da42619bd545590d0044b30d00ce4b\n\nshould be reverted, and replaced by the series to follow.\n\n-brandon\n"},{"id":"95606","messageId":"WFmMnhNsYRw3IUD8hZk59MeWg__Hu_J0586FDlwkv7_Bb80SFy_3xVB_YmBMzdF0byAGksl5Rnk@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"NRnq_JA3Ngz2v6EUhSwtVu5zewgvJgLsW85zZZqzrYE@cipher.nrlssc.navy.mil","subject":"repack and .keep series","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:01Z","receivedAt":"2008-11-12T17:59:01Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Here is an updated series built on top of master.\n\nWhat follows is the 6 patches that were sent, but are\nnot in next.\n\n-brandon\n"},{"id":"95603","messageId":"WFmMnhNsYRw3IUD8hZk59JmvLQlpuqo-hsW2L51xXUJ0ETOpVPtrZvZzBrrquDq1PlXDGK5BhsQ@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59MeWg__Hu_J0586FDlwkv7_Bb80SFy_3xVB_YmBMzdF0byAGksl5Rnk@cipher.nrlssc.navy.mil","subject":"[PATCH 1/6] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:02Z","receivedAt":"2008-11-12T17:59:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nObjects residing in pack files that have an associated .keep file are not\nsupposed to be repacked into new pack files, but they are.\n\nSigned-off-by: Brandon Casey \n---\n t/t7700-repack.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7700-repack.sh\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nnew file mode 100755\nindex 0000000..7aaff0b\n--- /dev/null\n+++ b/t/t7700-repack.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git repack works correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_failure 'objects in packs marked .keep are not repacked' '\n+\techo content1 > file1 &&\n+\techo content2 > file2 &&\n+\tgit add . &&\n+\tgit commit -m initial_commit &&\n+\t# Create two packs\n+\t# The first pack will contain all of the objects except one\n+\tgit rev-list --objects --all | grep -v file2 |\n+\t\tgit pack-objects pack > /dev/null &&\n+\t# The second pack will contain the excluded object\n+\tpacksha1=$(git rev-list --objects --all | grep file2 |\n+\t\tgit pack-objects pack) &&\n+\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n+\t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n+\tmv pack-* .git/objects/pack/ &&\n+\tgit repack -A -d -l &&\n+\tgit prune-packed &&\n+\tfor p in .git/objects/pack/*.idx; do\n+\t\tidx=$(basename $p)\n+\t\ttest \"pack-$packsha1.idx\" = \"$idx\" && continue\n+\t\tif git verify-pack -v $p | egrep \"^$objsha1\"; then\n+\t\t\tfound_duplicate_object=1\n+\t\t\techo \"DUPLICATE OBJECT FOUND\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone &&\n+\ttest -z \"$found_duplicate_object\"\n+'\n+\n+test_done\n+\n-- \n1.6.0.3.552.g12334\n"},{"id":"95605","messageId":"WFmMnhNsYRw3IUD8hZk59Bod6u3kdkWtabbRw1_B1XS8U8eP3drqEMEsFa_-Q_Qu-xrbOsLcdzo@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59JmvLQlpuqo-hsW2L51xXUJ0ETOpVPtrZvZzBrrquDq1PlXDGK5BhsQ@cipher.nrlssc.navy.mil","subject":"[PATCH 2/6] packed_git: convert pack_local flag into a bitfield and add pack_keep","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:03Z","receivedAt":"2008-11-12T17:59:03Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\npack_keep will be set when a pack file has an associated .keep file.\n\nSigned-off-by: Brandon Casey \n---\n cache.h | 3 ++-\n sha1_file.c | 5 +++++\n 2 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex eda7028..37ab457 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -686,7 +686,8 @@ extern struct packed_git {\n \tint index_version;\n \ttime_t mtime;\n \tint pack_fd;\n-\tint pack_local;\n+\tunsigned pack_local:1,\n+\t\t pack_keep:1;\n \tunsigned char sha1[20];\n \t/* something like \".git/objects/pack/xxxxx.pack\" */\n \tchar pack_name[FLEX_ARRAY]; /* more */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex ab2b520..f2b25bd 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -841,6 +841,11 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \t\treturn NULL;\n \t}\n \tmemcpy(p->pack_name, path, path_len);\n+\n+\tstrcpy(p->pack_name + path_len, \".keep\");\n+\tif (!access(p->pack_name, F_OK))\n+\t\tp->pack_keep = 1;\n+\n \tstrcpy(p->pack_name + path_len, \".pack\");\n \tif (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {\n \t\tfree(p);\n-- \n1.6.0.3.552.g12334\n"},{"id":"95604","messageId":"WFmMnhNsYRw3IUD8hZk59Ll3-y-Spr-gg7enTTiJoZ2xRB8K4w13wAlZBhI9MXJqCLarfq3OODE@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59Bod6u3kdkWtabbRw1_B1XS8U8eP3drqEMEsFa_-Q_Qu-xrbOsLcdzo@cipher.nrlssc.navy.mil","subject":"[PATCH 3/6] pack-objects: new option --honor-pack-keep","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:04Z","receivedAt":"2008-11-12T17:59:04Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This adds a new option to pack-objects which will cause it to ignore an\nobject which appears in a local pack which has a .keep file, even if it\nwas specified for packing.\n\nThis option will be used by the porcelain repack.\n\nSigned-off-by: Brandon Casey \n---\n Documentation/git-pack-objects.txt | 5 +++++\n builtin-pack-objects.c | 7 +++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8c354bd..f9fac2c 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -109,6 +109,11 @@ base-name::\n \tThe default is unlimited, unless the config variable\n \t`pack.packSizeLimit` is set.\n \n+--honor-pack-keep::\n+\tThis flag causes an object already in a local pack that\n+\thas a .keep file to be ignored, even if it appears in the\n+\tstandard input.\n+\n --incremental::\n \tThis flag causes an object already in a pack ignored\n \teven if it appears in the standard input.\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 15b80db..ddec341 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -71,6 +71,7 @@ static int reuse_delta = 1, reuse_object = 1;\n static int keep_unreachable, unpack_unreachable, include_tag;\n static int local;\n static int incremental;\n+static int ignore_packed_keep;\n static int allow_ofs_delta;\n static const char *base_name;\n static int progress = 1;\n@@ -703,6 +704,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t\treturn 0;\n \t\t\tif (local && !p->pack_local)\n \t\t\t\treturn 0;\n+\t\t\tif (ignore_packed_keep && p->pack_local && p->pack_keep)\n+\t\t\t\treturn 0;\n \t\t}\n \t}\n \n@@ -2048,6 +2051,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tincremental = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--honor-pack-keep\", arg)) {\n+\t\t\tignore_packed_keep = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--compression=\")) {\n \t\t\tchar *end;\n \t\t\tint level = strtoul(arg+14, &end, 0);\n-- \n1.6.0.3.552.g12334\n"},{"id":"95600","messageId":"WFmMnhNsYRw3IUD8hZk59PVSBNnWW0Ot3Am0gtsYesn_-APZGMYcRjOXaJqcBZo4pwvAKF-s_s0@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59Ll3-y-Spr-gg7enTTiJoZ2xRB8K4w13wAlZBhI9MXJqCLarfq3OODE@cipher.nrlssc.navy.mil","subject":"[PATCH 4/6] repack: don't repack local objects in packs with .keep file","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:05Z","receivedAt":"2008-11-12T17:59:05Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"If the user created a .keep file for a local pack, then it can be inferred\nthat the user does not want those objects repacked.\n\nThis fixes the repack bug tested by t7700.\n\nSigned-off-by: Brandon Casey \n---\n git-repack.sh | 2 +-\n t/t7700-repack.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex d39eb6c..8bb2201 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -83,7 +83,7 @@ case \",$all_into_one,\" in\n esac\n \n args=\"$args $local $quiet $no_reuse$extra\"\n-names=$(git pack-objects --non-empty --all --reflog $args file1 &&\n \techo content2 > file2 &&\n \tgit add . &&\n-- \n1.6.0.3.552.g12334\n"},{"id":"95601","messageId":"WFmMnhNsYRw3IUD8hZk59At-5jc1rZU5GaB35sb1epRZ6zwd7IMw9V94SQyEliIEyriM4J5j3AM@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59PVSBNnWW0Ot3Am0gtsYesn_-APZGMYcRjOXaJqcBZo4pwvAKF-s_s0@cipher.nrlssc.navy.mil","subject":"[PATCH 5/6] repack: do not fall back to incremental repacking with [-a|-A]","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:06Z","receivedAt":"2008-11-12T17:59:06Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"When repack is called with either the -a or -A option, the user has\nrequested to repack all objects including those referenced by the\nalternates mechanism. Currently, if there are no local packs without\n.keep files, then repack will call pack-objects with the\n'--unpacked --incremental' options which causes it to exclude alternate\npacked objects. So, remove this fallback.\n\nSigned-off-by: Brandon Casey \n---\n git-repack.sh | 11 ++++-------\n 1 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 8bb2201..4d313d1 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -71,13 +71,10 @@ case \",$all_into_one,\" in\n \t\t\t\texisting=\"$existing $e\"\n \t\t\tfi\n \t\tdone\n-\tfi\n-\tif test -z \"$args\"\n-\tthen\n-\t\targs='--unpacked --incremental'\n-\telif test -n \"$unpack_unreachable\"\n-\tthen\n-\t\targs=\"$args $unpack_unreachable\"\n+\t\tif test -n \"$args\" -a -n \"$unpack_unreachable\"\n+\t\tthen\n+\t\t\targs=\"$args $unpack_unreachable\"\n+\t\tfi\n \tfi\n \t;;\n esac\n-- \n1.6.0.3.552.g12334\n"},{"id":"95602","messageId":"WFmMnhNsYRw3IUD8hZk59C3ZoEDrEYCSt5_rIQMMTYQv4ZvQe32lQz-HznOAfvnsLxMiqGz6XW4@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59At-5jc1rZU5GaB35sb1epRZ6zwd7IMw9V94SQyEliIEyriM4J5j3AM@cipher.nrlssc.navy.mil","subject":"[PATCH 6/6] builtin-gc.c: use new pack_keep bitfield to detect .keep file existence","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-12T17:59:07Z","receivedAt":"2008-11-12T17:59:07Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Signed-off-by: Brandon Casey \n---\n builtin-gc.c | 12 +-----------\n 1 files changed, 1 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 7af65bb..781df60 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -131,19 +131,9 @@ static int too_many_packs(void)\n \n \tprepare_packed_git();\n \tfor (cnt = 0, p = packed_git; p; p = p->next) {\n-\t\tchar path[PATH_MAX];\n-\t\tsize_t len;\n-\t\tint keep;\n-\n \t\tif (!p->pack_local)\n \t\t\tcontinue;\n-\t\tlen = strlen(p->pack_name);\n-\t\tif (PATH_MAX <= len + 1)\n-\t\t\tcontinue; /* oops, give up */\n-\t\tmemcpy(path, p->pack_name, len-5);\n-\t\tmemcpy(path + len - 5, \".keep\", 6);\n-\t\tkeep = access(p->pack_name, F_OK) && (errno == ENOENT);\n-\t\tif (keep)\n+\t\tif (p->pack_keep)\n \t\t\tcontinue;\n \t\t/*\n \t\t * Perhaps check the size of the pack and count only\n-- \n1.6.0.3.552.g12334\n"},{"id":"95608","messageId":"7v7i78q0go.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"NRnq_JA3Ngz2v6EUhSwtVu5zewgvJgLsW85zZZqzrYE@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T18:10:47Z","receivedAt":"2008-11-12T18:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Jeff King wrote:\n>> On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:\n>> \n>>> This version replaces the use of 'head -n -1' with a grep, and should work on\n>>> all platforms.\n>> \n>> Hmm. I'm not sure what happened, but the version in 'next' has \"head -n\n>> -1\" in it.\n>\n> Well, there were so many revisions, I probably should have re-rolled the\n> whole series....\n> ...\n> if that's the way we want to go. I'm not partial to the phrase honor-pack-keep,\n> but I don't think ignore-pack-keep is appropriate, and it's the best I've come\n> up with.\n\nOk, care to send in a series in incremental updates format, please?\n"},{"id":"95611","messageId":"7vy6zoolha.fsf@gitster.siamese.dyndns.org","threadId":"16113","inReplyTo":"7v7i78q0go.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-12T18:19:45Z","receivedAt":"2008-11-12T18:19:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Brandon Casey writes:\n>\n>> Jeff King wrote:\n>>> On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:\n>>> \n>>>> This version replaces the use of 'head -n -1' with a grep, and should work on\n>>>> all platforms.\n>>> \n>>> Hmm. I'm not sure what happened, but the version in 'next' has \"head -n\n>>> -1\" in it.\n>>\n>> Well, there were so many revisions, I probably should have re-rolled the\n>> whole series....\n>> ...\n>> if that's the way we want to go. I'm not partial to the phrase honor-pack-keep,\n>> but I don't think ignore-pack-keep is appropriate, and it's the best I've come\n>> up with.\n>\n> Ok, care to send in a series in incremental updates format, please?\n\nNah, nevermind. I see you already sent a replacement series overriding what\nis in 'next'.\n"},{"id":"95617","messageId":"20081112191757.GB21401@coredump.intra.peff.net","threadId":"16113","inReplyTo":"7vzlk4q38q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-12T19:17:57Z","receivedAt":"2008-11-12T19:17:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 12, 2008 at 09:10:45AM -0800, Junio C Hamano wrote:\n\n> > Hmm. I'm not sure what happened, but the version in 'next' has \"head -n\n> > -1\" in it.\n> \n> You mean this one?\n> \n> commit 31d92611e45d1286b805e362dbc451936af24121\n\nYes, exactly.\n\n-Peff\n"},{"id":"95651","messageId":"E9qVE-t4Yve7ucbU-qz4xNKAAKLQr2fsS-7YejFpq3D1nvYFAzALqOj8GLVYLfJ_AlbpSpUJ_6o@cipher.nrlssc.navy.mil","threadId":"16113","inReplyTo":"WFmMnhNsYRw3IUD8hZk59At-5jc1rZU5GaB35sb1epRZ6zwd7IMw9V94SQyEliIEyriM4J5j3AM@cipher.nrlssc.navy.mil","subject":"[PATCH] t7700: test that 'repack -a' packs alternate packed objects","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-11-13T00:50:26Z","receivedAt":"2008-11-13T00:50:26Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Previously, when 'repack -a' was called and there were no packs in the local\nrepository without a .keep file, the repack would fall back to calling\npack-objects with '--unpacked --incremental'. This resulted in the created\npack file, if any, to be missing the packed objects in the alternate object\nstore. Test that this specific case has been fixed.\n\nSigned-off-by: Brandon Casey \n---\n\n\nHere is a test to demonstrate what\n [PATCH 5/6] repack: do not fall back to incremental repacking with [-a|-A]\nfixes.\n\nThis should apply cleanly to next since it includes\n a836cfa3 t7700: demonstrate mishandling of loose objects in an alternate ODB\nwhich has a few context lines showing through in the diff below.\n\n-brandon\n\n\n t/t7700-repack.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex 960bff4..3f602ea 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -53,5 +53,21 @@ test_expect_success 'loose objects in alternate ODB are not repacked' '\n \ttest -z \"$found_duplicate_object\"\n '\n \n+test_expect_success 'packed obs in alt ODB are repacked even when local repo is packless' '\n+\tmkdir alt_objects/pack\n+\tmv .git/objects/pack/* alt_objects/pack &&\n+\tgit repack -a &&\n+\tmyidx=$(ls -1 .git/objects/pack/*.idx) &&\n+\ttest -f \"$myidx\" &&\n+\tfor p in alt_objects/pack/*.idx; do\n+\t\tgit verify-pack -v $p | sed -n -e \"/^[0-9a-f]\\{40\\}/p\"\n+\tdone | while read sha1 rest; do\n+\t\tif ! ( git verify-pack -v $myidx | grep \"^$sha1\" ); then\n+\t\t\techo \"Missing object in local pack: $sha1\"\n+\t\t\treturn 1\n+\t\tfi\n+\tdone\n+'\n+\n test_done\n \n-- \n1.6.0.3.552.g12334\n"}]}