{"thread":{"id":"13123","subject":"[PATCH] git-fast-import: note 1M limit of mark number","startedAt":"2008-04-15T12:54:48Z","lastAt":"2008-04-16T07:04:32Z","messageCount":5,"participants":["Sam Vilain","Michael Haggerty","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74447","messageId":"20080415125222.C55C121CE8F@mail.utsl.gen.nz","threadId":"13123","inReplyTo":null,"subject":"[PATCH] git-fast-import: note 1M limit of mark number","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2008-04-15T12:54:48Z","receivedAt":"2008-04-15T12:54:48Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"The insert_mark() function in fast-import.c has this limit; note the\nlimitation in the documentation.\n\nSigned-off-by: Sam Vilain <sam@vilain.net>\n---\n Documentation/git-fast-import.txt |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex c29a4f8..b5cc3c2 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -410,6 +410,9 @@ or `refs/heads/42`), or an abbreviated SHA-1 which happened to\n consist only of base-10 digits.\n +\n Marks must be declared (via `mark`) before they can be used.\n++\n+Note that due to current internal limitations, you may not make marks\n+with a higher number than 1048575 (2^20-1).\n \n * A complete 40 byte or abbreviated commit SHA-1 in hex.\n \n-- \n1.5.4.rc2.85.g7c8f5\n"},{"id":"74453","messageId":"4804CECE.2040205@alum.mit.edu","threadId":"13123","inReplyTo":"20080415125222.C55C121CE8F@mail.utsl.gen.nz","subject":"Re: [PATCH] git-fast-import: note 1M limit of mark number","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2008-04-15T15:50:38Z","receivedAt":"2008-04-15T15:50:38Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Sam Vilain wrote:\n> The insert_mark() function in fast-import.c has this limit; note the\n> limitation in the documentation.\n> \n> Signed-off-by: Sam Vilain <sam@vilain.net>\n> ---\n>  Documentation/git-fast-import.txt |    3 +++\n>  1 files changed, 3 insertions(+), 0 deletions(-)\n> \n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index c29a4f8..b5cc3c2 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -410,6 +410,9 @@ or `refs/heads/42`), or an abbreviated SHA-1 which happened to\n>  consist only of base-10 digits.\n>  +\n>  Marks must be declared (via `mark`) before they can be used.\n> ++\n> +Note that due to current internal limitations, you may not make marks\n> +with a higher number than 1048575 (2^20-1).\n>  \n>  * A complete 40 byte or abbreviated commit SHA-1 in hex.\n>  \n\nOh.  Um.  That is an awkwardly small number nowadays.\n\ncvs2svn has been used for repositories with O(2^20) distinct file\nrevisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too\nsmall for some users.\n\nMoreover, cvs2git needs to generate marks for both file contents and for\ncommits.  It generates the latter by adding 1000000000 to the small\ninteger IDs that it uses internally.  If git-fast-import only allows\n20-bit integers, this makes me wonder why this hasn't broken\ndramatically in the past.  Pure numerological good fortune, combined\nwith weak range checking in git-fast-import?\n\nIn any case, this restriction will require changes in cvs2git.\n\nWhile I'm at it, let me also renew my suggestion that git-fast-import\nuse separate namespaces (\"markspaces\", so to speak) for file content\nmarks and for commit marks.  There is no reason for these distinct types\nof marks to be located in a shared space of integers.\n\nMichael\n"},{"id":"74476","messageId":"48051882.8000201@vilain.net","threadId":"13123","inReplyTo":"4804CECE.2040205@alum.mit.edu","subject":"Re: [PATCH] git-fast-import: note 1M limit of mark number","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2008-04-15T21:05:06Z","receivedAt":"2008-04-15T21:05:06Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Michael Haggerty wrote:\n>> ++\n>> +Note that due to current internal limitations, you may not make marks\n>> +with a higher number than 1048575 (2^20-1).\n>>  \n>>  * A complete 40 byte or abbreviated commit SHA-1 in hex.\n>>  \n> \n> Oh.  Um.  That is an awkwardly small number nowadays.\n> \n> cvs2svn has been used for repositories with O(2^20) distinct file\n> revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too\n> small for some users.\n\nRight.  But, if you're not making the importer you write for a\nconversion of that size restartable, you're insane.  So, marking more\nthan 1Mi *marks* in a single gfi session might not be so vital.\n\nIt only tripped me up because I was using a database sequence to\ngenerate the marks, which meant I hit the ceiling.\n\n> Moreover, cvs2git needs to generate marks for both file contents and for\n> commits.  It generates the latter by adding 1000000000 to the small\n> integer IDs that it uses internally.  If git-fast-import only allows\n> 20-bit integers, this makes me wonder why this hasn't broken\n> dramatically in the past.  Pure numerological good fortune, combined\n> with weak range checking in git-fast-import?\n\nPerhaps.  All I saw was that after I hit 1Mi for the mark ID, the mark\nnumbers in the returned file were drastically different from the ones I\nput in.  I had a glance over this code and it seemed likely to be a\nculprit - this docpatch is really more raising awareness of the problem.\n Obviously finding the fault and fixing it would be preferable.\n\n> While I'm at it, let me also renew my suggestion that git-fast-import\n> use separate namespaces (\"markspaces\", so to speak) for file content\n> marks and for commit marks.  There is no reason for these distinct types\n> of marks to be located in a shared space of integers.\n\nThere is a reason, it's because they're both just object IDs.  Is it\nreally that much of a drag?  I know what you mean though, it meant for\nmy code I had to keep track of which type each mark was.\n\nSam.\n"},{"id":"74528","messageId":"20080416065425.GC29771@spearce.org","threadId":"13123","inReplyTo":"48051882.8000201@vilain.net","subject":"Re: [PATCH] git-fast-import: note 1M limit of mark number","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-04-16T06:54:26Z","receivedAt":"2008-04-16T06:54:26Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sam Vilain <sam@vilain.net> wrote:\n> Michael Haggerty wrote:\n> >> ++\n> >> +Note that due to current internal limitations, you may not make marks\n> >> +with a higher number than 1048575 (2^20-1).\n> >>  \n> >>  * A complete 40 byte or abbreviated commit SHA-1 in hex.\n> > \n> > Oh.  Um.  That is an awkwardly small number nowadays.\n> > \n> > cvs2svn has been used for repositories with O(2^20) distinct file\n> > revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too\n> > small for some users.\n> \n> Right.  But, if you're not making the importer you write for a\n> conversion of that size restartable, you're insane.  So, marking more\n> than 1Mi *marks* in a single gfi session might not be so vital.\n> \n> It only tripped me up because I was using a database sequence to\n> generate the marks, which meant I hit the ceiling.\n\nUhm.  Wow.  gfi has a bug then; mark numbers are supposed to\nbe whatever uintmax_t is on your platform; for any systems that\nsupport a 64 bit off_t (and most do these days) that should be a\n64 bit integer value, which we all know easily exceeds 2^20-1.\n\nI'd rather see the bug fixed than a documentation patch.  But I'm\ntoo whacked out from heavy travel in the past two days flying\ncoast-to-coast and back to attempt debugging gfi and writing such\na fix patch tonight.  Maybe someone else will find it before I\ncan look at it later.  ;-)\n \n> > While I'm at it, let me also renew my suggestion that git-fast-import\n> > use separate namespaces (\"markspaces\", so to speak) for file content\n> > marks and for commit marks.  There is no reason for these distinct types\n> > of marks to be located in a shared space of integers.\n> \n> There is a reason, it's because they're both just object IDs.  Is it\n> really that much of a drag?  I know what you mean though, it meant for\n> my code I had to keep track of which type each mark was.\n\nYea.  I think I had pointed out the same point to Michael earlier\nwhen he asked about it.  I didn't want create two different tables\nof marks because then we either need to extend the language to say\nwhich table, or we have to infer it based on position.  But either\nway its just object IDs.\n\n-- \nShawn.\n"},{"id":"74529","messageId":"4805A500.5070707@alum.mit.edu","threadId":"13123","inReplyTo":"48051882.8000201@vilain.net","subject":"Re: [PATCH] git-fast-import: note 1M limit of mark number","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2008-04-16T07:04:32Z","receivedAt":"2008-04-16T07:04:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Sam Vilain wrote:\n> Michael Haggerty wrote:\n>>> ++\n>>> +Note that due to current internal limitations, you may not make marks\n>>> +with a higher number than 1048575 (2^20-1).\n>>>  \n>>>  * A complete 40 byte or abbreviated commit SHA-1 in hex.\n>>>  \n>> Oh.  Um.  That is an awkwardly small number nowadays.\n>>\n>> cvs2svn has been used for repositories with O(2^20) distinct file\n>> revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too\n>> small for some users.\n> \n> Right.  But, if you're not making the importer you write for a\n> conversion of that size restartable, you're insane.  So, marking more\n> than 1Mi *marks* in a single gfi session might not be so vital.\n\nWell, call me insane then.  Because there *are* no bulletproof\nincremental CVS importers, and it would be a lot of work to write one.\n(git-cvsimport doesn't count because it is far from bulletproof.)  So it\nis reasonable to choose a high-quality one-time conversion over a broken\nrestartable conversion.\n\n>> While I'm at it, let me also renew my suggestion that git-fast-import\n>> use separate namespaces (\"markspaces\", so to speak) for file content\n>> marks and for commit marks.  There is no reason for these distinct types\n>> of marks to be located in a shared space of integers.\n> \n> There is a reason, it's because they're both just object IDs.  Is it\n> really that much of a drag?  I know what you mean though, it meant for\n> my code I had to keep track of which type each mark was.\n\nThat's exactly my point.  That's at least two of us then who have had\nextra work to merge two separate ID concepts into a single integer\nsequence.  It's not a big deal, but it would be convenient if\ngit-fast-import allowed the sequences to overlap.\n\nMichael\n"}]}