{"thread":{"id":"5193","subject":"diff machinery cleanup","startedAt":"2006-08-10T08:24:55Z","lastAt":"2006-08-10T21:02:24Z","messageCount":6,"participants":["Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"25076","messageId":"20060810082455.GA30739@coredump.intra.peff.net","threadId":"5193","inReplyTo":null,"subject":"diff machinery cleanup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-08-10T08:24:55Z","receivedAt":"2006-08-10T08:24:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I'm trying to run two different 'diffs' in the same C program, and the\nresults of the second diff depend on whether or not the first diff has\nrun. A minimal case is below. When show_updated() is run, show_changed()\nresults in all files listed as \"unmerged\". If show_updated() is\ncommented out, the output for show_changed() is as expected.\n\n-- >8 --\n#include \"cache.h\"\n#include \"object.h\"\n#include \"commit.h\"\n#include \"diff.h\"\n#include \"revision.h\"\n\nvoid show_updated() {\n\tstruct rev_info rev;\n\tconst char *argv[] = { NULL, \"HEAD\", NULL };\n\tinit_revisions(&rev, NULL);\n\tsetup_revisions(2, argv, &rev, NULL);\n\trev.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;\n\trun_diff_index(&rev, 1);\n}\n\nvoid show_changed() {\n\tstruct rev_info rev;\n\tconst char *argv[] = { NULL, NULL };\n\tinit_revisions(&rev, NULL);\n\tsetup_revisions(1, argv, &rev, NULL);\n\trev.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;\n\trun_diff_files(&rev, 0);\n}\n\nint main(int argc, char **argv) {\n\tprintf(\"updated:\\n\");\n\tshow_updated();\n\tprintf(\"changed:\\n\");\n\tshow_changed();\n\treturn 0;\n}\n-- >8 --\n\nIt seems clear that there's some global magic touched by the first diff\nthat impacts the second. I have to give up on finding it for tonight,\nbut I'm hoping somebody who knows more about the code will find it\nobvious (or can tell me that I'm doing something else horribly wrong in\nthe above, or that these functions were never intended to be called\nwithin the same program).\n\n-Peff\n"},{"id":"25078","messageId":"7vejvpvsni.fsf@assigned-by-dhcp.cox.net","threadId":"5193","inReplyTo":"20060810082455.GA30739@coredump.intra.peff.net","subject":"Re: diff machinery cleanup","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-10T09:36:49Z","receivedAt":"2006-08-10T09:36:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It seems clear that there's some global magic touched by the first diff\n> that impacts the second. I have to give up on finding it for tonight,\n> but I'm hoping somebody who knows more about the code will find it\n> obvious (or can tell me that I'm doing something else horribly wrong in\n> the above, or that these functions were never intended to be called\n> within the same program).\n\nIn general, run_diff_X are _not_ designed to run twice.\n\nThe run_diff_index() function munges the index while doing its\nwork (e.g. mark_merge_entries() hoists unmerged entries to stage\n3 -- and worse yet creating duplicate entries for the same path\nat stage 3; read_tree() reads the entries into stage 1 so that\nit can be compared in-index with stage 0 entries).  The other\nfunction, run_diff_files() have the same assumption but does not\ntouch index if I recall correctly.\n\nIf you are working in \"next\" branch where Johannes's merge-recur\nwork introduced discard_cache(), you could fake this somehow\nstashing away a copy of the original index, and once you are\ndone with run_diff_index(), clean the slate by calling\ndiscard_cache() once you are done, and swap the original index\nin before running run_diff_files().\n\nTo solve this cleanly without doing the index munging hack, you\nwould (actually, I would) need to have a new path walker that\nwalks index, tree and working tree in parallel, which I was\nworking on in the git-status/git-commit rewrite I started and\ndiscarded a few days ago.\n"},{"id":"25085","messageId":"20060810103836.GA1317@coredump.intra.peff.net","threadId":"5193","inReplyTo":"7vejvpvsni.fsf@assigned-by-dhcp.cox.net","subject":"Re: diff machinery cleanup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-08-10T10:38:37Z","receivedAt":"2006-08-10T10:38:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2006 at 02:36:49AM -0700, Junio C Hamano wrote:\n\n> In general, run_diff_X are _not_ designed to run twice.\n\nOK, makes sense. As you probably guessed, the reason is for the\nrun-status in C.\n\n> If you are working in \"next\" branch where Johannes's merge-recur\n> work introduced discard_cache(), you could fake this somehow\n> stashing away a copy of the original index, and once you are\n> done with run_diff_index(), clean the slate by calling\n> discard_cache() once you are done, and swap the original index\n> in before running run_diff_files().\n\nOK, doing a discard_cache() between the call to run_diff_index and\nrun_diff_files seems to clear up the problem. But if I understand\ncorrectly, are you saying that run_diff_index has munged the index on\ndisk, and I really need to be poking at a temporary copy? If so, why\nisn't that a problem when running (e.g.) \"git-diff-index; git-ls-files\"?\n\n> To solve this cleanly without doing the index munging hack, you\n> would (actually, I would) need to have a new path walker that\n> walks index, tree and working tree in parallel, which I was\n> working on in the git-status/git-commit rewrite I started and\n> discarded a few days ago.\n\nThat does sound the cleanest, and it would enable a more useful status\nmessage, as you mentioned before. What caused you to stop working on it?\nInfeasible, or simply more infeasible than you would like right now?\n\n-Peff\n"},{"id":"25107","messageId":"7vzmecv7tp.fsf@assigned-by-dhcp.cox.net","threadId":"5193","inReplyTo":"20060810103836.GA1317@coredump.intra.peff.net","subject":"Re: diff machinery cleanup","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-10T17:06:42Z","receivedAt":"2006-08-10T17:06:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> OK, doing a discard_cache() between the call to run_diff_index and\n> run_diff_files seems to clear up the problem. But if I understand\n> correctly, are you saying that run_diff_index has munged the index on\n> disk, and I really need to be poking at a temporary copy? If so, why\n> isn't that a problem when running (e.g.) \"git-diff-index; git-ls-files\"?\n\nNo, run_diff_index munges the index in-core, and it does not\nwrites it out for obvious reasons.\n\nSome of the \"interrogation commands\" do munge the index without\nwriting it out because that is the easiest/cleanest way to\nimplement what they do.  ls-files does it to filter by paths,\nfor example.\n\n> That does sound the cleanest, and it would enable a more useful status\n> message, as you mentioned before. What caused you to stop working on it?\n> Infeasible, or simply more infeasible than you would like right now?\n\nFinding out somebody was already working on it?\n"},{"id":"25113","messageId":"20060810201049.GA4759@sigio.intra.peff.net","threadId":"5193","inReplyTo":"7vzmecv7tp.fsf@assigned-by-dhcp.cox.net","subject":"Re: diff machinery cleanup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-08-10T20:10:50Z","receivedAt":"2006-08-10T20:10:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 10, 2006 at 10:06:42AM -0700, Junio C Hamano wrote:\n\n> No, run_diff_index munges the index in-core, and it does not\n> writes it out for obvious reasons.\n\nAh, OK, that makes more sense. Is there any reason then that this\nwouldn't work (it certainly seems to, but I don't want to be causing\ninvisible problems that will come back later)? You seemed to outline a\nmuch more complex procedure in your original mail.\n  run_diff_index();\n  discard_cache();\n  ...\n  run_diff_files();\n\nI'd like to go with this simple solution at first for the C runstatus,\nand then hopefully move to simultaneous diffing later.\n\n-Peff\n"},{"id":"25115","messageId":"7virl0uwwv.fsf@assigned-by-dhcp.cox.net","threadId":"5193","inReplyTo":"20060810201049.GA4759@sigio.intra.peff.net","subject":"Re: diff machinery cleanup","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-10T21:02:24Z","receivedAt":"2006-08-10T21:02:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Ah, OK, that makes more sense. Is there any reason then that this\n> wouldn't work (it certainly seems to, but I don't want to be causing\n> invisible problems that will come back later)?\n\nIt should.  I wanted to avoid re-reading the index from the\nfilesystem for some reason I cannot justify anymore.  The\nresponse was sent before my first caffeine for the day, perhaps\nthat was the reason ;-).\n"}]}