Re: git: uh-oh - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: git: uh-oh |
Date | |
Msg-id | 29443.1282345155@sss.pgh.pa.us Whole thread Raw |
In response to | Re: git: uh-oh (Magnus Hagander <magnus@hagander.net>) |
List | pgsql-hackers |
Magnus Hagander <magnus@hagander.net> writes: > I have now pushed a complete copy of the latest migrated repository to > http://git.postgresql.org/gitweb?p=git-migration-test.git;a=summary. > This one has tkey keyword expansion on, which we decided we want. My > script that compares branch tips and tags to cvs now shows *zero* > differences. Which in itself is an improvement over the old version of > cvs2git :-) Cool --- this alone is probably worth the delay in converting. > I have not checked the state of the "newly added files issue" that > Robert found, nor in general verified anything other than branch tips > and tags so far. Anybody who has time to do that, please go right > ahead :-) I just spent some time comparing the REL8_3_STABLE history to what I have from cvs2cl. It is *much* better --- no more unrelated commits. I do see the "manufactured commits" that Robert is on about, but what is now apparent is that those correspond to artifact commits on the CVS side too. For example, here's what cvs2cl claims happened in the 8.3 branch on Feb 28 (all times EST = GMT-5): 2010-02-28 22:41 tgl * contrib/xml2/: Makefile, xpath.c, xslt_proc.c, expected/xml2.out,sql/xml2.sql (REL8_3_STABLE): Back-patch today's memorymanagementfixups in contrib/xml2.Prior to 8.3, these changes are not critical for compatibility withcore Postgres,since core had no libxml2 calls then. However thereis still a risk if contrib/xml2 is used along with libxml2functionalityin Perl or other loadable modules. So back-patch toall versions.Also back-patch addition of regressiontests. I'm not sure howmany of the cases are interesting without the interaction with corexml code, but a sillyregression test is still better than none atall. 2010-02-28 21:21 tgl * src/: backend/access/transam/xact.c, backend/utils/adt/xml.c,include/utils/xml.h (REL8_3_STABLE): Back-patch changes of2009-05-13in xml.c's memory management.I was afraid to do this when these changes were first made, but nowthat 8.4 hasseen some field use it should be all right toback-patch. These changes are really quite necessary in order togive xml.cany hope of co-existing with loadable modules that alsowish to use libxml2. 2010-02-28 16:31 tgl * contrib/xml2/: expected/xml2.out, sql/xml2.sql: Fix up memorymanagement problems in contrib/xml2.Get rid of the code thatattempted to funnel libxml2's memoryallocations into palloc. We already knew from experience with thecore xml datatypethat trying to do this is simply not reliable. Unlike the core code, I did not bother adding a lot ofPG_TRY/PG_CATCHlogic to try to ensure that everything is cleanedup on error exit. Hence, we might leak some memory ifone of thesefunctions fails partway through. Given the deprecated status ofthis contrib module and the fact that errorspartway through thefunctions shouldn't be too common, it doesn't seem worth worryingabout.Also fix a separate bug inxpath_table, that it did the wrongthings if given a result tuple descriptor with less than 2 columns. While such a caseisn't very useful in practice, we shouldn't failor stomp memory when it occurs.Add some simple regression tests basedon all the reported crashcases that I have on hand.This should be back-patched, but let's see if the buildfarm likesitfirst. Notice that that last entry doesn't say (REL8_3_STABLE). Which is correct, because *there was no such commit against 8.3*. This entry is quoting the commit message, and the commit time, of the HEAD commit that added xml2.sql and xml2.out --- and notice it only lists those two files, not the other ones touched by that HEAD commit. In the git conversion, there is a "manufactured commit" corresponding to this one, although the timestamp seems slightly different, and it appears to inject the HEAD versions of these test files into the branch. Then in the later git commit corresponding to the first cvs2cl entry above, there is a diff that makes the test files match the way they really look in 8.3. I suspect that this happened every time we did a back-branch file addition, and that in most cases the oddity got masked because the HEAD and back-branch commits happened at approximately the same time and with identical commit messages. So they got folded into one report by cvs2cl. In this example, with the messages being different and a few hours elapsed between the commits, we can see that some weirdness did happen in the CVS history too. This also becomes apparent when you look at cvsweb: http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/xml2/sql/xml2.sql The back-branch versions of the file are shown as being updates from the HEAD version 1.1, which is surely not the way things happened in reality, but ... So at this point I'm willing to buy Max and Michael's assertion that this is a faithful conversion of the CVS history. The fact that the commit messages are tagged as manufactured seems like it might be a good thing not a bad thing --- they're manufactured on the CVS side too. We need to do more testing of this conversion, but right at the moment I'm thinking it might be OK as-is. regards, tom lane
pgsql-hackers by date: